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.
There's no reason for those to exist anymore -- origiinally they were
added in a hopeful attempt to make use of parallel shader compilation,
but in practice that meant compiling at most two or three shaders at
once and still stalling until that was done, so not that great at all.
The new APIs provide much better opportunities for parallelism.
Fun fact:
CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());
is actually one character shorter than
CORRADE_INTERNAL_ASSERT_OUTPUT(GL::Shader::compile({vert, frag}));
so not even typing convenience would be a reason to keep these.
It makes no difference in this case, the State struct is not exported
even without the attribute. OTOH, it would need to have an EXPORT
attribute if it was desired to be exported, for example like done with
nested classes in GL::Framebuffer.
Similar to the change done in Corrade, see the commit for details:
878624ac36
Wow, this is probably the most backwards-compatibility code I've ever
written. Can't wait until I can drop all that.
Eheh. But given that nobody complained for so long, it means nobody uses
those ancient versions anymore. Or nobody uses DebugTools, that's more
like it.
As was done with GL::Buffer::data() back in 2017 -- the size parameter
is way too error prone this way (is it bytes? or multiples of
sizeof(T)?), plus arrayCast() contains a few extra checks that would
have to be replicated here to have the same level of robustness, etc.,
etc.
While branching on a compiler is rather common, checking a particular
compiler version should be needed only rarely. Thus minimize use of such
macros to make them easier to grep for.
It limits the support for CMake 3.12+, but it's much less verbose and I
don't expect people to use ancient CMake versions with IDEs like Xcode
or VS anyway, so this should be fine.
Basically mirroring the changes done for Corrade::Compare::*File*,
including storing the filenames as string views to avoid unnecessary
copies (and then paying extra attention to not pass temporaries to them
in its own tests).
Otherwise unrelated error messages such as file read failures would get
included, making the tests fail. The Debug output on the other hand
isn't used for any internal reporting.
Vladimír Vondruš