FillRect fills rows, not pixels - #14
Merged
Merged
Conversation
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Profiling a
Wallpaperafter moving it ontoDrawImageshowed 75% of its time inFillRect— not in the image at all.FillRectwas aPutPixelper pixel, and aPutPixelis 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. The image work was the smaller fish; it just happened to be the one holding the profiler.
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.
The small-fill case is there because a table of rows and a row of buttons is what the toolkit actually issues, and the per-row set-up has to earn its keep at that size too.
Proving the picture did not change
TestFillRectMatchesTheLoopItReplacedcompares against thePutPixelloop 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 shows up instead of hiding in zeroes.100% statement coverage.
🤖 Generated with Claude Code