Skip to content

Preserve batched Nifty atlas data across context restarts - #3065

Open
adrian-andrzjewski wants to merge 1 commit into
jMonkeyEngine:masterfrom
adrian-andrzjewski:fix/nifty-atlas-context-loss
Open

adrian-andrzjewski wants to merge 1 commit into
jMonkeyEngine:masterfrom
adrian-andrzjewski:fix/nifty-atlas-context-loss

Conversation

@adrian-andrzjewski

@adrian-andrzjewski adrian-andrzjewski commented Nov 12, 2026 •

Copy link
Copy Markdown

Summary

Nifty's batched backend uploads atlas subregions with Renderer.modifyTexture, but leaves the atlas Image CPU data unchanged. After a graphics-context restart, jME restores that stale backing image while Nifty still considers cached images uploaded.

Mirror atlas updates into independent RGBA8 backing data per atlas, while retaining the existing batched draw and GPU subupload path. Atlas clears reset the backing data and discard superseded queued updates. Unsupported conversions fail before mutating the atlas.

This addresses the OpenKeeper symptom discussed in tonihele/OpenKeeper#1109 without switching OpenKeeper to the standard renderer.

Validation

  • :jme3-niftygui:test: 6 tests pass on current master and the jME 3.9.0 baseline.
  • OpenKeeper test: 715 tests pass with a locally built patched jME 3.9 Nifty jar.
  • A temporary LWJGL3 framebuffer harness based on TestIssue1013 reproduced the original failure: 255,360 of 480,000 pixels changed after one restart. With this fix, the framebuffer was identical after two restarts.
  • CPU atlas-mirror microbenchmark on an AMD Ryzen 5 3600, JDK 21 (median of 7 samples): 32x32 = 2.89 us/update, 128x128 = 12.73 us/update, 512x512 = 144.13 us/update. This measures the added CPU copy only; it is not a GUI frame-time comparison.
  • No render-loop or batch submission code changes. Each 2048x2048 RGBA8 atlas retains 16 MiB of CPU backing data. Frame-time, draw-call, GPU-time, and context-recovery-duration comparisons were not collected.

OpenKeeper campaigns, direct map loading, HUD/minimap/cursor behavior, and repeated settings changes have not been manually validated because Keeper II map data is unavailable. The batched path avoids the standard-renderer route associated with the HUDBackground cache report, but these gameplay checks remain open.

Copy link
Copy Markdown
Author

@riccardobl Could you review this jme3-niftygui fix? It keeps the batched renderer and mirrors atlas updates into CPU backing data so jME can restore the textures after context recreation. Six focused tests pass on current master and 3.9, OpenKeeper's 715 tests pass with the patched 3.9 jar, and the TestIssue1013 framebuffer matched after two restarts (the baseline lost 53.2% of pixels). Campaign/map/cursor behavior and frame-time benchmarks remain unverified.

@jaime-jmebot jaime-jmebot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Adrian, thanks for putting together a focused fix for the context-loss issue. Keeping the batched rendering path intact while making each atlas image’s CPU backing reflect its subimage updates is a good fit for the problem, and the per-atlas storage also avoids the previous shared-buffer ownership problem.

I reviewed the update and clear paths alongside the new tests. The bounds and format checks happen before atlas data is changed, the RGBA8 fast path preserves buffer positions while copying rows, and the conversion path covers formats that can be represented through ImageRaster. Clearing an atlas also resets its restore payload and drops queued GPU updates that would otherwise undo the clear. The added tests exercise these important behaviors, including restoring from the image after resetting its GPU object.

I didn’t find anything that needs a change. The documented CPU-memory cost is a reasonable trade-off for retaining a reliable restore payload, and the reported context-restart harness gives useful coverage for the original failure. Looks good to me—thanks for tackling this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants