Basically the same logic as with Trade::AbstractImporter::openMemory(),
the doOpenData() signature is (yet again) changed to accept a pair of a
r-value Array reference and a DataFlags describing whether the memory is
owned or not. Compared to Trade, I don't anticipate any in-place
mutation of open fonts, so there's no DataFlags::Mutable.
There's also no DataFlags::Global because data of the opened font aren't
exposed further anywhere (so far at least, but I can imagine SVG data
eventually possibly passed through to importers, which then could be a
use case for this flag).
This is the final breaking change to the AbstractFont doOpenData() API.
Adapting of all other code to these changes will be done in a subsequent
commit, so right now a non-deprecated build still fails.
This was apparently never tested, so accidentally omitting the close()
call during a refactor wouldn't trigger any test failure until
accidental discovery months later. 100% coverage and yet.
This will eventually make it possible to create always-open font
implementations, as in those doProperties() can be called on-demand
without requiring a dummy implementation of doOpenFile() / doOpenData()
and doClose() that is there only to fill the Properties struct.
Thanks to the doOpenFile() / doOpenData() signature changing in the
previous commit I can do this in a backwards-compatible way, keeping the
original signature as a deprecated API. Changing just the return value
would be a hard breaking change.
As with previous commit, only AbstractFont tests themselves are adapted
to the changes, not the rest yet, so a non-deprecated build will now
fail.
The AbstractFont::openFile() and openData() now get an additional
argument specifying a font index to allow picking a concrete font index
for example in a TTC collection.
This index has to be specified upfront with no possibility to change it
afterwards, because that's how both FreeType and stb_truetype work. Thus
there are also new fileFontCount() and dataFontCount() APIs taking a
filename / data argument, instead of this being a fontCount() query on
an opened file. The implication from this is that -- unlike basically
all other APIs in Trade and elsewhere -- specifying an out-of-bounds
font index isn't an assertion (i.e., a programmer error) but rather a
graceful failure, because requireing the user to first call
fileFontCount() and then openFile() would mean having to open and
parse the same file twice, which is undesirable overhead.
While this isn't breaking for end users, as it's just a new optional
argument to openFile() and openData(), it's a breaking change for plugin
interfaces. The old doOpenFile() and doOpenData() interfaces are still
there to help transitioning existing plugins, but are marked as
deprecated. Delegating to those turned out to be quite involved, so
there are many new tests verifying this compatibility code path.
The plugin interface string version is bumped but in the next commits
I'm making use of the doOpenData() / doOpenFile() API breakage to do
more changes. Those won't result in further plugin interface changes, so
this set of commits should be treated as a single indivisible change to
the plugin interface. For this reason, uses of AbstractFont outside of
AbstractFontTest (including the MagnumFont plugin) aren't adapted yet
-- they pass tests as before, but compile only with deprecated APIs
enabled.
Co-authored-by: Andrew Snyder <asnyder@minitab.com>
It's done like this in all other code, not sure why not here. Likely
because debug output wasn't tested at all before and only got added
later, thus at the end? So make it consistent.
In some cases the plugin manager constructor was not tested at all,
leading to an uncovered line. For importers also verify that calling
close() on a non-opened instance doesn't call into doClose().
Again, the Audio importer isn't touched at all, as that interface is
scheduled for mergin into Trade.
So far, even though code coverage says it's all fine, things like data
conversion failures were not tested at all, or only by checking the
return value, but without verifying that the implementation got actually
called, and without checking that no superfluous message was printed by
accident.
The AbstractFontConverter interface is tested for failures as well even
though most of it is scheduled for rework (STL removal, etc.), as
otherwise it'd be likely that I forget to test the failure cases when
simply porting the old test code to the new APIs. On the other hand, the
Audio importer interface is not touched as it's scheduled for merging
into Trade.
Code from 2012 still wants to fight back. Turns out the user
implementation of doOpenFile() was never tested, only the fallback paths
to doOpenData(). Heh.
This makes the test added in previous commit pass on Chrome. This also
fixes the UI library there now, before it didn't show icons exactly due
to this, because they're uploaded with a non-zero offset (after text
glyphs) and Chrome requires the image height to be set in that case even
if uploading to the very first slice.
It works everywhere except on Chrome WebGL 2, which complains that pixel
unpack parameters are incorrect. Which is a very helpful error on its
own, fortunately I know already where the problem is.
It worked correctly with local testing, but now I enabled llvmpipe for
(desktop) GL testing on the CI where it's compiled with GCC 4.8 and *of
course* image.release() gets called before image.size(), resulting in
empty image being produced.
I bet the same happens with MSVC, it's just that nobody discovered yet.
Counterpart to GlyphCacheArrayGL. At first I expected that it'd need
TextureTools::DistanceFieldGL to be expanded with texture array support
but after enough massaging of brain matter I realized that not really,
since the procesing has to be done slice by slice anyway, and having to
upload the whole array just to have a temporary input for processing
would be a waste of memory.
At first I was like "ugh this is BAD" and made the texture and output
framebuffer resident. Then I realized that for the upcoming array
variant I need to reattach the output texture every time, so having a
resident framebuffer was not that useful anymore, and then I realized
that having a resident texture but calling setImage() on it is not any
better than making a temporary one every time, and making the resident
texture significantly larger just to accomodate any size that could
possibly be processed was even worse. So, ultimately it's just these
comments.
Right now it didn't really matter as the only subclass making use of
this was DistanceFieldGlyphCacheGL in which the processed format was
always R8Unorm, nevertheless it's better this way, and makes it work
with 16-bit output and such that might get added in the future.
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.
Was postponed in 8168a06bab because the
needed DebugTools::textureSubImage() was not there yet. Now I have (a
simplified version of) it done, so use it.
Vladimír Vondruš