Whoops, this got silently omitted during the massive refactor in
f7a6d79aa0 (Nov 23). Buffers *do* get
destroyed, VAOs not. If you got sudden GPU memory usage issues after a
recent Magnum update, this was why.
Need to make a constexpr style data for the UI library and it involves
various multiplications and such, so took that as an opportunity to
enable constexpr on all operators. No other functions such as max() so
far, as I don't really need those yet.
After a few abandoned iterations that involved adding constexpr
overloads only to the Vector2, Vector3 and Vector4 subclasses I ended up
with a rather minimal solution that makes the base Vector constexpr
already, and just about 50 extra lines in total.
In the original code from 2010, to avoid redundant code, the const
operations were delegating to compound assignment operations, i.e.
operator*() being implemented by making a copy of itself and then
delegating to operator*=(). Thus, as far as a Debug build is concerned,
one extra indirection for each. The new solution is *also* one
indirection (which is needed in order to expand the variadic sequence)
so it's not worse in Debug in any way, however it's one indirection less
in the Vector2, Vector3 and Vector4 subclasses as there it delegates
directly to the internal implementation instead of the base class
operator. On GCC at least, there's no measurable impact on build times
either -- the whole project builds in ~2:22 both before and after this
change.
The way the change is done also allows the new code to be compiled out
if C++14 constexpr is enabled, where the functions would simply delegate
to the compound assignments. I'm not planning to touch that any time
soon either.
This complier is making my hair gray. Fortunately the out-of-class
operator doesn't conflict with the in-class one, so it's purely an
additive workaround. Adding extra checks to all subclasses to be sure
this works correctly in all cases and not just in the base class.
Except for a vector and a row matrix multiplication operator, which
doesn't make sense to be a member. These are all now significantly
shorter thanks to not having to repeat that many template parameters,
and they can make use of direct access into the _data array for better
debug perf.
In particular verify also the compound assignment operators and that
they correctly return a reference to self. And make sure the
non-mutating operators can be called on const instances.
The tests were mostly written back in 2010 and it shows. It survived all
that time because I didn't need any larger refactor of the math library
until now, but I'm going to make some changes and it'll be embarrassing
to introduce nasty regressions because the test coverage was lacking.
For some reason a lot of these got forgotten in
b2c353bf21. Also adding a comment
explaining the difference, because it's likely to stop being obvious few
months from now.
Also adjusting two tests which were calling rotation() on matrices that
actually didn't have a correct rotation part, and it only slipped
through because of the bug in isOrthogonal().
Co-authored-by: John Turner <7strbass@gmail.com>
Amazing, I haven't been in this corner of the codebase for over six
months but the test setup and code coverage still makes it quite
straightforward to add a rather complex new feature without breaking
unrelated cases or leaving certain areas completely untested.
To be used with the recently added MaterialTools::removeDuplicates() for
example, or internally by other upcoming scene tools such as importer
filtering.
Usage pattern is very similar to the duplicate removal utilities in
MeshTools, except that here the process is a simple O(n^2 m) operation,
without any hashing. Turned out to be good enough, in the initial tests
at least.
This "type erased std::vector member" was done in the times before
growable arrays were a thing, and kind of made sense to go the extra way
to avoid a <vector> include in the header. Except that it made rather
unportable assumptions about std::vector size, which weren't correct for
example with _GLIBXX_ASSERTIONS set.
But what was *completely* unacceptable was that the vector was of one or
another type depending on the GL feature set present in the current
context. Apart from adding a lot of extra *nasty* logic to construction,
moves and destruction, this approach led to the mesh instance asking the
current context on destruction in order to know whether a destructor
should be called on std::vector<Buffer> or std::vector<AttributeLayout>.
Ugh.
Now it's a regular Array member (which isn't *that* heavy to need such
type-erased treatment, although it eventually could be), and thanks to
the AttributeLayout packing improvements in previous commits it's no
longer prohibitively wasteful to just abuse AttributeLayout instances to
store just owning Buffer instances alone -- doing so now wastes only 16
bytes per buffer, compared to 36 before. Given there's usually just one
or two vertex buffers per mesh (compared to attributes, which are
usually 4 or more), it should be fine.
The MeshGLTest::destructMovedOutInstance() test added few commits back
also no longer asserts on no GL context being present.
Saves 8 bytes (104 -> 96 bytes), which were before wasted on 4
byte padding before the 8-byte _indexBufferOffset member and 4 bytes
after the _indexBuffer, which is new there due to the Buffer being 8
instead of 12 bytes now.
On ES2 there's three 32-bit members less, which means this change only
moved the 4-byte after _indexBuffer to after _indexType, i.e. 88 bytes
before and 88 now as well.
Which, thanks to a 3-byte padding being now just 1 byte, makes the
Buffer class 8 bytes large instead of 12. And in turn, the internal
Mesh::AttributeLayout struct is now 40 bytes instead of 48 as there's no
longer an extra 4 bytes of padding to satisfy 8-byte alignment of the
offset member. Still can go lower than that.
Want to construct them without a GL context present, and Optional is too
wasteful. Also adding it to the AbstractGlyphCache base, where it skips
also allocating the internal PIMPL state, because it's not going to get
used for anything in a NoCreate'd instance anyway.
The test added in previous commit now passes. Besides the behavior being
different for single- and multi-bind calls, an additional "interesting"
behavior is that the glBindBufferBase() / glBindBufferRange()
apparently also creates the GL object, if not already (in contrast to
the multi-bind APIs which *require* the GL objects to be created
before).
Vladimír Vondruš