Fix stencil testing remaining enabled after it is disabled

[toaster0123]

Problem

Turning stencil testing off can leave it enabled in OpenGL.

For example, enable stencil testing with Never for both faces, then apply a disabled RenderState with the same operations and functions. The second state should let fragments through, but GLRenderer skips glDisable(GL_STENCIL_TEST). On a framebuffer with stencil storage, the following draw can remain invisible.

The renderer compares the requested enable flag with context.stencilTest, but never updates that cached flag. It stays false after enabling the test. Reapplying an unchanged enabled state also repeats all stencil calls.

Fix

Keep the cached enable flag up to date. Also cache and compare both faces' reference values and comparison masks. Those values must remain part of change detection: fixing only the enable flag would otherwise stop reference-only and mask-only updates from reaching OpenGL.

Validation

• Added 12 focused tests. Three fail on unchanged upstream e8cf975be583a668d4121cdce334bfb0c5e75af0; all pass with the fix. Coverage includes disable/re-enable, unchanged states, front/back references and masks, operations/functions, and reapplying state after context invalidation.
• Native offscreen test on Mesa llvmpipe, OpenGL ES 3.2: the reproducer leaves a blue pixel on baseline because the requested red draw is still rejected. With the fix, the native stencil enable query becomes false and the pixel is red. Five related controls also pass. Each version performs 12 draws and 12 RGBA readbacks with zero GL errors.
• Clean :jme3-core:build and the desktop, effects, and plugins test suites pass. Core has 495 tests, with one pre-existing skipped test and no failures. Renderer Checkstyle tasks pass; the new test has no warnings. Existing source warnings remain.
• Whole-repository clean build was attempted but cannot run here because the Android screenshot project has no configured Android SDK.

The native test uses a test-only JNA bridge to system GLES. It validates these renderer decisions and observed software-driver pixels; it does not establish behavior on every GPU, production backend, or scene. General recovery from arbitrary external GL state is outside this change.

Supplemental native validation: standalone runner, pinned source hashes and sanitized native logs. This is separate from the Gradle tests in the PR.

[jaime-jmebot]

Quick check on the change-detection condition: after caching stencilTest, does it still start with state.isStencilTest() ||? If so, every applyRenderState with an enabled stencil state re-enters this block and re-issues glStencilFuncSeparate / glStencilOp / glStencilMask each frame, so the "repeats all stencil calls" half of the bug described in the PR would still be there.

Using the cached flag as a != comparison (or dropping the raw state.isStencilTest() term entirely) would cover both directions — enable and disable — and make the "unchanged state" path meaningful.

if (state.isStencilTest() != context.stencilTest

• One thought: since the cached flag is now maintained, the bare state.isStencilTest() || term is redundant and it defeats the unchanged-state optimization. Could you confirm whether it was replaced by a != comparison?

[toaster0123]

The published condition already starts with context.stencilTest != state.isStencilTest(); there is no raw state.isStencilTest() || term. The cache is then assigned the requested flag inside the block. See the condition and assignment at fa5dad4. Both enable and disable transitions are therefore detected, while an unchanged enabled state can skip the block.

[jaime-jmebot]

You're right, and thanks for the correction — my earlier comment was based on a truncated hunk that started mid-condition, so I inferred a leading state.isStencilTest() || term that isn't there. The diff shows the condition opening with context.stencilTest != state.isStencilTest(), refs/masks comparisons appended, and the cached flag assigned inside the block, which matches what you describe. Both transitions are detected and an unchanged enabled state skips the block.

With that dropped, the only things still open from my side are the two smaller notes: asserting stencil call counts in the unchanged-state test, and a pointer to where the native JNA/GLES validation lives. Neither blocks this fix.

[jaime-jmebot]

Nice and focused — mocking glEnable / glDisable / glStencilFuncSeparate / glStencilOp / glStencilMask is the right level for this.

One suggestion for hardening: in the "reapply an unchanged enabled state" test, also assert the count of stencil calls on the mock GL (or that no further stencil calls were recorded after the first apply). Right now a regression that re-issues every stencil call each frame could still pass if the test only checks that no glDisable happened.

• Also: the native JNA/GLES offscreen test described under Validation isn't part of this diff. Will it land in a follow-up, or should the PR description point to where it lives?

[toaster0123]

The unchanged-enabled test already clears the mock's history after the first apply, then uses verifyNoInteractions(gl) after the second. That requires zero additional GL calls of any kind, including every stencil function/operation call: test lines 70–78.

The supplemental native harness is now public on the fork's validation-only branch: runner, readable Java, pinned hashes and native logs. It was rerun from those public files against the exact baseline and PR heads. The README explains the failing-baseline expectations, installed dependencies and software-driver limits. It is separate from the Gradle regression suite in this focused PR.

[jaime-jmebot]

Both points check out now:

• clearInvocations(gl) + verifyNoInteractions(gl) on the unchanged-enabled test is exactly the call-count assertion I was after — stricter, actually, since it catches any redundant GL call, not just the stencil ones.
• Good call on making the native harness public on the validation-only branch. That makes the supplemental evidence checkable without relying on private artifacts.

No further concerns from me. Approving. 👍

[jaime-jmebot]

Nice catch on the stale context.stencilTest flag — caching the enable state plus both faces' refs and masks is the right shape, and the reasoning that refs/masks must stay in change detection is well spelled out.

A few small things:

• Please confirm the change-detection condition now compares the cached flag with != rather than still starting with the raw state.isStencilTest() ||; otherwise every enabled state re-issues the full stencil sequence each frame.
• The new tests could assert stencil call counts for the unchanged-state case, so a regression that re-issues the calls is caught too.
• Cached mask defaults of -1 match GL's all-ones default, so no spurious first-frame calls — nice.
• The native JNA/GLES validation isn't in this diff; it'd be good to note where it lives before merge.

Otherwise this looks clean and well tested. 👍

[jaime-jmebot]

I checked the actual diff and confirmed the author is correct — the change-detection condition starts with context.stencilTest != state.isStencilTest(), not the raw state.isStencilTest() || term I originally flagged. My earlier comment was based on a truncated hunk, so I retracted that concern in the existing thread and left only the two smaller non-blocking notes standing.

[jaime-jmebot]

Both of my earlier notes are now resolved — the unchanged-state test asserts zero GL interactions (stricter than call counts), and the native validation harness is public and reproducible from pinned hashes.

• The root cause fix is right: keeping context.stencilTest in sync and comparing it with != means both enable and disable transitions reach GL, and refs/masks stay in change detection so reference-only and mask-only updates still propagate.
• Defaults in RenderContext.init() (0 refs, -1 masks) match GL's state, so no spurious calls on the first frame.
• The test suite covers the disable/re-enable, unchanged-state, per-face ref/mask, ops/func, and post-invalidateState() paths. Clean build and core tests pass.

Thanks for the careful write-up and the extra validation work. Nice catch on the stale flag. 👍