Skip to content

FlipFaceCull corrupts shared RenderState for backward-normal geometry #2914

Description

@DieterDePaepe

(Created with help from AI)

com.jme3.material.Material.updateRenderState() flips a geometry's face cull mode
when the geometry's world scale has an odd number of negative components (mirrored
winding, so front/back are reversed):

} else if (techniqueDef.getRenderState() != null) {
    finalRenderState = mergedRenderState.copyFrom(RenderState.DEFAULT);
    finalRenderState = techniqueDef.getRenderState().copyMergedTo(additionalState, finalRenderState);
} else {
    finalRenderState = mergedRenderState.copyFrom(RenderState.DEFAULT);
    finalRenderState = RenderState.DEFAULT.copyMergedTo(additionalState, finalRenderState);
}
// test if the face cull mode should be flipped before render
if (finalRenderState.isFaceCullFlippable() && isNormalsBackward(geometry.getWorldScale())) {
    finalRenderState.flipFaceCull();
}

The flip is correct in intent, but it mutates the wrong object. RenderState.copyMergedTo
begins:

public RenderState copyMergedTo(RenderState additionalState, RenderState state) {
    if (additionalState == null) {
        return this;   // <-- returns the *shared* receiver, not `state`
    }
    ...
}

A Material's additionalState is null by default it is only created lazily,
the first time getAdditionalRenderState() is called. So for any material whose
additional render state has never been touched, copyMergedTo(null, merged) returns
this: either techniqueDef.getRenderState() (shared by every material using that
.j3md technique) or the RenderState.DEFAULT singleton. updateRenderState then
calls finalRenderState.flipFaceCull() on that shared object, permanently
flipping its cull mode.

Result: rendering one mirrored geometry flips front-face culling for every other
geometry that shares the technique's RenderState (which, for anything using
Unshaded.j3md, Lighting.j3md, etc. without an explicit additional render state,
is all of them). Every subsequent front-facing surface is culled away, and because
the shared object stays mutated, the corruption persists across frames until another
mirrored draw flips it back — so it presents as intermittent "geometry disappears /
turns inside-out" that's maddening to reproduce.

Reproduction

  • A scene where several geometries share one technique's RenderState (i.e. plain
    materials that never call getAdditionalRenderState()).
  • One geometry given a world transform with a single negative scale component
    (e.g. setLocalScale(-1, 1, 1)), mirroring it.
  • Render. The mirrored geometry itself looks correct, but every geometry drawn
    after it in the frame is culled on the wrong face — front faces vanish, the model
    reads as see-through / inside-out. Toggle the mirrored geometry off and the
    corruption may or may not clear depending on draw order.

The fix

Only ever flip a RenderState the material owns. If copyMergedTo handed back a
shared alias, copy it into the material's private mergedRenderState first:

if (finalRenderState.isFaceCullFlippable() && isNormalsBackward(geometry.getWorldScale())) {
    if (finalRenderState != mergedRenderState) {
        finalRenderState = mergedRenderState.copyFrom(finalRenderState);
    }
    finalRenderState.flipFaceCull();
}

Correct for all three branches: in the forced and non-null-additionalState paths
finalRenderState already is mergedRenderState (the guard is a no-op); in the
buggy null-additionalState paths it's the shared alias, so we copy before mutating
and the shared object is left untouched.

isNormalsBackward / flipFaceCull semantics are unchanged; only the object the
flip lands on changes. No public API change.

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

Status
tracked

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions