CesiumGS / CesiumGS/cesium

Fix remaining framebuffer binding-cache bypasses (`_bind`/`_unBind` outside `Context.bindFramebuffer`)

Open
#13,664 0 comments 0 reactions 0 assignees View on GitHub
category - graphics type - bug type - cleanup
Dominant language
JavaScript
Stars
15.7k
Forks
3.9k
Avg merge
4d 6h
Merged PRs (30d)
34

Description

### What happened?

This stems from https://github.com/CesiumGS/cesium/issues/13661 which was fixed in https://github.com/CesiumGS/cesium/pull/13662.

Please read up on that issue and PR to get full context first.

## Background

#13662 fixed a one-frame black flash caused by the `Framebuffer` constructor issuing `gl.bindFramebuffer` calls without updating `Context._currentFramebuffer`. `Context.bindFramebuffer` skips the GL call when the target matches its cache, so any code that changes the real GL binding behind the cache's back makes subsequent draws land on the wrong framebuffer until the next real rebind (or `endFrame`).

#13662 patched only the constructor. The same bypass pattern is still live in several places that can, in theory, run mid-frame:

- `Framebuffer.js` `status` getter binds `this`, unbinds to `null`, cache untouched. Read during updates by `OIT.js`, `ShadowMap.js`, and `FramebufferManager.js`
- `Texture.js` `loadFramebufferSource` (used by `Texture.fromFramebuffer`). Binds `source.framebuffer`, unbinds to `null`.
- `CubeMap.js` `copyFace` constructs + binds a scratch framebuffer.
- `TextureAtlas.js` does an atlas resize during billboard/label update. This follows the same construct + bind/unbind pattern.

Each is a variant of the #13662 bug: harmless until some caller sequence puts a cached bind before it and draws after it in the same frame.

## Proposed fix

Rather than patching each site with `context._currentFramebuffer = undefined` (repeating the cross-class reach noted in #13662), we could make `Context` the sole writer of framebuffer binding state. Arguably, it should probably be this way already...

1. Expose the binding through the context, e.g. `context.bindFramebuffer(framebuffer)` / a scoped helper like `context.withFramebuffer(fb, fn)` that saves and restores both the GL binding and the cache.
2. Convert the sites above (and the `Framebuffer` constructor) to use it; `_bind`/`_unBind` become private implementation details of `Context` or are removed.
3. Keep the #13662 spec and add equivalents: reading `status`, `Texture.fromFramebuffer`, `CubeMap.copyFace`, and an atlas resize must all leave `context._currentFramebuffer` consistent with the GL binding.

## Notes

- `Context.endFrame` already resets the cache each frame, which is why these have mostly gone unnoticed — the desync window is at most the remainder of one frame.
- Refs: #13662 (constructor fix + root-cause analysis).

### Reproduction steps

N/A - theoretical from code analysis. https://github.com/CesiumGS/cesium/issues/13661 is the one we actually observed, now fixed.

In theory, we could write some unit tests to prove these bypass flaws.

### Sandcastle example

_No response_

### Environment

Browser: Firefox / Chrome / all
CesiumJS Version: Latest (main / 1.144)
Operating System: macOS / all

### AI acknowledgment

- [x] I used AI to generate this issue report.
- [x] (If the above is checked) I have reviewed the AI-generated content before submitting.

Contributor guide

Open the contributing guide

Research direction

Read issues #13661 and #13662 first for the framebuffer cache context, then inspect Context, Framebuffer.js, Texture.js, CubeMap.js, and TextureAtlas.js. Trace the listed _bind/_unBind paths and the existing #13662 spec. Done means the affected operations leave Context._currentFramebuffer consistent with the GL binding and equivalent coverage exists for status, Texture.fromFramebuffer, CubeMap.copyFace, and atlas resize.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
computer-graphics
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.