One less executable to build, and we need to test more variants. The
original measured thing ("when to remove duplicates") is no longer really
relevant.
Interestingly enough the fuzzy variant isn't that much slower.
Something fishy going on in there, caused by the algorithm overwriting
the key values (and the map relying on them being immutable).
Interestingly enough the fuzzy variants works on GCC's libstdc++ even
though the key data get changed every entry, fails only on libc++.
Basically using the same idea as with the discrete version -- having the
second dimension dynamic, together with restricting the implementation to
just Float and Double.
According to the SubdivideRemoveDuplicatesBenchmark, this makes the
implementation slightly slower. I presume this is due to how minmax and
offsets are calculated which is quite cache-inefficient as it goes over
the same memory block multiple times. Added a TODO for later.
I spotted a potential bug -- and it clearly *is* a bug. The test doesn't
reduce the data size in the last dimension, leaving a duplicate (and
unused) item at the end.
I need a variant of removeDuplicatesFuzzyInPlace() that doesn't allocate
the output array but instead puts the data into a pre-existing location.
The discrete / non-fuzzy variant had this already, but this one not.
A breaking change, sorry, but I don't want to add yet another layer of
backwards compatibility on APIs that are in master for just a month or
so. This is a test for how many people actually use these APIs -- if
nobody complains, great!
Conflating the fuzzy operation with the discrete one wasn't a good idea,
as people could be unintentionally using the (slower) fuzzy variant on
data that could be easily deduplicated using the discrete variant. One
such case is in the icosphere primitive, and I'm going to look at that
right now.
A bit unfortunate that the test needs ES3.2 and GS, but I got nothing
better right now. Not handling ObjectId yet, for that I need to
implement instancing first (so yes, GCC/Clang will still warn about an
unhandled switch case).
Until now, except for an attribute-less index-less mesh, the vertex
count was only implicitly taken from passed attributes, but it was
severely limiting:
- There was no way to set vertex count for an attribute-less
indexed mesh, which didn't make sense
- All code that made non-owning MeshData instances referencing another
MeshData had to explicitly handle the attribute-less corner case to
avoid vertex count getting lost
- Offset-only attributes couldn't be used to specify static layout of
meshes with dynamic vertex count, causing unnecessary extra
allocations especially in the Primitives library.
The combineIndexArrays() and combineIndexedArrays() API is replaced with
a more generic combineIndexedAttributes(), and thanks to that we also
don't need STL-based duplicate() and removeDuplicates().
Originally this was done in order to make handling of deserialized data
much simpler (as for those attributes also need to only contain an
offset into some unknown data array), but seems this could be very
useful elsewhere as well -- for example when the layout is known
beforehand but the actual data not yet -- such as in the Line and
Gradient primitives (going to switch them to this in the next commit).
What still unfortunately has to be known in advance is the actual vertex
count (as supplying it directly to MeshData would mean adding 6 new
constructor overloads, and there's enough of those already). Might
revisit later.