And use static functions with an explicit "self" pointer instead. Those
have half the size (8 vs 16 bytes on 64bit x86), which in turn reduces
the state tracker memory use by about 750 bytes. On desktop GL with an
Intel GPU & Mesa this reduces the state tracker allocation size by almot
10%, from 8.3 kB to 7.6 kB. Not bad.
Apart from small memory savings, this also removes the need to include
the full class definiton from the State headers on MSVC (because
on that compiler the member function pointer size is different based on
whether the type definition is known or not, IMAGINE THAT BEING A
FEATURE AND NOT A BUG), leading to less header dependencies and better
incremental compile times there.
This was already done in some cases (and the Vk library used this from
the beginning), and as I'm about to add some more extension-dependent
functionality it felt like a good time to finish that change, finally.
In some cases the *Implementation() could even be dropped in favor of
pointing to the GL API directly (such as is already done for various
glUniform*() calls), that'd be another step -- this is good enough for
now.
And this, this change allows the growable Array to use malloc() instead
of new, and thus also realloc(), saving unnecessary reallocations if the
memory can be grown in-place. All because Containers::Pair is trivially
copyable while std::pair wasn't.
There isn't any good reason to use the STL anymore.
Hm, and here I used the "capability" of std::pair that allowed it to
store references. I don't even want to know what all was involved to
support that, Containers::Reference is much easier to reason about.
Apparently even `= {}` was broken for std::pair once, not to mention the
unnecessary extra overhead with this type not being trivially copyable.
Good riddance.
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>
Most of these are just inputs, so a compatibility StringStl.h include
will do, the only exception is the callback for which there needs to
stay a deprecated overload (which is internally delegated from the
StringView one).
Also explicitly testing with non-null-terminated strings -- the APIs
take an explicit size so it shouldn't be a problem, but it's always good
to have this verified independently. Drivers are crap, you know.
One consequence of no longer using an impossible-to-forward-declare
std::string is that I had to deinline the DebugGroup constructor because
it no longer worked with just a forward-declared StringView.
The previous callback pointer was needed just to disambiguate, so it can
be a bool; the user pointer can be set from the caller already instead
of being done in each variant again.
Because it somewhat confusingly may have implied that it's really
composed of 8-bit bools, and not bits. The same reasoning was used to
pick the name for Corrade's Containers::BitArray.
Backwards compatibility aliases are in place as usual, however the
internal BoolVectorConverter is now BitVectorConverter and there
unfortunately cannot be any backwards compatibility. This breaks only
GLM and Eigen integration in the magnum-integration repo, which I'm
fixing immediately. I don't expect any user code to use this internal
helper. For regular vectors maybe, for this one definitely not.
Since glGetBufferSubData() is only exposed on Emscripten 2.0.17 and up,
both functions are not available on older versions. This is to avoid any
accidental foot guns since it explodes at compile time. The webgl2 CI
will be upgraded to 2.0.17 in a later commit.
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.
Using a tuple was very useful, helpful and exciting as I had to make an
explicit comment about what element is what, and then having to remember
the order in all places that accessed the tuple.
Using a struct however is rather boring as the fields are just named
there, I don't need any complex std::get<4>() and extra comments
explaining what is where and it's just not so adventurous anymore. The
build time of a non-deprecated MagnumGL library also dropped from 5.9
seconds to 5.85. SAD!
All the tests were updated to explicitly check that non-null-terminated
strings get handled properly (the GL label APIs have an explicit size,
so it *should*, but just in case). Also, because various subclasses
override the setter to return correct type for method chaining and the
override has to be deinlined to avoid relying on a StringView include,
the tests are now explicitly done for each leaf class, instead of the
subclass
The <string> being removed from the base class for all GL objects may
affect downstream projects which relied on it being included. In case of
Magnum, the breakages were already fixed in the previous commit.
Compile time improvement for the MagnumGL library alone is 0.2 second or
4% (6.1 seconds before, 5.9 after). Not bad, given that there's three
more files to compile and strings are still heavily used in other GL
debug output APIs and all shader stuff. For a build of just the GL
library and all tests, it goes down from 28.9 seconds to 28.1. Most
tests also still rely quite heavily on std::stringstream for debug
output testing, so the numbers still could go further.
This used to somehow get dragged along with <string> on all platforms we
tested on, but once we get rid of <string> we'd have to include
<algorithm> instead. Not acceptable, also it's much shorter and less
error-prone this way.
It's quite easy if WEBGL_debug_renderer_info is present, but if it isn't
then our only hope is to check for ANGLE-specific extensions. Which may
cause some false positives, but since we need to enable various
workarounds for ANGLE to avoid issues, it's better than having the
detection too conservative.
The original ANGLE detection based on line size is probably not so
relevant anymore, but let's keep it there.
Was browsing the extension registry looking for something else and found
this instead. It used to be ES catching up with desktop, now it's the
other way around.
Because its usage is rather specific, there wasn't no
extension-dependent dispatch happening and thus the extension was never
marked as used. But that was confusing since it actually is, so add an
explicit branch just to list the extension.
It was used just for certain corner cases that weren't covered by
{EXT,OES}_draw_elements_base_vertex already, but because of a wrongly
understood extension interaction the EXT/OES multidraw entrypoint (which
isn't implemented on ANGLE) was used from these as well.
And while trying to disable the two extensions for the MeshGLTest I
discovered the test half-expects the ANGLE variant to be implemented and
so I finished it for both the single-draw and multi-draw case.
The extension interaction that makes base-vertex multidraw calls broken
on ANGLE will be fixed in the following commit.
Yes, this is only ever provided by an ANGLE extension, and as you might
have guessed, The Great Google Overlords didn't bother caring to have
this supported outside of their browser.
Since the main point of the low-level API is to make use of the
EXT_multi_draw_arrays / ANGLE_multi_draw / WEBGL_multi_draw extensions
for a reduced driver overhead *and* the builtin gl_DrawID variable, it
just doesn't make sense to provide a fallback to draw() in a loop. When
the extensions are not available, the user has to do extra work in order
to supply proper UBO draw offset for each draw call, and thus the
fallback path has basically no practical use.
On the other hand, draw() taking the MeshView instances is kept as-is --
this one is rather old and so breaking compatibility would be an
unnecessary pain point. Plus, since it has to allocate the contiguous
arrays internally, it's not desirable for any fast-path rendering
anyway.
Unlike most other extensions, this one has to be explicitly enabled in
Emscripten in order to be used. Which thus done as part of other "driver
workarounds" done on startup. To avoid that, the extension can be
explicitly disabled, and thanks to the previous commit the disabling
will be performed before the extension is attempted to be enabled.
Because a MeshView might not be the best thing to have when you are
submitting a batch of thousand draws. It takes a strided array views to
allow for more flexibility, but can also detect if the input is already
contiguous and use it as-is.
UNFORTUNATELY the GL 1.0 legacy still continues to stink and so there
has to be a 64-bit-specific overload which is the *actual* variant that
doesn't allocate because glMultiDrawElements takes a `void**` for INDEX
OFFSETS and it's IN BYTES! Which foolish soul designed such a thing back
in the 1860s, I wonder. There's no reason to not have an index offset
in elements because all indices have to have the same type ANYWAY. And
yes, I wasted about three hours debugging driver crashes because I
THOUGHT this parameter takes offset in elements, not bytes.
Also note: on 32-bit platforms this depends on latest Corrade with the
CORRADE_TARGET_32BIT definition. Spent an embarrassing amount of time
wondering why all local builds but Emscripten work.
Vladimír Vondruš