8000 graphics/gl: forget deleted textures in the binding cache by ricande · Pull Request #691 · not-fl3/miniquad · GitHub
[go: up one dir, main page]

Skip to content

graphics/gl: forget deleted textures in the binding cache - #691

Open
ricande wants to merge 1 commit into
not-fl3:masterfrom
ricande:gl-forget-deleted-texture
Open

ricande wants to merge 1 commit into
not-fl3:masterfrom
ricande:gl-forget-deleted-texture

Conversation

@ricande
@ricande ricande commented Oct 15, 2026 •
Copy link
Copy Markdown

delete_texture deletes the GL texture but leaves it in GlCache. GL rebinds the units that had the texture to 0, but the cache still records it as bound. If the next texture created gets the same GL name, bind_texture takes it as already bound and skips glBindTexture, so the upload goes to whatever is really bound and the new texture stays empty.

macroquad hits this when its font atlas grows right after a flush that ended with text (for example when switching back from a render-target camera): the new atlas texture is never filled, and all text is drawn as black rectangles from then on. I believe this is the cause of not-fl3/macroquad#1006.

Change

GlCache::forget_texture resets the cache entries, including the stored binding used by store_texture_binding/restore_texture_binding, that refer to the deleted texture. It makes no GL calls, so the bindings of other textures are not touched. It replaces the commented-out //self.cache.clear_texture_bindings();, which was added in f08f001 as a fix for stale bindings and commented out in 65f1da6. Unlike clear_texture_bindings(), it does not rebind every unit to 0.

Reproduction and verification

Minimal reproduction: https://github.com/ricande/miniquad-atlas-repro (about 100 lines: text drawn into a render target, read back, then into the window, with sizes that make the atlas grow). Results on Linux, Mesa 26.0.8, Intel UHD Graphics 630, macroquad 0.4.16, miniquad 0.4.11 (src/graphics/gl.rs and src/graphics/gl/cache.rs are the same on master), 100 frames per run:

Build Runs with black text
unchanged 6 of 10
with this change 0 of 10, and 0 of 12 instrumented runs

With logging added to macroquad's Atlas::texture, the new atlas texture always got the same GL name as the old one, and without the change the text turned black in exactly the runs where the atlas was deleted while GL had it bound (5 of 8 runs) and in none of the others. With the change, 3 of the 12 instrumented runs deleted the atlas while it was bound, and none failed.

This was verified on that one machine only; I have not been able to check the other reports in not-fl3/macroquad#1006 (other GPUs and systems). Related, but not verified here: #633 (binding an already deleted texture) involves the same stale cache entries.

delete_texture deleted the GL texture but left it in GlCache. GL rebinds
units that had the texture to 0, but the cache still recorded it as bound.
If a texture created next got the same GL name, bind_texture took it as
already bound and skipped glBindTexture, so its upload went to whatever was
really bound and the new texture stayed empty.

This happens to macroquad's font atlas when it grows right after a flush
that ended with text: the new atlas texture is never filled and all text is
drawn as black rectangles (not-fl3/macroquad#1006).

GlCache::forget_texture resets the cache entries, including the stored
binding, that refer to the deleted texture. It makes no GL calls, so
bindings of other textures are not touched; this replaces the commented-out
clear_texture_bindings() call.
ricande added a commit to ricande/miniquad-atlas-repro that referenced this pull request Oct 15, 2026
ricande added a commit to ricande/RustyHusky that referenced this pull request Oct 15, 2026
Text could turn into black rectangles when macroquad's font atlas grew
right after a render-target pass (not-fl3/macroquad#1006): miniquad's
delete_texture left the deleted texture in its binding cache. Vendor
miniquad 0.4.11 exactly as published plus one patch (proposed upstream as
not-fl3/miniquad#691) and use it through [patch.crates-io]; a script, also
run in CI, checks that the copy is the crate plus the patch.

Remove the earlier view::end_batch workaround, which made the failure
rarer but did not prevent it. docs/UPSTREAM_FIXES.md records the symptom,
reproduction, cause, patch, verification and how to drop the copy.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ricande
ricande commented Oct 15, 2026
Copy link
Copy Markdown
Author

Additional verification on a local Apple M2 Mac: the existing reproduction's text-failure criterion also triggers with the macOS OpenGL backend, and the proposed fix passed the small comparison batch.

Environment: MacBook Air (Apple M2, 8 GB RAM), macOS 26.5.2 (25F84), native arm64 release build, rustc 1.99.1, macroquad 0.4.16 and miniquad 0. 8000 4.11. The program reports GL renderer: Apple M2. Miniquad's Metal backend was not tested.

Method: Used https://github.com/ricande/miniquad-atlas-repro at de7a4f90b297d10db736898cff4be9fd30387cee, with unchanged src/main.rs, running the release executable with DEBUG=1 and 100 frames. Compared published miniquad 0.4.11 against a separate local copy with the executable code changes from this PR at fbc59729b24dd6c3c4052a729431fa54ffe42b0d, selected through [patch.crates-io]. The repro's lockfile needed its registry source/checksum restored for miniquad; dependency versions were unchanged.

Build Text failures / completed runs Details
Unmodified miniquad 0.4.11 2 / 4 Both failures at frame 70, exit code 1; two runs completed 100 frames with exit code 0
With this fix 0 / 3 All three completed 100 frames with exit code 0

Expected: the white text produces nonzero white-pixel counts throughout all 100 frames. Both failing runs logged:

frame 69: bound after the target pass (4, 0), white 27155
UNSUPPORTED (log once): POSSIBLE ISSUE: unit 0 GLD_TEXTURE_INDEX_2D is unloadable and bound to sampler type (Float) - using zero texture because texture unloadable
frame 70: bound after the target pass (4, 0), white 0
RESULT: black text in the render target from frame 70

Separate resource failure: A fifth baseline run reported MTLCommandBufferErrorDomain Code=8, Insufficient Memory / kIOGPUCommandBufferCallbackErrorOutOfMemory, and was terminated after frame 77 without a RESULT. It is excluded from the 2/4 result, not counted as this text bug. Neither of the two text-failure logs contained that memory error. Subsequent runs were bounded to 60 seconds and stopped automatically on a GPU-memory error.

This verifies the reproduction's pixel-based text failure on macOS/Apple M2 and provides supporting evidence for the fix there. The sample is small, atlas packing is randomized without a shared seed between builds, and atlas deletion/name reuse was not separately instrumented in this test. These results do not establish that every report in not-fl3/macroquad#1006 is resolved.

@ricande
ricande commented Oct 15, 2026
Copy link
Copy Markdown
Author

Additional WebGL regression coverage for this patch: test sources, instructions and three browser-run logs.

On Apple M2 / macOS 26.5.2 / Chrome 155 (ANGLE Metal), both WebGL1 and WebGL2 reproduce a stale-cache restore without any application reuse of a deleted handle:
allocate A and B → bind/draw A → delete A → update only live B. Baseline master 39c928fb7e09fd44e45f37fe9a98503968bca412 restores deleted A from its cache; applying this PR at fbc59729b24dd6c3c4052a729431fa54ffe42b0d eliminates that diagnostic. Unlike the atlas reproduction, this does not require texture-name reuse.

Controls: keeping A alive gives no diagnostic in either variant; deliberately reusing deleted A gives the diagnostic in both. Three rounds of all 12 combinations (36 instances) completed and rendered the live texture correctly with GL error 0. All 12 packaged configurations also rebuilt successfully using committed lockfiles.

This is a concrete connection to the diagnostic in #633, not verification that its fatal mobile crash is fixed. No crash occurred here; mobile Safari/Chrome and the reporter's actual scene-transition sequence still need testing. The existing PR is sufficient for the cache path demonstrated here; no additional fix or PR is proposed.

ricande added a commit to ricande/RustOut that referenced this pull request Oct 18, 2026
- Recording no longer waits for ffmpeg: push_frame never blocks; a full,
  bounded queue stops the encoder with a clear error while the game goes on.
  ffmpeg processes have time limits, their output is drained, and closing
  or discarding stops them at once. Ported from RustyHusky, with regression
  tests for hanging, slow and fast encoders.
- Build against miniquad 0.4.11 with the texture-cache fix proposed in
  not-fl3/miniquad#691 (black text rectangles, not-fl3/macroquad#1006);
  vendored crate plus patch, checked by a script.
- Minimum Rust version 1.87 (was documented as 1.82).
- GitHub Actions CI on Linux, Windows and macOS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@viniciusfigueiras
Copy link
Copy Markdown

Tested on Windows/NVIDIA

I reproduced the black text issue in my game using macroquad 0.4.16 and miniquad 0.4.11 on Windows with NVIDIA OpenGL.
I tested the original miniquad 0.4.11 against the same version with only this PR's changes applied.

  • Forced atlas growth with the atlas bound last: 4/4 runs failed without the patch, 0/6 with it. Tested at 2925×1950 and 1080×720.
  • With a different texture bound last: 0 failures in either version.
  • In an instrumented failing run, the GL texture name was reused after atlas growth and querying its width returned 0. With the patch, atlas growth completed normally.

I also ran our game's GPU checks at 1080×720. L2 depth comparisons had 0 differing pixels, static-state audits were clean, and scissor checks found no violations with or without the patch. Both builds compiled. The same 12 unrelated desktop tests failed in both versions.

The patch fixed every reproduction I tried on this setup. Thanks for the fix!

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

0