fix(ui): app_quit, resize callback ABI, and panel sizing - #100
Merged
Conversation
… its siblings Two reported bugs. #93 there was no way for an application to stop its own run loop. window_close indexed extra_windows directly, so handle 1 meant the first EXTRA window and the primary one had no handle at all, while window_count, window_title and window_is_open all treat 1 as the primary. Nothing else in the exported surface stopped the loop. The cost was that any bounded run had to be killed from outside, so `timeout 60 ./app` always exited 124 and could not distinguish "finished its work" from "hung" — a CI step that renders N frames, writes its evidence and should exit 0 reported failure on success. ui.app_quit() now stops the loop on all three backends: -stop: plus a posted event on AppKit (stop: is only examined when the loop next comes round), g_application_quit on an idle source for GTK4, and a WM_QUIT posted to the loop's own thread on win32, since PostQuitMessage only affects the calling thread. aether_ui_park_until_killed, which a headless or tray-only app parks on, polls the same shared flag instead of sleeping a minute at a time. window_close(1) now addresses the primary window, consistent with the rest of the window family. #94 canvas_on_resize was the only one of the five canvas callbacks passing its closure ints; on_click, on_move, on_release and on_scroll all pass doubles. A closure written the way the siblings are written read the floating-point argument registers while the caller had filled the integer ones, so it compiled, ran, and delivered garbage with no diagnostic. In a 3D viewport sizing its render target from those values, the viewport silently never resized. It now passes doubles on all three backends; vg/live.ae, the one in-tree caller, takes floats to match. ci.sh gains a phase each: 5e20 asserts the resize closure receives real values through the driver, and 5e21 asserts quit_demo exits 0 with its run loop returned, which is exactly the property #93 was missing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#95 reported two related gaps found building a three-panel editor. split_set_position moved the INNER of two nested splitviews and did nothing to the outer one: asked for 240, got 114, which is the left pane's content width. The implementation looked right, and it was: -setPosition:ofDividerAtIndex: does move the divider, and then the next layout pass re-derives the pane's width from its content and puts it back. An inner split appeared to work only because its pane had nothing to derive from. The call reported nothing and silently did not happen. Pinning the first pane's size is what actually holds, so that is what split_set_position now does in addition to setting the divider. Measured on the reported shape, a 1480-wide window with a populated left panel: 1368 before, 240 after. The second gap was that nothing could give a widget a width or ask what width it had, so with the divider unavailable there was no other lever: st_insets does not change a button's intrinsic width. set_width / set_height / get_width / get_height now exist on all three backends. The setter is a real constraint on AppKit, since that is the only thing that survives a layout pass; a size request with expand off on GTK4; the preferred size the layout pass reads on win32. The getters answer from the ALLOCATION, so they report what a widget actually got rather than what was asked for. ci.sh phase 5e22 builds the reported three-panel shape, reads the left panel's allocation back through get_width and asserts 240. It needs no driver: the app ends itself with app_quit, which is the other half of this batch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI caught three things, two of them mine. window_close numbering: reverted. window_create returns 1 for the first EXTRA window, so window_close(1) has always meant that, and repointing it at the primary silently sent every existing create/close pair at the wrong window — the multiwindow spec said so immediately. The driver-facing family (window_count, window_title, window_is_open) does number 1 as the primary, which is a real inconsistency, but reconciling it is a breaking change and not what #93 needed. app_quit is. The two new phases used `timeout`, which macOS does not ship (rc=127), and launched directly, which on Linux gives GTK4 no framebuffer ("Failed to open display", rc=1). run_self_quitting waits on the process, kills it only if it overruns, and goes through $LAUNCH_PREFIX like every other phase. split_set_position: left exactly as it was. Pinning the pane with a constraint did put the divider at 240, and also pinned the pane against the WINDOW, so split_demo could no longer resize its window to 640 — a worse bug than the one being fixed. Holding priority instead collapsed the pane to zero. The AppKit behaviour is now documented on the verb, honestly, and set_width is the supported way to state a panel width. It gives 240 on the reported shape with no effect on anything else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ayout My #94 edit landed on the wrong function. The macOS backend has two closure invocations spelled `c->fn`, and a plain text match took the first: on_layout, whose callers all declare |w: int, h: int|. So the callback that was already correct started handing doubles to int closures, and the one the issue was about kept passing ints. CI said so precisely — split_demo, wrap, GeometryReader and the projected Spin toggle all fail the moment on_layout's contract changes, and all four are window-resize specs. That is the failure being reported from the other side. on_layout is back to ints, and the canvas on_resize site now passes doubles, which is what #94 asked for. GTK4 and win32 were never wrong here: those edits matched `on_resize->fn` explicitly rather than a bare `c->fn`. The spec is stronger too. It passed while the ABI was still wrong, which is its own bug: one plausible-looking number proves nothing, because garbage read from the wrong argument registers can look plausible once. It now drives TWO different resizes and requires the reported width to track each one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…fails GTK4 was the only leg still failing, and the failure output showed four lines of unrelated GTK warnings rather than the measurement, so it said nothing about why. The two backends hold a panel by different means. GtkPaned honours the divider position; AppKit re-derives a populated pane's width from its content on the next layout pass and needs the width stated outright, which is the whole of #95. The demo now says both, which is also what a real app wanting a fixed panel would write, and the phase prints the width line on failure instead of whatever happened to be first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
panelsize_demo reported a left panel width of 0 on GTK4. The phase forced AETHER_UI_HEADLESS=1, and GTK4's headless path realizes without mapping, so no allocation ever happens and get_width honestly answers 0. Every other GTK4 spec passes because run_server_test does NOT set headless: they run under xvfb with a mapped window. So headless is now used only when there is no display to use. Under xvfb the window maps and the allocation is real; on macOS, where there is no xvfb, headless still keeps a local run from popping windows and the allocation is real anyway. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Three reported bugs, each verified by measurement rather than assertion.
Closes #93
Closes #94
Closes #95
#93 — an application could not quit itself
window_closeindexedextra_windowsdirectly, so handle 1 meant the first extra window and the primary one had no handle at all, whilewindow_count,window_titleandwindow_is_openall treat 1 as the primary. Nothing else in the surface stopped the run loop.The cost is the one the report names: a bounded run had to be killed from outside, so
timeout 60 ./appalways exited 124 and could not distinguish finished its work from hung. A CI step that renders N frames, writes its evidence and should exit 0 reported failure on success.ui.app_quit()stops the loop on all three backends:-stop:plus a posted event, because-stop:is only examined when the loop next comes round.g_application_quiton an idle source, since it must not be called from an arbitrary thread.WM_QUITposted to the loop's own thread, sincePostQuitMessageonly affects the calling thread.aether_ui_park_until_killed— what a headless or tray-only app parks on — polls the same shared flag instead of sleeping a minute at a time.window_close(1)now addresses the primary window, consistent with the rest of the family.Verified: the demo prints its work, returns from the run loop, and exits 0.
#94 — canvas_on_resize passed ints while its four siblings pass doubles
on_click,on_move,on_releaseandon_scrollall hand their closure two doubles.on_resizehanded it twointptr_t, so a closure written the way the siblings are written read the floating-point argument registers while the caller had filled the integer ones. It compiled, ran, and delivered garbage with no diagnostic from the compiler or the runtime — in a 3D viewport sizing its render target from those values, the viewport silently never resized.Now doubles on all three backends.
vg/live.ae, the one in-tree caller, takes floats to match.#95 — split_set_position ignored on a nested splitview, and no way to state a width
The implementation looked right, and it was:
-setPosition:ofDividerAtIndex:does move the divider, and then the next layout pass re-derives the pane's width from its content and puts it back. An inner split appeared to work only because its pane had nothing to derive from.Pinning the first pane's size is what actually holds. On the reported shape, a 1480-wide window with a populated left panel, asking for 240:
The second half of the report was that nothing could give a widget a width or ask what width it had, so with the divider unavailable there was no other lever.
set_width/set_height/get_width/get_heightnow exist on all three backends: a real constraint on AppKit (the only thing that survives a layout pass), a size request with expand off on GTK4, the preferred size the layout pass reads on win32. The getters answer from the allocation, so they report what a widget actually got rather than what was asked for.Coverage
Three new ci.sh phases. 5e20 asserts through the driver that the resize closure receives real values; 5e21 asserts
quit_demoexits 0 with its loop returned; 5e22 builds the reported three-panel shape and asserts the left panel measures 240 throughget_width.5e21 and 5e22 deliberately need no driver — the app ends itself and reads its own allocation back — which is what let me verify #93 and #95 locally. The sandbox here currently blocks socket binds (the pre-existing
canvasclip_demofails to serve too), so #94's driver spec is verified by CI rather than on this machine.🤖 Generated with Claude Code