Fix sampler-incomplete texture binding and stuck Cocoa swap chain on macOS Apple Silicon#418
Open
troyedwardsjr wants to merge 3 commits into
Open
Conversation
added 3 commits
May 27, 2026 01:28
…ncomplete default texture 0
…e Cocoa to flush the visible NSView on macOS
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes two real OpenGL bugs in
OpenGLRenderer3Dthat together prevent the live SDL window from rendering on macOS Apple Silicon. Either bug alone is sufficient to leave the window pure black. Both fixes are cross-platform-correct improvements; only the symptom is macOS-specific.Full technical writeup: PATCHES.md.
Companion issue: #417.
Summary of changes
1. Sampler incompleteness on units 1/2/3 (renderer correctness)
RenderVertexBufferleaves sampler units 1/2/3 bound to default texture name0whenever the active material lacks a normal/specular/illumination map. The shader (simple.frag) samples those units unconditionally. On any GL Core Profile, default texture0is sampler-incomplete (its built-inGL_TEXTURE_MIN_FILTER = GL_NEAREST_MIPMAP_LINEARdemands mipmaps it doesn't have); Apple's driver enforces this by substituting a "zero texture" and emitting:Lighting then collapses to black through the simple → lighting → postprocess chain.
Fix: Create a 1×1 white dummy texture at context init with complete sampler state (
GL_LINEARfilters,GL_CLAMP_TO_EDGEwraps,BASE_LEVEL=MAX_LEVEL=0). Bind it to units 1/2/3 instead of default texture0whenever the material has no real texture for that unit. Cleanup likewise.Cost: one 1×1 texture in VRAM; same number of
glBindTexturecalls per frame as before.2. Stuck Cocoa swap chain on macOS Apple Silicon (presentation)
Even with #1 fixed, the window stays black on macOS. Diagnostic:
glReadPixelsafterSDL_GL_SwapWindowreturns correct pixels (this is how the offscreen video path captures frames), so GL is rendering correctly to the back buffer and the GL-side swap completes but Apple's GL→Cocoa bridge on Apple Silicon doesn't actually flush the front buffer to the visibleNSViewwithout explicit nudging.Fix: Two-line sandwich around
SDL_GL_SwapWindowinOpenGLRenderer3D::SwapBuffers:Plus a one-line change in
sdl_glfuncs.hsoglFinishis actually loaded (was declaredSDL_PROC_UNUSED).Neither call is macOS-specific.
glFinishbefore swap is longstanding low-latency-rendering best practice;SDL_PumpEventsfrom the main thread is a no-op outside macOS where the event loop is already pumped. Net cost on Apple Silicon: one extra GL sync + one event pump per frame (~100µs total at typical framerates). No correctness risk on any platform.Commits
(Commit messages still carry the
try-sampler:/try-finish-pump:prefix from the original branch labels in the working fork; happy to squash and rewrite to clean upstream-style messages if you prefer.)What this PR does NOT change
academy_run_to_score_with_keeperepisode dump on patched and unpatched builds; bot-vs-bot scoring + state evolution match).Test plan
Manual visual verification:
python -m gfootball.play_game --players=bot:left_players=1 --level=academy_3_vs_1_with_keeper --action_set=full --real_time=true --render=trueon macOS Apple Silicon → pure black window, plus warning.Automated:
python -m gfootball.examples.run_ppo2 --level=academy_empty_goal_close --total_timesteps=10000produces identical training curves before/after on a fixed seed (Bug 1 is the only semantic change, and the dummy texture's effect on samplers is a no-op because the shader's sample of a unit-1 texture is multiplied by zero wheneverhas_normalis false in the lighting math — verified by inspection oflighting.frag).Alternative considered
Adding
has_normal/has_specular/has_illuminationpreprocessor guards insidesimple.fragso the shader doesn't sample units 1/2/3 when the material lacks those maps. Equivalent in effect, but requires either compiling separate shader variants for each combination (cartesian explosion) oruniform bool has_normalchecks (which most drivers handle fine but are slightly less GPU-friendly). The dummy-texture approach is the smaller, more conservative change.License & attribution
Apache 2.0, preserved. Commits are signed off. Bug 1 patch is original; Bug 2 follows a longstanding macOS SDL workaround pattern documented in many community discussions over the years.