Took me quite a while to realize what was going on, but in retrospect
it's obvious -- the rasterizer just *rounds* the sub-pixel-positioned
glyph quads to nearest pixels. Which then can cause the neighboring
glyph data to leak to it (because the texture is then sampled not
directly on the edge pixel, but slightly outside of it), or it can also
cut away the edge, when it gets rounded in the other direction.
This was a problem with the original -- laughably inefficient -- atlas
packer as well, but because that packer had excessive padding around
everything, only the second edge-cutting artifact manifested, and that
one is rather subtle unless you know what to look for.
This means the packing is now slightly worse than before and sizes that
previously worked may no longer fit anymore. But since the new atlas
packer is relatively new (well, from September, time sure flies
different here), and the improvement compared to the original packer is
still quite massive, I don't think this is a problem.
Want to construct them without a GL context present, and Optional is too
wasteful. Also adding it to the AbstractGlyphCache base, where it skips
also allocating the internal PIMPL state, because it's not going to get
used for anything in a NoCreate'd instance anyway.
If Magnum and Corrade get installed into the same directory,
target_include_directories() or target_link_libraries() with Corrade
before Magnum will result in the (usually stale) installed Magnum
headers being picked over the local ones. Which is unwanted, so try to
always put the local Magnum include path first.
Tested manually by installing to an arbitrary location and editing
configure.h to contain an #error. That failed for the Text library, and
with these changes it now doesn't fail anymore, but that's not a
guarantee that I managed to fix all such cases.
Especially given that nullptr causes an assert. All call sites basically
ended up passing a &font and all that extra annoyance just doesn't make
sense.
Given this API is still relatively recent, I'm not bothering with
backwards compatibility.
Which also allows the internals to be a bit simpler / potentially more
efficient, as the implementation can now access the glyph cache contents
directly, without a getter.
The new API makes it possible to alias storage for the glyph offsets,
advances and IDs (20 bytes per glyph) directly with the output positions
+ texcoords (16 bytes per glyph, times four) by splitting the retrieval
in two steps.
I wonder how long will it take before this bites back, heh.
In basically all cases it's two independent operations so forcing them
to be done together doesn't really bring any potential efficiency
advantages. On the other hand, splitting them allows allows the caller
to better make use of available memory, as the new
renderGlyphQuadsInto() allows the input and output arrays to be aliased.
Bumping AbstractFont plugin interface versions as this is a breaking
change.
The awful original STL-heavy public API is kept the same for now, it's
just the internals being now implemented using brand new APIs that are
actually usable with multiple fonts, font sizes and runs with different
scripts/languages/directions. There's also preparation for configurable
vertical text layouting, although for now the functionality asserts that
horizontal text is used.
This also makes Renderer.h header available on non-GL builds, as the new
APIs don't rely on a class full of GL objects anymore. The class will
get eventually renamed and moved to a dedicated RendererGL.h header, but
for now this partial update has to suffice.
To be used as a placeholder argument in the reworked renderer APIs. No
intention to deal with the complexities of this right now, just taking
it to not have to deprecate & replace the API when this feature actually
gets implemented.
So it's possible to shape the text even before having all glyphs ready.
That's one reason, second reason is that this is a more common behavior
-- it usually doesn't make sense to make the text jump based on whether
it's "zaxaca", "KEKEKE" or "yqpyq".
The original alignment based on glyph bounds is now moved into dedicated
`*GlyphBounds` variants. Additionally the `*LeftGlyphBounds` were
changed to subtract the initial glyph offset as well, `*Integer` now
rounds only in the direction where it's needed because a division by 2
happened, and there's a set of `*Bottom*` values that somehow weren't
there before.
Still not good enough regression testing for the changes I need to make.
This was absolutely not verifying correct behavior of the other vertical
alignment values for multiline text.
Otherwise the assertion from TextureTools::DistanceField fires only
later during image upload, which can cause great confusion, not being
sure what's to blame, etc.
Such as is the case with fillGlyphCache(), which just maintains the
union of all glyph rectangles. Sorry for having that temporarily broken.
Also I don't really like the solution here, but was the best I could
come up with, after a bunch of failed attempts to try to do this
directly in the TextureTools::DistanceField.
Just cannot use gl_FragCoord in here. That's it, that's the fix. What's
however COMPLETELY unexpected is that this simple change made the
process significantly faster on my Intel GPU, from ~815 µs to 670! I
can't even pretend I understand what's going on here, but maybe doing
less math in the fragment shader when calculating the texture
coordinates (and thus possibly the driver having a better idea how to
prefetch or schedule?) is what made this faster? Or maybe it's due to
one uniform input less, and two interpolated values instead of four on
the way from the vertex shader?
Empty input was asserting originally, but such strict behavior doesn't
seem to be useful in practice as all application code would need to
explicitly make sure that shape() isn't called if the input or the input
range is empty. In particular, I hit this assert as soon as I rebuilt
and ran magnum-player, and the assertion didn't really uncover any bug
or make anything better by firing.
Treating zero glyph output as a failure also isn't good -- rendering
for example a string of whitespace may also result in zero glyphs on the
output, which isn't a failure. Plus it's currently unclear what should
actually be a failure -- neither FreeType nor HarfBuzz really have a
concept of the shaping operation failing, hb_shape() returns void -- so
special-casing glyphCount() being zero as a failure just doesn't make
sense.
This thus means the doScript(), doLanguage() and doDirection()
implementations are now called always, regardless of whether shape() was
called at all, or whether it produced any glyphs. This allows the shaper
for example to give back a detected script etc even if the actual
input range to shape was empty.
This class won't print the deprecation warning in that case. I'm a bit
disappointed that the Clang changelogs are always so vague, there's
often never a possibility to figure out which version a particular bug
was fixed in apart from testing each and every.
Replaces the previous, grossly inefficient AbstractLayouter which was
performing one virtual call per glyph (!). It's now also reusable,
meaning it doesn't need to be allocated anew for every new shaped text,
and it no longer requires each and every font plugin to implement the
same redundant glyph data fetching from the glyph cache, scaling etc. --
all that is meant to be done by the users of AbstractShaper, i.e.
Renderer. The independency on a glyph cache theorerically also means it
can be used for a completely different, non-texture-based way to render
text (such as direct path drawing directly on the GPU), although I won't
be exploring that path now.
It also exposes an interface for specifying script, language,
direction and typographic features. Such interface will be currently
only implemented in HarfBuzz, but that's the intent -- to provide a
flexible enough interface to support all possible use cases that a font
or a font plugin may support, instead of exposing a least common
denominator and then having no easy way to shape a text in a non-Latin
script or use a fancy OpenType feature the chosen font has.
The old public interface is preserved for backwards compatibility,
marked as deprecated, however the virtual APIs are not, as supporting
that would be too nasty. I don't think any user code ever implemented a
font plugin so this should be okay.
To ensure smooth transition with no regressions, the Renderer class and
MagnumFont tests still use the old API in this commit, and their test
pass the same way as they did before (except for two removed MagnumFont
test cases which tested errors that are now an assertion in the
deprecated layout() API and thus cannot be tested from the plugin
anymore). Porting them away from the deprecated API will be done in
separate commits.
Will be used for the upcoming new AbstractShaper API. It's based on the
ISO script codes, matching the HarfBuzz script enum, with the intention
of being passed to it as-is.
It doesn't need to access anything else than what the base API provides,
doesn't render with it, and doesn't expose the instance via any getter
either.
Even though the Renderer is scheduled for a total rework and changing
the old API may feel like throwaway work, this change alone doesn't
break anything and allows me to test certain new corner cases.
It was all just way too random and completely detached from real-world
cases. In particular, the mesh rectangle and the texture rectangle were
completely different, which just cannot happen, and was completely
disregarding presence of a glyph cache, making it impossible to adapt to
the upcoming AbstractLayouter / AbstractShaper rework.
It now also explicitly verifies behavior of various Alignment values
instead of just testing them at random, and there's also a new TODO
related to those.
And thus the DistanceFieldGlyphCache subclass as well. The
deinlined destructor wasn't really needed as the GL::Texture has its
destructor deinlined as well, so it wouldn't cause too much extra work
for the compiler to have it implicit.
Also, I suspect the destructor was just a leftover from when there was
no AbstractGlyphCache base.
Vladimír Vondruš