UInstancedStaticMeshComponent::RemoveInstance invalidates instance indexes

General Feedback & Requests
, , , , , ,
1

Branch: Launcher

Description:

Calling UInstancedStaticMeshComponent::RemoveInstance(int32 InstanceIndex) invalidates all indexes > removed index.

This would have been ok (not good but acceptable) if the description at least suggested this might happen. However, neither the description of AddInstance nor RemoveInstance provides any insight in why things get totally ■■■■■■ up after removing an instance.

RemoveInstance description:

/** Remove the instance specified. Returns True on success. */

AddInstance description:

** Add an instance to this component. Transform is given in local space of this component. */

RemoveInstance code:

bool UInstancedStaticMeshComponent::RemoveInstance(int32 InstanceIndex)
{
	if (!PerInstanceSMData.IsValidIndex(InstanceIndex))
	{
		return false;
	}

	// Request navigation update
	PartialNavigationUpdate(InstanceIndex);

	// remove instance

//==============================================================

PerInstanceSMData is a TArray; RemoveAt removes the index and shrinks the array

	PerInstanceSMData.RemoveAt(InstanceIndex);

//==============================================================

#if WITH_EDITOR
	// remove selection flag if array is filled in
	if (SelectedInstances.IsValidIndex(InstanceIndex))
	{
		SelectedInstances.RemoveAt(InstanceIndex);
	}
#endif

	// update the physics state
	if (bPhysicsStateCreated)
	{
		// TODO: it may be possible to instead just update the BodyInstanceIndex for all bodies after the removed instance. 
		ClearAllInstanceBodies();
		CreateAllInstanceBodies();
	}

	// Indicate we need to update render state to reflect changes
	ReleasePerInstanceRenderData();
	MarkRenderStateDirty();

	return true;
}

Invalidating the only way of tracking something you create is serious malpractice. I advise you to change that. If not, please update the description in the header file to properly portray that those are volatile.

2

Hey TheOriginalArkless-

I understand your issue however it is not something I’m familiar with myself. To help me test reproducing this on my end could you post the code class where you’re setting up your instanced static mesh and calling RemoveInstance() so that I can try to follow your setup?

Cheers

3

Reproduction steps:

UFUNCTION(Blueprintcallable, Category = "test")
void doStuff(UInstancedStaticMeshComponent * instancer) {
    int32 instance1 = instancer->AddInstance(someTransform);
    int32 instance2 = instancer->AddInstance(someTransform);
    //do whatever you want
    instancer->RemoveInstance(instance1)
    instancer->UpdateInstanceTransform(instance2, someOtherTransform, true, true);
}

instancer->UpdateInstanceTransform(instance2… does not change the instance that reaturned instance2 (the int) when created

4

From my understanding, when you remove the element at the specified index the array shrinks, so the element at “Index+1” moves to “Index”, and everything else rearranges accordingly. In this case, after removing instance1, the element that was stored in the location of instance2 would now be in the instance1 location. In the call to UpdateInstanceTransform() do you see the original instance2 element update if you replace “instance2” with “instance1”.

5

I have not investigated further but I’ll do so soonish.

6

Yep, works exactly that way (as expected - but i’m not into just stating things without testing).

Code used to test (could upload project if you really need it):

	UInstancedStaticMeshComponent * visualizationInstances;
	int32 instance1;
	int32 instance2;

	UFUNCTION(Blueprintcallable, Category = "test")
		void TickVisualization(float time) {
			FTransform old;
			visualizationInstances->GetInstanceTransform(instance1, old, true);
			FVector pos = old.GetTranslation() + FVector(10.f, 0.f, 0.f);
			old.SetTranslation(pos);
			visualizationInstances->UpdateInstanceTransform(instance1, old, true, true);
		};
	UFUNCTION(Blueprintcallable, category = "test")
		void setVisualization(UInstancedStaticMeshComponent * visualizer) {
			visualizationInstances = visualizer;
			instance1 = visualizer->AddInstance(FTransform(FVector(0, 0, 0)));
			instance2 = visualizer->AddInstance(FTransform(FVector(0, 0, 0)));
			visualizer->RemoveInstance(instance1);
		};
7

The documentation for TArray::RemoveAt mentions that the array will resize (if suitable) after removing the specified index. When you make the call to RemoveInstance and the subsequent call to RemoveAt is made, the array re-sizes to eliminate the gap (when index 2 is removed, index 3 moves into index 2’s location rather than leaving index 2 null or filled with a garbage value).

8

TArray does, UInstancedStaticMeshComponent doesn’t. I am not so into reading the code of everything I use. The UE source code is kinda long.

9

Since the code is working as intended I can submit a report to have the documentation updated to be more clear in how RemoveInstance() works.

10

Thank. This is a really urgent thing. Won’t help me anymore but will reduce the deveopment time of others a lot.

11

Did someone updated the documentation? Where can I find it? Thanks.

12

Hey marco-

The link to the documentation page for UInstancedStaticMeshComponent::RemoveInstance can be found at the following link. It does not appear that the page has been updated to mention the array shrinking after removing the instance yet. I have updated the original report for better visibility.

Here is the current documentation for RemoveInstance():

13

Can you please make it more clear in the documentation, that the instance_idx is an always continous index with no gaps and not a handle? That’s what I assumed, but wasn’t sure. Thank you.

14

Oh geez, this makes it a LOT harder to use instanced meshes for a large number of objects with their own lifetimes. Thanks for the thread, saved me a lot of time trying to figure out the problem – now I just have to figure out the solution.

This is certainly not an ideal implementation.

15

My personal suggestion is copying the code and changing the variable from TArray to your own implementation with fixed indices.
I would share mine but I went ahead and personalized so it won’t help you.

My suggestion would be index mapping. An easy to understand method is the following:

Instances
InstanceLinks
ExternalLinks

Instance[x] = some instance
InstanceLinks[x] = y (index for externalLinks)
ExternalLinks[y] = x (index for Instance)

remove x:
Instance[x] = Instance[Instance.lastIndex]
InstanceLinks[x] = InstanceLinks[Instance.lastIndex]
Instance.lastIndex--
ExternalLinks[InstanceLinks[x]] = x
16
1 Like
17

Any method for implementing this post UE 4.20?
I’ve been struggling with it for a while now.