Restore texture and framebuffer bindings after invalidation

[toaster0123]

Problem

After another OpenGL renderer finishes, an application can call invalidateState() as documented and still get the wrong texture or render target on the next jME operation.

Two concrete cases:

• External code leaves texture unit 3 active. After invalidation, setTexture(0, greenTexture) binds the green texture on unit 3, while a shader sampling unit 0 still sees its old red texture
• External code leaves another framebuffer bound. After invalidation, setFrameBuffer(null) returns without selecting the main framebuffer, so the following clear and draw affect the external target. This also happens when the platform's main framebuffer has a nonzero name

The cache was reset to unit 0 and a null/default framebuffer without restoring those native bindings. Matching those assumed values was then treated as a reason to skip the required GL calls.

Fix

Keep unknown binding knowledge in RenderContext:

• Mark the active texture unit unknown, so the next texture operation activates its requested unit
• Track framebuffer-binding validity separately from the cached name/object, and only suppress a bind when validity, Java association and the expected native name all match
• Preserve fresh-context defaults and repeated-call elision. Selecting the screen still works without framebuffer-object support

The other consumers of this cache are handled too. A copy made while the binding is unknown snapshots native read/draw targets before attachment updates, restores them separately, and leaves the cache unknown. Known copies keep their existing query-free path. Known framebuffer metadata is recorded together; framebuffer-only invalidation preserves texture knowledge. Override, deletion and immediate main-target configuration decisions do not trust an unknown cached value or mistake an unrelated native target for the main framebuffer.

Validation

• The original 14-case binding model fails six cases and passes eight controls on upstream and the exact published heads of Fix render-target mipmap storage reuse and GLES3 half-float filtering #2983, Fix stale framebuffer attachments, leaked renderbuffers, and nonzero-default copies #2984 and Reapply cached render state after external OpenGL changes #2993. The unchanged model passes all 14 with this fix
• The committed regression suite has 43 renderer cases plus six RenderContext lifecycle/validity contracts
• Native Mesa GLES and desktop tests reproduce the wrong sampled color and wrong clear/draw target before the fix. Final standalone and composed runs pass 132 phases, 1,602 assertions, 128 draws and 556 pixel readbacks with zero GL errors
• Controls cover fresh defaults, uploaded/new textures, parameter updates, repeated invalidation, unchanged repeats, explicit targets, zero/nonzero presentation targets, overrides, and copies with separate read/draw bindings and attachment updates
• Core/desktop/LWJGL3 builds, effects/plugins checks and core Javadoc pass: 610 reported tests, zero failures/errors, two existing skips. Added-line Checkstyle reports no new warnings
• The explicit composition passes 662 core tests with zero failures/errors and one existing skip
• Native controls also verify retained texture knowledge through copies and known deletion followed by main-target selection, for zero and nonzero presentation targets
• Independent source review found no remaining blockers

This branch is based directly on current upstream. A separate tested composition includes #2983 (e7c87ee), #2984 (f1646c1) and #2993 (4c89905); it preserves their storage, lifecycle, nonzero-default and render-state changes. Composition requires retaining context.invalidate() followed by invalidateBindings(), combining the validity guards with #2984's default-target handling, and keeping #2983's mipmap handling inside #2984's attachment-update logic.

Scope and limits

On standalone nonzero-default initialization, the stricter native-name check can add one conservative initial bind because the baseline cache still starts at zero. Repeated selections remain elided. The #2984 composition already synchronizes that initial native name.

This fixes binding selection, not external texture/attachment mutation or every OpenGL state category. General sRGB-state invalidation and exception-time copy restoration are outside this patch. The copy tests verify safe use of unknown bindings; the public API does not promise preservation of arbitrary external state.

Native evidence uses Mesa llvmpipe through a test-only JNA bridge, not every GPU or a production backend. The whole-repository local build is blocked by the missing Android SDK. The completed scoped checks above do not claim that whole-repository build passed.

[jaime-jmebot]

Nice work — the diagnosis is right: resetting the cache to "unit 0 / main framebuffer" without restoring the native binding is exactly the kind of thing that bites users after they embed another GL renderer. Moving "we don't know" into RenderContext is the correct shape for the fix, and the fresh-default / repeated-call controls are a good guard against regressing into redundant binds.

A few things I'd like tightened before this lands:

1. invalidateBindings() clears the binding knowledge but leaves boundFBO stale

boundTextureUnit = -1;
frameBufferBindingValid = false;
boundFB = null;          // cleared
// boundFBO keeps the old FBO name

boundFB/boundFBO are public fields, so a consumer that forgets isFrameBufferBindingValid() sees "no framebuffer object bound, but FBO 12" — a combination that can't happen natively. Since the validity flag alone is enough to force the next real bind, I'd reset it here too:

boundFBO = 0;

That means RenderContextBindingTest.invalidateBindingsLeavesFramebufferNameUnchanged should flip to asserting 0 — fine, it was only pinning the current behaviour.

2. The new public setter is a footgun of the same kind this PR fixes

setFrameBufferBindingValid(true) lets anyone declare the cache valid without an actual glBindFramebuffer, which re-creates the original bug from the other side. A single call that records all three values at once can't drift:

public void setFrameBufferBinding(FrameBuffer fb, int fbo) {
    this.boundFB = fb;
    this.boundFBO = fbo;
    this.frameBufferBindingValid = true;
}

Symmetry point: boundTextureUnit = -1 is an undocumented sentinel with no isTextureUnitBindingValid() counterpart, so nothing stops a future GL_TEXTURE0 + context.boundTextureUnit from producing an invalid enum. A small accessor (or at least a comment on the field) would close that.

3. Quick check: does reset() clear the flag too?

Only init() appears to set frameBufferBindingValid = true in this diff. copyContext starts by resetting the destination context, so a stale true alongside boundFB == null would be exactly the broken state — worth confirming (or adding the reset there).

4. sRGB follow-up, understood as out of scope

With an unknown binding, both toggles in setMainFrameBufferSrgb are now skipped, which is the safe direction. The open question is the other end: when jME later re-binds the main framebuffer, does anything re-apply the cached mainFrameBufferSrgb, or does the native state stay stale until the app toggles sRGB again? No blocker here — just worth a tracked issue if the answer is "nothing re-applies it".

5. Small test nit that would have real value

The recording driver already guards against bogus framebuffer IDs:

assertTrue(framebuffer >= 0, "Unknown framebuffer ID passed to GL");

Adding the same check for the active unit closes the symmetric hole and turns a leaked -1 into a failing test instead of a silently recorded activeUnit = -1:

if (name.equals("glActiveTexture")) {
    int unit = (Integer) args[0] - GL.GL_TEXTURE0;
    assertTrue(unit >= 0, "Unknown texture unit passed to GL");
    activeUnit = unit;
}

Overall the approach looks right and the control set (fresh defaults, repeated invalidation, explicit targets, nonzero presentation target, copies with separate read/draw bindings) is solid.

[jaime-jmebot]

• The root cause and the direction of the fix look right: carrying "unknown" binding knowledge in RenderContext and only eliding a bind when the cache is genuinely valid is the clean way to solve it.
• Fresh-context defaults and repeated-call elision are preserved, and the control set around them (explicit targets, nonzero presentation target, copies with separate read/draw bindings) gives good confidence there's no extra-bind regression.
• Small hygiene items before merge: clear boundFBO alongside boundFB in invalidateBindings(), consider replacing setFrameBufferBindingValid(boolean) with a single call that records framebuffer + name + validity together, and confirm reset() clears the validity flag too. Details and a suggested test guard are in my comment above.
• sRGB after an unknown binding is intentionally left out of scope — just want the "is the cached mainFrameBufferSrgb re-applied on the next main-target bind?" question tracked.

[toaster0123]

@jaime-jmebot Addressed the recording-helper and test suggestions in 809b988.

• Known bindings now use one method to record the Java association, native name and validity together. A separate framebuffer-only invalidator preserves texture knowledge during copy restoration. Tests also cover known deletion followed by null selection with a nonzero presentation target; the shortcut now checks the expected native name as well as object identity.
• I kept boundFBO as an explicitly untrusted value while invalid. Null boundFB means no Java association, not necessarily native name zero: a nonzero presentation or externally managed FBO can legitimately have that shape. Resetting the number to zero would not make it safe for a consumer to ignore validity. The field/method docs and tests now make this distinction explicit.
• reset() calls init() and deliberately restores known fresh-context defaults, including valid=true. External invalidation then marks binding knowledge unknown. The reset contract is tested; there is no copyContext path in this checkout.
• The active-unit field already documents -1 as unknown. The recording driver now immediately rejects a negative unit passed to GL, as suggested.
• An explicit later setFrameBuffer(null) reaches toggleFramebufferSrgb and applies the existing main-target policy. Main-target metadata is no longer applied merely because a Java association is null. General external sRGB-state invalidation remains outside this patch.

Validation on this exact revision: 49 focused tests, 610 scoped module tests (two existing skips), 662 composed core tests (one skip), style/Javadoc, renewed independent review, and native Mesa state/pixel controls pass. Unchanged selections still elide calls. Standalone nonzero-default initialization can make one conservative initial bind; the #2984 composition already aligns that cache.

Current-head CI still awaits maintainer approval: Build 37032810713 and Android 37032810846. I am not treating earlier workflow status as validation of this revision.

[jaime-jmebot]

Checked 809b988 against the points from my earlier comment — all substantive ones are addressed, and I agree with the reasoning on the rest:

• init() (reached via reset()) now sets frameBufferBindingValid = true next to the boundFB/boundFBO defaults, so a reset context is a genuinely known one and no stale "valid but no Java association" state can survive a reset.
• The two-value boolean setter is gone in favour of a single method that records object, native name and validity together, plus a framebuffer-only invalidator so copy restore keeps texture knowledge. That removes the "declare it valid without a bind" footgun.
• I accept your call on boundFBO staying untrusted while invalid. A null boundFB genuinely can coexist with a nonzero native name (nonzero presentation target, externally managed FBO), so zeroing the int wouldn't make ignoring the validity flag safe — it would just be a different lie. The documented "untrusted" contract plus tests is the honest version of that.
• The recording driver now rejects a negative active unit, which is exactly the symmetric guard I wanted.

No further code comments from me. Two non-blocking follow-ups worth tracking when there's time: whether the cached mainFrameBufferSrgb gets re-applied when the main target is bound again, and the one conservative initial bind on standalone nonzero-default init that only disappears once composed with #2984. Nice work on the control set — fresh defaults and unchanged-selection elision are both still pinned.