From 05a2ecb606cc761e6d916050e2bb3439a08ff753 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Tue, 11 Aug 2026 12:09:03 +0200 Subject: [PATCH] FillRect fills rows, not pixels Profiling a Wallpaper after moving it onto DrawImage showed 75% of its time in FillRect -- not in the image at all. FillRect was a PutPixel per pixel, and a PutPixel is a translation, two bounds tests, a clip test and a blend: 700,000 of them to paint one window-sized background. This is the primitive the toolkit leans on hardest. Every background, every button, every table row, every focus ring goes through it, so the cost was being paid by every widget on every frame, not by images. Where a fill may write is a rectangle -- the destination intersected with the surface and with the clip -- and it is decided once. An opaque fill then writes the SAME four bytes everywhere, so one row is built and the rest of the rectangle is that row copied. The row itself is built by writing one pixel and doubling it, so N pixels cost log2(N) copies rather than N stores. A translucent fill still composites pixel by pixel, because its result depends on what was underneath. full window 42612 ns/op vs 1751507 per-pixel 41x 35 table rows 8164 ns/op vs 285603 per-pixel 35x TestFillRectMatchesTheLoopItReplaced compares against the PutPixel loop byte for byte over 18 cases: single pixel, single row, single column, odd widths, off each edge, entirely off the surface, empty, negative height, fully transparent, translucent, clipped, clipped away, translated, and translated AND clipped. The ground is painted a non-black colour first so a blend that wrongly behaves as a copy cannot hide in zeroes. 100% statement coverage. Co-Authored-By: Claude Opus 4.8 --- fill_test.go | 149 +++++++++++++++++++++++++++++++++++++++++++++++++++ pixel.go | 58 ++++++++++++++++++-- 2 files changed, 202 insertions(+), 5 deletions(-) create mode 100644 fill_test.go diff --git a/fill_test.go b/fill_test.go new file mode 100644 index 0000000..d0e792f --- /dev/null +++ b/fill_test.go @@ -0,0 +1,149 @@ +// Copyright (c) 2026 the go-widgets/painter authors. All rights reserved. +// Use of this source code is governed by a BSD-3-Clause license that can be +// found in the LICENSE file at the root of this repository. + +package painter + +import "testing" + +// fillReference is the loop FillRect used to be: a PutPixel per pixel, which +// applies the translation, the surface bounds, the clip and the blend. It is +// the yardstick, because a fill that is fast and different is not an +// optimisation. +func fillReference(p *PixelPainter, r Rect, c RGBA) { + for y := r.Y; y < r.Y+r.H; y++ { + for x := r.X; x < r.X+r.W; x++ { + p.PutPixel(x, y, c) + } + } +} + +func TestFillRectMatchesTheLoopItReplaced(t *testing.T) { + opaque := RGBA{R: 200, G: 100, B: 50, A: 255} + half := RGBA{R: 200, G: 100, B: 50, A: 128} + + for _, tc := range []struct { + name string + r Rect + c RGBA + clip *Rect + tx, ty int + }{ + {name: "opaque, whole surface", r: Rect{X: 0, Y: 0, W: 20, H: 20}, c: opaque}, + {name: "opaque, inset", r: Rect{X: 3, Y: 4, W: 6, H: 5}, c: opaque}, + {name: "opaque, single pixel", r: Rect{X: 7, Y: 7, W: 1, H: 1}, c: opaque}, + {name: "opaque, single column", r: Rect{X: 2, Y: 2, W: 1, H: 9}, c: opaque}, + {name: "opaque, single row", r: Rect{X: 2, Y: 2, W: 9, H: 1}, c: opaque}, + {name: "opaque, odd width", r: Rect{X: 1, Y: 1, W: 7, H: 3}, c: opaque}, + {name: "off the left and top", r: Rect{X: -4, Y: -3, W: 9, H: 8}, c: opaque}, + {name: "off the right and bottom", r: Rect{X: 15, Y: 16, W: 9, H: 8}, c: opaque}, + {name: "entirely off the surface", r: Rect{X: 40, Y: 40, W: 5, H: 5}, c: opaque}, + {name: "empty", r: Rect{X: 2, Y: 2, W: 0, H: 5}, c: opaque}, + {name: "negative height", r: Rect{X: 2, Y: 2, W: 5, H: -1}, c: opaque}, + {name: "fully transparent", r: Rect{X: 0, Y: 0, W: 10, H: 10}, c: RGBA{R: 9, G: 9, B: 9}}, + {name: "translucent", r: Rect{X: 2, Y: 2, W: 8, H: 6}, c: half}, + {name: "opaque, clipped", r: Rect{X: 0, Y: 0, W: 20, H: 20}, c: opaque, clip: &Rect{X: 3, Y: 5, W: 6, H: 4}}, + {name: "translucent, clipped", r: Rect{X: 0, Y: 0, W: 20, H: 20}, c: half, clip: &Rect{X: 3, Y: 5, W: 6, H: 4}}, + {name: "opaque, clipped away", r: Rect{X: 0, Y: 0, W: 5, H: 5}, c: opaque, clip: &Rect{X: 15, Y: 15, W: 2, H: 2}}, + {name: "translated", r: Rect{X: 1, Y: 1, W: 5, H: 4}, c: opaque, tx: 6, ty: 7}, + {name: "translated and clipped", r: Rect{X: 0, Y: 0, W: 12, H: 12}, c: opaque, tx: 3, ty: 3, clip: &Rect{X: 1, Y: 1, W: 6, H: 6}}, + } { + t.Run(tc.name, func(t *testing.T) { + run := func(f func(p *PixelPainter)) *PixelPainter { + p := newPix(20, 20) + // A non-black ground, so a blend that wrongly behaves as a copy + // shows up instead of hiding in zeroes. + for i := 0; i < len(p.Buf); i += 4 { + p.Buf[i], p.Buf[i+1], p.Buf[i+2], p.Buf[i+3] = 30, 60, 90, 255 + } + if tc.tx != 0 || tc.ty != 0 { + p.PushTranslate(tc.tx, tc.ty) + } + if tc.clip != nil { + p.PushClip(*tc.clip) + } + f(p) + if tc.clip != nil { + p.PopClip() + } + if tc.tx != 0 || tc.ty != 0 { + p.PopTranslate() + } + return p + } + + got := run(func(p *PixelPainter) { p.FillRect(tc.r, tc.c) }) + want := run(func(p *PixelPainter) { fillReference(p, tc.r, tc.c) }) + + for i := range got.Buf { + if got.Buf[i] != want.Buf[i] { + px := i / 4 + t.Fatalf("pixel %d,%d byte %d: FillRect %d, the loop it replaced %d", + px%20, px/20, i%4, got.Buf[i], want.Buf[i]) + } + } + }) + } +} + +// A Buf shorter than Width*Height*4 is tolerated rather than fatal, the same +// way PutPixel tolerates it. +func TestFillRectShortBuffer(t *testing.T) { + p := &PixelPainter{Buf: make([]byte, 4*2*4), Width: 4, Height: 4} + p.FillRect(Rect{X: 0, Y: 0, W: 4, H: 4}, RGBA{R: 1, G: 2, B: 3, A: 255}) + if p.Buf[3] == 0 { + t.Error("the rows that did fit were not filled") + } + + // The translucent path walks the same rows and must tolerate it too. + q := &PixelPainter{Buf: make([]byte, 4*2*4), Width: 4, Height: 4} + q.FillRect(Rect{X: 0, Y: 0, W: 4, H: 4}, RGBA{R: 1, G: 2, B: 3, A: 128}) + if q.Buf[3] == 0 { + t.Error("the rows that did fit were not blended") + } +} + +// The fill that every widget makes: a window-sized opaque background. +func BenchmarkFillRect(b *testing.B) { + p := newPix(1000, 700) + r := Rect{X: 0, Y: 0, W: 1000, H: 700} + c := RGBA{R: 30, G: 60, B: 90, A: 255} + b.ResetTimer() + for i := 0; i < b.N; i++ { + p.FillRect(r, c) + } +} + +func BenchmarkFillRectPerPixel(b *testing.B) { + p := newPix(1000, 700) + r := Rect{X: 0, Y: 0, W: 1000, H: 700} + c := RGBA{R: 30, G: 60, B: 90, A: 255} + b.ResetTimer() + for i := 0; i < b.N; i++ { + fillReference(p, r, c) + } +} + +// Small fills are what a table of rows and a row of buttons actually issue, and +// there the per-row set-up has to earn its keep too. +func BenchmarkFillRectSmall(b *testing.B) { + p := newPix(1000, 700) + c := RGBA{R: 30, G: 60, B: 90, A: 255} + b.ResetTimer() + for i := 0; i < b.N; i++ { + for y := 0; y < 700; y += 20 { + p.FillRect(Rect{X: 4, Y: y, W: 180, H: 18}, c) + } + } +} + +func BenchmarkFillRectSmallPerPixel(b *testing.B) { + p := newPix(1000, 700) + c := RGBA{R: 30, G: 60, B: 90, A: 255} + b.ResetTimer() + for i := 0; i < b.N; i++ { + for y := 0; y < 700; y += 20 { + fillReference(p, Rect{X: 4, Y: y, W: 180, H: 18}, c) + } + } +} diff --git a/pixel.go b/pixel.go index 76f5f45..0dedbc5 100644 --- a/pixel.go +++ b/pixel.go @@ -89,13 +89,61 @@ func NewPixelPainter(buf []byte, width, height int) *PixelPainter { return &PixelPainter{Buf: buf, Width: width, Height: height} } -// FillRect fills r with c. Out-of-bounds bytes are dropped so a -// widget that ranges past the edge doesn't panic. +// FillRect fills r with c. Out-of-bounds bytes are dropped so a widget that +// ranges past the edge doesn't panic. +// +// This is the primitive the toolkit leans on hardest -- every background, every +// button, every table row -- and it used to be a PutPixel per pixel, which is a +// shift, two bounds tests, a clip test and a blend each: 700,000 of them for a +// window-sized fill. Where a fill may write is a rectangle, decided once; and +// an opaque fill writes the SAME four bytes everywhere, so one row is built and +// the rest of the rectangle is that row copied. A translucent fill still +// composites pixel by pixel, because its result depends on what was underneath. func (p *PixelPainter) FillRect(r Rect, c RGBA) { - for y := r.Y; y < r.Y+r.H; y++ { - for x := r.X; x < r.X+r.W; x++ { - p.PutPixel(x, y, c) + if r.W <= 0 || r.H <= 0 || c.A == 0 { + return + } + r = shiftRect(p.off, r) + + eff := intersect(r, Rect{X: 0, Y: 0, W: p.Width, H: p.Height}) + if n := len(p.clip); n > 0 { + eff = intersect(eff, p.clip[n-1]) + } + if eff.W <= 0 || eff.H <= 0 { + return + } + + first := -1 + for y := eff.Y; y < eff.Y+eff.H; y++ { + dstRow := y * p.Width * 4 + lo, hi := dstRow+eff.X*4, dstRow+(eff.X+eff.W)*4 + // A caller may hand over a Buf shorter than Width*Height*4, exactly as + // PutPixel tolerates; a row that does not fit is skipped, not fatal. + if hi > len(p.Buf) { + continue + } + + if c.A != 0xFF { + for off := lo; off < hi; off += 4 { + p.blendInto(off, c) + } + continue + } + + if first >= 0 { + copy(p.Buf[lo:hi], p.Buf[first:first+eff.W*4]) + continue + } + + // Build the first row by writing one pixel and doubling it: each copy + // moves as many bytes as are already there, so a row of N pixels costs + // log2(N) copies rather than N stores. + row := p.Buf[lo:hi] + row[0], row[1], row[2], row[3] = c.R, c.G, c.B, 0xFF + for filled := 4; filled < len(row); filled *= 2 { + copy(row[filled:], row[:filled]) } + first = lo } }