From 63418c705c815909f0398d9c4777c94eec293d0a Mon Sep 17 00:00:00 2001 From: foxnne Date: Sun, 18 Oct 2026 11:10:53 -0500 Subject: [PATCH 1/2] layout: opening the view picker no longer stalls a frame Opening a place's view picker drew every view that shows nowhere for the first time in one frame, and about ten frames later captured all of them in another: 19-26 ms of main-thread work each (Debug, loaded machine). Now those views are drawn offscreen one at a time. As the picker draws its cards it chooses the next: a card in view whose view drew nowhere and has no picture (`State.offscreen`: its registry index, checked against its id's hash). `captureUnplaced`, before the picker draws as before, warms that one view (ten frames and a reveal's fade) and captures it, one draw a frame, so what it opens stacks under the picker and its picture shows on its card that frame. A view whose card leaves the list early is dropped. A failed capture is remembered for the picker's session (`State.recordCaptureFailed`); three in a row that cannot start, or more failures than are remembered, end capture for the session. The picker's work is in the profiler: "picker", "picker: capture unplaced" and "picker: capture". Co-Authored-By: Claude Opus 5.5 --- app/layout/Layout.zig | 109 +++++++++++++++++++++++++++++++----------- app/layout/Picker.zig | 25 ++++++++++ app/layout/State.zig | 54 ++++++++++++++++++++- tests/integration.zig | 97 +++++++++++++++++++++++++++++++++++++ 4 files changed, 257 insertions(+), 28 deletions(-) diff --git a/app/layout/Layout.zig b/app/layout/Layout.zig index 4b9cfd225..be35d2d22 100644 --- a/app/layout/Layout.zig +++ b/app/layout/Layout.zig @@ -667,7 +667,8 @@ fn pick(items: []const *Surface, current: ?[]const u8) ?*Surface { /// /// While the picker is collecting (`State.snapshots_wanted`), a surface without a snapshot is /// drawn through a texture target and the result both kept and put on screen — the pixels are -/// the same ones, so nothing blinks, and the surface still ran exactly once. +/// the same ones, so nothing blinks, and the surface still ran exactly once. Not once capture has +/// failed for it, or here at all (`State.captureFailed`, `State.capture_unavailable`). pub fn draw(self: *Layout, s: *Surface) !dvui.App.Result { var hasher = std.hash.Wyhash.init(0); hasher.update(s.id); @@ -683,7 +684,7 @@ pub fn draw(self: *Layout, s: *Surface) !dvui.App.Result { /// `draw` without the fade-in: for a swap, whose overlay already carries the change. fn drawSolid(self: *Layout, s: *Surface) !dvui.App.Result { self.drawn.append(self.arena, s) catch {}; - if (self.state.snapshots_wanted and self.state.snapshot(s.id) == null) { + if (self.state.snapshots_wanted and !self.state.capture_unavailable and self.state.snapshot(s.id) == null and !self.state.captureFailed(s.id)) { if (try self.drawCaptured(s)) |r| return r; } return surfaceDraw(s); @@ -691,7 +692,9 @@ fn drawSolid(self: *Layout, s: *Surface) !dvui.App.Result { /// `s.draw` into a texture the size of the current parent's content, then that texture onto the /// screen where the surface would have drawn. Null when there is nothing to capture into — a -/// zero-sized parent, or a backend without render targets — and the caller draws normally. +/// zero-sized parent, or a backend without render targets — and the caller draws normally. A +/// failure is remembered for the picker's session (`State.capture_unavailable`, +/// `State.capture_failed`), so it is not tried every frame. /// /// Through `dvui.Picture` rather than a bare `renderTarget` switch: dvui defers part of its /// drawing (strokes after fills, subwindow content) to queues flushed later, and a bare switch @@ -699,8 +702,14 @@ fn drawSolid(self: *Layout, s: *Surface) !dvui.App.Result { /// flushes them inside `stop`, which is the difference between a snapshot of a scroll area and /// a snapshot of its background. fn drawCaptured(self: *Layout, s: *Surface) !?dvui.App.Result { + const prof = core.profile.begin("fizzy", "picker: capture"); + defer prof.end(); const rs = dvui.parentGet().data().contentRectScale(); - var pic = dvui.Picture.start(rs.r) orelse return null; + if (rs.r.empty()) return null; + var pic = dvui.Picture.start(rs.r) orelse { + self.state.recordCaptureFailed(s.id, false); + return null; + }; // Some backends leave a fresh target uninitialised; the cross-fade learned this the hard way. pic.texture.clear(); const old_clip = dvui.clipGet(); @@ -708,7 +717,11 @@ fn drawCaptured(self: *Layout, s: *Surface) !?dvui.App.Result { const result = surfaceDraw(s); dvui.clipSet(old_clip); pic.stop(); - const texture = dvui.textureFromTarget(pic.texture) catch return try result; + const texture = dvui.textureFromTarget(pic.texture) catch { + self.state.recordCaptureFailed(s.id, true); + return try result; + }; + self.state.capture_unstarted = 0; dvui.renderTexture(texture, .{ .r = pic.r, .s = rs.s }, .{}) catch {}; self.state.takeSnapshot(self.gpa, s.id, .{ .texture = texture, .natural = .{ .w = pic.r.w / rs.s, .h = pic.r.h / rs.s } }); return try result; @@ -737,26 +750,57 @@ pub fn closeFloat(self: *Layout, name: []const u8) void { Floats.close(self, name, .home); } -/// Snapshot every surface that drew nowhere this frame, by drawing each once offscreen at a -/// fixed size. Only while the picker is collecting and only for surfaces still missing a -/// snapshot, so a frame with nothing to do costs a lookup. The application calls this after -/// its shape has run, from the base window. +/// Draw the view the picker chose for a picture (`State.offscreen`, picked from the cards it +/// shows) offscreen at a fixed size, one draw a frame: a few frames of warm-up, then its capture. +/// Only while the picker is collecting, so a frame with nothing to do costs a check. The +/// application calls this after its shape has run and before the picker draws, from the base +/// window: what the view opens (a popup, a float) stacks under the picker, and a picture taken +/// here shows on its card the same frame. +/// +/// Only a view that drew nowhere this frame (`drawn`) is drawn here: one on screen is captured +/// where it lives (`draw`), and an offscreen picture of the workspace is a picture of an empty box. /// -/// "Drew nowhere" is decided by `drawn`, not by the missing snapshot: the picker opens -/// mid-frame, after some regions have already run, and those surfaces must be captured where -/// they live on the *next* frame rather than photographed offscreen now — an offscreen -/// picture of the workspace is a picture of an empty box. +/// Offscreen, a view costs what it draws: its first draw lays out a whole subtree from nothing, +/// and its capture is the one draw that renders any of it (the warm-up draws are clipped away). +/// The picker opening used to pay both for every view, each in one frame. One at a time, a view +/// scrolled or filtered out of the list is never drawn for it, and a card waiting for its picture +/// shows an empty tile. pub fn captureUnplaced(self: *Layout) void { - if (!self.state.snapshots_wanted) return; - var pending = false; - outer: for (self.host.surfaces.items, 0..) |*s, i| { - if (s.hidden or self.state.snapshot(s.id) != null) continue; - for (self.drawn.items) |d| if (d == s) { - pending = true; // drew this frame before the request: in-place capture next frame - continue :outer; - }; + const state = self.state; + if (!state.snapshots_wanted or state.capture_unavailable) return; + const prof = core.profile.begin("fizzy", "picker: capture unplaced"); + defer prof.end(); + // Opened this frame, after some views drew: they are captured where they live on the next. + // (One drawn since is captured as it draws, `drawSolid`; one with no room to draw in waits, + // asking for nothing, until it has some.) + var pending = state.snapshots_requested; + state.snapshots_requested = false; + + if (state.offscreen) |*o| step: { + const surfaces = self.host.surfaces.items; + // Where the picker found it, unless the registry changed since: then wherever it is now. + if (o.index >= surfaces.len or std.hash.Wyhash.hash(0, surfaces[o.index].id) != o.id) { + o.index = for (surfaces, 0..) |c, i| { + if (std.hash.Wyhash.hash(0, c.id) == o.id) break i; + } else { + state.offscreen = null; + break :step; + }; + } + const s = &surfaces[o.index]; + // Hidden, given its picture on screen, or drawn there this frame: not drawn twice. + const on_screen = for (self.drawn.items) |d| { + if (d == s) break true; + } else false; + // Its card left the list before it got far (filtered out, scrolled away): dropped, to be + // chosen again if its card comes back. Further in, it is finished. + const out_of_view = !o.in_view and o.warm < offscreen_warmup_frames / 2; + if (s.hidden or on_screen or out_of_view or state.snapshot(s.id) != null) { + state.offscreen = null; + break :step; + } var box = dvui.box(@src(), .{ .dir = .vertical }, .{ - .id_extra = i, + .id_extra = @truncate(o.id), .rect = .{ .x = -20_000, .y = -20_000, .w = offscreen_capture.w, .h = offscreen_capture.h }, .background = true, .color_fill = .{ .color = dvui.themeGet().color(.content, .fill) }, @@ -765,15 +809,20 @@ pub fn captureUnplaced(self: *Layout) void { // A subtree drawn for the first time is not yet what it will look like: dvui lays it // out from last frame's sizes, which it has none of, and anything inside that reveals // itself starts hidden. So it is drawn unphotographed for a few frames first — the same - // settle-then-show the reveal does on screen, just without an audience. - const warm = dvui.dataGet(null, box.data().id, "_warm", u8) orelse 0; - if (warm < offscreen_warmup_frames) { + // settle-then-show the reveal does on screen, just without an audience: a few frames, and + // the reveal's whole fade, however fast the frames come. + const now = dvui.frameTimeNS(); + if (o.warm == 0) o.since_ns = now; + if (o.warm < offscreen_warmup_frames or now - o.since_ns < offscreenWarmupNs()) { _ = surfaceDraw(s) catch {}; - dvui.dataSet(null, box.data().id, "_warm", warm + 1); + o.warm +|= 1; pending = true; - continue; + break :step; } + // Taken or failed (`drawCaptured` remembers which), it is done: the picker, drawing next, + // shows the picture and chooses the next view. _ = self.drawCaptured(s) catch null; + state.offscreen = null; } if (pending) dvui.refresh(null, @src(), null); } @@ -784,6 +833,12 @@ const offscreen_capture: dvui.Size = .{ .w = 360, .h = 480 }; /// Long enough for a 120 ms reveal to finish at 60 fps, with a little to spare. const offscreen_warmup_frames: u8 = 10; +/// How long the warm-up lasts at the least: a reveal's fade (`core.anim.reveal`) at the +/// interface's speed (`core.motion`), so a picture is never taken mid-fade on a fast display. +fn offscreenWarmupNs() i128 { + return @as(i128, core.motion.duration((core.anim.RevealOptions{}).duration_micros)) * std.time.ns_per_us; +} + /// Draw whichever surface is selected for these keywords, into the current parent. Everything /// it does is reachable through `matching` / `selected` / `draw`; `region` uses it to fill a /// declared region's space. diff --git a/app/layout/Picker.zig b/app/layout/Picker.zig index 52c562d15..652f6006a 100644 --- a/app/layout/Picker.zig +++ b/app/layout/Picker.zig @@ -77,7 +77,12 @@ pub fn close(self: *Picker, gpa: std.mem.Allocator) void { pub fn draw(self: *Picker, f: *Layout) void { if (self.lifted) return self.drawLifted(f); if (!self.is_open) return; + const prof = core.profile.begin("fizzy", "picker"); + defer prof.end(); const state = f.state; + // Whether the view being drawn for its picture still has its card in view: its card says so + // as it draws (`nextOffscreen`). + if (state.offscreen) |*o| o.in_view = false; const gpa = f.gpa; const region = blk: { @@ -525,6 +530,25 @@ const Hit = union(enum) { lifted: struct { tile: dvui.Rect.Physical, event_num: u16 }, }; +/// Choose surface `s` (at `index` in the registry) to be drawn offscreen for its picture +/// (`State.offscreen`, `Layout.captureUnplaced`) when none is: its card in view, as the list laid +/// it out, its view drawn nowhere this frame, and no picture or failed capture yet. Not on the +/// card's first frame, before it has a size: every card then packs into the top of the list. The +/// card of the view already chosen says whether it is still in view. +fn nextOffscreen(f: *Layout, s: *const sdk.Surface, index: usize, card_data: *dvui.WidgetData) void { + const state = f.state; + if (state.capture_unavailable) return; + const in_view = !dvui.firstFrame(card_data.id) and !card_data.borderRectScale().r.intersect(dvui.clipGet()).empty(); + if (state.offscreen) |*o| { + if (o.index == index and in_view) o.in_view = true; + return; + } + if (!in_view or state.snapshot(s.id) != null or state.captureFailed(s.id)) return; + for (f.drawn.items) |d| if (d == s) return; + state.offscreen = .{ .index = index, .id = std.hash.Wyhash.hash(0, s.id) }; + dvui.refresh(null, @src(), null); +} + /// One surface: its snapshot scaled to fit, its title, its owner, and a highlight when it is in /// the region. A press that moves far enough lifts the card into a drag instead of clicking. fn card(f: *Layout, s: *const sdk.Surface, on: bool, id_extra: usize) Hit { @@ -542,6 +566,7 @@ fn card(f: *Layout, s: *const sdk.Surface, on: bool, id_extra: usize) Hit { }); defer bw.deinit(); const id = bw.data().id; + nextOffscreen(f, s, id_extra, bw.data()); // For demo tapes (`docs/AUTOMATION.md`): the card, to lift a view out of the picker by. core.anchor.mark(bw.data(), "picker:{s}", .{s.id}); diff --git a/app/layout/State.zig b/app/layout/State.zig index cc6acba3f..600680b15 100644 --- a/app/layout/State.zig +++ b/app/layout/State.zig @@ -87,8 +87,27 @@ shows: std.StringHashMapUnmanaged(Region.Shows) = .empty, /// unloaded) while the picker is open. snapshots: std.StringHashMapUnmanaged(Snapshot) = .empty, /// While true, `Layout.draw` captures any surface it draws that has no snapshot yet, and -/// `Layout.captureUnplaced` draws the rest offscreen once. Set by `openPicker`. +/// `Layout.captureUnplaced` draws the ones whose cards the picker shows offscreen, one at a time +/// (`offscreen`). Set by `openPicker`. snapshots_wanted: bool = false, +/// Set by `openPicker`, taken by `Layout.captureUnplaced`: the views that drew before the picker +/// opened, this frame, are captured where they live on the next, which this asks for. +snapshots_requested: bool = false, +/// The one view `Layout.captureUnplaced` is drawing offscreen for its picture, chosen by the picker +/// from the cards it shows (`Picker.draw`): where it is in the registry, checked against its id's +/// hash (`std.hash.Wyhash`, seed 0) before each use; the frames of warm-up drawn, and when the +/// first was (`dvui.frameTimeNS`); whether the list had its card in view on its last draw. +offscreen: ?struct { index: usize, id: u64, warm: u8 = 0, since_ns: i128 = 0, in_view: bool = true } = null, +/// No picture can be taken here: nothing more is captured, on screen or off, and no frame asked +/// for one, until the picker reopens. Set once `capture_unstarted_limit` captures in a row could +/// not even start (a backend without texture targets), or `capture_failed` is full. +capture_unavailable: bool = false, +/// Captures in a row, each of a different view, that could not start (`dvui.Picture.start`). +capture_unstarted: u8 = 0, +/// The views (id hashes) whose capture failed since the picker opened: not tried again until it +/// reopens (`captureFailed`, `recordCaptureFailed`). +capture_failed: [16]u64 = undefined, +capture_failed_len: usize = 0, /// The one surface picker, if open. On the state rather than in whichever pane opened it so the /// settings table and a region's corner button open the same thing, and the app draws it in /// one place above everything else (`Picker.draw`). @@ -665,6 +684,38 @@ pub fn discardSnapshots(self: *State, gpa: std.mem.Allocator) void { } self.snapshots.clearRetainingCapacity(); self.snapshots_wanted = false; + self.snapshots_requested = false; + self.offscreen = null; + self.capture_unavailable = false; + self.capture_unstarted = 0; + self.capture_failed_len = 0; +} + +/// Whether surface `id`'s capture failed since the picker opened (`capture_failed`). +pub fn captureFailed(self: *const State, id: []const u8) bool { + const hash = std.hash.Wyhash.hash(0, id); + for (self.capture_failed[0..self.capture_failed_len]) |f| { + if (f == hash) return true; + } + return false; +} + +/// How many captures in a row may fail to start, each a different view, before capture counts as +/// unavailable: one alone is that view's (too large a target, say), not the backend's. +const capture_unstarted_limit = 3; + +/// Surface `id`'s capture failed: it is not tried again until the picker reopens. `started` is +/// whether its picture could begin at all (`dvui.Picture.start`); several in a row that could not, +/// or more failures than are remembered, and capture is given up for the session +/// (`capture_unavailable`) rather than tried view after view. +pub fn recordCaptureFailed(self: *State, id: []const u8, started: bool) void { + self.capture_unstarted = if (started) 0 else self.capture_unstarted + 1; + if (self.capture_unstarted >= capture_unstarted_limit or self.capture_failed_len == self.capture_failed.len) { + self.capture_unavailable = true; + return; + } + self.capture_failed[self.capture_failed_len] = std.hash.Wyhash.hash(0, id); + self.capture_failed_len += 1; } /// Open the picker for region `name`, anchored at `anchor` (or centred when null), and start @@ -672,6 +723,7 @@ pub fn discardSnapshots(self: *State, gpa: std.mem.Allocator) void { pub fn openPicker(self: *State, gpa: std.mem.Allocator, name: []const u8, anchor: ?dvui.Point.Natural) void { self.discardSnapshots(gpa); self.snapshots_wanted = true; + self.snapshots_requested = true; self.picker.open(gpa, name, anchor); } diff --git a/tests/integration.zig b/tests/integration.zig index 3ca3196a6..382ebb614 100644 --- a/tests/integration.zig +++ b/tests/integration.zig @@ -2595,6 +2595,103 @@ test "the picker lists each view under the place it is made for, the open place try std.testing.expect(dvui.tagGet("picker.empty") == null); } +/// Views no place shows, each counting its draws: what the picker draws offscreen for its cards. +const UnplacedViews = struct { + const ids = [_][]const u8{ "test.u0", "test.u1", "test.u2", "test.u3", "test.u4", "test.u5", "test.u6" }; + var draws: [ids.len]u32 = @splat(0); + + fn drawFor(comptime n: usize) fn (?*anyopaque) anyerror!dvui.App.Result { + return struct { + fn draw(_: ?*anyopaque) anyerror!dvui.App.Result { + draws[n] += 1; + return .ok; + } + }.draw; + } + + /// Whether the picker's list had view `n`'s card in view on the last frame. + fn shown(n: usize) !bool { + var buf: [64]u8 = undefined; + return (try dvui.testing.tagGet(try std.fmt.bufPrint(&buf, "picker:{s}", .{ids[n]}))).visible; + } + + /// Step a frame; fails if two of the views were drawn in it. + fn step() !void { + const before = draws; + _ = try dvui.testing.step(PickerFrame.frame); + var drew: usize = 0; + for (before, draws) |b, a| drew += a - b; + try std.testing.expect(drew <= 1); + } +}; + +test "picker: a view that draws nowhere is drawn for its card one at a time, only while the list shows it" { + var ctx = try shim.init(std.testing.allocator); + defer ctx.deinit(std.testing.allocator); + + const editor = ctx.editor; + editor.app.gpa = std.testing.allocator; + defer editor.app.layout.regions.deinit(editor.app.gpa); + defer editor.app.layout.regions_building.deinit(editor.app.gpa); + defer editor.app.layout.deinitQualified(editor.app.gpa); + defer editor.app.layout.deinitAssignments(editor.app.gpa); + defer editor.app.layout.deinitExtents(editor.app.gpa); + defer editor.app.layout.picker.close(editor.app.gpa); + defer editor.app.layout.discardSnapshots(editor.app.gpa); + + UnplacedViews.draws = @splat(0); + // Only views no place shows: one on screen would be captured first, and fail, and with no + // texture targets that ends capture for the session before any is drawn offscreen. + inline for (UnplacedViews.ids, 0..) |id, n| { + try editor.app.host.registerSurface(.{ .id = id, .title = id, .keywords = &.{"inspector"}, .draw = UnplacedViews.drawFor(n) }); + } + + PickerFrame.editor = editor; + defer PickerFrame.editor = null; + try dvui.testing.settle(PickerFrame.frame); + + editor.app.layout.openPicker(editor.app.gpa, "Sidebar", null); + // The frame it opens on, and the list's first, when no card has a size yet: no card shows, + // and nothing is drawn for one. + for (0..2) |_| try UnplacedViews.step(); + try std.testing.expectEqual(@as([UnplacedViews.ids.len]u32, @splat(0)), UnplacedViews.draws); + + // The cards in view are the list's first ones, in order: some of the views and not all, in the + // shim's 600 by 400 window. The views are drawn one at a time, first card first. The testing + // backend has no texture targets, so no capture can start: after three in a row, capture is + // given up for the session, and nothing more is drawn. + try std.testing.expectEqual(dvui.Size.Natural{ .w = 600, .h = 400 }, dvui.windowRect().size()); + for (0..600) |_| try UnplacedViews.step(); + try std.testing.expect(editor.app.layout.capture_unavailable); + var shown: usize = 0; + while (shown < UnplacedViews.ids.len and try UnplacedViews.shown(shown)) shown += 1; + try std.testing.expect(shown > 0 and shown < UnplacedViews.ids.len); + for (0..UnplacedViews.ids.len) |i| try std.testing.expectEqual(i < shown, try UnplacedViews.shown(i)); + for (UnplacedViews.draws, 0..) |n, i| try std.testing.expectEqual(i < @min(shown, 3), n > 0); + const settled = UnplacedViews.draws; + for (0..30) |_| try UnplacedViews.step(); + try std.testing.expectEqual(settled, UnplacedViews.draws); + + // Reopened, with the views in view already failed: the ones left are past the fold, and are + // not drawn. + UnplacedViews.draws = @splat(0); + editor.app.layout.openPicker(editor.app.gpa, "Sidebar", null); + for (0..shown) |i| editor.app.layout.recordCaptureFailed(UnplacedViews.ids[i], true); + for (0..200) |_| try UnplacedViews.step(); + try std.testing.expectEqual(@as([UnplacedViews.ids.len]u32, @splat(0)), UnplacedViews.draws); + + // Filtered: with those views gone from the list, the next card moves up into view and its view + // is drawn. (Hidden rather than typed or scrolled: the testing backend's input does not reach + // the popup.) + for (0..shown) |i| editor.app.host.setSurfaceHidden(UnplacedViews.ids[i], true); + for (0..200) |_| try UnplacedViews.step(); + try std.testing.expect(try UnplacedViews.shown(shown)); + try std.testing.expect(UnplacedViews.draws[shown] > 0); + for (shown..UnplacedViews.ids.len) |i| { + if (!try UnplacedViews.shown(i)) try std.testing.expectEqual(@as(u32, 0), UnplacedViews.draws[i]); + } +} + test "a hide_when_empty panel stays while its view is dragged onto main" { var ctx = try shim.init(std.testing.allocator); defer ctx.deinit(std.testing.allocator); From 68d7701e7bfa50d942a554b45386625ad0081e0e Mon Sep 17 00:00:00 2001 From: foxnne Date: Sun, 18 Oct 2026 15:29:01 -0500 Subject: [PATCH 2/2] layout: picker review: offscreen-only unavailability, unbounded failures Capture is given up for the picker's session only after three offscreen captures in a row cannot start (their target is small and fixed); an on-screen view whose capture fails is only marked failed. Failed views go in a per-session set (no cap, no global disable). Whether the chosen view's card left the list is decided from frame stamps (`card_ns` against `Picker.list_ns`), so a picker draw that returns early never drops it. A view on screen without its picture asks for a frame again, unless it failed or has no room. One predicate, `mayDrawOffscreen`, decides for both the picker and `captureUnplaced`. Co-Authored-By: Claude Opus 5.5 --- app/layout/Layout.zig | 58 +++++++++++++++++++++++++------------- app/layout/Picker.zig | 25 +++++++++-------- app/layout/State.zig | 65 +++++++++++++++++++------------------------ tests/integration.zig | 2 +- 4 files changed, 80 insertions(+), 70 deletions(-) diff --git a/app/layout/Layout.zig b/app/layout/Layout.zig index be35d2d22..fc2040d90 100644 --- a/app/layout/Layout.zig +++ b/app/layout/Layout.zig @@ -158,6 +158,9 @@ extents_changed: bool = false, /// Every surface `draw` ran this frame, so `captureUnplaced` knows which ones genuinely drew /// nowhere — as opposed to drew before the picker asked. Arena-backed, per frame. drawn: std.ArrayListUnmanaged(*Surface) = .empty, +/// Surfaces drawn this frame in a place with no room, so not captured (`drawCaptured`): no frame +/// is asked for them until they have some. Arena-backed, per frame. +unsized: std.ArrayListUnmanaged(*Surface) = .empty, /// Open regions, innermost last. Every region pushes; its `deinit` pops. This is what lets /// `split` know which axis it divides and which neighbour it resizes, without a shape having to @@ -693,8 +696,8 @@ fn drawSolid(self: *Layout, s: *Surface) !dvui.App.Result { /// `s.draw` into a texture the size of the current parent's content, then that texture onto the /// screen where the surface would have drawn. Null when there is nothing to capture into — a /// zero-sized parent, or a backend without render targets — and the caller draws normally. A -/// failure is remembered for the picker's session (`State.capture_unavailable`, -/// `State.capture_failed`), so it is not tried every frame. +/// failure is remembered for the picker's session (`State.recordCaptureFailed`), so it is not +/// tried every frame. /// /// Through `dvui.Picture` rather than a bare `renderTarget` switch: dvui defers part of its /// drawing (strokes after fills, subwindow content) to queues flushed later, and a bare switch @@ -705,9 +708,12 @@ fn drawCaptured(self: *Layout, s: *Surface) !?dvui.App.Result { const prof = core.profile.begin("fizzy", "picker: capture"); defer prof.end(); const rs = dvui.parentGet().data().contentRectScale(); - if (rs.r.empty()) return null; + if (rs.r.empty()) { + self.unsized.append(self.arena, s) catch {}; + return null; + } var pic = dvui.Picture.start(rs.r) orelse { - self.state.recordCaptureFailed(s.id, false); + self.state.recordCaptureFailed(self.gpa, s.id); return null; }; // Some backends leave a fresh target uninitialised; the cross-fade learned this the hard way. @@ -718,10 +724,9 @@ fn drawCaptured(self: *Layout, s: *Surface) !?dvui.App.Result { dvui.clipSet(old_clip); pic.stop(); const texture = dvui.textureFromTarget(pic.texture) catch { - self.state.recordCaptureFailed(s.id, true); + self.state.recordCaptureFailed(self.gpa, s.id); return try result; }; - self.state.capture_unstarted = 0; dvui.renderTexture(texture, .{ .r = pic.r, .s = rs.s }, .{}) catch {}; self.state.takeSnapshot(self.gpa, s.id, .{ .texture = texture, .natural = .{ .w = pic.r.w / rs.s, .h = pic.r.h / rs.s } }); return try result; @@ -770,11 +775,15 @@ pub fn captureUnplaced(self: *Layout) void { if (!state.snapshots_wanted or state.capture_unavailable) return; const prof = core.profile.begin("fizzy", "picker: capture unplaced"); defer prof.end(); - // Opened this frame, after some views drew: they are captured where they live on the next. - // (One drawn since is captured as it draws, `drawSolid`; one with no room to draw in waits, - // asking for nothing, until it has some.) - var pending = state.snapshots_requested; - state.snapshots_requested = false; + // A view on screen without its picture (it drew before the picker opened, this frame) is + // captured where it lives on the next. Not one whose capture failed, or that has no room. + var pending = false; + for (self.drawn.items) |d| { + if (state.snapshot(d.id) != null or state.captureFailed(d.id)) continue; + for (self.unsized.items) |u| { + if (u == d) break; + } else pending = true; + } if (state.offscreen) |*o| step: { const surfaces = self.host.surfaces.items; @@ -788,14 +797,11 @@ pub fn captureUnplaced(self: *Layout) void { }; } const s = &surfaces[o.index]; - // Hidden, given its picture on screen, or drawn there this frame: not drawn twice. - const on_screen = for (self.drawn.items) |d| { - if (d == s) break true; - } else false; - // Its card left the list before it got far (filtered out, scrolled away): dropped, to be - // chosen again if its card comes back. Further in, it is finished. - const out_of_view = !o.in_view and o.warm < offscreen_warmup_frames / 2; - if (s.hidden or on_screen or out_of_view or state.snapshot(s.id) != null) { + // Its card left the list before it got far (filtered out, scrolled away): the list drew + // since without it. Dropped, to be chosen again if the card comes back; further in, it is + // finished. + const out_of_view = o.card_ns < state.picker.list_ns and o.warm < offscreen_warmup_frames / 2; + if (out_of_view or !self.mayDrawOffscreen(s)) { state.offscreen = null; break :step; } @@ -821,7 +827,8 @@ pub fn captureUnplaced(self: *Layout) void { } // Taken or failed (`drawCaptured` remembers which), it is done: the picker, drawing next, // shows the picture and chooses the next view. - _ = self.drawCaptured(s) catch null; + const started = if (self.drawCaptured(s)) |r| r != null else |_| true; + state.noteOffscreenStart(started); state.offscreen = null; } if (pending) dvui.refresh(null, @src(), null); @@ -833,6 +840,17 @@ const offscreen_capture: dvui.Size = .{ .w = 360, .h = 480 }; /// Long enough for a 120 ms reveal to finish at 60 fps, with a little to spare. const offscreen_warmup_frames: u8 = 10; +/// Whether surface `s` may be drawn offscreen for its picture (`captureUnplaced`, chosen by +/// `Picker.draw`): capture works here, it has no picture and no failed capture, it is not hidden, +/// and it drew nowhere this frame — one on screen is captured where it lives, never drawn twice. +pub fn mayDrawOffscreen(self: *const Layout, s: *const Surface) bool { + const state = self.state; + if (state.capture_unavailable or s.hidden) return false; + if (state.snapshot(s.id) != null or state.captureFailed(s.id)) return false; + for (self.drawn.items) |d| if (d == s) return false; + return true; +} + /// How long the warm-up lasts at the least: a reveal's fade (`core.anim.reveal`) at the /// interface's speed (`core.motion`), so a picture is never taken mid-fade on a fast display. fn offscreenWarmupNs() i128 { diff --git a/app/layout/Picker.zig b/app/layout/Picker.zig index 652f6006a..7603072e0 100644 --- a/app/layout/Picker.zig +++ b/app/layout/Picker.zig @@ -50,6 +50,10 @@ anchor: ?dvui.Point.Natural = null, /// `ViewDrag`. The popup is closed; `draw` drives the drag instead until the /// button comes up. lifted: bool = false, +/// When the list last laid its cards out (`dvui.frameTimeNS`): a view being drawn for its picture +/// whose card was not in view then has left the list (`Layout.captureUnplaced`). A draw that +/// returned before the list leaves it alone. +list_ns: i128 = 0, /// Card geometry, in points. The preview keeps the snapshot's aspect inside this box. const preview: dvui.Size = .{ .w = 150, .h = 100 }; @@ -80,9 +84,6 @@ pub fn draw(self: *Picker, f: *Layout) void { const prof = core.profile.begin("fizzy", "picker"); defer prof.end(); const state = f.state; - // Whether the view being drawn for its picture still has its card in view: its card says so - // as it draws (`nextOffscreen`). - if (state.offscreen) |*o| o.in_view = false; const gpa = f.gpa; const region = blk: { @@ -278,6 +279,7 @@ pub fn draw(self: *Picker, f: *Layout) void { defer core.widgets.scrollShadows(scroll); var grid = dvui.box(@src(), .{ .dir = .vertical }, .{ .expand = .horizontal }); + self.list_ns = dvui.frameTimeNS(); defer grid.deinit(); var row: ?*dvui.BoxWidget = null; @@ -532,20 +534,19 @@ const Hit = union(enum) { /// Choose surface `s` (at `index` in the registry) to be drawn offscreen for its picture /// (`State.offscreen`, `Layout.captureUnplaced`) when none is: its card in view, as the list laid -/// it out, its view drawn nowhere this frame, and no picture or failed capture yet. Not on the -/// card's first frame, before it has a size: every card then packs into the top of the list. The -/// card of the view already chosen says whether it is still in view. +/// it out, and the view one that may be (`Layout.mayDrawOffscreen`). Not on the card's first +/// frame, before it has a size: every card then packs into the top of the list. The card of the +/// view already chosen stamps that it is still in view (`card_ns`). fn nextOffscreen(f: *Layout, s: *const sdk.Surface, index: usize, card_data: *dvui.WidgetData) void { const state = f.state; - if (state.capture_unavailable) return; - const in_view = !dvui.firstFrame(card_data.id) and !card_data.borderRectScale().r.intersect(dvui.clipGet()).empty(); + if (dvui.firstFrame(card_data.id) or card_data.borderRectScale().r.intersect(dvui.clipGet()).empty()) return; + const now = dvui.frameTimeNS(); if (state.offscreen) |*o| { - if (o.index == index and in_view) o.in_view = true; + if (o.index == index) o.card_ns = now; return; } - if (!in_view or state.snapshot(s.id) != null or state.captureFailed(s.id)) return; - for (f.drawn.items) |d| if (d == s) return; - state.offscreen = .{ .index = index, .id = std.hash.Wyhash.hash(0, s.id) }; + if (!f.mayDrawOffscreen(s)) return; + state.offscreen = .{ .index = index, .id = std.hash.Wyhash.hash(0, s.id), .card_ns = now }; dvui.refresh(null, @src(), null); } diff --git a/app/layout/State.zig b/app/layout/State.zig index 600680b15..d3a92a16b 100644 --- a/app/layout/State.zig +++ b/app/layout/State.zig @@ -90,24 +90,21 @@ snapshots: std.StringHashMapUnmanaged(Snapshot) = .empty, /// `Layout.captureUnplaced` draws the ones whose cards the picker shows offscreen, one at a time /// (`offscreen`). Set by `openPicker`. snapshots_wanted: bool = false, -/// Set by `openPicker`, taken by `Layout.captureUnplaced`: the views that drew before the picker -/// opened, this frame, are captured where they live on the next, which this asks for. -snapshots_requested: bool = false, /// The one view `Layout.captureUnplaced` is drawing offscreen for its picture, chosen by the picker /// from the cards it shows (`Picker.draw`): where it is in the registry, checked against its id's /// hash (`std.hash.Wyhash`, seed 0) before each use; the frames of warm-up drawn, and when the -/// first was (`dvui.frameTimeNS`); whether the list had its card in view on its last draw. -offscreen: ?struct { index: usize, id: u64, warm: u8 = 0, since_ns: i128 = 0, in_view: bool = true } = null, +/// first was; when the list last had its card in view (`dvui.frameTimeNS`, as `Picker.list_ns`). +offscreen: ?struct { index: usize, id: u64, warm: u8 = 0, since_ns: i128 = 0, card_ns: i128 } = null, /// No picture can be taken here: nothing more is captured, on screen or off, and no frame asked -/// for one, until the picker reopens. Set once `capture_unstarted_limit` captures in a row could -/// not even start (a backend without texture targets), or `capture_failed` is full. +/// for one, until the picker reopens. Set once `offscreen_unstarted_limit` offscreen captures in a +/// row could not even start (`noteOffscreenStart`): their target is small and fixed, so that is +/// the backend (no texture targets), not the view. capture_unavailable: bool = false, -/// Captures in a row, each of a different view, that could not start (`dvui.Picture.start`). -capture_unstarted: u8 = 0, -/// The views (id hashes) whose capture failed since the picker opened: not tried again until it -/// reopens (`captureFailed`, `recordCaptureFailed`). -capture_failed: [16]u64 = undefined, -capture_failed_len: usize = 0, +/// Offscreen captures in a row, each of a different view, that could not start. +offscreen_unstarted: u8 = 0, +/// The views (id hashes) whose capture failed since the picker opened, gpa-owned: not tried again +/// until it reopens (`captureFailed`, `recordCaptureFailed`). +capture_failed: std.AutoHashMapUnmanaged(u64, void) = .empty, /// The one surface picker, if open. On the state rather than in whichever pane opened it so the /// settings table and a region's corner button open the same thing, and the app draws it in /// one place above everything else (`Picker.draw`). @@ -684,38 +681,33 @@ pub fn discardSnapshots(self: *State, gpa: std.mem.Allocator) void { } self.snapshots.clearRetainingCapacity(); self.snapshots_wanted = false; - self.snapshots_requested = false; self.offscreen = null; self.capture_unavailable = false; - self.capture_unstarted = 0; - self.capture_failed_len = 0; + self.offscreen_unstarted = 0; + self.capture_failed.clearAndFree(gpa); } /// Whether surface `id`'s capture failed since the picker opened (`capture_failed`). pub fn captureFailed(self: *const State, id: []const u8) bool { - const hash = std.hash.Wyhash.hash(0, id); - for (self.capture_failed[0..self.capture_failed_len]) |f| { - if (f == hash) return true; - } - return false; + return self.capture_failed.contains(std.hash.Wyhash.hash(0, id)); } -/// How many captures in a row may fail to start, each a different view, before capture counts as -/// unavailable: one alone is that view's (too large a target, say), not the backend's. -const capture_unstarted_limit = 3; +/// Surface `id`'s capture failed (its picture could not start, or gave no texture): it is not +/// tried again until the picker reopens. Nothing else is affected. +pub fn recordCaptureFailed(self: *State, gpa: std.mem.Allocator, id: []const u8) void { + self.capture_failed.put(gpa, std.hash.Wyhash.hash(0, id), {}) catch {}; +} -/// Surface `id`'s capture failed: it is not tried again until the picker reopens. `started` is -/// whether its picture could begin at all (`dvui.Picture.start`); several in a row that could not, -/// or more failures than are remembered, and capture is given up for the session -/// (`capture_unavailable`) rather than tried view after view. -pub fn recordCaptureFailed(self: *State, id: []const u8, started: bool) void { - self.capture_unstarted = if (started) 0 else self.capture_unstarted + 1; - if (self.capture_unstarted >= capture_unstarted_limit or self.capture_failed_len == self.capture_failed.len) { - self.capture_unavailable = true; - return; - } - self.capture_failed[self.capture_failed_len] = std.hash.Wyhash.hash(0, id); - self.capture_failed_len += 1; +/// How many offscreen captures in a row may fail to start, each a different view, before capture +/// counts as unavailable here. +const offscreen_unstarted_limit = 3; + +/// An offscreen capture began (`started`), or could not: its target is small and fixed, so several +/// in a row that could not mean the backend has no texture targets, and capture is given up for +/// the picker's session (`capture_unavailable`). +pub fn noteOffscreenStart(self: *State, started: bool) void { + self.offscreen_unstarted = if (started) 0 else self.offscreen_unstarted + 1; + if (self.offscreen_unstarted >= offscreen_unstarted_limit) self.capture_unavailable = true; } /// Open the picker for region `name`, anchored at `anchor` (or centred when null), and start @@ -723,7 +715,6 @@ pub fn recordCaptureFailed(self: *State, id: []const u8, started: bool) void { pub fn openPicker(self: *State, gpa: std.mem.Allocator, name: []const u8, anchor: ?dvui.Point.Natural) void { self.discardSnapshots(gpa); self.snapshots_wanted = true; - self.snapshots_requested = true; self.picker.open(gpa, name, anchor); } diff --git a/tests/integration.zig b/tests/integration.zig index 382ebb614..7a441d114 100644 --- a/tests/integration.zig +++ b/tests/integration.zig @@ -2676,7 +2676,7 @@ test "picker: a view that draws nowhere is drawn for its card one at a time, onl // not drawn. UnplacedViews.draws = @splat(0); editor.app.layout.openPicker(editor.app.gpa, "Sidebar", null); - for (0..shown) |i| editor.app.layout.recordCaptureFailed(UnplacedViews.ids[i], true); + for (0..shown) |i| editor.app.layout.recordCaptureFailed(editor.app.gpa, UnplacedViews.ids[i]); for (0..200) |_| try UnplacedViews.step(); try std.testing.expectEqual(@as([UnplacedViews.ids.len]u32, @splat(0)), UnplacedViews.draws);