fix(rendering): actually suppress FBO resize-to-zero on minimize - #5372
fix(rendering): actually suppress FBO resize-to-zero on minimize#5372soloturn wants to merge 1 commit into
Conversation
#5081: game screen goes dark after minimizing and restoring, via a third-party minimize gesture tool (TaskbarX) rather than native Windows minimize/restore. This is the same symptom #4980 already tried to fix, and the fix it shipped has an inverted condition, so it never worked - not for native minimize either, just less commonly hit. GLFW reports the framebuffer as 0x0 while a window is minimized. DisplayResolutionDependentFbo.propertyChange and PerspectiveCamera's own listener both react to DISPLAY_RESOLUTION_CHANGE firing at all - neither reads the old/new values on the event, they only care whether it fired - and propertyChange regenerates every resolution-dependent FBO at whatever size the display device reports at that moment. So the intent of #4980's fix was: while minimized, don't let this event fire, so FBOs never get regenerated at 0x0 and never need to be un-broken on restore. PropertyChangeSupport.firePropertyChange suppresses notification when old equals new, which #4980 used as the on/off switch - but picked the value that differs from newValue(1) while minimized (0) and the value that matches while not (1), the opposite of "stop it while minimized". So minimizing always fired (0 != 1, unchanged from before #4980), while every ordinary window resize - not just minimize/restore - stopped firing (1 == 1, newly suppressed), since the value passed no longer had anything to do with whether the size actually changed. Flips it: (1, 1) while minimized so the compare matches and nothing fires, (0, 1) otherwise so a genuine resize still does. Root-caused and fixed by reading, not reproduced - this needs an actual Windows session with a real or synthetic minimize gesture, which I don't have here. The fix follows directly from documented GLFW behaviour (0x0 framebuffer while iconified) and PropertyChangeSupport's own equals-based suppression, not from observing the bug. No existing tests cover this class - updateViewport needs a live GLFW window/context, not something headless-testable. Compiles clean (:engine:compileJava). Fixes #5081 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesViewport notification handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized rendering fix changes minimize-time framebuffer resize signaling so normal resizes continue to propagate; no actionable merge-blocking risk remains after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Fixes #5081 - game screen goes dark after minimizing (via a third-party minimize tool) and restoring.
This is #4980's fix, with an inverted condition
LwjglDisplayDevice.updateViewportalready tries to guard against this - added by #4980 for an earlier report of the identical symptom - but the guard never actually worked.GLFW reports the framebuffer as 0x0 while a window is minimized.
DisplayResolutionDependentFbo.propertyChangeandPerspectiveCamera's ownDISPLAY_RESOLUTION_CHANGElistener both react to the event firing at all, not to its old/new values -propertyChangeregenerates every resolution-dependent FBO at whatever size the display device reports at that moment. #4980's intent was to stop that event firing while minimized, usingPropertyChangeSupport.firePropertyChange's own behaviour of skipping notification when old equals new as the switch:This picks the value backwards: while minimized,
i=0differs fromnewValue=1, so the event still fires - unchanged from before #4980 landed. While not minimized,i=1matchesnewValue=1, so it's suppressed - a second, unrelated regression, since that's exactly when a resize should propagate.Fix
Flip which value goes with which case:
(1, 1)while minimized so the compare matches and nothing fires,(0, 1)otherwise so a genuine resize still does.What I could and couldn't verify
Root-caused and fixed by reading, not reproduced - confirming this needs an actual Windows session with a real or synthetic minimize gesture, which I don't have here. The fix follows directly from documented GLFW behaviour (0x0 framebuffer while iconified) and
PropertyChangeSupport's own equals-based suppression semantics, not from observing the bug happen.No existing tests cover this class -
updateViewportneeds a live GLFW window/context, not something headless-testable.:engine:compileJavais clean.