subagent transcript
session 18e25685 · workflow wf_23df5b1c
delegated task
You are an adversarial verifier for a FreeCAD-wasm code review. A reviewer claims: SEVERITY: low CATEGORY: correctness-edge FILE: src/Gui/WasmGLFixedF
▸ Magik
You are an adversarial verifier for a FreeCAD-wasm code review. A reviewer claims:
SEVERITY: low
CATEGORY: correctness-edge
FILE: src/Gui/WasmGLFixedFunc.cpp
SUMMARY: Client-array indexed-draw sizing references F.arrays.vertex._count which is never assigned, so the VBO-indices-with-client-vertices fallback can under-size the gather and corrupt/miss geometry.
DETAIL/EVIDENCE: At line 179, when indices live in a bound ELEMENT_ARRAY_BUFFER but vertices are client memory, maxIndex is set from F.arrays.vertex._count (never set anywhere), falling back to count-1. If the true max index exceeds count-1 the scratch Float32Array (line 150) is too small and the gather reads past it (NaN) or omits vertices. Niche combination (Coin usually keeps indices and vertices together) but a real latent bug. FIX: read back the element buffer (getBufferSubData) for the true max index, or record the vertex count in ffPointer and store it as _count.
RECOMMENDATION: (none)
Independently verify by reading the actual file(s) and running git/grep as needed (repo at /home/magik6k/lcad-wasm/freecad-port/FreeCAD, toolchain at /opt/toolchains, deploy at /home/magik6k/lcad-wasm/freecad-port/deploy-gui). Default to REFUTED if you cannot confirm the evidence. Adjust severity to what the evidence actually supports.
Fable
thinking
I should check the file to confirm that.
Fable
⚙ Read {"file_path": "/home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/Gui/WasmGLFixedFunc.cpp", "offset": 120, "limit": 120}
Fable
thinking
I need to check if `_count` is actually assigned anywhere in the code, since line 179 references it but it might never be initialized.
Fable
⚙ Bash {"command": "grep -n \"_count\" /home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/Gui/WasmGLFixedFunc.cpp", "description": "grep for _count usages"}
Fable
thinking
I'm checking where `_count` gets referenced and whether it's ever actually assigned anywhere in the `F.arrays.vertex` definition.
Fable
⚙ Bash {"command": "grep -n \"arrays\\|\\.vertex\\|count\\b\\|glbuf\\|\\.on\\b\\|ptr\\b\\|stride\\|size\\b\" /home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/Gui/WasmGLFixedFunc.cpp | head -60", "description": "grep for arrays and vertex spec fiel…
Fable
thinking
I'm checking the ffPointer function to verify that `_count` isn't being initialized anywhere in that section.
Fable
⚙ Read {"file_path": "/home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/Gui/WasmGLFixedFunc.cpp", "offset": 345, "limit": 20}
Fable
thinking
So `_count` is never actually defined on the vertex spec object—it only has properties like `on`, `size`, `type`, `stride`, `ptr`, `vbo`, and `glbuf`. That means `F.arrays.vertex._count` is always undefined, causing the ternary to default to `count-1`. Now I'm tracing through the actual bug scenario where we have vertices in client memory and indices in a buffer.
When `isElements && vertClient` is true, `F._maxIndex` gets set to `count-1` since `_count` is undefined. Then in the bind phase, `maxV` becomes `count`, and the scratch array is sized to `count*sz`. But here's the problem: for indexed drawing, `count` is the number of indices being drawn, not the number of vertices. The actual max …
Fable
I've verified all the load-bearing claims. Let me confirm my findings.
The evidence checks out:
1. **Line 179** reads `F.arrays.vertex._count ? F.arrays.vertex._count-1 : count-1` — confirmed.
2. **`_count` is never assigned anywhere.** `grep -n "_count"` returns only line 179 (the read). The vertex spec object is defined at line 65 as `{on,size,type,stride,ptr,vbo}` and populated in `ffPointer` (lines 351-357), which sets only `size,type,stride,ptr,glbuf`. No `_count` property is ever written, so the ternary always evaluates the falsy branch and falls back to `count-1`.
3. **The under-sizing path is real.** When `isElements && vertClient && elemBuf` (indices in a bound ELEMENT_ARRAY_BUFFER, vertices in client memory), `F._maxIndex = count-1`, so `maxV = count` (line 149) and the scratch `Float32Array` is `count*sz` (line 150). For indexed draws, `count` is the number of *indices*, not vertices; the true maximum vertex index can exceed `count-1` (e.g., 3 indices {5,100,200}). The gather then produces only `count` vertices, and `drawElements` at line 193 reads the VBO-resident indices which reference beyond the gathered range → garbage/NaN or missing geometry.
4. The developer explicitly acknowledged this as an assumption in the comment at lines 177-178.
Minor imprecision in the write-up: the gather loop itself stays within its own `maxV` bound; the corruption actually happens at the subsequent `drawElements` reading past the under-sized gathered VBO. Net effect is the same (corrupt/missing geometry), so this doesn't change the verdict.
Severity: low is appropriate — the triggering combination (VBO indices + client vertices) is a niche mix, and in the common indexed case (vertices reused, indices > vertex count) `count-1` happens to over-size safely. It's a genuine latent bug but rarely hit.
Fable
⚙ StructuredOutput {"verdict": "CONFIRMED", "reason": "Verified against src/Gui/WasmGLFixedFunc.cpp. Line 179 references F.arrays.vertex._count, and grep confirms _count is never assigned anywhere in the file; the vertex spec (line 65) and its only writer ffP…