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.
Similarly like pixel size is stored in uncompressed images, block size
makes it possible to perform size checks on passed data, slice the images
and so on. It only took over a decade to get that done.
The block properties coming from CompressedPixelStorage are currently
expected to be either not set at all or exactly match what's stored in
the image for given format. The PixelStorage will get eventually
deprecated in favor of a simpler and more flexible representation, but
that's another big chunk of work so it's first done like this.
The GL library tests currently blow up on various assertions and it
isn't yet updated to make use of the known format properties instead of
querying them from GL. That'll be done in the following commits.
Similarly as done in Aug 2024 in Corrade. When these were a part of the
function signature, they ended up being encoded into the exported
symbol. There are still cases of StridedArrayView slice() having
enable_if in the signature, which amounts to about 18 kB symbols in all
libMagnum*-d.so libraries, but apart from that this is the state before:
$ strings libMagnum*-d.so | grep enable_if | grep -v slice | wc -c
29591
And this is after. All of those are coming from STL, thus from
old or deprecated APIs that still use std::vector, std::tuple and such,
and from the few std::sort() uses.
$ strings libMagnum*-d.so | grep enable_if | grep -v slice | wc -c
4103
In a non-deprecated build it's just this, which is a 10x reduction.
Can't really do much about these maybe exceút for implementing my own
swap() specializations (sigh?), but I think it's fine.
$ strings libMagnum*-d.so | grep enable_if | grep -v slice | wc -c
2904
I also made it consistently use
typename std::enable_if<..., int>::type = 0
instead of
class = typename std::enable_if<...>::type
because the former works correctly also in presence of overloads and
having it used consistently everywhere makes it easier to grep & change
later. All SFINAE is now also excluded from Doxygen output, because it
doesn't make much sense there. It's better to just explain the
restriction in words than with this nasty hack.
The ambiguity with StridedArrayView1D<const char> was there always, I
just didn't hit that anywhere so far. With the recent changes in
Corrade, where StridedArrayView2D<const T> is constructible from
StridedArrayView1D<T> as well in addition to const T, the ambiguity gets
hit by a test. So test both variants and add an overload that resolves
those.
There's no other such case in either MeshIndexData or SceneFieldData, as
the StridedArrayView1D<const void> and StridedArrayView2D<const char>
constructor variants always have differing arguments. Neither it happens
in case of (ARM) platforms where char is unsigned.
They got added in 6d41597d1d in 2018 to
selectively suppress a deprecation warning. But later on the deprecated
API got removed and these stayed, being useless. So just call the
ADL pixelFormatSize() APIs directly.
For compressed images the expected message was completely wrong, meaning
it'd fail in any case, and they were missing a SKIP on no-assert builds.
Then, for GL::[Compressed]BufferImage the assertions in setData() were
never tested at all.
Because there's no format that'd have more than 256-byte pixels anyway,
the theoretically biggest one would be RGBA64F or some such with 32
bytes. Nevertheless, an assert is now in place to verify the bounds as
well as ensuring the pixel size is not zero.
They're not parsed since 6b22a11170
(2020), so there's no point in keeping those workarounds. They're only
kept in utility application sources as they're parsed for pages, and in
tweakable implementations where it's easier to just copypaste the whole
ifdef expression from the header every time instead of modifying it to
not include DOXYGEN_GENERATING_OUTPUT.
And document that. Because the pixel size cannot be determined for it,
and one has to either pass it explicitly or use the templated overload
that figures it out implicitly via ADL. This asserted before, but only
deep inside in pixelFormatSize(), which may be confusing.
I need to do a similar treatment for compressed images with block size
properties so let's first make it behave properly for uncompressed.
I mean, yeah, it's all bad, but at least it works now. The upcoming
internal representation will not be this silly CompressedPixelStorage
anymore and then it will become a bit less bad.
For a proper language-lawyer-safe implementation I'd explicitly call
destructors and then in-place-new the other instance and such, but
that's two more branches and thus twice as many chances to mess up.
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.
Back in 2020 when I wrote this I didn't really expect the MeshData to be
directly used for much more than putting them on a GPU, mostly because
that used to be the primary use case with the old MeshData2D /
MeshData3D. So the documentation was focusing mainly on populating a GPU
mesh, and any docs for CPU-side access were added rather hastily.
As now the asset processing use case is much larger, the original docs no
longer made sense. Let's hope this is better.
Which is good, those are all either anchors in the theme itself, or are
group names for which Doxygen doesn't make anchors anymore (but m.css
does). Plain HTML link still works as before, so let's do that instead.
Vladimír Vondruš