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.
I think I tried this at least once in the past already, and failed the
same way. So just document that it's missing to prevent me from wasting
time on this again in the future.
The MAGNUM_GL_ABSTRACTSHADERPROGRAM_SUBCLASS_DRAW_IMPLEMENTATION() and
MAGNUM_GL_ABSTRACTSHADERPROGRAM_SUBCLASS_DISPATCH_IMPLEMENTATION() *is*
documented so that one stays not underscored.
One obvious use case is rendering a text that's just a bunch of \n
characters. With the current code, it would result in a completely empty
rectangle no matter how many newlines there would be, which isn't right
because -- even if nothing is actually visible -- it would break
anything that relies on the bounding rectangle for alignment and
positioning of other content.
Another case is for example the Ui library, which -- once ported to
use the new Renderer instead of the lower-level APIs -- wants to use the
bounding rectangle for cursor and selection positioning. And if it would
be empty for empty text, it'd mean empty input boxes would have
invisible cursor. Not good.
The old Renderer implementation seems to have handled this correctly --
but in f3d6ab4916, when porting to the new
APIs, I discarded that from the test, thinking it was some unnecessary
behavior. It wasn't, it was just never tested directly so I forgot it
was needed.
This was quite nasty, a multi-day effort to trim this down and then
increasingly growing disappointment as I discovered it was affecting
basically any use of the API.
With seemingly innocent code like below, it happened that the
implementation part from MagnumMath.hpp got included twice because
MagnumMathBatch.hpp includes MagnumMath.hpp as a dependency, leading to
duplicate symbol definitions. This should be a completely valid use
case, so I'm adding guards to ensure the implementation is always only
added if not already.
#define MAGNUM_MATH_IMPLEMENTATION
#include <MagnumMath.hpp>
#include <MagnumMathBatch.hpp>
A corresponding test case is going to get added to the singles repo.
Am I overdoing it? Most unpremultiplication code I found doesn't even
deal with treating alpha being zero correctly, while here I'm handling
also the cases of RGB channels going over alpha, and for packed
formats trying to match precision of the same done with pack()/unpack().
The equations were using some strange undefined `a` which made it look
like it's dealing with alpha. Just turn that into a constant like
Wikipedia does, and link there as well. Additionally, one of the `c`'s
wasn't correctly a lowercase bold vector.
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.
So far it contains just the uniform definitions and utilities related
to line drawing, nothing else, but especially the line utilities were
needed to be able to build MeshTools tests on a GL-less build.
The tests all pass exactly as they did before, apart from slightly
different assertion messages. I moved the guts from Renderer.cpp to
RendererGL.cpp as the code relies on both GL and STL, to not have it
spread over too many places.
Apart from the fact that it blew up when the text was indeed empty,
which is fixed now (ugh), it was kinda obvious with the current
implementation but won't be when the guts get replaced with something
reasonable. So ensure we don't break existing use cases.
Vladimír Vondruš