From da6d5acb1652e5ec9f2bcc234c5e645072064c10 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sun, 18 Oct 2026 16:17:20 +0200 Subject: [PATCH] perf(paint): normalise blitImage's source once, not once per pixel Round 175's own PR (#288) flagged blitImage as a known, smaller instance of the exact same anti-pattern blitTileClipped had: every blitted pixel went through the image.Image/color.Color interface (heap-allocating to box the returned value, and double-applying alpha the same way At().RGBA() always premultiplies, which was then passed as blendPixel's straight colour argument alongside the same alpha again as coverage). Unmeasured at the time; measured now with BenchmarkBlitImageLargePhoto (a 1920x1080 solid image): the cost is real, tens of milliseconds for a single full-bleed photo. blitImage normalises src to *image.NRGBA once internally (toNRGBA, already added in #288) and reads it via NRGBAAt directly. No call site changes needed since the function's own signature is unchanged -- all 5 of its callers (icon-only button, leading/trailing button icons, object-fit image tiling's non-tiling branch, rotated-box compositing) benefit automatically. Co-Authored-By: Claude Sonnet 5 --- paint/background_paint_test.go | 16 ++++++++++++++++ paint/paint.go | 21 +++++++++++++++++---- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/paint/background_paint_test.go b/paint/background_paint_test.go index abaa5d1..98b42b3 100644 --- a/paint/background_paint_test.go +++ b/paint/background_paint_test.go @@ -492,6 +492,22 @@ func BenchmarkPaintBgBitmapRepeat(b *testing.B) { } } +// BenchmarkBlitImageLargePhoto exercises a plain blit (no repeat +// multiplier, unlike BenchmarkPaintBgBitmapRepeat above) at a realistic +// full-bleed photo size, since blitImage has the IDENTICAL +// src.At(...).RGBA() pattern blitTileClipped had before round 175's fix — +// flagged in that round's PR as a known, smaller instance of the same +// anti-pattern, unmeasured at the time. One element, not stamped many +// times, so the question here is whether a SINGLE large image's own pixel +// count alone makes the interface-boxing/premultiply cost worth fixing. +func BenchmarkBlitImageLargePhoto(b *testing.B) { + dst := white(1920, 1080) + src := solidImage(1920, 1080, color.RGBA{10, 20, 200, 255}) + for i := 0; i < b.N; i++ { + blitImage(dst, src, 0, 0, dst.Rect) + } +} + func TestBgMode(t *testing.T) { if bgMode(nil) != resample.Bicubic { t.Error("nil style should default to bicubic") diff --git a/paint/paint.go b/paint/paint.go index dbc3590..2eec395 100644 --- a/paint/paint.go +++ b/paint/paint.go @@ -1660,8 +1660,19 @@ func toNRGBA(src image.Image) *image.NRGBA { // blitImage draws src onto dst with its top-left at (dx, dy), confined to clip // (an ancestor's overflow clip; always within the image bounds). +// +// src is normalised to *image.NRGBA once, here, rather than read through the +// image.Image/color.Color interface per pixel — the same fix round 175 made +// for blitTileClipped's background-repeat path (see toNRGBA's own doc +// comment), flagged there as a known, smaller instance of the same pattern: +// no repeat multiplier here, but a full-bleed photo is still hundreds of +// thousands to millions of pixels for one element, and the interface read +// also double-applied alpha the same way (At().RGBA() already premultiplies; +// passing that as blendPixel's straight colour argument alongside the same +// alpha again as coverage), invisible at alpha=255. func blitImage(dst *image.RGBA, src image.Image, dx, dy int, clip image.Rectangle) { - b := src.Bounds() + tile := toNRGBA(src) + b := tile.Bounds() for sy := b.Min.Y; sy < b.Max.Y; sy++ { ty := dy + (sy - b.Min.Y) if ty < clip.Min.Y || ty >= clip.Max.Y { @@ -1672,9 +1683,11 @@ func blitImage(dst *image.RGBA, src image.Image, dx, dy int, clip image.Rectangl if tx < clip.Min.X || tx >= clip.Max.X { continue } - r, g, bl, a := src.At(sx, sy).RGBA() - cov := uint8(a >> 8) - blendPixel(dst, tx, ty, css.Color{R: uint8(r >> 8), G: uint8(g >> 8), B: uint8(bl >> 8), A: 255}, cov) + c := tile.NRGBAAt(sx, sy) + if c.A == 0 { + continue + } + blendPixel(dst, tx, ty, css.Color{R: c.R, G: c.G, B: c.B, A: 255}, c.A) } } }