BatchedMesh: `deleteGeometry` - how to re-activate?

[marwie]

Description

When I call deleteGeometry the instance is marked as inactive
But how can I use this already allocated space in the buffers again?

• addGeometry will not re-use available space ad the moment
• calling setGeometry will not activate the instance again

Solution

Keep track of freed space / search all inactive holes and try to re-use it when calling addGeometry

Alternatives

• User code keeps track of freed space and setGeometry will also set active[index] true
• setVisibleAt also sets active to true again
• add setActiveAt + an API to query available space at index x (could be a generator method or some mechanism to prevent accidental mutation of internal state)

Additional context

No response

[gkjohnson]

This was mentioned in #27897 but if you want to show the geometry again why are you using deleteGeometry? In the future the "active" state should be used in the "optimize" function to identify and compress sections of the geometry buffers that are no longer in use.

I know I mention in the other issue that it might be okay if it's set to true but realistically if the geometry has been "optimized" it will not be possible to set the geometry visible again and the given Id should be considered inaccessible if the geometry has been "deleted". In the long run applications should not be using "delete" if they want to use the geometry id again.

[marwie]

But couldn't the state active=false still be used for optimization if/when setGeomtry sets state to true again if we agree on that setGeometry effectively communicates an intention of this instance being active / the instance being actively used?
I mean: what's the reason for not being able to re-activate an instance? If

Right now I think due to the lack of a way to optimize the buffers maybe there shouldnt be no deleteGeometry call at all (or I shouldn't use it at all) since it does just punch holes in the buffers that can not be re-used - instead I should just change it to setVisible(false) and keep track of "available space" (I already do that by now)

Regarding optimization a bit offtopic maybe:
I'd also be curious how to deal with changed IDs? Would addGeometry return an handle/id that can still be used after optimization (because BatchedMesh has a mapping internally of geometryId <> index) ?

[gkjohnson]

But couldn't the state active=false still be used for optimization if/when setGeomtry sets state to true again

It's no more optimized than just setting visible to false. Again, I'm still not understanding why you're using deleteGeometry instead of setVisible in this case if you need to re-enable it.

if we agree on that setGeometry effectively communicates an intention of this instance being active / the instance being actively used?

Once the geometry has been "compressed" (ie the active geometry shifted to take the space used by the inactive geometry) then no, that id will not be able to be used anymore and it's likely the functions should throw an error if a now-deleted id is used. It would be confusing to differentiate between deleted-but-still-present and deleted-forever ids.

Right now I think due to the lack of a way to optimize the buffers maybe there shouldnt be no deleteGeometry call at all

I agree that the function doesn't have a lot of value over setVisibleAt, at the moment, but we shouldn't encourage people to use it in an unintended way. If anything the function should just be commented out and removed from the API until someone implements an "optimize" function.

I'd also be curious how to deal with changed IDs? Would addGeometry return an handle/id that can still be used after optimization (because BatchedMesh has a mapping internally of geometryId <> index) ?

Yes. Part of the "optimize" step needs to involve remapping ids to the updated geometry indices and fixes the triangle and vertex ranges.

[marwie]

I have both cases where instances can be deleted or just be hidden.
I'll change my code to just use visible for now - no problem.

Once the geometry has been "compressed" (ie the active geometry shifted to take the space used by the inactive geometry) then no, that id will not be able to be used anymore and it's likely the functions should throw an error if a now-deleted id is used. It would be confusing to differentiate between deleted-but-still-present and deleted-forever ids.

How am I then able to get the ids/indices for already added instances if the previously returned instance id is now invalid? I'm talking about cases where we have active [true, true, false, true] and addGeometry returned ids for [0, 1, 2, 3] - when I now call optimize the array becomes [true, true, true] and suddenly ID 3 is invalid even tho it wasn't deleted (?) - am I understanding this right?

But perhaps the addGeometry would in this case not directly return the index in the internal array but another identifier that stays valid.

If anything the function should just be commented out and removed from the API until someone implements an "optimize" function.

That was what I meant with maybe there shouldnt be no deleteGeometry call at all since it cannot be used in a meaningful way right now.

[gkjohnson]

But perhaps the addGeometry would in this case not directly return the index in the internal array but another identifier that stays valid.

This is what I intended above. If Id 2 is deleted, then geometry 3 is shifted to the left into position 2, and id 3 now refers to the geometry in the position that was for id 2. Ids for undeleted geometry should not become invalid. This mapping needs to be retained internally:

Yes. Part of the "optimize" step needs to involve remapping ids to the updated geometry indices and fixes the triangle and vertex ranges.

[marwie]

Ok great - that's what I hoped 😊

So i guess in conclusion for this issue:

I change my code to use setVisible
Should I update the PR to comment out deleteGeometry instead?

[gkjohnson]

Should I update the PR to comment out deleteGeometry instead?

I'm fine with this - it should be removed from the docs, too, which may be the more important change.