Interesting, the shader reinterpret code was added in 2016 but nothing
mentions *why* it was needed, what driver actually needs that. In 2025
all drivers I tested with are happy just with a passthrough.
Which means, this reinterpret is still only ever done in a single
overload of these APIs, reading to a buffer or reading a cube map
doesn't have this code path.
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".
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.
These were removed in e7aeaf78d0 for the
2020.06 release, but somehow these declarations, deprecated back in
2019.10, were still left there. Since they were useless anyway, I don't
even list this in the changelog.
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.
Otherwise it may happen that the clock gets adjusted mid-frame, leading
to an underflow and an assertion like this:
Assertion _measurements[i]._movingSum + data >= _measurements[i]._movingSum failed at src/Magnum/DebugTools/FrameProfiler.cpp:233
The clock adjustment is known to be happening rather frequently under
WSL2.
Ugh. Was using this to verify that the glyph cache was correctly
populated, only to end up with a GL error that I thought was coming from
the glyph cache itself and not here. Wasted too much time on that.
An ad-hoc solution was already done in DebugTools::screenshot(), now I
need it in another place. While not as fast as the O(1) mapping from
the generic format to the API-specific ones due to the potentially
linear lookup, it definitely could be useful in general.
Using Containers::Pair allows me to make certain Range APIs constexpr
that weren't possible in C++11 before. Compared to std::pair it's also
trivially copyable, which is a nice property when storing it in various
growable containers.
As usual, the <Corrade/Containers/PairStl.h> include is in place to help
people with porting, although in many cases this change will be
breaking. I had to do it at some point anyway, so the earlier it is the
better.
Not that C++ STL and exceptions would be anything to take inspiration
from, but there's std::out_of_range. Python IndexError is also specified
as "index out of range", not "bounds".
Partially needed to avoid build breakages because Corrade itself
switched as well, partially because a cleanup is always good. Done
except for (STL-heavy) code that's deprecated or SceneGraph-related APIs
that are still quite full of STL as well.
Originally it was just assuming that any Vector3ub or Color3ub is a
normalized format. That was kinda enough for many cases, but it started
to get annoying with sRGB image comparisons, as those had to be
manually reinterpret with a sRGB-less format in order to pass.
Now the pixel format detection looks at the expected image format as
well, and if the underlying type and component count matches, it
inherits the sRGB and normalized property from it as well. If not, it
falls back to an integer format for vectors, and normalized format for
colors. For vectors this is different from previous behavior but
shouldn't cause any problem in practice -- the only result will be that
the image comparison fails with a different message for pixel format
mismatch than before.
This now also properly and fully tests the pixelFormatFor() helper, and
adds a missing Color3 specialization of it.
Integer / packed formats are the majority of uses for this utility, and
for them the additional complexity with NaN and infinity handling isn't
needed at all.
Running ShadersMeshVisualizerGLTest with this change in a Debug build
led to its runtime being reduced by about 35%, the total test time
excluding benchmarks went from 14.5 seconds to 11. Not bad.
Mainly important for Shader::addSource() to prevent it from creating a
needless copy, but doesn't hurt to do the same also for
uniformLocation(), bindAttributeLocation() etc. -- it'll avoid a runtime
strlen() in that case at least.
Same as in the previous commit, most cases are inputs so a StringStl.h
compatibility include will do, the only breaking change is
GL::Shader::sources() which now returns a StringIterable instead of a
std::vector<std::string> (ew).
Awesome about this whole thing is that The Shader API now allows
creating a shader from sources coming either from string view literals
or Utility::Resource completely without having to allocate any strings
internally, because all those can be just non-owning references wrapped
with String::nullTerminatedGlobalView(). The only parts which aren't
references are the #line markers, but (especially on 64bit) those can
easily fit into the 22-byte (or 10-byte on 32bit) SSO storage.
Also, various Shader constructors and assignment operators had to be
deinlined in order to avoid having to include the String header, which
would be needed for Array destruction during a move.
Co-authored-by: Hugo Amiard <hugo.amiard@wonderlandengine.com>
The memory wasn't accessed in that case so everything is fine, but this
started tripping up the new assertions in ArrayView element access.
Fixed by moving (and thus duplicating) the access to where it is
actually needed.
Also removed the outdated comment -- there's no _currentData anymore
anywhere, not sure what was that meant to be.
Those were temporarily added until Trade::AbstractImporter is ported
away from std::string, and that's done for quite some time already, so
these are no longer needed.
Vladimír Vondruš