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.
The test passes now. This made the benchmark run significantly faster.
~200 µs instead of 670. Are clears really taking up more time than all
those texture fetches? Strange.
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?
The output is now *much* closer to the Vector shader output (mean
difference went from ~6 to 1.79), and in the multi-draw case it's
clearly visible that it's no longer slightly weirdly uneven.
The test files are now also much smaller as the originals were created
before RLE was implemented in TgaImageConverter.
The original implementation wrongly assumed that the input and output
pixel centers align, which would only be a case if the ratio of the
input and output sizes would be odd. Which it in practice isn't, usually
it's a 1024x1024 texture scaled down to 128x128 or something like that.
The flipped test cases added in the previous commit now pass.
According to the benchmark, the new code is very slightly slower (~815
µs vs ~805 before). The new code isn't really more complex than the old
one, it just does slightly different work -- there are new corner case
in the initial logic for marking the pixel inside or outside, on the
other hand some corner cases that had to be handled in the previous case
are no longer a thing.
If it's not, it's a programmer error (i.e., don't use Luminance or
packed formats, won't work), and since there's no way for the API to
report a failure in a programmatic way, this was causing hard-to-track
errors.
And the thing that changed for SwiftShader 4.1 was added EXT_texture_rg
support, which made the test work. So no, this was not a SwiftShader bug
at all.
Use a different overload to not have to create a temp framebuffer every
time, move also the error checking outside the loop and increase the
iteration count to actually have a result that isn't wildly different
every time.
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.
No reason not to, even though the move is destructive. Also unblocks
Text::DistanceFieldGlyphCache which wasn't movable due to this and one
other problem.
It's too annoying otherwise, especially if padding is involved. Turns
out that if padding is zero, these don't really contribute to the layout
in any way.
The internals are still rather ew, that's for another time, but the goal
here was to make it compile and correctly handle the new variability in
created and passed glyph cache instances. In particular:
- The MagnumFont createGlyphCache() now uploads the texture image
directly, skipping a copy through the CPU-side image which may not
have a size matching the input image. That's kind of nasty (and too
tied to OpenGL), but will be cleaned up once the GlyphCache APIs are
fixed to allow this in a nicer way.
- The MagnumFontConverter will now correctly select a font if the cache
contains more than one, properly deal with glyph caches that either
do or not do image processing, and will fail for array glyph caches
as that's not supported by the current format. It also now has an
early special-case handling for glyph caches with processed images,
where the actual image may have different size (and possibly format),
to match what MagnumFont expects.
The class now supports incremental filling, multiple fonts, texture
arrays, removes all reliance on STL containers and is finally properly
documented.
To avoid complete breakage of every use, as much as possible was kept as
deprecated APIs -- in particular the reserve() with the nasty
std::vectors, the insert() that assumes a 2D cache and a single font
and textureSize() that returns a 2D vector. Those behave the same as
before, but will assert if the cache is an array or contains more than
one font.
On the other hand, begin() / end() access with std::unordered_map iterators
(ew!) was removed as the internals simply aren't a hashmap anymore. The
image() that returned an Image2D is now used to fill the glyph cache
instead of querying its potentially processed contents, and returns a
MutableImageView3D. I considered keeping it and adding sourceImage()
instead, but such naming turned out to be too inconsistent. For querying
processed image data (such as with the distance field cache) there's a
new processedImage() query, guarded by new GlyphCacheFeature bits -- if
both ImageProcessing and ProcessedImageDownload is set, it can be used
to retrieve the processed image (so, similar as ImageDownload was
before), and if neither is set, the cache contents are queryable
directly through image(), without needing any special support from
the GPU API.
Existing code is updated only in the minimal way possible to ensure that
no serious breakage was introduced by reimplementing the deprecated APIs
on top of the new backend. Porting away from deprecated APIs will be
done in next commits. The GlyphCache and DistanceFieldGlyphCache have
their public API kept intact for now, as a similar rework will be needed
for them as well.
Additionally, the MagnumFont and MagnumFontConverter plugins aren't
compiling yet as they require substantial changes to deal with the new
glyph cache features. That is not the case with other plugins in the
magnum-plugins repository tho, for those the backwards compatibility
"just works". On the other hand, since layout of the AbstractGlyphChange
changed, I'm bumping the AbstractFont plugin interface version to
force-trigger a rebuild of dependent projects. Because I ran a stale
magnum-player binary, it worked without crashing or GL errors but just
didn't show ANY text whatsoever due to ABI differences, and I wasted
some precious minutes before realizing that a simple rebuild would fix
it.
Ugh. Was using this to verify that the glyph cache was correctly
populated, only to end up with a GL error that I thought was coming from
the glyph cache itself and not here. Wasted too much time on that.
Need some regression tests for the upcoming glyph cache rework as
otherwise it'll be too miserable. It now fills the glyph cache image
with some non-trivial data and compares to them, and checks the filled
glyphs for the created glyph cache too.
This also fixes a regression introduced when porting away from STL in
47a1295ab8, where UTF-8 layouting was
reporting extra glyphs at the end. Now UTF-8 support is properly tested
both in the MagnumFontConverter plugin and MagnumFont.
Vladimír Vondruš