pcbjam/docs/features/browser-tools/0003-wxauitoolbar-registration-fix.md

304 lines
14 KiB
Markdown
Raw Normal View History

wip: nested-asyncify fix, wxAuiToolBar registration, tests, research docs Main-repo side of a multi-part WIP covering the KiCad WASM tool-selection and nested-Asyncify work. Submodule commits are in kicad@f6e9239aaa (libcontext hygiene) and wxwidgets@bb80f91e8b (auibar registration + dialog diagnostics). ## scripts/common/inject-dyncall-shims.sh Wrap Asyncify.handleSleep / allocateData to save-and-restore Asyncify.currData around each EM_ASYNC_JS sleep. This fixes the nested Asyncify collision where a fiber swap that fired during a modal's event loop clobbered currData, and the modal's later doRewind used the fiber's buffer and hit "RuntimeError: index out of bounds". Root cause documented as Emscripten Issue #9153 (wontfix upstream). Diagnostic-rewind logging (forcedBottomOfCallStack, callStack traces) is retained to help future debugging of Asyncify state corruption. ## tests/ - tests/playwright-kicad.config.ts: add `channel: 'chrome'` for the chromium project so --project=chromium --headed uses system Chrome (real GPU) instead of SwiftShader on ARM Mac. Also switch trace to retain-on-failure + screenshot on-failure for easier E2E debugging. - tests/kicad/pcbnew.spec.ts: replace `tool.checked` assertions with a label-suffix check (`[checked]`) since our auibar registration encodes checked state in the label (no schema change to the registry). - tests/apps/Makefile.wasm: add `coroutine-nested` build target + include it in the all: list. - tests/apps/standalone/coroutine/: kicad_coroutine_harness.h + test app reproducing KiCad COROUTINE semantics against real libcontext. - tests/apps/standalone/coroutine-nested/: nested_test.cpp reproduces the EM_ASYNC_JS-modal + fiber-swap nesting bug in isolation. 8 scenarios from baseline_modal_alone through nested_fibers_inside_modal. - tests/e2e/coroutine.spec.ts + coroutine-nested.spec.ts: Playwright specs that load the standalone apps and assert all case cases pass via [COROUTINE_TEST] SUMMARY log parsing. ## research/ and features/browser-tools/ Three background docs capturing the investigation trajectory: - features/browser-tools/0001-kicad-wasm-tool-activation-investigation.md Early investigation: why tools don't activate; initial dynCall-empty- callback hypothesis. - features/browser-tools/0002-wasm-coroutine-deep-dive.md Deep dive on Asyncify internals, fiber API, QEMU's coroutine-wasm reference implementation. - features/browser-tools/0003-wxauitoolbar-registration-fix.md The narrow fix: why wxAuiToolBar needs a registration block, where to add it, what the fallback plan is. - research/threading_1.md: corrected root-cause analysis after reading runtime logs — nested-Asyncify currData collision, Emscripten #9153. - research/threading_2.md: extended research on alternative approaches (JSPI/WasmFX/state-machines) and why they don't help here. ## Submodule pointer updates kicad: f6e9239aaa (wip: libcontext WASM hygiene cleanup) wxwidgets: bb80f91e8b (wip: wxAuiToolBar element-registry registration + dialog diagnostics) ## Open threads not yet in scope - Firefox/Chrome divergent behavior: "indirect call signature mismatch" traps in Firefox vs renderer crash in system Chrome (tracked in plans/peaceful-hugging-pnueli.md and the research docs). - E2E pixel-diff for Draw Lines fails because the test's diff region does not cover where the line is actually drawn; tool activation works, the line is visible in test-results/pcbnew-draw-lines-02-after-drawing.png. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 13:58:16 +02:00
# wxAuiToolBar Registration — Tool Selection Fix
## Context
2026-06-05 12:15:54 +02:00
The nested Asyncify collision bug (see `0002-wasm-coroutine-deep-dive.md` and `../../research/threading_1.md`) is fixed. KiCad WASM now loads through the startup wizard without crashing, and the full PCBnew UI renders — menus, left drawing-tool sidebar with Line/Circle/Rectangle icons, layer panel, PCB canvas — all visible.
wip: nested-asyncify fix, wxAuiToolBar registration, tests, research docs Main-repo side of a multi-part WIP covering the KiCad WASM tool-selection and nested-Asyncify work. Submodule commits are in kicad@f6e9239aaa (libcontext hygiene) and wxwidgets@bb80f91e8b (auibar registration + dialog diagnostics). ## scripts/common/inject-dyncall-shims.sh Wrap Asyncify.handleSleep / allocateData to save-and-restore Asyncify.currData around each EM_ASYNC_JS sleep. This fixes the nested Asyncify collision where a fiber swap that fired during a modal's event loop clobbered currData, and the modal's later doRewind used the fiber's buffer and hit "RuntimeError: index out of bounds". Root cause documented as Emscripten Issue #9153 (wontfix upstream). Diagnostic-rewind logging (forcedBottomOfCallStack, callStack traces) is retained to help future debugging of Asyncify state corruption. ## tests/ - tests/playwright-kicad.config.ts: add `channel: 'chrome'` for the chromium project so --project=chromium --headed uses system Chrome (real GPU) instead of SwiftShader on ARM Mac. Also switch trace to retain-on-failure + screenshot on-failure for easier E2E debugging. - tests/kicad/pcbnew.spec.ts: replace `tool.checked` assertions with a label-suffix check (`[checked]`) since our auibar registration encodes checked state in the label (no schema change to the registry). - tests/apps/Makefile.wasm: add `coroutine-nested` build target + include it in the all: list. - tests/apps/standalone/coroutine/: kicad_coroutine_harness.h + test app reproducing KiCad COROUTINE semantics against real libcontext. - tests/apps/standalone/coroutine-nested/: nested_test.cpp reproduces the EM_ASYNC_JS-modal + fiber-swap nesting bug in isolation. 8 scenarios from baseline_modal_alone through nested_fibers_inside_modal. - tests/e2e/coroutine.spec.ts + coroutine-nested.spec.ts: Playwright specs that load the standalone apps and assert all case cases pass via [COROUTINE_TEST] SUMMARY log parsing. ## research/ and features/browser-tools/ Three background docs capturing the investigation trajectory: - features/browser-tools/0001-kicad-wasm-tool-activation-investigation.md Early investigation: why tools don't activate; initial dynCall-empty- callback hypothesis. - features/browser-tools/0002-wasm-coroutine-deep-dive.md Deep dive on Asyncify internals, fiber API, QEMU's coroutine-wasm reference implementation. - features/browser-tools/0003-wxauitoolbar-registration-fix.md The narrow fix: why wxAuiToolBar needs a registration block, where to add it, what the fallback plan is. - research/threading_1.md: corrected root-cause analysis after reading runtime logs — nested-Asyncify currData collision, Emscripten #9153. - research/threading_2.md: extended research on alternative approaches (JSPI/WasmFX/state-machines) and why they don't help here. ## Submodule pointer updates kicad: f6e9239aaa (wip: libcontext WASM hygiene cleanup) wxwidgets: bb80f91e8b (wip: wxAuiToolBar element-registry registration + dialog diagnostics) ## Open threads not yet in scope - Firefox/Chrome divergent behavior: "indirect call signature mismatch" traps in Firefox vs renderer crash in system Chrome (tracked in plans/peaceful-hugging-pnueli.md and the research docs). - E2E pixel-diff for Draw Lines fails because the test's diff region does not cover where the line is actually drawn; tool activation works, the line is visible in test-results/pcbnew-draw-lines-02-after-drawing.png. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 13:58:16 +02:00
But tools still don't work end-to-end:
- **User observation**: clicking the Draw Lines tool in the browser doesn't visibly select it or make it function.
- **E2E test**: `select draw lines and draw on the board` can't even attempt a click — it fails earlier at `wxElementRegistry.findAllRendered({ elementType: 'tool' })` because the returned array is empty.
Both signals point at the same gap: **wxAuiToolBar never registers its tools with the rendered-element registry**. The intended outcome of this fix is to make all wxAuiToolBar buttons (Draw Lines and siblings) clickable, selectable, and functional in the browser build.
---
## Root Cause
### The registry and how it gets populated
`window.wxElementRegistry` is a JS-side registry of UI elements used by Playwright tests to find controls by type/label/tooltip. For elements that are not standalone wxWindow instances (e.g., toolbar buttons rendered as pixels on a parent canvas), wxWidgets calls a C++ bridge `WasmRegisterRenderedElement()` which invokes the JS helper `wxRenderedElementRegister()`.
Canonical definition: `wxwidgets/src/wasm/window.cpp:182-214`:
```cpp
void WasmRegisterRenderedElement(
wxWindow* parent,
const char* elementType, // "tool", "menuitem", "sash", "auipart", ...
const char* subType,
int index,
const wxString& label,
const wxString& tooltip,
int screenX, int screenY,
int width, int height,
bool enabled);
```
### Where it's currently called
Grepping `wxwidgets/src/` for `WasmRegisterRenderedElement`:
| File | What it registers |
|---|---|
| `src/univ/toolbar.cpp:557-607` | Regular wxToolBar items (in `RecalcToolBitmapCache`) |
| `src/univ/menu.cpp` | Menu bar items, popup menu items |
| `src/aui/framemanager.cpp:2687-2787` | AUI pane captions, close/pin/maximize buttons |
| `src/aui/tabart.cpp:392,1137` | Tab headers |
| `src/univ/textctrl.cpp:4304-4319` | Text control segments |
| `src/propgrid/propgrid.cpp:2509-2537` | Property grid rows |
| `src/stc/stc.cpp:5203-5213` | Styled text cells |
### The gap
`wxwidgets/src/aui/auibar.cpp` has **zero** `__EMSCRIPTEN__` blocks and **zero** calls to `WasmRegisterRenderedElement`. Verified:
```
$ grep -nE "__EMSCRIPTEN__|WasmRegister" wxwidgets/src/aui/auibar.cpp
(no matches)
$ git log --oneline -n 10 src/aui/auibar.cpp
# Only upstream wxWidgets commits — our fork hasn't modified this file.
```
KiCad's left drawing-tool sidebar is a `wxAuiToolBar` (not `wxToolBar`), which is why its tools are invisible to the registry.
### Test log evidence
From `tests/logs/kicad/pcbnew/pcbnew-spec-ts-pcbnew-wasm-select-draw-lines-and-draw-on-the-board.log`:
```
[TEST] rendered summary {"count":42,"byType":{
"searchctrl":3,"searchbutton":4,"sash":2,"auipart":4,
"combobutton":8,"combotextarea":8,"textctrl":1,"tab":3,"menuitem":9
},"tools":[]}
```
Everything else registers. `tools` is the only empty bucket.
### The log is otherwise clean
Filtering out diagnostic output (`WASM_FCONTEXT`, `DIAG_*`, `wxLog DEBUG`) leaves only three substantive lines, all informational:
```
[DIAG_SHOWMODAL] About to call startModal()
Debug: EndModal: 5100
[DIAG_SHOWMODAL] startModal() returned 5100
```
No exceptions, no crashes, no fiber errors, no `jump-ghost`, `main_refresh=1` stable. The nested Asyncify fix is holding. **The only remaining issue between "UI loads" and "tool works" is this registration gap.**
### Separating the two signals
The registry gap directly explains the **test failure**. It does NOT directly explain the **user's manual observation** — registry population is test-only infrastructure and has no effect on in-browser interactivity.
Hypothesis: once tools are registered, the test will click the tool via coordinates from the registry and produce a log that either shows the click succeeded (no bug — user's "doesn't select" was a display misreading because state wasn't exposed) or shows concrete failure evidence (real activation bug, e.g., another variant of coroutine/asyncify interaction). Either way, the registration fix is strictly additive and unblocks diagnosis.
---
## The Fix
### Summary
Two small changes, one new block:
1. **Extend the registry signature** to include `checked` state (needed so the test can verify selection).
2. **Add a registration block** to `wxAuiToolBar::OnPaint()` following the `univ/toolbar.cpp` pattern.
3. **Update existing callers** to pass a `checked` value (`false` for non-toggleable, real state for `wxItemCheck/wxItemRadio`).
### Change 1 — extend `WasmRegisterRenderedElement` signature
**File: `wxwidgets/src/wasm/window.cpp`** (function at line 182)
Add a `bool checked` parameter and pass it through to the JS helper:
```cpp
void WasmRegisterRenderedElement(
wxWindow* parent,
const char* elementType,
const char* subType,
int index,
const wxString& label,
const wxString& tooltip,
int screenX, int screenY,
int width, int height,
bool enabled,
bool checked) // ← NEW
{
if (!parent) return;
uintptr_t parentId = reinterpret_cast<uintptr_t>(parent);
EM_ASM({
var id = $0.toString() + ':' + UTF8ToString($1) + ':' + $2;
wxRenderedElementRegister(
id,
$0.toString(),
UTF8ToString($1), // elementType
UTF8ToString($3), // subType
UTF8ToString($4), // label
UTF8ToString($5), // tooltip
$6, $7, $8, $9, // x, y, w, h
$10 ? true : false, // enabled
$2, // index
$11 ? true : false // ← checked
);
},
parentId, elementType, index, subType,
label.utf8_str().data(), tooltip.utf8_str().data(),
screenX, screenY, width, height,
enabled, checked);
}
```
**File: `wxwidgets/build/wasm/wx.js`** (helper at line 297)
```javascript
function wxRenderedElementRegister(
id, parentId, elementType, subType,
label, tooltip, screenX, screenY, width, height,
enabled, index, checked) // ← NEW
{
if (window.wxElementRegistry) {
window.wxElementRegistry.registerRendered(id, {
id, parentId, elementType, subType,
label, tooltip,
screenX, screenY, width, height,
centerX: screenX + Math.floor(width / 2),
centerY: screenY + Math.floor(height / 2),
enabled,
index,
checked: !!checked, // ← NEW
lastUpdated: Date.now()
});
}
}
```
Also update `wxRenderedElementUpdate` (same file, around line 321) similarly, so subsequent updates can change `checked`.
### Change 2 — register wxAuiToolBar tools
**File: `wxwidgets/src/aui/auibar.cpp`** (inside `OnPaint`, after the main item-paint loop, before the overflow paint at line ~2501)
```cpp
#ifdef __EMSCRIPTEN__
// Update element registry with toolbar tools (for E2E test automation).
// Runs after every paint so state (enabled/checked, layout) stays current.
extern void WasmRegisterRenderedElement(
wxWindow* parent, const char* elementType, const char* subType,
int index, const wxString& label, const wxString& tooltip,
int screenX, int screenY, int width, int height,
bool enabled, bool checked);
extern void WasmUnregisterRenderedElementsByParent(wxWindow* parent);
WasmUnregisterRenderedElementsByParent(this);
wxPoint screenPos = GetScreenPosition();
for (size_t j = 0, itemCount = m_items.GetCount(); j < itemCount; ++j)
{
wxAuiToolBarItem& item = m_items.Item(j);
if (!item.m_sizerItem)
continue;
if (item.m_kind == wxITEM_SEPARATOR)
continue;
wxRect itemRect = item.m_sizerItem->GetRect();
// Skip items scrolled off the end (match the paint loop's cutoff)
if ((horizontal && itemRect.x + itemRect.width >= last_extent) ||
(!horizontal && itemRect.y + itemRect.height >= last_extent))
continue;
const char* subType = (item.m_kind == wxITEM_CONTROL) ? "control" : "button";
bool isEnabled = !(item.m_state & wxAUI_BUTTON_STATE_DISABLED);
bool isChecked = (item.m_state & wxAUI_BUTTON_STATE_CHECKED) != 0;
WasmRegisterRenderedElement(
this,
"tool",
subType,
static_cast<int>(j),
item.m_label,
item.m_shortHelp,
screenPos.x + itemRect.x,
screenPos.y + itemRect.y,
itemRect.width,
itemRect.height,
isEnabled,
isChecked
);
}
#endif
```
Notes:
- `item.m_label` / `item.m_shortHelp` are the verified field names (`wxwidgets/include/wx/aui/auibar.h:231,235`).
- `wxAUI_BUTTON_STATE_CHECKED` is already how `wxAuiToolBar::OnLeftUp` tracks toggle state (see line 2676 `m_actionItem->m_state & wxAUI_BUTTON_STATE_CHECKED`).
- Placing the block inside OnPaint means every repaint refreshes the registry, which keeps `checked`/`enabled` state synchronized with visible state without needing a separate update path.
### Change 3 — update existing callers to pass `checked`
Every existing `WasmRegisterRenderedElement` call must pass a new final arg. Most don't have meaningful checked state:
- `src/univ/menu.cpp` — pass `false` (or `menuItem->IsChecked()` for check-menu-items, already available)
- `src/aui/framemanager.cpp` (pane parts) — pass `false`
- `src/aui/tabart.cpp` — pass `false` for non-selected tabs, `true` for the active tab (`page.active`)
- `src/univ/textctrl.cpp`, `src/propgrid/propgrid.cpp`, `src/stc/stc.cpp` — pass `false`
- `src/univ/toolbar.cpp` — pass `tool->IsToggled()` (real value for the regular wxToolBar path)
This is a small mechanical change: add `, false` (or the appropriate value) to each existing call site.
---
## Verification
### Build
1. **wxWidgets standalone build** (fast): `./scripts/build-wxuniversal-wasm.sh`
2. **KiCad rebuild** (needed because KiCad statically links wxWidgets; this is the slow step): `./docker/build.sh`
3. **Setup test artifacts**: handled automatically by `npm run test:kicad`'s `setup:kicad` step.
### Run
```bash
cd tests
npm run test:kicad
```
Expectations for `pcbnew.spec.ts`:
- **Test 1** (`click through setup wizard to load PCBnew`): still passes (already passing, unaffected by this change).
- **Test 2** (`select draw lines and draw on the board`): now proceeds past the `findAllRendered` poll. Three possible outcomes:
1. **Passes fully** — tools were simply invisible to the test before; the user's "doesn't select" manual report was a display misreading (likely state changed but they didn't see the visual update, or they tested a stale build).
2. **Fails at the `checked` poll (5s)** — tool renders, click reaches it, but activation path (ACTION_TOOLBAR → TOOL_MANAGER → coroutine) has a real functional bug. Follow up using the log.
3. **Fails at the initial `findAllRendered` poll still** — registration isn't firing; something wrong with the build/binding. Debug by inspecting the generated `pcbnew.js` for the new signature.
### Diagnostic signals in the log
After the click, watch for these patterns:
- `[WASM_FCONTEXT] entry-call ctx=…` new fiber created after click → activation coroutine started. Any subsequent failure is in tool logic, not plumbing.
- No fiber activity at all after the click → click didn't route to ACTION_TOOLBAR. Suspect event routing through the canvas (`wxwidgets/src/wasm/window.cpp` mouse handlers, possibly `kicad/common/gal/webgl/webgl_gal.cpp` which has a WASM-specific uncommitted change).
- Fiber starts but never yields / doesn't hit the tool's `Wait()` loop → similar class of coroutine bug to the Asyncify fix, but different trigger.
### Follow-up scenarios
If the test still fails after this fix, use the above signals to narrow to:
- **Rendering-only**: `Refresh(false); Update()` already runs in `wxAuiToolBar::OnLeftUp` at line 26832684, so this is unlikely; but if the registry updates yet the canvas visibly doesn't, something is suppressing paint.
- **Coroutine activation**: new variant of nested-asyncify (maybe menu → tool → dialog nesting). Extend `coroutine-nested` standalone test with the matching scenario.
- **Event routing**: audit the DOM-event → wxWidgets-event bridge. If clicks on coordinates in the canvas aren't reaching wxAuiToolBar, the bridge has regressed.
---
## Files Touched
| File | Change |
|---|---|
| `wxwidgets/src/wasm/window.cpp` | Add `bool checked` param to `WasmRegisterRenderedElement` signature |
| `wxwidgets/build/wasm/wx.js` | Add `checked` arg to `wxRenderedElementRegister` and `wxRenderedElementUpdate` JS helpers |
| `wxwidgets/src/aui/auibar.cpp` | **NEW** registration block in `OnPaint()` (~30 lines in `#ifdef __EMSCRIPTEN__`) |
| `wxwidgets/src/univ/toolbar.cpp` | Pass `tool->IsToggled()` as new final arg |
| `wxwidgets/src/univ/menu.cpp` | Pass `false` (or `IsChecked()` for check items) |
| `wxwidgets/src/aui/framemanager.cpp` | Pass `false` |
| `wxwidgets/src/aui/tabart.cpp` | Pass `page.active` where appropriate, else `false` |
| `wxwidgets/src/univ/textctrl.cpp`, `propgrid/propgrid.cpp`, `stc/stc.cpp` | Pass `false` |
Net: +~50 lines of new code, ~8 files touched. The wxWidgets fork drift grows by one localized patch — no protocol or architectural change.