From 22189cc777431d9902fe418a9cc41bd0172ab4eb Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 11 Sep 2026 18:36:02 +0200 Subject: [PATCH] cocoa: convert only the damaged rows, so reporting damage does something MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Incremental present was wired end to end and saved nothing. Measured on a window presenting at 60 Hz, same binary, one bool changed: 26.2% of a core with whole-surface present, 26.8% with damage reporting. Slightly worse, not better. Sampling the process said why. -drawRect: wrapped the WHOLE framebuffer in a fresh NSBitmapImageRep and drew it once, relying on AppKit's clip to limit what reached the screen — which it does. What it does not limit is the conversion: -[NSView displayIfNeeded] -> drawRect: -> -[NSImageRep drawInRect:fromRect:...] -> -[NSBitmapImageRep CGImage] A rep has no backing CGImage until it is drawn, so every present converted every row whatever the invalid region said. The clip narrowed the blit and left the conversion where it was. A run of rows is contiguous in the framebuffer, so it can be wrapped as a bitmap of its own. presentRects now hands its rectangles to -drawRect: through the window, DrawBands reduces them to merged row runs, and each run is drawn as its own rep into its own destination rect, so the conversion is proportional to what changed. WHAT IT IS WORTH, measured rather than implied: a window repainting a small region every frame at 60 Hz costs 22.2% of a core before and 19.1% after, two runs each, consistent in direction. That is ~14% off the frame cost — real and reproducible, and well short of what the profile suggested. The conversion was about a seventh of the cost, not the bulk of it; the rest sits below this layer and is not addressed here. An AppKit-initiated draw — an expose, a resize, the first display — finds no pending rectangles (they are consumed by the draw that reads them, and presentFull clears them) and falls back to one band covering the buffer, which is exactly the draw this replaces. The row arithmetic lives in mapping.go, the OS-independent half the Linux lane measures, and is covered by construction: bands merge when they touch as well as when they overlap, clamp to the buffer, drop what falls outside, and BandDest places each band where the whole-buffer draw would have put those same rows — with adjacent bands meeting exactly, since a gap between two of them is a stale line across the window. Co-Authored-By: Claude Opus 5 --- internal/cocoa/cocoa_darwin.go | 72 ++++++++++++++---- internal/cocoa/mapping.go | 88 +++++++++++++++++++++- internal/cocoa/mapping_test.go | 132 +++++++++++++++++++++++++++++++++ 3 files changed, 277 insertions(+), 15 deletions(-) diff --git a/internal/cocoa/cocoa_darwin.go b/internal/cocoa/cocoa_darwin.go index b2a37ef..c2ee902 100644 --- a/internal/cocoa/cocoa_darwin.go +++ b/internal/cocoa/cocoa_darwin.go @@ -180,8 +180,13 @@ type Window struct { // See Options.Passive. passive bool - root toolkit.Widget - dmg damageRenderer + root toolkit.Widget + dmg damageRenderer + // pending is the damage presentRects asked AppKit to redraw, handed to + // -drawRect: so it converts only those rows. Empty means "whole buffer", + // which is what an AppKit-initiated draw — an expose, a resize, the first + // display — needs, since its invalid region is none of our business. + pending []toolkit.Rect buttonHeld bool dnd *dnd.Controller @@ -322,30 +327,59 @@ func viewDrawRect(self objc.ID, _ objc.SEL) { } w.mu.Lock() buf, bw, bh := w.buf, w.w, w.h + pending := w.pending + w.pending = nil w.mu.Unlock() if len(buf) == 0 || bw == 0 || bh == 0 { return } - rep := newBitmapRep(buf, bw, bh) - if rep == 0 { - return - } bounds := objc.Send[nsRect](self, selBounds) - // The full drawInRect: form with respectFlipped:YES honours the flipped - // view so the buffer's row 0 lands at the top of the window; fromRect zero = - // whole image; fraction 1.0; hints nil. The op is Copy in the ordinary - // opaque path; in translucent mode it is SourceOver so the framebuffer's - // transparent holes (punched over material regions) reveal the effect views - // composited behind this view. + // The op is Copy in the ordinary opaque path; in translucent mode it is + // SourceOver so the framebuffer's transparent holes (punched over material + // regions) reveal the effect views composited behind this view. op := nsCompositingCopy if w.translucent { op = nsCompositingSourceOver } - objc.Send[objc.ID](rep, selDrawInRectFull, bounds, nsRect{}, uint(op), 1.0, true, objc.ID(0)) - rep.Send(selRelease) + bands := DrawBands(pending, bh) + if len(bands) == 0 { + bands = []Band{{Y: 0, H: bh}} // AppKit asked; it gets everything + } + for _, b := range bands { + w.drawBand(buf, bw, bh, b, bounds, op) + } runtime.KeepAlive(buf) } +// drawBand converts and blits one run of framebuffer rows. +// +// A run of rows is contiguous in the buffer, so it can be wrapped as a bitmap of +// its own — and that is the saving. AppKit converts the rep it is handed to a +// CGImage in order to draw it, over every row the rep spans, whatever the clip +// says: a rep over the whole buffer converts the whole buffer even when one line +// of text changed. Sampling a window presenting at 60 Hz put +// -[NSBitmapImageRep CGImage] at the top of the draw path, which is why +// reporting damage at all had saved nothing measurable (26.2% of a core without, +// 26.8% with). +func (w *Window) drawBand(buf []byte, bw, bh int, b Band, bounds nsRect, op int) { + if b.H <= 0 || b.Y < 0 || b.Y+b.H > bh { + return + } + stride := bw * 4 + rep := newBitmapRep(buf[b.Y*stride:], bw, b.H) + if rep == 0 { + return + } + defer rep.Send(selRelease) + x, y, dw, dh := BandDest(b, bh, bounds.Origin.X, bounds.Origin.Y, + bounds.Size.W, bounds.Size.H) + dst := nsRect{Origin: nsPoint{X: x, Y: y}, Size: nsSize{W: dw, H: dh}} + // respectFlipped:YES honours the flipped view so the band's row 0 lands at + // the top of its destination; fromRect zero = the whole (banded) image; + // fraction 1.0; hints nil. + objc.Send[objc.ID](rep, selDrawInRectFull, dst, nsRect{}, uint(op), 1.0, true, objc.ID(0)) +} + // newBitmapRep wraps an RGBA buffer in an NSBitmapImageRep that references (does // not copy) the bytes. buf must outlive the rep's use (the caller keeps it // alive). The returned rep is owned by the caller (alloc/init) and released @@ -1039,6 +1073,9 @@ func (w *Window) presentFull() { if w.view == 0 { return } + w.mu.Lock() + w.pending = nil + w.mu.Unlock() w.view.Send(selSetNeedsDisplay, true) w.view.Send(selDisplayIfNeeded) } @@ -1049,6 +1086,13 @@ func (w *Window) presentRects(rects []toolkit.Rect) { if w.view == 0 || len(rects) == 0 { return } + // Handed to -drawRect: through the window rather than read from AppKit's + // dirty region, because these are OUR rectangles and the callback's NSRect + // rides in the float registers undeclared. It is consumed there, so a draw + // AppKit starts for its own reasons finds nothing and redraws everything. + w.mu.Lock() + w.pending = rects + w.mu.Unlock() for _, r := range rects { x, y, rw, rh := DirtyRect(r, w.scale) w.view.Send(selSetNeedsDisplayRect, nsRect{Origin: nsPoint{X: x, Y: y}, Size: nsSize{W: rw, H: rh}}) diff --git a/internal/cocoa/mapping.go b/internal/cocoa/mapping.go index 5f9202d..5a927c9 100644 --- a/internal/cocoa/mapping.go +++ b/internal/cocoa/mapping.go @@ -25,7 +25,11 @@ // everything here. package cocoa -import "github.com/go-widgets/toolkit" +import ( + "slices" + + "github.com/go-widgets/toolkit" +) // NSEventModifierFlags bits used by the backend. AppKit reports device- // independent modifier state in the high bits of the flags mask. @@ -355,3 +359,85 @@ func unitToByte(v float64) uint8 { return uint8(v*255 + 0.5) } } + +// Band is a contiguous run of framebuffer ROWS to convert and blit: Y is the +// first row, H the count. A band is always full width. +type Band struct{ Y, H int } + +// DrawBands reduces damage rectangles to the row runs a draw has to touch, +// merged, ordered and clamped to a buffer bufH rows tall. Rectangles that fall +// wholly outside contribute nothing; an empty result means there is nothing to +// draw. +// +// Rows, not rectangles, because a run of rows is CONTIGUOUS in the framebuffer +// and can therefore be wrapped as a bitmap of its own. That is the whole point: +// -drawRect: builds an NSBitmapImageRep over the buffer and AppKit converts it +// to a CGImage to draw it, and that conversion covers every row the rep spans +// whatever the clip says. Sampling a window presenting at 60 Hz put +// -[NSBitmapImageRep CGImage] at the top of the draw path, under +// -[NSImageRep drawInRect:fromRect:...] under -[NSView displayIfNeeded]: the +// invalid region limited what reached the screen, not what was converted, so +// reporting damage saved nothing at all (measured: 26.2% of a core without +// damage reporting, 26.8% with). A rep spanning only the changed rows makes the +// conversion proportional to the change. +// +// The x span is deliberately dropped. Narrowing columns would need a +// non-contiguous sub-image, which is a copy — and the rows are where the cost +// is: a changed line of text spans a handful of rows out of a thousand. +func DrawBands(rects []toolkit.Rect, bufH int) []Band { + if bufH <= 0 { + return nil + } + var spans []Band + for _, r := range rects { + y0, y1 := r.Y, r.Y+r.H + if y0 < 0 { + y0 = 0 + } + if y1 > bufH { + y1 = bufH + } + if y1 <= y0 { + continue + } + spans = append(spans, Band{Y: y0, H: y1 - y0}) + } + if len(spans) == 0 { + return nil + } + slices.SortFunc(spans, func(a, b Band) int { return a.Y - b.Y }) + out := []Band{spans[0]} + for _, s := range spans[1:] { + last := &out[len(out)-1] + // Touching counts as overlapping: two bands that meet edge to edge are + // one run of rows, and splitting them would convert the seam twice. + if s.Y <= last.Y+last.H { + if end := s.Y + s.H; end > last.Y+last.H { + last.H = end - last.Y + } + continue + } + out = append(out, s) + } + return out +} + +// BandDest is where a band of framebuffer rows lands in the flipped view, given +// the view's bounds in points and the buffer's height in device pixels. +// +// Full width, because DrawBands drops the x span; the y arithmetic is the whole +// of it, and it is the part that can be wrong. The scale is taken from the +// buffer and the bounds rather than from the window's stored scale, so a band +// lands exactly where the whole-buffer draw would have put those same rows -- +// including on the frame after a backing-scale change, when the two disagree +// for one draw. +// +// A bounds with no height cannot say where anything goes: the band is returned +// at the origin with no height, which draws nothing. +func BandDest(b Band, bufH int, ox, oy, ow, oh float64) (x, y, w, h float64) { + if bufH <= 0 || oh <= 0 { + return ox, oy, ow, 0 + } + perRow := oh / float64(bufH) + return ox, oy + float64(b.Y)*perRow, ow, float64(b.H) * perRow +} diff --git a/internal/cocoa/mapping_test.go b/internal/cocoa/mapping_test.go index b2e5eb7..2440856 100644 --- a/internal/cocoa/mapping_test.go +++ b/internal/cocoa/mapping_test.go @@ -302,3 +302,135 @@ func TestUnitToByte(t *testing.T) { } } } + +func TestDrawBands(t *testing.T) { + cases := []struct { + name string + in []toolkit.Rect + bufH int + want []Band + }{{ + name: "nothing to draw", + in: nil, bufH: 100, want: nil, + }, { + name: "a buffer with no rows", + in: []toolkit.Rect{{Y: 0, H: 10}}, bufH: 0, want: nil, + }, { + // The case the whole change exists for: a line of text changed, and the + // conversion should cover those rows rather than the window. + name: "one small rectangle", + in: []toolkit.Rect{{X: 40, Y: 120, W: 200, H: 14}}, bufH: 1000, + want: []Band{{Y: 120, H: 14}}, + }, { + name: "two apart stay apart", + in: []toolkit.Rect{{Y: 10, H: 5}, {Y: 100, H: 5}}, bufH: 1000, + want: []Band{{Y: 10, H: 5}, {Y: 100, H: 5}}, + }, { + name: "out of order", + in: []toolkit.Rect{{Y: 100, H: 5}, {Y: 10, H: 5}}, bufH: 1000, + want: []Band{{Y: 10, H: 5}, {Y: 100, H: 5}}, + }, { + name: "overlapping merge", + in: []toolkit.Rect{{Y: 10, H: 20}, {Y: 20, H: 20}}, bufH: 1000, + want: []Band{{Y: 10, H: 30}}, + }, { + // Edge to edge is one run: converting the seam twice costs more than + // the row it saves. + name: "touching merge", + in: []toolkit.Rect{{Y: 10, H: 10}, {Y: 20, H: 10}}, bufH: 1000, + want: []Band{{Y: 10, H: 20}}, + }, { + name: "one swallowed by another", + in: []toolkit.Rect{{Y: 10, H: 100}, {Y: 20, H: 5}}, bufH: 1000, + want: []Band{{Y: 10, H: 100}}, + }, { + name: "clamped to the buffer", + in: []toolkit.Rect{{Y: -20, H: 30}, {Y: 990, H: 40}}, bufH: 1000, + want: []Band{{Y: 0, H: 10}, {Y: 990, H: 10}}, + }, { + name: "wholly outside contributes nothing", + in: []toolkit.Rect{{Y: -50, H: 10}, {Y: 2000, H: 10}, {Y: 5, H: 0}}, bufH: 1000, + want: nil, + }} + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := DrawBands(c.in, c.bufH) + if len(got) != len(c.want) { + t.Fatalf("DrawBands = %v, want %v", got, c.want) + } + for i := range got { + if got[i] != c.want[i] { + t.Fatalf("DrawBands = %v, want %v", got, c.want) + } + } + }) + } +} + +// Whatever the rectangles, every row one of them names must end up inside a +// band: a row left out is a row the screen keeps stale, because the +// framebuffer persists between frames. +func TestDrawBandsCoversEveryRowItWasGiven(t *testing.T) { + const bufH = 200 + rects := []toolkit.Rect{ + {Y: 190, H: 30}, {Y: 3, H: 1}, {Y: 3, H: 40}, {Y: -5, H: 7}, {Y: 120, H: 0}, + } + bands := DrawBands(rects, bufH) + for _, r := range rects { + for y := max(r.Y, 0); y < min(r.Y+r.H, bufH); y++ { + in := false + for _, b := range bands { + if y >= b.Y && y < b.Y+b.H { + in = true + break + } + } + if !in { + t.Fatalf("row %d was asked for and lies outside every band %v", y, bands) + } + } + } +} + +func TestBandDest(t *testing.T) { + // A retina window: 1600 buffer rows over 800 points, so two pixels a point. + // The band at rows 200..214 lands at points 100..107. + if x, y, w, h := BandDest(Band{Y: 200, H: 14}, 1600, 0, 0, 400, 800); x != 0 || y != 100 || w != 400 || h != 7 { + t.Fatalf("BandDest retina = (%v,%v,%v,%v), want (0,100,400,7)", x, y, w, h) + } + // Scale 1: rows are points. + if _, y, _, h := BandDest(Band{Y: 30, H: 10}, 600, 0, 0, 400, 600); y != 30 || h != 10 { + t.Fatalf("BandDest scale1 = (y%v,h%v), want (y30,h10)", y, h) + } + // The bounds origin is carried, not assumed to be zero. + if x, y, _, _ := BandDest(Band{Y: 0, H: 10}, 600, 12, 34, 400, 600); x != 12 || y != 34 { + t.Fatalf("BandDest origin = (%v,%v), want (12,34)", x, y) + } + // The whole buffer as one band covers the whole bounds, which is what the + // AppKit-initiated draw falls back to: it must be pixel-identical to the + // draw this replaces. + if x, y, w, h := BandDest(Band{Y: 0, H: 1600}, 1600, 0, 0, 400, 800); x != 0 || y != 0 || w != 400 || h != 800 { + t.Fatalf("BandDest whole = (%v,%v,%v,%v), want the full bounds", x, y, w, h) + } + // Nothing to scale by: no height, so nothing is drawn. + if _, _, _, h := BandDest(Band{Y: 0, H: 10}, 0, 0, 0, 400, 800); h != 0 { + t.Fatalf("BandDest with no rows h = %v, want 0", h) + } + if _, _, _, h := BandDest(Band{Y: 0, H: 10}, 600, 0, 0, 400, 0); h != 0 { + t.Fatalf("BandDest with no bounds h = %v, want 0", h) + } +} + +// The bands of one frame must tile the bounds exactly as the whole-buffer draw +// would: a gap between two adjacent bands is a line of stale pixels across the +// window. +func TestBandDestsOfAdjacentBandsMeetExactly(t *testing.T) { + const bufH, oh = 1600, 800.0 + a := Band{Y: 100, H: 20} + b := Band{Y: 120, H: 30} + _, ay, _, ah := BandDest(a, bufH, 0, 0, 400, oh) + _, by, _, _ := BandDest(b, bufH, 0, 0, 400, oh) + if ay+ah != by { + t.Fatalf("band ends at %v and the next starts at %v; the seam is stale", ay+ah, by) + } +}