Loops and Bubbles

June 19, 2018

We’ve just got a couple of small bug fixes here. Nothing major enough to warrant a full blog post individually – but still worth sorting out.

The Rogue For-Loop

Digging through some physics code while looking for optimisations to integrate into a client project we came across this in UInstancedStaticMeshComponent::OnDestroyPhysicsState().

int32 PSceneIndex = INDEX_NONE;  
for(const FBodyInstance* BI : InstanceBodies)  
{  
  if(BI)  
  {  
    if(BI->SceneIndexSync)  
    {  
      PSceneIndex = BI->SceneIndexSync;  
      break;  
    }  
    else if(BI->SceneIndexAsync)  
    {  
      PSceneIndex = BI->SceneIndexAsync;  
      break;  
    }  
  }  
}

Nothing wrong with that I hear you say! And you’d be right. That is until you’d looked at the whole function.

void UInstancedStaticMeshComponent::OnDestroyPhysicsState()
{
  int32 PSceneIndex = INDEX_NONE;
  for(const FBodyInstance* BI : InstanceBodies)
  {
    if(BI)
    {
      if(BI->SceneIndexSync)
      {
        PSceneIndex = BI->SceneIndexSync;
        break;
      }
      else if(BI->SceneIndexAsync)
      {
        PSceneIndex = BI->SceneIndexAsync;
        break;
      }
    }
  }

  USceneComponent::OnDestroyPhysicsState();

  // Release all physics representations
  ClearAllInstanceBodies();
}

Nope, that for loop is doing very little. Looking through the history of this file showed us that there used to be (in 4.15 and below) another block of code dealing with releasing aggregates after the call to ClearAllInstanceBodies() which used the PSceneIndex variable.

#if WITH_PHYSX

  if(PSceneIndex != INDEX_NONE)
  {
    PxScene* PScene = GetPhysXSceneFromIndex(PSceneIndex);
    SCOPED_SCENE_WRITE_LOCK(PScene);

    // releasing Aggregates, they shouldn't contain any Bodies now, because they are released above
    for (auto* Aggregate : Aggregates)
    {
      Aggregate->release();
    }
  }
  
  Aggregates.Empty();
#endif //WITH_PHYSX

But this had been removed by 4.16, obviously deletion of the loop used to find the value of PSceneIndex had been missed out by accident. Nothing super major there, but no need for pointless code either!

Badly Intersecting Comment Bubbles

The second change that we’re presenting is a minor quality of life improvement.

Currently the z-order of blueprint nodes and comment bubbles is a little mixed up. The image below shows what we mean. As you can see the z-order of the components is as follows: comment bubble -> node body -> comment title -> texture preview!

Fortunately this is easily fixed by some small modifications to SGraphPanel::OnPaint() in SGraphPanel.cpp.

First some const keywords need deleting from the declarations of NodeShadowsLayerId and NodeLayerId so we can alter the values if needed (~ line 125 in 4.19).

// Save a LayerId for wires, which appear below nodes but above comments
// We will draw them later, along with the arrows which appear above nodes.
const int32 WireLayerId = LayerId++;

int32 NodeShadowsLayerId = LayerId;
int32 NodeLayerId = NodeShadowsLayerId + 1;

Then a flag is added to identify if a comment node is being drawn (~ line 158 in 4.19).

// If a comment node, draw in the dedicated comment slots
bool bCommentNode = false; // <- Add this
{
  UObject* NodeObj = ChildNode->GetObjectBeingDisplayed();
  if (NodeObj && NodeObj->IsA(UEdGraphNode_Comment::StaticClass()))
  {
    ShadowLayerId = CommentNodeShadowLayerId;
    ChildLayerId = CommentNodeLayerId;
    bCommentNode = true; // <- And add this
  }
}

Finally we adjust the layer id values after drawing the node, but before drawing its overlay (~ line 236 in 4.19).

if (bNodeIsVisible)
{
  ...
  
  int32 CurWidgetsMaxLayerId;
  {
    ...
    // Draw the node.O
    CurWidgetsMaxLayerId = CurWidget.Widget->Paint(NewArgs, CurWidget.Geometry, MyCullingRect, OutDrawElements, ChildLayerId, NodeStyleToUse, !DisplayAsReadOnly.Get() && ShouldBeEnabled( bParentEnabled ) );
  }
  
  /* Add from here */
  // Make sure the main node is rendered above the comment node's title
  if (bCommentNode && CurWidgetsMaxLayerId > NodeShadowsLayerId)
  {
    NodeShadowsLayerId = CurWidgetsMaxLayerId;
    NodeLayerId = NodeShadowsLayerId + 1;
    MaxLayerId = NodeLayerId;
  }
  /* to here */
  
  // Draw the node's overlay, if it has one.
  ...
}

The result: nodes sit completely above the comment bubbles!

Credit(s): Rogue For-Loop: Josef Gluyas (Coconut Lizard)
Intersecting Comment Bubble: Yuchen Mei (Coconut Lizard)
Status: Currently unfixed in UE4.20
Github Pulls: Rogue For-Loop: github.com/EpicGames/UnrealEngine/pull/4676
Intersecting Comment Bubble: github.com/EpicGames/UnrealEngine/pull/4793

blueprints c++programming ue4