Unlike _rgb and _rgbf these are not constexpr, but I'll eventually add
table-based constexpr variants into a new ConstexprLiterals namespace
for all sRGB and half-float variants.
This means I don't get the parsed number as an integer but have to parse
it myself from individual chars, on the other hand it allows me to check
that it's indeed hexadecimal and has correct length, because writing a
24-bit color literal with _rgbaf, or forgetting one digit, etc. was a
common source of accidental bugs.
The new code is larger and more complex, so I verified that it doesn't
have too large effect on compile times. On GCC the MathColorTest built
in ~1.20 seconds before and ~1.25 after, on Clang it was 1.24 vs 1.26.
So there's a minor increase, but it's small enough to warrant the
increased robustness. In total, the whole codebase with tests builds in
~120 seconds both before and after, so I suppose unless the codebase
consists of just color literals alone (which could be the case for e.g.
some stylesheets), it's completely fine.
And it found bugs in some test code already. Fix for those is in next
commit.
The original behavior was extremely imprecise. I remember hitting this
in the UI library, where it was happily telling me that a vector is
zero, and I spent ages debugging only to replace it with an equality
comparison that behaved correctly. Let's fix that properly.
Not file contents yet because the file can start with a XML preamble,
seven megabytes of comments and only then there's <svg. I'm not in a
mood for that today.
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.
It just binds a layer of it to a framebuffer internally so no shader
changes or extra construction flags are needed. Originally I thought
about making the input an array as well, but ultimately that just
doesn't make sense -- the processing would need to be done slice by
slice anyway and you don't want to allocate the whole excessively sized
texture just for it to be used once, and only a part of it every time.
Wanted to add a proper variant that reads all layers at once into an
Image3D, but that requires the Image APIs to be made less crappy first.
So it's just this for noew because now I badly need it for
Text::DistanceFieldGlyphCacheArrayGL tests.
I fixed some minor English crimes in the method docs and then realized
this is all just bad and the utter uselessness and lack of information
led to way too many confused questions over the years. So let's do it
properly, finally, ugh.
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.
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().
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.
So far the Renderer doesn't work with that, and neither the builtin
Vector shaders are be able use it, but gotta start somewhere.
I wanted to have the texture contents tested on ES, but it turns out
that implementing DebugTools::textureSubImage() for arrays is blocked on
another feature I badly need to finish first. Sigh.
I just don't see a point in those. PBOs are for when a roundtrip through
a CPU memory would be wasteful, but these utils are mainly for use in
tests. Definitely not for being called several times per frame, because
the temporary framebuffer creation just doesn't make sense. Not sure
what was I thinking in 2016 when I added those, apart from "feature
parity for no practical reason".
I'm going to make a new Renderer class that's unlike the old one, and
isn't templated anymore either, so gotta make room for it first. I assume
that all existing code used the Renderer2D / Renderer3D typedefs so this
should be pretty much painless.
Ultimately this class will be gone, with only (then deprecated)
AbstractRenderer left, and Renderer2D / Renderer3D being aliases to it
-- internally it doesn't actually do anything dfferent, even the
positions are two-component in both cases.
As the time goes, I have less and less patience trying to figure these
things out. It's likely my fault for using the not-well-tested DSA APIs
or something.
For integer attributes, that is. This never worked and the MeshGLTest
even had an explicit test case for this, but it took me until now to
realize that this would be best caught at compile time. So now it is.
But since this functionality dates back to 2010, just removing the
values would likely break a lot of code (well, broken code, but still),
so the disallowed values are now deprecated, giving a compile time
warning when used, and are removed only on non-deprecated builds.
This warning isn't enabled by default by either -Wall or -Wextra, and
for a good reason. When I do that, it fires also for all uses of
Platform::Application, basically any subclass causes that warning to be
printed as well, even if given subclass is used just once, in an
anonymous namespace, and only through the MAGNUM_APPLICATION_MAIN()
macro. The only solution on Clang is to make such subclasses `final`,
but on GCC not even that helps and one is forced to really make the base
destructor virtual. Which doesn't achieve anything, only adds yet
another entry to the vtable.
So, in the SceneGraph it fixes one particular use case where the warning
was triggered, and since the fix isn't so invasive I'm fine with that.
For Platform I have no idea how an acceptable fix would look like, so I
hope nobody needs any of that anytime soon.
See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102168, especially
the part saying "This warning should not be used.".
The views made it look like one cannot use Trade::ImageData etc. as
inputs, and the reason for using them was because there was a
load-bearing const in the view type that allowed slice() to work. The
slice() API is fixed now so this doesn't need to be silly anymore.
Now it's less code, and it also no longer results in random edge
artifacts because the padding area wasn't correctly uploaded. I'm going
to do the same change in the FreeTypeFont and StbTrueTypeFont next.
Because otherwise the users likely have to do something similar on their
end to perform a texture upload etc., which means they'll either take
the easy path and upload everything including the unused area, or they
introduce various bugs in the process, leading to random artifacts,
especially when it comes to padding.
Which is exactly what I think is causing random test failures in the Ui
library text rendering, because the glyph cache filling process in
plugins is calculating the rectangle too tight, without considering
padding. Gonna fix that now.
Vladimír Vondruš