Fix persistent GL_INVALID_OPERATION spam and resolution-change bugs on Android
build / build (push) Successful in 2m44s
build / build (push) Successful in 2m44s
- opengl_end_frame() bound framebuffer 0 on Android, which has no valid target there (real presentation goes through the OpenXR swapchain's own FBOs) - gl4es reported this every frame. Skip it there. - render_hack_cameras() re-entered opengl_start_frame()/opengl_end_frame() for camera-monitor renders that never touch the GL screen; excluded them to match use_opengl()'s existing behavior. Also drains a stray error left by xrPollEvent(). - ChangeScreenSize() never called opengl_resize() on Android, freezing the 3D render target at launch size after an in-game resolution change; and xr_get_game_resolution() used the swapchain's fixed size instead of the current screen resolution, misplacing the laser-pointer cursor after such a change. - Removes all temporary bisection diagnostics added while tracking down the GL_INVALID_OPERATION source.
This commit is contained in:
@@ -402,6 +402,10 @@ each one does:
|
||||
`SDL_SetWindowFullscreen`/`SetWindowSize`/`SetWindowPosition` calls on
|
||||
Android, since there's no real desktop-style window to resize and
|
||||
calling them desyncs SDL's cached window size from the real surface.
|
||||
Also calls `opengl_resize()` directly in `ChangeScreenSize()`
|
||||
(see README 5.5.6) - skipping `SDL_SetWindowSize()` means no native
|
||||
resize event ever fires on Android to trigger it when the in-game
|
||||
resolution changes, so it would otherwise never re-run there.
|
||||
- **`10-android-openxr-cmake.patch`** - wires up the OpenXR loader plus
|
||||
EGL as link/include dependencies for the CMake build.
|
||||
- **`11-android-openxr-present.patch`** - the core OpenXR present-path
|
||||
@@ -464,6 +468,46 @@ each one does:
|
||||
silently forever instead of falling back. Adds an `#else` branch that
|
||||
returns failure and logs a `WARN`, so `InitDecXMI()`'s existing
|
||||
fallback-to-dummy-driver logic actually triggers.
|
||||
- **`18-android-opengl-immediate-mode-attribs.patch`** - fixes a black (and,
|
||||
after this patch's first revision, flat-colored/untextured) screen when
|
||||
switching to the OpenGL/3D render mode in-game (see README 5.5.5). Every
|
||||
`glBegin(GL_TRIANGLE_STRIP)`-based draw in `OpenGL.cc` -
|
||||
`opengl_swap_and_restore()`'s screen-composite quad,
|
||||
`opengl_light_tmap()` (every textured wall/floor polygon, via
|
||||
`draw_vertex()`), `opengl_bitmap()` (sprites), and `opengl_draw_poly()`
|
||||
(flat/gouraud polygons, also via `draw_vertex()`) - used
|
||||
`glVertexAttrib2f`/`glVertexAttrib1f`/`glVertex3f` immediate mode to set
|
||||
per-vertex texcoords/light, which gl4es only captures per-vertex for
|
||||
attribute 0 (position); custom attributes end up constant for the whole
|
||||
draw call. Adds `android_imm_begin()`/`android_imm_vertex()`/
|
||||
`android_imm_end()`, a small shared immediate-mode-emulation shim that
|
||||
buffers vertices and flushes them as one real `glDrawArrays()` call, and
|
||||
switches every one of those call sites to it, Android-only. The
|
||||
starfield (`opengl_begin_stars()`/`opengl_draw_star()`/
|
||||
`opengl_end_stars()`, `GL_POINTS` rather than `GL_TRIANGLE_STRIP`, up to
|
||||
`NUM_STARS`=1000 per frame) had the same bug for its per-star brightness
|
||||
and gets its own similarly-shaped `android_stars_begin()`/
|
||||
`android_stars_vertex()`/`android_stars_end()` buffer. Also skips the
|
||||
unconditional `SDL_GL_MakeCurrent(window, context)` call every
|
||||
rendering function in this file had at its top, Android-only - a
|
||||
worthwhile cleanup on its own merits, but *not* the source of the
|
||||
persistent per-frame `GL_INVALID_OPERATION` (1282) logcat spam during
|
||||
3D gameplay that this was first suspected to be (see README 5.5.8 for
|
||||
that bug's actual root cause and fix, in patch 20 below).
|
||||
- **`19-android-fix-hackcam-opengl-frame.patch`** - `fr_start_view()`/
|
||||
`fr_send_view()` (`frsetup.c`) called `opengl_start_frame()`/
|
||||
`opengl_end_frame()` again for hack-camera security-monitor renders
|
||||
(which target a private software bitmap, not the GL-backed screen),
|
||||
since `should_opengl_swap()` doesn't exclude them the way
|
||||
`use_opengl()` already does. Adds the same exclusion to both call
|
||||
sites (see README 5.5.8).
|
||||
- **`20-android-no-default-framebuffer.patch`** - the actual fix for the
|
||||
persistent per-frame `GL_INVALID_OPERATION` (1282) in README 5.5.8:
|
||||
`opengl_end_frame()`'s `BindFrameBuffer(nullptr)` bind to framebuffer/
|
||||
renderbuffer id 0 has no valid target on Android (real presentation
|
||||
goes through the OpenXR swapchain's own FBOs, set up separately in
|
||||
`xr_swapchain.c`) and is also functionally dead there regardless -
|
||||
skipped entirely, Android-only.
|
||||
|
||||
`android/gl4es-patches/`:
|
||||
|
||||
@@ -600,6 +644,189 @@ path (still silent, but with a clear `WARN` in logcat) instead of
|
||||
silently reporting success while never opening anything - a much faster
|
||||
diagnosis if this ever recurs.
|
||||
|
||||
#### 5.5.5. Black, then flat-colored/untextured screen after switching to OpenGL rendering in-game
|
||||
|
||||
The engine has a `Ctrl+G`-bound in-game toggle (`toggle_opengl()`,
|
||||
`Prefs.c`) that cycles between the plain software renderer and OpenGL
|
||||
rendering. On Quest, switching to OpenGL rendering (either mode:
|
||||
unfiltered or bilinear) produced a black screen for the 3D view, with
|
||||
`OpenGL error: 1282` (`GL_INVALID_OPERATION`) in logcat right at the switch.
|
||||
|
||||
**Root cause:** `opengl_swap_and_restore()` draws the offscreen-rendered
|
||||
3D view onto a screen-filling quad using `glBegin(GL_TRIANGLE_STRIP)` /
|
||||
`glVertexAttrib2f()`/`glVertexAttrib1f()` / `glVertex3f()` immediate
|
||||
mode to supply per-vertex texcoords and light. This is exactly the same
|
||||
bug already found and fixed for the UI-overlay quad in
|
||||
`11-android-openxr-present.patch`'s `android_draw_surface_as_quad()`:
|
||||
under gl4es's GLES2 immediate-mode emulation, `glVertexAttrib*()` sets a
|
||||
*constant* value for the whole draw call, not a fresh value per vertex
|
||||
the way desktop GL's immediate mode does - only attribute 0 (position,
|
||||
driven by `glVertex3f`) gets real per-vertex capture. The 3D-view quad
|
||||
draw predates that fix and was never updated to match, so it kept using
|
||||
the broken pattern.
|
||||
|
||||
This patch's first revision only fixed that one quad, which did clear the
|
||||
black screen - but the next on-device test showed a "blocky" world
|
||||
instead: every wall/floor polygon rendered as a single solid color
|
||||
instead of its texture. The same broken immediate-mode pattern turned out
|
||||
to be used by every other textured/lit polygon draw in the file too:
|
||||
`opengl_light_tmap()` (every textured wall/floor polygon in the 3D view,
|
||||
via the shared `draw_vertex()` helper), `opengl_bitmap()` (sprites), and
|
||||
`opengl_draw_poly()` (flat/gouraud-shaded polygons, also via
|
||||
`draw_vertex()`) - each polygon's texcoord/light attribute ended up
|
||||
constant across all of its vertices (whatever was last set), instead of
|
||||
varying per vertex, hence the flat, solid-colored blocks.
|
||||
|
||||
**Fix:** `18-android-opengl-immediate-mode-attribs.patch` adds
|
||||
`android_imm_begin()`/`android_imm_vertex()`/`android_imm_end()`, a small
|
||||
shared immediate-mode-emulation shim (buffers vertices, then issues one
|
||||
real `glDrawArrays()` call), and switches every glBegin/glVertexAttrib/
|
||||
glVertex3f call site above to it, Android-only - the desktop path is
|
||||
unaffected and keeps the original immediate-mode code, which works
|
||||
correctly under real desktop OpenGL.
|
||||
|
||||
The same patch also fixes the starfield
|
||||
(`opengl_begin_stars()`/`opengl_draw_star()`/`opengl_end_stars()`), found
|
||||
while auditing the file for every other instance of this pattern rather
|
||||
than through an on-device symptom: each star is a separate `GL_POINTS`
|
||||
vertex with its own brightness set via the same
|
||||
`glVertexAttrib1f`/`glVertex3f` immediate-mode pair, so every star drawn
|
||||
between one begin/end call would have rendered at whatever brightness was
|
||||
set for the last one. Gets its own `android_stars_begin()`/
|
||||
`android_stars_vertex()`/`android_stars_end()` buffer, shaped for
|
||||
`GL_POINTS` and up to `NUM_STARS` (`GameSrc/rendtool.c`) vertices rather
|
||||
than `android_imm_*()`'s small triangle-strip buffer.
|
||||
|
||||
#### 5.5.6. Display breaks and stops responding to input after changing resolution in-game
|
||||
|
||||
Picking a different resolution from the in-game options menu (e.g.
|
||||
`1024x768`) produced a single `OpenGL error: 1282` in logcat and left the
|
||||
display frozen on the last frame - `VrApi`'s FPS counter kept running at
|
||||
90fps (the OpenXR compositor was still submitting frames), but nothing
|
||||
in the menu responded to further input afterward.
|
||||
|
||||
**Root cause:** `ChangeScreenSize()` (`ShockBitmap.c`) calls
|
||||
`SDL_RenderSetLogicalSize()` to the new resolution and reallocates
|
||||
`drawSurface`/`offscreenDrawSurface` accordingly, but on desktop the
|
||||
actual GL-side work - `opengl_resize()` in `OpenGL.cc`, which
|
||||
(re)creates `backupBuffer` (the offscreen render target the 3D view
|
||||
draws into) at the new size - only runs later, from the
|
||||
`SDL_WINDOWEVENT_RESIZED`/`SIZE_CHANGED` event that
|
||||
`SDL_SetWindowSize()` generates. `09-android-no-window-resize.patch`
|
||||
skips that `SDL_SetWindowSize()` call on Android for an unrelated
|
||||
reason (see its own entry above) - which also means no such event, and
|
||||
therefore no `opengl_resize()` call, ever happens on Android when the
|
||||
in-game resolution changes. `backupBuffer` stayed stuck at whatever size
|
||||
it was created with at launch while the rest of the game moved on to
|
||||
the new resolution, and that mismatch is what broke rendering and left
|
||||
the menu unable to correctly hit-test further clicks.
|
||||
|
||||
**Fix:** `ChangeScreenSize()` now calls `opengl_resize()` directly on
|
||||
Android, right after updating `gScreenWide`/`gScreenHigh` (matching the
|
||||
desktop event's actual timing, since `SDL_PollEvent()` always delivers
|
||||
it after `ChangeScreenSize()` has already returned) - added to
|
||||
`09-android-no-window-resize.patch` since this is a direct, foreseeable
|
||||
consequence of that patch's own change, not a new mechanism.
|
||||
|
||||
#### 5.5.7. Laser-pointer cursor disappears after changing resolution in-game
|
||||
|
||||
After 5.5.6's fix, changing resolution stopped breaking the display, but
|
||||
the laser-pointer-driven mouse cursor (see section 5's architecture
|
||||
overview) would disappear and stop being able to click anything
|
||||
afterward.
|
||||
|
||||
**Root cause:** `xr_get_game_resolution()` (`xr_session.c`), which
|
||||
`xr_input.c` uses to convert a game-quad ray hit's normalized u,v into
|
||||
the pixel coordinates it feeds the engine via `xr_mouse_move()`, returned
|
||||
the game quad's OpenXR swapchain's fixed pixel size
|
||||
(`g_game_swapchain.width/height`) - set once at launch and never resized.
|
||||
`android_draw_surface_as_quad()` (`OpenGL.cc`,
|
||||
`11-android-openxr-present.patch`) always stretch-blits `drawSurface` to
|
||||
fill that swapchain image via a single full-quad draw, regardless of
|
||||
`drawSurface`'s own pixel size, so the swapchain's fixed size has no
|
||||
actual relationship to the coordinate space `MouseX`/`MouseY` live in -
|
||||
that's `drawSurface`'s own current size (`gScreenWide`/`gScreenHigh`),
|
||||
which `ChangeScreenSize()` can change at runtime independently of the
|
||||
swapchain. Using the stale swapchain size there mapped every ray hit
|
||||
into the wrong coordinate space, moving the mouse position out of the
|
||||
engine's actual screen bounds.
|
||||
|
||||
**Fix:** `xr_get_game_resolution()` now returns `gScreenWide`/
|
||||
`gScreenHigh` instead. `xr_get_game_quad_extent()` (the quad's shape and
|
||||
distance in the VR world) is intentionally left as-is, still tied to the
|
||||
swapchain's fixed size - that's a real, unchanging property of the
|
||||
swapchain image itself, unrelated to this bug.
|
||||
|
||||
#### 5.5.8. Persistent `OpenGL error: 1282` logcat spam during 3D gameplay
|
||||
|
||||
Every logcat capture during actual 3D gameplay showed a continuous
|
||||
stream of `OpenGL error: 1282` (roughly twice per frame) from the one
|
||||
pre-existing `glGetError()` check in `opengl_swap_and_restore()`, with
|
||||
no visible effect on rendering (textures, resolution changes, and the
|
||||
cursor all worked correctly per sections 5.5.5-5.5.7's fixes). 1282 is
|
||||
`GL_INVALID_OPERATION` (0x0502) - not `GL_INVALID_ENUM` (0x0500/1280),
|
||||
which earlier debugging had mistakenly assumed given the visual
|
||||
symptoms resembled the immediate-mode bug in 5.5.5.
|
||||
|
||||
Two real, unrelated bugs turned out to be involved, found by adding a
|
||||
temporary chain of `glGetError()` checkpoints across the whole
|
||||
per-frame render path (`mainloop()` → `render_run()` → `fr_rend()` →
|
||||
`fr_prepare_view()`/`fr_start_view()`/`fr_send_view()`) and bisecting
|
||||
which ones read clean:
|
||||
|
||||
**xrPollEvent() leaves a stray error.** `xr_poll_events()`
|
||||
(`xr_session.c`) left gl4es reporting `GL_INVALID_OPERATION` on every
|
||||
call while the session is running, regardless of what this app's own
|
||||
code does. Fixed by draining it (`while (glGetError() != GL_NO_ERROR)`)
|
||||
right after the polling loop.
|
||||
|
||||
**A hack-camera render re-entered the OpenXR frame setup.**
|
||||
`render_hack_cameras()` (`render.c`) calls `fr_rend()` - and so
|
||||
`fr_start_view()`/`fr_send_view()` - a second time per real frame for
|
||||
each active security-camera monitor, rendering into a private software
|
||||
(`FR_DOUBLEB_MASK`) bitmap rather than the actual GL-backed screen.
|
||||
`use_opengl()` already excludes hack-camera/pickup-mode views from the
|
||||
GL texture-mapping functions (`FR_PICKUPM_MASK|FR_HACKCAM_MASK`), but
|
||||
`should_opengl_swap()` - the separate check gating
|
||||
`opengl_start_frame()`/`opengl_end_frame()` in those two functions -
|
||||
didn't, so those ran again mid-frame for a render that was never
|
||||
GL-backed. `19-android-fix-hackcam-opengl-frame.patch` adds the same
|
||||
exclusion to both call sites. This is a genuine bug fixed on its own
|
||||
merits, but bisecting further showed it wasn't actually the source of
|
||||
the 1282 spam - removing it just relocated the checkpoint that caught
|
||||
the error, without changing whether it fired.
|
||||
|
||||
**Actual root cause: binding "framebuffer 0" on Android has no valid
|
||||
target.** `opengl_end_frame()`'s `BindFrameBuffer(nullptr)` binds
|
||||
framebuffer/renderbuffer id 0 to mean "the real window surface" - true
|
||||
on desktop, where that's SDL's actual GL window. On Android, this app's
|
||||
real presentation target is never that window's own EGL surface at
|
||||
all - it's the OpenXR swapchain's per-image FBOs
|
||||
(`xr_swapchain_create()`/`xr_swapchain_acquire()`, `xr_swapchain.c`).
|
||||
Whatever framebuffer/renderbuffer 0 actually resolves to for gl4es in
|
||||
this configuration, binding it reliably set a persistent
|
||||
`GL_INVALID_OPERATION` - confirmed by bisecting through the *entire*
|
||||
render path with per-call checkpoints, all clean right up until the one
|
||||
placed immediately after this exact call. The bind is also functionally
|
||||
dead regardless of the error: by the time anything next draws, Shock.c's
|
||||
`SDLDraw()` has already run `xr_frame_begin()` →
|
||||
`xr_swapchain_acquire()`, which rebinds to that frame's real swapchain
|
||||
FBO before `opengl_swap_and_restore()` (the only thing that draws before
|
||||
the next `opengl_start_frame()`'s own `BindFrameBuffer(&backupBuffer)`)
|
||||
runs - so nothing ever actually renders while framebuffer 0 is bound.
|
||||
`20-android-no-default-framebuffer.patch` skips the call on Android
|
||||
entirely. Confirmed via logcat: this fully eliminated the error across
|
||||
an extended play session with 3D rendering active throughout, with no
|
||||
change in what actually renders.
|
||||
|
||||
An earlier theory (that the OpenXR runtime's own GL work inside
|
||||
`xrAcquireSwapchainImage`/`xrWaitSwapchainImage`/`xrEndFrame` was
|
||||
leaving stray driver-level errors on the shared EGL context) prompted a
|
||||
dlsym'd, gl4es-bypassing `glGetError()` drain at those call sites. Once
|
||||
the above was fixed, testing showed that drain was never actually
|
||||
necessary, so it was removed rather than kept as unproven defensive
|
||||
code.
|
||||
|
||||
## 6. License
|
||||
|
||||
The original tooling in this repository (the Docker build image, build
|
||||
|
||||
Reference in New Issue
Block a user