Partial fix for Issue #2087 (NegativeScaleTest)

[codex128]

This PR partially fixes #2087 by "flipping" the face cull mode as the render state is applied to the renderer if the geometry world scale is found to be negative. This does not alter the normal vectors, so shaders still receive backwards normals and models still may not appear quite right (especially with normal maps), but this fixes the face culling issue, at least.

[stephengold]

Did you test these changes on the "NegativeScaleTest" model?

[codex128]

Oops, no, I didn't. Tested it out just now, and yes, the changes fixed the test model so there are no Xs anymore. I think it may have also fixed the shading on the spheres, but I couldn't tell for certain.

[stephengold]

Looks good to me.

[stephengold]

Hey @codex128 ! Thanks for your contribution to JMonkeyEngine.

[Trass3r]

According to git bisect this broke our rendering as soon as shadows are enabled.
There's an incorrect glDepthMask(false) for some of the rendered opaque geometry, I guess from the PreShadow material.
Can anyone confirm?

[codex128]

My first guess would be that RenderState#set is problematic, since my change uses set, while the previous code did not. Are you using a shadow filter or a shadow renderer?

[Trass3r]

Yeah from my debugging might be ForcedRenderState related.
Happens with both renderer and filter.

[codex128]

I've opened #2130 which should fix this issue.

[stephengold]

Thank you @codex128 for following up on issue #2129.

[JohnLKkk]

I just found a problem locally. This modification seems to have caused an exception with ShadowFilter (running TestDirectionalLightShadow.java results in a black screen). I found this issue at commitID=5975b3f791d18fae311a10e209b3ed40b5e5c60f.

[JohnLKkk]

private void updateRenderState(Geometry geometry, RenderManager renderManager, Renderer renderer, TechniqueDef techniqueDef) { if (renderManager.getForcedRenderState() != null) { mergedRenderState.set(renderManager.getForcedRenderState()); } else if (techniqueDef.getRenderState() != null) { // copyMergedTo writes to mergedRenderState techniqueDef.getRenderState().copyMergedTo(additionalState, mergedRenderState); } else { RenderState.DEFAULT.copyMergedTo(additionalState, mergedRenderState); } // test if the face cull mode should be flipped before render if (mergedRenderState.isFaceCullFlippable() && isNormalsBackward(geometry.getWorldScale())) { mergedRenderState.flipFaceCull(); } renderer.applyRenderState(mergedRenderState); }
It should be changed to:
private void updateRenderState(Geometry geometry, RenderManager renderManager, Renderer renderer, TechniqueDef techniqueDef) { if (renderManager.getForcedRenderState() != null) { mergedRenderState.set(renderManager.getForcedRenderState()); } else if (techniqueDef.getRenderState() != null) { // copyMergedTo writes to mergedRenderState mergedRenderState.set(techniqueDef.getRenderState().copyMergedTo(additionalState, mergedRenderState)); } else { mergedRenderState.set(RenderState.DEFAULT.copyMergedTo(additionalState, mergedRenderState)); } // test if the face cull mode should be flipped before render if (mergedRenderState.isFaceCullFlippable() && isNormalsBackward(geometry.getWorldScale())) { mergedRenderState.flipFaceCull(); } renderer.applyRenderState(mergedRenderState); }

[stephengold]

@JohnLKkk Thank you for reporting the issue! Two things:

1. When quoting multiple lines of code at GitHub, please use triple backticks so line breaks and indentation will be preserved, like so:

public static int main(String... args) {
    return 0;
}

2. Please open a new PR with your fix so it can be reviewed and integrated ASAP.

[codex128]

I believe this problem was fixed in #2130.
TestDirectionalLightShadow.java is working fine for me on that branch.

[JohnLKkk]

@JohnLKkk Thank you for reporting the issue! Two things:

1. When quoting multiple lines of code at GitHub, please use triple backticks so line breaks and indentation will be preserved, like so:

public static int main(String... args) {
    return 0;
}

2. Please open a new PR with your fix so it can be reviewed and integrated ASAP.

Thank you for your guidance.