fix(3d): blank viewer after raytracing round-trip — owner-context FFP routing + VAO isolation (gl1)
Switching OpenGL → raytracing → OpenGL could leave the viewer showing only the background gradient, with mid-session "[gl1] WebGL context changed" thrash and INVALID_OPERATION storms on BOTH WebGL contexts. Traced mechanism: the shim's FFP draw routing keyed on one process-global client-array flag; an interrupted fixed-function window (MODEL_3D::BeginDrawMulti loops, or the switch-back reload re-recording display lists with the GL context lock released across JSPI suspensions) left it set, after which the raytracer blit's and the 2D GAL's glDrawArrays were routed through the FFP pipeline — and one misrouted draw permanently repointed the VICTIM's own VAO attributes at shim buffers (the blit's attribute 0 collides with ATTR_POSITION), so both stayed broken/blank even after the flag cleared. Shim fixes (wasm/gl1): - Owner-context routing gate: __wrap_glDrawArrays/Elements route into the FFP pipeline only under the shim's owner context (adopted at the first FFP client-state mutation or programSync in a context); foreign-context draws always pass through — the 2D GAL can never be misrouted and the context guard can never thrash. - VAO isolation (ScopedDefaultVAO): draw executors do their attribute setup on VAO 0 and restore the caller's binding — a misrouted draw can no longer corrupt the caller. - contextSync() resets the whole client-array mirror on a context change (enables/pointers/VBO names all described the dead context). kicad pointer bump (d6e3dc1a87a): blit preamble disables the four client arrays (same-context firewall) + DoRePaint hidden-parent early return now clears m_is_currently_painting like its six siblings (a standalone sufficient cause of a permanently blank viewer). TDD (each observed red before its fix, green after; harness = authoritative): - T1 VAO corruption, T2 foreign-context routing + guard thrash, T3 stale client-state surviving context recreation — tests/e2e/3d-webgl.spec.ts over new harness choreography (appQuad/ffpMakeStale/createSecondContext/ quadDrawFresh). Parity stays 47/47 with zero drift. - tests/kicad/3d-viewer-engine-toggle.spec.ts (new, CI-skipped like the deadlock spec): real round-trip happy-path gate — board re-renders, zero [gl1] lines, zero INVALID_OPERATION. (The raytraced image itself never displays on the wasm build — the pre-existing inert-toggle KNOWN ISSUE in 3d-viewer-deadlock.spec.ts, out of scope here; the engine switch and the poisoning reload path run regardless.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
b824eb007a
commit
daaff1f973
13 changed files with 542 additions and 8 deletions
|
|
@ -34,11 +34,28 @@ Link sites (both read `sources.txt` + `wrapped_symbols.txt`):
|
|||
|
||||
## Draw routing
|
||||
|
||||
A `glDrawArrays`/`glDrawElements` call is FFP traffic iff `GL_VERTEX_ARRAY`
|
||||
client state is enabled: only GL1 code calls `glEnableClientState`, while the
|
||||
raytracer blit (`eda_3d_canvas_wasm.cpp`) and the 2D WebGL GAL drive their own
|
||||
GLSL programs and never touch client state — their draws pass through
|
||||
untouched.
|
||||
A `glDrawArrays`/`glDrawElements` call is FFP traffic iff BOTH hold:
|
||||
|
||||
- the CURRENT WebGL context is the shim's **owner context** (`contextIsOwner()`,
|
||||
adopted at the first FFP client-state mutation or `programSync()` in a
|
||||
context — a draw under any other context is a modern-GL consumer by
|
||||
definition and always passes through, even mid-display-list-recording);
|
||||
- `GL_VERTEX_ARRAY` client state is enabled (only GL1 code calls
|
||||
`glEnableClientState`; the raytracer blit and the 2D WebGL GAL drive their
|
||||
own GLSL programs).
|
||||
|
||||
The owner gate exists because the client-state flag is process-global and an
|
||||
interrupted FFP window (e.g. `MODEL_3D::BeginDrawMulti` loops crossing a JSPI
|
||||
suspension) can leave it set — without the gate, the 2D GAL's draws got routed
|
||||
through the FFP pipeline (the engine-toggle blank-viewer bug). Two further
|
||||
hardenings from the same bug:
|
||||
|
||||
- **VAO isolation** (`ScopedDefaultVAO`, gl1_draw.cpp): draw executors bind
|
||||
VAO 0 for their attribute setup and restore the caller's binding — a routed
|
||||
draw arriving with the caller's VAO bound (the blit's attribute 0 collides
|
||||
with `ATTR_POSITION`) must never scribble it.
|
||||
- `contextSync()` resets the client-array mirror entirely on a context change:
|
||||
enables, pointers and captured VBO names all described the dead context.
|
||||
|
||||
## Invariants that are easy to break (learned from the native goldens)
|
||||
|
||||
|
|
|
|||
|
|
@ -230,6 +230,12 @@ void stateAlphaFunc( GLenum func, GLclampf ref );
|
|||
// immediate-mode state: the change can be detected mid-scene-rebuild, and
|
||||
// those modules are context-agnostic CPU state.
|
||||
void contextSync();
|
||||
// True when the CURRENT WebGL context is the shim's owner context (or no owner
|
||||
// exists yet). The __wrap_* draw interceptors gate FFP routing on this: only
|
||||
// the owner context's draws can be fixed-function traffic — a draw under any
|
||||
// other context (the 2D GAL, a recreated canvas) always passes through, no
|
||||
// matter what the global client-array mirror says. (gl1_state.cpp)
|
||||
bool contextIsOwner();
|
||||
// Per-TU cache drops invoked by contextSync() on a context change.
|
||||
void shadersDropContextObjects(); // FFP program + uniform locations + fail latch
|
||||
void drawDropContextObjects(); // stream/scratch VBOs
|
||||
|
|
|
|||
|
|
@ -40,11 +40,40 @@ enum
|
|||
};
|
||||
|
||||
|
||||
// The shim's attribute setup assumes the DEFAULT VAO (see the VAO-policy note
|
||||
// above) — but a routed draw can arrive with the CALLER's VAO bound (the
|
||||
// raytracer blit and the 2D GAL both bind their own VAO right before drawing,
|
||||
// and their attribute 0 collides with ATTR_POSITION). Scribbling attribute
|
||||
// state into that VAO corrupts the caller PERMANENTLY — the engine-toggle
|
||||
// blank-viewer amplifier. Every draw executor therefore isolates itself on
|
||||
// VAO 0 and restores the caller's binding afterwards.
|
||||
class ScopedDefaultVAO
|
||||
{
|
||||
public:
|
||||
ScopedDefaultVAO()
|
||||
{
|
||||
glGetIntegerv( GL_VERTEX_ARRAY_BINDING, &m_prev );
|
||||
if( m_prev != 0 )
|
||||
glBindVertexArray( 0 );
|
||||
}
|
||||
~ScopedDefaultVAO()
|
||||
{
|
||||
if( m_prev != 0 )
|
||||
glBindVertexArray( (GLuint) m_prev );
|
||||
}
|
||||
|
||||
private:
|
||||
GLint m_prev = 0;
|
||||
};
|
||||
|
||||
|
||||
void drawImmVertices( GLenum mode, const ImmVertex* verts, GLsizei count )
|
||||
{
|
||||
if( !programSync() )
|
||||
return;
|
||||
|
||||
ScopedDefaultVAO vaoGuard;
|
||||
|
||||
if( !s_streamVBO )
|
||||
glGenBuffers( 1, &s_streamVBO );
|
||||
|
||||
|
|
@ -203,6 +232,8 @@ void drawArraysWithSources( GLenum mode, GLsizei count, const AttribSource aSrc[
|
|||
return;
|
||||
}
|
||||
|
||||
ScopedDefaultVAO vaoGuard;
|
||||
|
||||
GLint prevArrayBuffer = 0;
|
||||
glGetIntegerv( GL_ARRAY_BUFFER_BINDING, &prevArrayBuffer );
|
||||
|
||||
|
|
|
|||
|
|
@ -407,6 +407,15 @@ void glColorMaterial( GLenum face, GLenum mode )
|
|||
|
||||
static ClientArray* clientArraySlot( GLenum cap )
|
||||
{
|
||||
// FFP client-state mutation is the earliest signal that fixed-function
|
||||
// code is working in the CURRENT context — adopt/refresh ownership here,
|
||||
// BEFORE the mutation lands in the mirror. This is what moves ownership
|
||||
// to a recreated 3D context (its scene rebuild calls gl*Pointer /
|
||||
// glEnableClientState long before its first draw), and what guarantees a
|
||||
// context change wipes stale client-array state before new state is
|
||||
// recorded. Only FFP callers reach these names; the 2D GAL never does.
|
||||
contextSync();
|
||||
|
||||
State& s = S();
|
||||
|
||||
switch( cap )
|
||||
|
|
@ -456,6 +465,7 @@ static GLuint currentArrayBufferBinding()
|
|||
|
||||
void glVertexPointer( GLint size, GLenum type, GLsizei stride, const GLvoid* ptr )
|
||||
{
|
||||
contextSync(); // see clientArraySlot()
|
||||
ClientArray& a = S().clientArrays[CA_VERTEX];
|
||||
a.size = size;
|
||||
a.type = type;
|
||||
|
|
|
|||
|
|
@ -78,6 +78,18 @@ void __wrap_glGetFloatv( GLenum pname, GLfloat* params )
|
|||
|
||||
void __wrap_glDrawArrays( GLenum mode, GLint first, GLsizei count )
|
||||
{
|
||||
// Only the shim's owner context can carry FFP traffic. A draw under any
|
||||
// other context (the 2D GAL, the raytracer blit after a canvas swap) is a
|
||||
// modern-GL consumer by definition — pass it through even if the global
|
||||
// client-array mirror was left enabled by an interrupted FFP window, and
|
||||
// even while a display list is recording (a foreign draw must never be
|
||||
// swallowed into the owner's open list).
|
||||
if( !contextIsOwner() )
|
||||
{
|
||||
__real_glDrawArrays( mode, first, count );
|
||||
return;
|
||||
}
|
||||
|
||||
if( dlistRecording() )
|
||||
{
|
||||
dlistRecordDrawArrays( mode, first, count );
|
||||
|
|
@ -98,6 +110,13 @@ void __wrap_glDrawArrays( GLenum mode, GLint first, GLsizei count )
|
|||
|
||||
void __wrap_glDrawElements( GLenum mode, GLsizei count, GLenum type, const GLvoid* indices )
|
||||
{
|
||||
// Same owner-context gate as __wrap_glDrawArrays.
|
||||
if( !contextIsOwner() )
|
||||
{
|
||||
__real_glDrawElements( mode, count, type, indices );
|
||||
return;
|
||||
}
|
||||
|
||||
if( dlistRecording() )
|
||||
{
|
||||
GL1_WARN_ONCE( "glDrawElements inside glNewList is not supported — dropped" );
|
||||
|
|
|
|||
|
|
@ -41,6 +41,16 @@ static int currentContextId()
|
|||
// first contextSync() under a live context.
|
||||
static int s_ownerContext = 0;
|
||||
|
||||
bool contextIsOwner()
|
||||
{
|
||||
int cur = currentContextId();
|
||||
|
||||
// No live context: nothing can be drawn anyway. No owner yet: the first
|
||||
// FFP consumer under any context becomes the owner (adopted in
|
||||
// contextSync() on its first programSync()).
|
||||
return cur != 0 && ( s_ownerContext == 0 || cur == s_ownerContext );
|
||||
}
|
||||
|
||||
void contextSync()
|
||||
{
|
||||
int cur = currentContextId();
|
||||
|
|
@ -61,8 +71,13 @@ void contextSync()
|
|||
State& s = S();
|
||||
s.boundTexture2D = 0;
|
||||
|
||||
// Client-array state dies with its context: the enables, the captured
|
||||
// VBO names AND the CPU pointers all described the dead context's
|
||||
// world. Keeping `.enabled` was the engine-toggle bug's tail — a flag
|
||||
// left set by the old context kept routing modern-GL draws through the
|
||||
// FFP pipeline in the new one.
|
||||
for( int i = 0; i < CA_COUNT; ++i )
|
||||
s.clientArrays[i].boundBuffer = 0;
|
||||
s.clientArrays[i] = ClientArray();
|
||||
|
||||
// The new program starts with default-initialized uniforms; force a
|
||||
// full re-upload on its first sync.
|
||||
|
|
|
|||
Loading…
Reference in a new issue