Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes from PoolVector to LocalVector and pre-reserving vectors rather than push_back.
Tests show more than factor of 3 speedup for merging 200,000 boxes.
Notes
LocalVector
is generally more efficient thanPoolVector
for single threaded operations.MeshInstance::merge_meshes()
function, it may be exposed further in 3.6 so it makes sense to make it as fast as possible, especially for use at runtime at level load, rather than pre-baking.Side issue - copy initialization in LocalVector
I noticed that both when assigning a
Vector
toLocalVector
and also for assigningPoolVector
, it seems to have to be done as two expression as follows, rather thanLocalVector blah = PoolVector(foo);
. I just copied the code for assigningVector
, but it would be nice if this can be solved:If you try it as a single expression there is a compile error.
EDIT: Solved the above, the problem was this needs copy initialization rather than assignment operator overload.
LocalVector
needed an operatorLocalVector::LocalVector(const PoolVector<T> &p_from)
. I've also added one forLocalVector::LocalVector(const Vector<T> &p_from)
as I had previously noted this expression didn't work. The PR is now changed to use this short form rather than the longer form assignment quoted above.