From 58e9a2e235672932bb341fc6657b31635e2db9ec Mon Sep 17 00:00:00 2001 From: Tristan Ross Date: Tue, 25 Aug 2026 19:54:01 -0700 Subject: [PATCH] fix: problems on cosmic --- lib/phantom/app.zig | 26 ++- lib/phantom/backend/prism.zig | 194 +++++++++++++++---- lib/phantom/icon/builtin.zig | 52 ++++++ lib/phantom/text/Font.zig | 11 ++ lib/phantom/window.zig | 338 ++++++++++++++++++++++++++++++---- 5 files changed, 542 insertions(+), 79 deletions(-) diff --git a/lib/phantom/app.zig b/lib/phantom/app.zig index 44b1d65..ead7022 100644 --- a/lib/phantom/app.zig +++ b/lib/phantom/app.zig @@ -104,10 +104,28 @@ pub const App = struct { // stdout alone and never falls back to stderr. const is_tty = std.Io.File.stdout().isTty(init.io) catch false; // Asked of lattice, not guessed from the environment: see - // `selectBackend`, and `window.available` for what lattice is asked. - const window_possible = phantom.window.available(init.gpa, init.io, init.environ_map); - switch (selectBackend(init.environ_map, window_possible, is_tty)) { - .gpu => return phantom.window.App.run(init, root, .{}), + // `selectBackend`, and `window.open` for what lattice is asked. + // + // The window it opens IS the window the session runs on. Opening one is + // the only honest way to know a window is possible, so the answer comes + // with the thing itself, and dropping it here would mean connecting a + // second time for something already in hand. + const opts = phantom.window.Options{}; + var opened = phantom.window.open(init.gpa, init.io, init.environ_map, opts); + // Whatever this function does next, an unused window is given back. Every + // path that takes it over clears this first, so it is closed once or not + // at all. + defer if (opened) |*o| o.close(); + + switch (selectBackend(init.environ_map, opened != null, is_tty)) { + .gpu => { + const win = opened orelse + // Only reachable through `PHANTOM_BACKEND=gpu`, which asks + // for the window path on a machine that has no window. + return error.NoWindowBackend; + opened = null; + return phantom.window.App.runOn(init, win, root, opts); + }, .tui => return phantom.Tui.run(init, root, .{}), .none => { // A clear message, because "it did nothing" is the worst outcome here. diff --git a/lib/phantom/backend/prism.zig b/lib/phantom/backend/prism.zig index 6a5dd4d..e1fa13a 100644 --- a/lib/phantom/backend/prism.zig +++ b/lib/phantom/backend/prism.zig @@ -440,6 +440,41 @@ pub const PrismBackend = struct { return self.atlas.ensureCoverage(self.gpa, key, cov); } + /// Append the two triangles that draw one atlas bitmap, with its top-left at + /// `x`, `y` in device pixels and tinted by `color`. + /// + /// A glyph, an icon and a mark standing in for a missing glyph are the same + /// thing here: coverage in one atlas, one pipeline, one tint by vertex + /// colour. The caller works out where the bitmap goes, which is the only + /// part that differs between them. + fn appendCoverageQuad( + self: *PrismBackend, + batch: *Batcher, + entry: GlyphAtlas.Entry, + x: f32, + y: f32, + color: geom.Color, + viewport: geom.PhysicalSize, + ) !void { + const w: f32 = @floatFromInt(entry.w); + const h: f32 = @floatFromInt(entry.h); + const tl = toClip(x, y, viewport, self.flip_y); + const tr = toClip(x + w, y, viewport, self.flip_y); + const br = toClip(x + w, y + h, viewport, self.flip_y); + const bl = toClip(x, y + h, viewport, self.flip_y); + const cr = color.r; + const cg = color.g; + const cb = color.b; + const ca = color.a; + // Two triangles (CCW): TL-TR-BR, TL-BR-BL. + try batch.tverts.append(self.gpa, .{ .x = tl[0], .y = tl[1], .u = entry.u0, .v = entry.v0, .r = cr, .g = cg, .b = cb, .a = ca }); + try batch.tverts.append(self.gpa, .{ .x = tr[0], .y = tr[1], .u = entry.u1, .v = entry.v0, .r = cr, .g = cg, .b = cb, .a = ca }); + try batch.tverts.append(self.gpa, .{ .x = br[0], .y = br[1], .u = entry.u1, .v = entry.v1, .r = cr, .g = cg, .b = cb, .a = ca }); + try batch.tverts.append(self.gpa, .{ .x = tl[0], .y = tl[1], .u = entry.u0, .v = entry.v0, .r = cr, .g = cg, .b = cb, .a = ca }); + try batch.tverts.append(self.gpa, .{ .x = br[0], .y = br[1], .u = entry.u1, .v = entry.v1, .r = cr, .g = cg, .b = cb, .a = ca }); + try batch.tverts.append(self.gpa, .{ .x = bl[0], .y = bl[1], .u = entry.u0, .v = entry.v1, .r = cr, .g = cg, .b = cb, .a = ca }); + } + /// Rasterize `list` into `target`. The list is walked ONCE and the draws are /// emitted in list order, so a primitive covers everything before it and nothing /// after it. Consecutive primitives of one kind still share a single draw. @@ -528,6 +563,44 @@ pub const PrismBackend = struct { // Cast the type-erased font pointer back to the concrete type. const font: *text.Font = @ptrCast(@alignCast(run.font)); for (run.glyphs) |g| { + // A face with no glyph for this codepoint draws `.notdef`, + // which is a replacement box. The bundled faces are display + // faces and cover little outside ASCII, so an interface that + // writes a tick or a chevron in ordinary text gets boxes here + // while a terminal, drawing with its own font, gets the real + // character. Standing a built-in mark in its place closes + // that split: the two backends show the same thing, and the + // caller writes the character it means. + // + // The mark goes in a square box the size of the text, resting + // on the baseline. Every built-in is drawn inside a margin on + // a centred grid, so that lands a tick at about cap height + // and the midline dots of an ellipsis at about half of it, + // which is where each belongs beside text. Layout already + // reserved this codepoint's advance, so the mark drops into + // the space the box would have taken and nothing shifts. + if (!font.hasGlyph(g.cp)) { + if (icon_builtin.iconForCodepoint(g.cp)) |id| { + const side = run.size; + const entry = try self.ensureIcon(id, .{ .width = side, .height = side }); + if (entry.w == 0 or entry.h == 0) continue; + const baseline = run.origin.y + run.ascent + g.y; + // The icon arm below places a bitmap at + // `box_top + box_height + entry.top`. With the box + // bottom on the baseline that is + // `(baseline - side) + side + entry.top`, so the + // side cancels and the baseline carries it. + try self.appendCoverageQuad( + &batch, + entry, + run.origin.x + g.x + @as(f32, @floatFromInt(entry.left)) - cur_off.x, + baseline + @as(f32, @floatFromInt(entry.top)) - cur_off.y, + run.color, + viewport, + ); + continue; + } + } const entry = try self.atlas.ensure(self.gpa, font, run.size, g.cp); // Skip zero-size glyphs (whitespace, missing). if (entry.w == 0 or entry.h == 0) continue; @@ -541,24 +614,7 @@ pub const PrismBackend = struct { // origin.y + ascent (origin is the run's top-left). const gx: f32 = run.origin.x + g.x + @as(f32, @floatFromInt(entry.left)) - cur_off.x; const gy: f32 = run.origin.y + run.ascent + g.y + @as(f32, @floatFromInt(entry.top)) - cur_off.y; - const gw: f32 = @floatFromInt(entry.w); - const gh: f32 = @floatFromInt(entry.h); - // Four corners in clip space, paired with atlas UVs. - const tl = toClip(gx, gy, viewport, self.flip_y); - const tr = toClip(gx + gw, gy, viewport, self.flip_y); - const br = toClip(gx + gw, gy + gh, viewport, self.flip_y); - const bl = toClip(gx, gy + gh, viewport, self.flip_y); - const cr = run.color.r; - const cg = run.color.g; - const cb_c = run.color.b; - const ca = run.color.a; - // Two triangles (CCW): TL-TR-BR, TL-BR-BL. - try batch.tverts.append(self.gpa, .{ .x = tl[0], .y = tl[1], .u = entry.u0, .v = entry.v0, .r = cr, .g = cg, .b = cb_c, .a = ca }); - try batch.tverts.append(self.gpa, .{ .x = tr[0], .y = tr[1], .u = entry.u1, .v = entry.v0, .r = cr, .g = cg, .b = cb_c, .a = ca }); - try batch.tverts.append(self.gpa, .{ .x = br[0], .y = br[1], .u = entry.u1, .v = entry.v1, .r = cr, .g = cg, .b = cb_c, .a = ca }); - try batch.tverts.append(self.gpa, .{ .x = tl[0], .y = tl[1], .u = entry.u0, .v = entry.v0, .r = cr, .g = cg, .b = cb_c, .a = ca }); - try batch.tverts.append(self.gpa, .{ .x = br[0], .y = br[1], .u = entry.u1, .v = entry.v1, .r = cr, .g = cg, .b = cb_c, .a = ca }); - try batch.tverts.append(self.gpa, .{ .x = bl[0], .y = bl[1], .u = entry.u0, .v = entry.v1, .r = cr, .g = cg, .b = cb_c, .a = ca }); + try self.appendCoverageQuad(&batch, entry, gx, gy, run.color, viewport); } }, .icon => |ic| { @@ -576,23 +632,7 @@ pub const PrismBackend = struct { // primitive's origin. const ix: f32 = ic.origin.x + @as(f32, @floatFromInt(entry.left)) - cur_off.x; const iy: f32 = ic.origin.y + ic.size.height + @as(f32, @floatFromInt(entry.top)) - cur_off.y; - const iw: f32 = @floatFromInt(entry.w); - const ih: f32 = @floatFromInt(entry.h); - const tl = toClip(ix, iy, viewport, self.flip_y); - const tr = toClip(ix + iw, iy, viewport, self.flip_y); - const br = toClip(ix + iw, iy + ih, viewport, self.flip_y); - const bl = toClip(ix, iy + ih, viewport, self.flip_y); - const cr = ic.color.r; - const cg = ic.color.g; - const cb_c = ic.color.b; - const ca = ic.color.a; - // Two triangles (CCW): TL-TR-BR, TL-BR-BL, as the glyph path above. - try batch.tverts.append(self.gpa, .{ .x = tl[0], .y = tl[1], .u = entry.u0, .v = entry.v0, .r = cr, .g = cg, .b = cb_c, .a = ca }); - try batch.tverts.append(self.gpa, .{ .x = tr[0], .y = tr[1], .u = entry.u1, .v = entry.v0, .r = cr, .g = cg, .b = cb_c, .a = ca }); - try batch.tverts.append(self.gpa, .{ .x = br[0], .y = br[1], .u = entry.u1, .v = entry.v1, .r = cr, .g = cg, .b = cb_c, .a = ca }); - try batch.tverts.append(self.gpa, .{ .x = tl[0], .y = tl[1], .u = entry.u0, .v = entry.v0, .r = cr, .g = cg, .b = cb_c, .a = ca }); - try batch.tverts.append(self.gpa, .{ .x = br[0], .y = br[1], .u = entry.u1, .v = entry.v1, .r = cr, .g = cg, .b = cb_c, .a = ca }); - try batch.tverts.append(self.gpa, .{ .x = bl[0], .y = bl[1], .u = entry.u0, .v = entry.v1, .r = cr, .g = cg, .b = cb_c, .a = ca }); + try self.appendCoverageQuad(&batch, entry, ix, iy, ic.color, viewport); }, }; try batch.flush(); @@ -1180,3 +1220,87 @@ test "the atlas keeps one mark at two heights apart" { try std.testing.expectEqual(short.h, again.h); try std.testing.expectEqual(short.u0, again.u0); } + +/// Render one codepoint as a text run and give back the target's pixels, for the +/// fallback tests below. The caller frees the returned slice. +fn renderOneCodepoint(gpa: std.mem.Allocator, dev: prism.Device, font: *text.Font, cp: u21) ![]u8 { + var backend = try PrismBackend.init(dev, gpa); + defer backend.deinit(); + const W: u32 = 64; + const H: u32 = 64; + const target = try dev.createResource(.{ .image = .{ .width = W, .height = H, .format = .rgba8_unorm, .usage = .{ .render_target = true } } }); + defer dev.destroyResource(target); + const ctx = try dev.createContext(); + defer ctx.deinit(); + + var list = dl.DisplayList{}; + defer list.deinit(gpa); + const glyphs = [_]dl.PositionedGlyph{.{ .cp = cp, .x = 0, .y = 0 }}; + try list.append(gpa, .{ .text = .{ + .glyphs = &glyphs, + .text = "", + .font = font, + .size = 32, + .color = geom.Color.rgb(1, 1, 1), + .origin = geom.PhysicalOffset{ .x = 8, .y = 8 }, + .ascent = 40, + } }); + try backend.render(ctx, target, geom.PhysicalSize{ .width = 64, .height = 64 }, list, geom.Color.rgb(0, 0, 0)); + return gpa.dupe(u8, try dev.mapResource(target)); +} + +test "a codepoint the face cannot draw becomes its built-in mark" { + // The bundled faces are display faces with almost nothing outside ASCII, so + // a tick or a chevron written in ordinary text came out as the replacement + // box glyph 0 draws, while a terminal showed the real character from its own + // font. The two backends have to agree. + const gpa = std.testing.allocator; + try requireRaster(gpa); + const sel = prism.drivers.createBestDevice(gpa) orelse return error.NoPrismDevice; + defer sel.device.deinit(); + var font = try text.Font.load(gpa, text.builtin.neuropol_bytes); + defer font.deinit(gpa); + + // The premise: neither of these is in the face. If one ever is, this test is + // measuring something else and says so rather than passing quietly. + try std.testing.expect(!font.hasGlyph('\u{25B8}')); + try std.testing.expect(!font.hasGlyph('\u{2603}')); + + // U+25B8 has a mark; U+2603, a snowman, does not and keeps the box. + const marked = try renderOneCodepoint(gpa, sel.device, &font, '\u{25B8}'); + defer gpa.free(marked); + const unmarked = try renderOneCodepoint(gpa, sel.device, &font, '\u{2603}'); + defer gpa.free(unmarked); + + // Both are glyph 0 to the face, so without the substitution these are the + // same pixels. That is the whole of the bug, and this is what separates + // them. + try std.testing.expect(!std.mem.eql(u8, marked, unmarked)); + + // And the mark drew something, rather than the substitution quietly + // dropping a codepoint it could not blit. + var lit: u32 = 0; + var i: usize = 0; + while (i < marked.len) : (i += 4) { + if (marked[i] > 40) lit += 1; + } + try std.testing.expect(lit > 10); +} + +test "the two spellings of one mark draw the same thing" { + // U+25B6 and U+25B8 are the large and small right-pointing triangles. A + // caller writes whichever it prefers and means the same mark, so both reach + // the same built-in and draw identically. + const gpa = std.testing.allocator; + try requireRaster(gpa); + const sel = prism.drivers.createBestDevice(gpa) orelse return error.NoPrismDevice; + defer sel.device.deinit(); + var font = try text.Font.load(gpa, text.builtin.neuropol_bytes); + defer font.deinit(gpa); + + const large = try renderOneCodepoint(gpa, sel.device, &font, '\u{25B6}'); + defer gpa.free(large); + const small = try renderOneCodepoint(gpa, sel.device, &font, '\u{25B8}'); + defer gpa.free(small); + try std.testing.expectEqualSlices(u8, large, small); +} diff --git a/lib/phantom/icon/builtin.zig b/lib/phantom/icon/builtin.zig index dd513a8..420cbff 100644 --- a/lib/phantom/icon/builtin.zig +++ b/lib/phantom/icon/builtin.zig @@ -40,6 +40,10 @@ pub const Id = enum(u32) { minus = 9, rule_vertical = 10, rule_horizontal = 11, + + /// Three dots on the midline, for elided content. U+22EF, which no bundled + /// face has either. + ellipsis = 12, }; /// What a cell backend draws in place of the mark. @@ -84,6 +88,41 @@ pub fn cellMarkFor(id: Id) ?CellMark { .minus => .{ .cp = '-' }, .rule_vertical => .{ .cp = '\u{2502}', .tile = true }, .rule_horizontal => .{ .cp = '\u{2500}', .tile = true }, + .ellipsis => .{ .cp = '\u{22EF}' }, + }; +} + +/// The mark to draw in place of `cp` when the face has no glyph for it, or null +/// when nothing here means that codepoint. +/// +/// The bundled faces are display faces: every non-ASCII codepoint probed +/// resolves to glyph 0, so text carrying one of these draws a replacement box in +/// pixel mode while a terminal draws it correctly from its own font. That split +/// is what this closes. A caller writes the character it means, in ordinary +/// text, and both modes show the same thing. +/// +/// This is `cellMarkFor` read backwards, plus the near neighbours: a caller +/// reaching for a right-pointing triangle may write the small one or the large +/// one, and both mean the same mark. Nothing here is a substitution between +/// DIFFERENT marks, only between spellings of one. +/// +/// Only codepoints with a real mark belong here. A face that is missing a letter +/// is a fault to see, not to paper over. +pub fn iconForCodepoint(cp: u21) ?Id { + return switch (cp) { + '\u{2713}' => .check, + '\u{2717}' => .cross, + // Both sizes of each geometric triangle. U+25B8 and U+25BE are what a + // terminal interface usually reaches for, being lighter beside text. + '\u{25C0}', '\u{25C2}' => .chevron_left, + '\u{25B6}', '\u{25B8}' => .chevron_right, + '\u{25B2}', '\u{25B4}' => .chevron_up, + '\u{25BC}', '\u{25BE}' => .chevron_down, + '\u{2192}' => .arrow_right, + '\u{2502}' => .rule_vertical, + '\u{2500}' => .rule_horizontal, + '\u{22EF}' => .ellipsis, + else => null, }; } @@ -102,6 +141,7 @@ pub fn pathFor(id: Id) path.Path { .minus => minus, .rule_vertical => rule_vertical, .rule_horizontal => rule_horizontal, + .ellipsis => ellipsis, }; } @@ -383,6 +423,18 @@ const minus = path.Path{ .verbs = &.{ .{ .line = .{ .x = 19, .y = 12 } }, } }; +/// Three dots on the midline. Drawn as three of the shortest strokes the round +/// cap can make, so each reads as a dot rather than a dash: a zero length +/// segment would be culled before it reached the rasterizer. +const ellipsis = path.Path{ .verbs = &.{ + .{ .move = .{ .x = 5, .y = 12 } }, + .{ .line = .{ .x = 5.01, .y = 12 } }, + .{ .move = .{ .x = 12, .y = 12 } }, + .{ .line = .{ .x = 12.01, .y = 12 } }, + .{ .move = .{ .x = 19, .y = 12 } }, + .{ .line = .{ .x = 19.01, .y = 12 } }, +} }; + /// Full bleed and butt capped, so stacking one per row draws a continuous rail /// with no seam at the row boundaries. This is U+2502's job in a terminal, and /// the reason it is here is that no bundled font has that glyph. diff --git a/lib/phantom/text/Font.zig b/lib/phantom/text/Font.zig index bf80e9c..203a203 100644 --- a/lib/phantom/text/Font.zig +++ b/lib/phantom/text/Font.zig @@ -79,6 +79,17 @@ pub fn descent(self: *const Font) i16 { return self.metrics.descent; } +/// Whether this face can draw `cp` at all. +/// +/// Glyph 0 is the `.notdef` box every face reserves for a codepoint it does not +/// cover, so a face that "draws" it draws a replacement box. The bundled faces +/// are display faces and cover little outside ASCII, which is what makes this +/// worth asking: see `backend/prism.zig`, which substitutes a built-in mark +/// rather than blit the box. +pub fn hasGlyph(self: *const Font, cp: u21) bool { + return self.metrics.glyphIndex(cp) != 0; +} + /// Horizontal advance of `cp` at `px_size` pixels (device space). Metrics only, /// no rasterization, no allocation. pub fn advance(self: *const Font, cp: u21, px_size: f32) f32 { diff --git a/lib/phantom/window.zig b/lib/phantom/window.zig index b558b6d..e07acc7 100644 --- a/lib/phantom/window.zig +++ b/lib/phantom/window.zig @@ -13,8 +13,40 @@ const compositor_builds = phantom.backend.prism.builds_here; const lattice = if (compositor_builds) @import("lattice") else void; -/// Whether a window can actually be opened here, asked of lattice rather than -/// guessed from the environment. +/// A window that has been opened, before a session has been built on it. +/// +/// This exists so the question and the answer are the same act. `open` cannot +/// tell a caller a window is possible without opening one, and a caller that +/// then threw that away and opened a second would pay for the whole connection +/// twice: a round trip, the globals, the driver, the seat bind. +/// +/// The lattice `Context` is a backend pointer and a flag, with every piece of +/// real state behind that pointer, so handing one over is a copy of two words +/// and not a move of anything live. +/// +/// Whoever holds this owns it. `Session.initOn` takes it; anyone else calls +/// `close`. +pub const Opening = if (compositor_builds) struct { + ctx: lattice.Context, + surface: lattice.Surface, + + /// Give back the window and the connection, for a caller that asked whether + /// a window was possible and is not going to draw in one. + pub fn close(self: *Opening) void { + self.ctx.destroySurface(self.surface.id); + self.ctx.deinit(); + self.* = undefined; + } +} else struct { + /// Never built: `open` reports null wherever the compositor path does not + /// compile. The empty shape is what keeps `open`'s signature analyzable on + /// a target where `lattice` is a `void`. + pub fn close(self: *Opening) void { + _ = self; + } +}; + +/// Open a window, or report that this machine cannot. /// /// lattice resolves its own backend from `WAYLAND_DISPLAY`, `DISPLAY`, /// `XDG_SESSION_TYPE` and the DRM device, and its rules are not the obvious @@ -23,29 +55,67 @@ const lattice = if (compositor_builds) @import("lattice") else void; /// one, and the way it would disagree is that phantom would choose the window /// backend under X11 and lattice would then quietly render to nothing. /// -/// So this asks by opening a context and reading back what lattice gave it. A -/// backend with no outputs has no screen to put a window on, which is what the -/// headless backend reports, and it is the signal available today without -/// lattice having to expose the resolved backend kind. +/// So this does not predict. It opens the context, checks that lattice gave it +/// a render device and a screen, and then CREATES THE SURFACE, because that is +/// the step that fails on a backend which answers every earlier question and +/// still cannot put a window up. A probe that stopped before it would report +/// yes and leave the real start to fail with `NotImplemented`. /// -/// The context is opened and closed again, which costs a compositor round trip -/// once at startup. That is worth more than a guess that can be wrong. -pub fn available(gpa: std.mem.Allocator, io: std.Io, environ: *const std.process.Environ.Map) bool { +/// Null means no window here, with everything this opened already closed. +pub fn open( + gpa: std.mem.Allocator, + io: std.Io, + environ: *const std.process.Environ.Map, + opts: Options, +) ?Opening { // An `if` on a comptime-known condition drops the untaken branch before // analysis, which is what keeps `lattice` (a `void` wherever the compositor // path does not build) from being reached for members it does not have. An - // early `return false` above would NOT do that: the rest of the body sits at + // early `return null` above would NOT do that: the rest of the body sits at // function scope and is analyzed either way. An aarch64-macos build is what // caught this. if (compositor_builds) { - var ctx = lattice.Context.init(gpa, io, environ, .{}) catch return false; - defer ctx.deinit(); + var ctx = lattice.Context.init(gpa, io, environ, .{ + .initial_width = opts.width, + .initial_height = opts.height, + .driver = opts.driver, + }) catch return null; // A render device is as necessary as a screen: without one there is // nothing to draw the window's contents with. - if (ctx.renderDevice() == null) return false; - return ctx.outputs().len > 0; + if (ctx.renderDevice() == null) { + ctx.deinit(); + return null; + } + // No outputs is what the headless backend reports, and a backend with no + // screen has nowhere to put a window. + if (ctx.outputs().len == 0) { + ctx.deinit(); + return null; + } + const surface = ctx.createSurface(.{ + .title = opts.title, + .width = opts.width, + .height = opts.height, + .color = lattice.ColorConfig.sdr(.xrgb8888), + }) catch { + ctx.deinit(); + return null; + }; + return .{ .ctx = ctx, .surface = surface }; } - return false; + return null; +} + +/// Whether a window can be opened here. +/// +/// The yes or no on its own, for a caller with nothing to do with the window it +/// proves. It opens one and closes it again, so a caller that WILL go on to +/// draw should call `open` and keep what it gets: that is the same answer +/// without paying for the connection twice. +pub fn available(gpa: std.mem.Allocator, io: std.Io, environ: *const std.process.Environ.Map) bool { + var opened = open(gpa, io, environ, .{}) orelse return false; + opened.close(); + return true; } /// Fold a lattice key event into the shared phantom key event. lattice has @@ -122,18 +192,33 @@ pub const App = struct { // lattice type it names, from being analyzed on a target where lattice // is a `void`. It is the one gate in this file, so a caller never needs // one of its own: `app.zig` calls this unconditionally. + if (compositor_builds) { + const opened = open(process.gpa, process.io, process.environ_map, opts) orelse + return error.NoWindowBackend; + return runOn(process, opened, root, opts); + } + // Only reachable through `PHANTOM_BACKEND=gpu`, since `open` reports null + // here and nothing else selects this backend. + return error.NoWindowBackend; + } + + /// The same, on a window that is already open. + /// + /// `app.zig` takes this path: it has to open a window to know whether it can + /// have one, and this is what keeps that window instead of dropping it and + /// connecting a second time. Takes ownership of `opened`, whether it returns + /// an error or not. + pub fn runOn(process: std.process.Init, opened: Opening, root: phantom.Root, opts: Options) !void { if (compositor_builds) { // A Session holds pointers into its own fields, so it must be built // at an address that does not move: see `tui.Tui.run`, which says // the same of its own session for the same reason. var session: Session = undefined; - try session.init(process.gpa, process.io, process.environ_map, root, opts); + try session.initOn(process.gpa, process.io, opened, root, opts); defer session.deinit(); while (try session.step()) {} return; } - // Only reachable through `PHANTOM_BACKEND=gpu`, since `available` reports - // false here and nothing else selects this backend. return error.NoWindowBackend; } }; @@ -177,6 +262,8 @@ pub const Session = struct { last_point: phantom.PhysicalOffset, /// Physical pixels for each logical pixel, applied to incoming pointer /// coordinates as well as to layout. See `step` for why it is 1.0 today. + /// Whether the last `step` drew. Read through `drewFrame`. + drew_frame: bool, scale: f32, running: bool, @@ -195,18 +282,42 @@ pub const Session = struct { environ: *const std.process.Environ.Map, root: phantom.Root, opts: Options, + ) !void { + const opened = open(gpa, io, environ, opts) orelse return error.NoWindowBackend; + return self.initOn(gpa, io, opened, root, opts); + } + + /// Build a session on a window that is already open, taking it over. + /// + /// For a caller that asked whether a window was possible and got one back + /// from `open`: that connection is the connection to draw on, and opening a + /// second would pay the whole cost again for an answer it already has. + /// + /// This takes ownership either way. On success `deinit` gives the window + /// back; on failure this closes it before returning, so a caller that gets + /// an error must NOT call `deinit` and must NOT call `Opening.close`. + pub fn initOn( + self: *Session, + gpa: std.mem.Allocator, + io: std.Io, + opened: Opening, + root: phantom.Root, + opts: Options, ) !void { self.gpa = gpa; self.io = io; self.opts = opts; - self.ctx = try lattice.Context.init(gpa, io, environ, .{ - .initial_width = opts.width, - .initial_height = opts.height, - .driver = opts.driver, - }); - errdefer self.ctx.deinit(); + self.ctx = opened.ctx; + self.surface = opened.surface; + errdefer { + self.ctx.destroySurface(self.surface.id); + self.ctx.deinit(); + } + // `open` already found a device, so this cannot fail through that path. + // It is still asked rather than assumed, because the answer is the one + // the backend is built on. const dev = self.ctx.renderDevice() orelse return error.NoRenderDevice; self.backend = try phantom.backend.PrismBackend.init(dev.*, gpa); errdefer self.backend.deinit(); @@ -215,14 +326,6 @@ pub const Session = struct { // goldens leave this false. self.backend.flip_y = true; - self.surface = try self.ctx.createSurface(.{ - .title = opts.title, - .width = opts.width, - .height = opts.height, - .color = lattice.ColorConfig.sdr(.xrgb8888), - }); - errdefer self.ctx.destroySurface(self.surface.id); - self.diag_writer = std.Io.File.stderr().writerStreaming(io, &self.diag_buf); self.arena = std.heap.ArenaAllocator.init(gpa); errdefer self.arena.deinit(); @@ -277,6 +380,7 @@ pub const Session = struct { self.last_point = phantom.PhysicalOffset.zero; self.scale = 1.0; self.running = true; + self.drew_frame = false; } /// Put back everything `init` set up, tree first and window last, so nothing @@ -296,13 +400,29 @@ pub const Session = struct { /// Draw a frame if the compositor is ready for one, then take in whatever /// events have arrived. Returns false once the window has closed, at which /// point the caller stops calling it and calls `deinit`. + /// + /// The return value is whether the session is still running and NOT whether + /// it drew: a compositor that is not ready for a frame yet is the normal + /// case, not the end of the session. Ask `drewFrame` for that, which is a + /// question only this backend raises. `tui.Session.step` draws every time, + /// because a terminal has nothing to hold a frame back. pub fn step(self: *Session) !bool { if (!self.running) return false; - if (self.ctx.renderAvailable(self.surface.id)) try self.renderFrame(); + self.drew_frame = self.ctx.renderAvailable(self.surface.id); + if (self.drew_frame) try self.renderFrame(); try self.ctx.poll(self.opts.poll_ms, handleEvent, self); return self.running; } + /// Whether the last `step` drew a frame. + /// + /// False when the compositor was not ready for one, which is what `step` + /// alone cannot tell a caller that tracks repaints. A `step` that returned + /// false, or one that has not run yet, reports false: nothing was drawn. + pub fn drewFrame(self: *const Session) bool { + return self.drew_frame; + } + /// Stop at the end of the current turn. Safe to call from a widget callback /// during `step`: the loop reads it on the way out. pub fn requestStop(self: *Session) void { @@ -313,19 +433,18 @@ pub const Session = struct { var bctx = phantom.BuildContext{ .arena = self.arena.allocator(), .owner = &self.owner }; const ts = std.Io.Clock.now(.awake, self.io); self.owner.scheduler.tick(ts.nanoseconds); - self.owner.flushDirty(&bctx); - // A rebuild can add or remove focusable nodes, so the traversal order is - // rebuilt from the tree rather than kept incrementally. - try self.focus_mgr.collect(self.gpa, self.el); + // The size comes FIRST, before anything builds. A widget reads it through + // `MediaQuery.of` during its build, so metrics published after + // `flushDirty` are the metrics of the frame before: a window resize then + // takes two frames to show, and the first of them lays out the new size + // with the old numbers. Asking the surface for its target is what makes + // the current size known, so that has to move up here too. const rt = try self.ctx.renderTarget(self.surface.id); const vp = phantom.PhysicalSize{ .width = @floatFromInt(rt.width), .height = @floatFromInt(rt.height), }; - self.canvas.clear(); - const ro = self.el.renderObject() orelse return error.NoRootRenderObject; - // Native has no buffer scaling (lattice sets neither set_buffer_scale // nor fractional-scale), so the surface, the framebuffer AND the // wl_pointer surface-local coordinates are all the SAME physical pixels @@ -345,6 +464,14 @@ pub const Session = struct { .dpr = self.scale, .text_scale = 1.0, }); + + self.owner.flushDirty(&bctx); + // A rebuild can add or remove focusable nodes, so the traversal order is + // rebuilt from the tree rather than kept incrementally. + try self.focus_mgr.collect(self.gpa, self.el); + + self.canvas.clear(); + const ro = self.el.renderObject() orelse return error.NoRootRenderObject; _ = ro.layout(phantom.BoxConstraints.tightScaled(vp, self.scale)); try ro.paint(&self.canvas, phantom.PhysicalOffset.zero); try self.backend.render( @@ -479,3 +606,134 @@ test "the default options open the window phantom has always opened" { // A frame at 60Hz, which is what `App.run` waited before the split. try std.testing.expectEqual(@as(?u32, 16), o.poll_ms); } + +// --------------------------------------------------------------------------- +// Session tests +// +// lattice's headless backend carries a real prism device, a surface and a +// render target, so a whole `Session` runs on it with no compositor. That is +// what makes the two behaviours below testable at all: everything else about +// the window path needs a display. +// --------------------------------------------------------------------------- + +/// A window session standing on lattice's headless backend, with the pieces a +/// test needs to reach: `h` to move the surface size or hold a frame back. +const Fixture = struct { + threaded: std.Io.Threaded, + h: *lattice.backends.Headless, + session: Session, + + fn open(f: *Fixture, gpa: std.mem.Allocator, root: phantom.Root) !void { + try phantom.backend.prism.requireRaster(gpa); + f.threaded = std.Io.Threaded.init(gpa, .{}); + errdefer f.threaded.deinit(); + const io = f.threaded.io(); + + f.h = try lattice.backends.Headless.init(gpa); + var ctx = lattice.Context.initWithBackend(f.h.backend()); + errdefer ctx.deinit(); + const surface = try ctx.createSurface(.{ + .title = "test", + .width = 800, + .height = 600, + .color = lattice.ColorConfig.sdr(.xrgb8888), + }); + // `initOn` takes the window over, which is the seam this exercises. + try f.session.initOn(gpa, io, .{ .ctx = ctx, .surface = surface }, root, .{ .poll_ms = 0 }); + } + + fn close(f: *Fixture) void { + f.session.deinit(); + f.threaded.deinit(); + } +}; + +test "a step that drew and a step that did not are told apart" { + // `step` returns whether the session is still running, so a compositor that + // is not ready for a frame and a session that is still going look the same + // from the outside. A caller tracking repaints needs the other answer. + const gpa = std.testing.allocator; + var f: Fixture = undefined; + f.open(gpa, phantom.Root.plain(testRoot)) catch |e| switch (e) { + error.SkipZigTest => return error.SkipZigTest, + else => return e, + }; + defer f.close(); + + // Nothing has run yet, so nothing has been drawn. + try std.testing.expect(!f.session.drewFrame()); + + f.h.setRenderAvailable(true); + try std.testing.expect(try f.session.step()); + try std.testing.expect(f.session.drewFrame()); + + // The compositor holds the next frame back. The session is still running, + // and that is exactly the case the return value cannot express. + f.h.setRenderAvailable(false); + try std.testing.expect(try f.session.step()); + try std.testing.expect(!f.session.drewFrame()); + + // And it recovers, rather than latching off. + f.h.setRenderAvailable(true); + try std.testing.expect(try f.session.step()); + try std.testing.expect(f.session.drewFrame()); +} + +/// Records the viewport width every time it builds. A resize has to reach a +/// build, and this is what reports which frame's numbers it saw. +var seen_width: f32 = 0; + +var reader_state: ?*SizeReader.State = null; + +const SizeReader = struct { + pub const State = struct { + base: phantom.StateBase = .{}, + pub fn build(s: *State, bctx: *phantom.BuildContext) anyerror!phantom.Widget { + // The framework owns the state, so the test reaches it through here. + reader_state = s; + seen_width = phantom.MediaQuery.of(bctx).size.width; + return (phantom.ColoredBox{ .color = phantom.Color.rgb(0, 0, 0) }).widget(); + } + }; + pub fn widget(self: *const SizeReader) phantom.Widget { + return phantom.StatefulWidget(SizeReader, self); + } +}; + +var size_reader = SizeReader{}; + +fn sizeReaderRoot(_: *phantom.BuildContext) phantom.Widget { + return size_reader.widget(); +} + +fn testRoot(_: *phantom.BuildContext) phantom.Widget { + return (phantom.ColoredBox{ .color = phantom.Color.rgb(0, 0, 0) }).widget(); +} + +test "a build sees the size of the frame it is building, not the one before" { + // `renderFrame` used to publish the viewport AFTER `flushDirty`, so a widget + // reading `MediaQuery.of` during its build got the previous frame's numbers. + // A window resize then took two frames to show, and the first of them laid + // out the new size with the old width. + const gpa = std.testing.allocator; + var f: Fixture = undefined; + f.open(gpa, phantom.Root.plain(sizeReaderRoot)) catch |e| switch (e) { + error.SkipZigTest => return error.SkipZigTest, + else => return e, + }; + defer f.close(); + + f.h.setRenderAvailable(true); + _ = try f.session.step(); + try std.testing.expectEqual(@as(f32, 800), seen_width); + + // The surface is now wider, which is what a compositor resize looks like + // from the render target's side. + f.h.tw = 1024; + // Dirty the tree so the next frame rebuilds. Without this `flushDirty` has + // nothing to do and the test would pass whatever the order was. + phantom.markNeedsBuild(reader_state.?); + seen_width = 0; + _ = try f.session.step(); + try std.testing.expectEqual(@as(f32, 1024), seen_width); +}