I wanted to just ignore these, but the RT builds for some reason have
warnings-as-error enabled by default and, UGH, I just want this thing to
finally pass the CI.
Not only the implementation is a template hell full of its own problems,
the internals are also just wrapping trivial bits of STL without adding
anything of value on top, so ultimately any code that actually used this
thing ended up being slower to build, more verbose, harder to reason
about and with thousands of tiny little inefficiencies all over the
place. So let's just finally say no to this idea from 2012.
"These seemed like a good idea at the time." But that was in 2013 and
while that might have been acceptable back then, it's not anymore, it's
all just a complex abstraction that prevents any reasonable resource
reuse and forces a dedicated draw call for each use.
Just do this directly from the corresponding Primitives instead.
Deprecating those allows me to eventually deprecate the (also
overengineered) ResourceManager class, at which point Magnum should be
finally free from the worst design decisions from 15 years ago.
Unfortunately several examples still rely on it (while it only makes
them more complex, not better), have to wait until those are cleaned up
first.
I'm going to deprecate the (awfully inefficient)
DebugTools::ForceRenderer and ObjectRenderer, and need at least
something else to direct the users to. This should suffice.
The NoInit constructor no longer results in an Array with a custom
deleter which would prevent the resulting MeshData from being used in
plugins, and all contents are subsequently overwritten, so there's no
reason to not use it.
And expand tests with corner cases that I previously forgot about. Can't
really provide any benchmarking info, since the Rungholt scene from
McGuire archives uses quads and negative indices.
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>
They're now uncovered, which only makes it harder to spot missing
coverage in non-deprecated code. And testing is simply a matter of
copypasting the existing tests.
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.
Nope, that was a very stupid change. The *vector* min() / max() is used
in intersect(), and this change broke the bindings build (likely along
with random other code).
This reverts commit 8c228c69c9. Partially,
keeping the extra added includes and also documenting why the
Functions.h are included.
Disabling SDL_AUDIO seems to work with 2.32.10, so if there was a bug it
no longer seems to be. On the other hand, SDL_EVENTS is something the
application implementation does need, so I'm not sure why I even
attempted disabling those.
For some reason it had the full set of elements on ES2 as well, i.e.
something that definitely shouldn't be the case. Unfortunately C doesn't
warn if the array isn't initialized for all elements, setting the
remaining to 0, so this was uncaught until now. Now if I accidentally
make the array too small somewhere, it'll blow up inside
BufferState.cpp.
Because I had a stupid bug in the test and wasn't testing all of them.
And then the function itself had a bug that made it no-op exactly when
it shouldn't be a no-op.
Vladimír Vondruš