Window: replace input/render events with a single AppWindowHandler#39
Open
codymullins wants to merge 2 commits into
Open
Window: replace input/render events with a single AppWindowHandler#39codymullins wants to merge 2 commits into
codymullins wants to merge 2 commits into
Conversation
… handler A window has exactly one owner, so its input, render, resize, and focus callbacks move from multicast `event Action<T>` to a single `AppWindow.Handler` (an `AppWindowHandler`). Subclass it, or use the `CallbackAppWindowHandler` adapter for lambda-style hookup. FrameLoop is the window's handler while attached: it drives the frame and forwards input, focus, and post-reconfigure resize to an app handler set on `FrameLoop.Handler`, so apps set that instead of the window's. AppHost.WindowClosed stays an event — it is a true broadcast notification where independent listeners legitimately observe each window's close. The internal IWindowBackend seam stays events too: a single internal subscriber and an implementation detail. Samples and the windowed harness move to the handler.
…t directly FrameLoop.Handler is now a get-only CallbackAppWindowHandler the loop owns, so callers set callbacks on it (loop.Handler.KeyInput = ...) instead of newing their own handler. Drops the per-caller construction in the samples and the windowed harness.
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.
Why
A window has exactly one owner, so
event Action<T>was the wrong tool for its input/render/resize/focus callbacks: it advertises multicast and arbitrary subscribe/unsubscribe ordering that a single-owner window never wants. This moves those callbacks to a single handler object.(Decision recap: synchronous handler, not async
ValueTask— these callbacks fire on the main-thread pump and per-window render thread, and the input/render path is hot, so async there would block the pump or churn state machines on the hottest path.ValueTask<T>stays in reserve for a future decision callback, of which there are none today.)What changed
AppWindowHandler(new) — abstract class with virtual no-opOnRenderFrame/OnResized/OnPointerInput/OnKeyInput/OnTextInput/OnFocusChanged. Subclass it.CallbackAppWindowHandler(new) — a sealed adapter whose callbacks are plain single delegates (not multicast), for lambda-style hookup in samples and quick code.WindowFocusEvent(new) — payload record struct for focus.AppWindow— the six public events become onepublic AppWindowHandler? Handler { get; set; }.FrameLoop— is the window's handler while attached. It drives the frame and forwards input, focus, and post-reconfigure resize to an app handler set onFrameLoop.Handler. Apps setloop.Handlerinstead of the window's, and keep drawing throughOnRender.Deliberately left as events
AppHost.WindowClosed— a genuine broadcast lifecycle notification; independent listeners (the app, a resource manager, logging) legitimately observe each window's close.eventis the right tool here.IWindowBackendseam — single internal subscriber (AppWindow) and an implementation detail; converting it is churn across both backends with no public-API benefit. Possible follow-up.Not included (approved shape vs. shipped)
OnCloseRequestedappeared in the approved sketch but is omitted: there is no close-request/veto plumbing today (close is fire-and-forget viaRequestClose), so shipping it would be uninvoked public API. It is the natural home for a close veto when that lands.Verification
dotnet build Skyline.slnxclean, 0 warnings (incl.Skyline.Appleon the macOS workload).Skyline.Tests— 62 passed. Windowed harness — 94 checks passed.tools/format-check.shclean.Sequencing
Landed after #36 (focus) and #37 (pointer enter/leave) merged, then migrated the whole surface in one sweep, so no new
event Action<T>was added in the old style first.