There's really no need to allocate 56 MB of image *and* texture data
just to verify the constructor is called. This makes the Emscripten test
OOM and there's really no need for that.
Same approach as done e.g. in the Ui library -- taking advantage of the
base class already allocating an internal state struct, deriving from it
and putting the state there instead of either having it as a class
member (at the cost of extra header dependency) or as a separate state
struct (at the cost of extra allocation). The only downside is a virtual
destructor in the state struct, but compared to the alternatives that's
completely fine.
Neither a driver bug nor something wrong with the code. It's just
that with a tight flush rectangle the target texture may have some random
garbage left around the edges, causing the comparison to fail, so it's
now explicitly cleared upfront.
Doesn't happen when running the offending test alone (because I suppose
the memory is coming fresh from the driver, being zeroed out for security
purposes), only when running after the others (where I suspect it's now
reusing previous partially-filled memory which it didn't before). Doesn't
happen on NVidia either.
Compared to Corrade, the improvement in compile time is about a minute
cumulative across all cores, or about 8 seconds on an 8-core system (~2
minutes before, ~1:52 after). Not bad at all. And this is with a
deprecated build, the non-deprecated build is 1:48 -> 1:41.
Like with TextureTools::DistanceField, to make room for non-GL
implementations. Old names are deprecated aliases, same for headers. The
only remaining bit in the Text library is the Renderer class, but for
that one I first need to invent a non-shitty API so I can just deprecate
the old thing as a whole and create something reasonable from scratch in
RendererGL.
Along with the bits in Text library this is one of the last things that
still assume OpenGL present by default.
As usual, the old name and header is now a deprecated typedef.
The internal GL texture format (especially the R8 vs Luminance mess on
ES2) is now considered an implementation detail and shouldn't affect
common use in any way.
The format is now required always, in order to prepare for use cases
where colored glyphs are a thing as well. Additionally, to match the
recent change in AbstractGlyphCache, the processed format is specified
separately, allowing the input and processed formats to be decoupled.
Which ultimately fixes the regression on ES2 and WebGL 1 where it was no
longer possible to call font.fillyGlyphCache() on a
DistanceFieldGlyphCache.
Also, as there's now a generic format on input, another ES2-specific
issue is now fixed as well, in particular a case where a GL error
would be emitted on drivers with EXT_texture_storage because an unsized
format is passed to setStorage(). This was a problem since a long time
ago, but I ignored it because it didn't affect WebGL 1 and all drivers
that exposed EXT_texture_storage exposed EXT_texture_rg, effectively
circumventing this issue. Or so I think, at least.
The constructors taking either a GL::TextureFormat or no format at all
are now deprecated aliases to the new functionality.
RGB was a hopeful intention that never really worked in practice. Or, if
it worked, it couldn't be queried back, and the driver used RGBA
internally anyway.
The refactor in 6707534ce6 somehow didn't
account for the need to have a different pixel format for the input and
the processed texture, which is needed on ES2 / WebGL 1 because there's
no renderable single-channel format. Which means distance field text
rendering was broken since then.
This commit is the first part of fixing that regression. It makes the
AbstractGlyphCache aware of the processing being done, decoupling the
input and processed format and size. Which also allows the base
implementation to provide interfaces that so far were only limited to
DistanceFieldGlyphCache, thus eventually making it possible for font
plugins to supply a pregenerated distance field image through
fillGlyphCache() and not just through createGlyphCache().
The two DistanceFieldGlyphCache APIs that are now directly on the
AbstractGlyphCache are now deprecated aliases to the new functionality.
The actual fix for the ES2 / WebGL 1 regression will come in the next
commit, as it's pretty much a separate step that involves updating the
GlyphCache as well.
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?
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.
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.
Not that C++ STL and exceptions would be anything to take inspiration
from, but there's std::out_of_range. Python IndexError is also specified
as "index out of range", not "bounds".
About time this got done. This also has an XFAIL for the case where a
distance field image is processed with an offset, have to fix the
underlying issue in TextureTools first.
Also added a range assertion for the distance field image setter to
match what the abstract base does, together with a corresponding getter.
At the moment just the GL library itself w/o the tests, and without
backwards compatibility aliases. The following types were left in the
root namespace, despite being in the GL/ directory, as they will get
moved back soon:
* Image, CompressedImage and their dimensional typedefs
* ImageView, CompressedImageView and their dimensional typedefs
* PixelStorage
Not PixelFormat etc., that one will stay in the GL namespace and a
completely new PixelFormat enum will be provided in the root namespace.
Minimal updates (just the include guards) so Git is hopefully able to
detect the rename and track the history properly.
Everything except Magnum::GL doesn't compile now.
Everything what was in src/ is now in src/Corrade, everything from
src/Plugins is now in src/MagnumPlugins, everything from external/ is in
src/MagnumExternal. Added new CMakeLists.txt file and updated the other
ones for the moves, no other change was made. If MAGNUM_BUILD_DEPRECATED
is set, everything compiles and installs like previously except for the
plugins, which are now in MagnumPlugins and not in Magnum/Plugins.
Better API for handling more than one application screens (context
switching, event propagation etc.). Taken from Push The Box, updated to
current coding style and templated.
Vladimír Vondruš