Skip to content

vello_common: Add a fast path in tiling for lines fully contained in the viewport#1635

Merged
LaurenzV merged 2 commits into
mainfrom
laurenz/tile_opt_2
May 15, 2026
Merged

vello_common: Add a fast path in tiling for lines fully contained in the viewport#1635
LaurenzV merged 2 commits into
mainfrom
laurenz/tile_opt_2

Conversation

@LaurenzV
Copy link
Copy Markdown
Collaborator

Based on #1634. push_row is quite expensive, because a lot of logic is needed to account for culling. This PR adds a fast path for the common case where the line is completely contained in the viewport. This gets me to:

tile_aaa/Ghostscript_Tiger
                        time:   [89.356 µs 90.769 µs 92.179 µs]
                        change: [-3.0779% -0.9448% +1.1767%] (p = 0.39 > 0.05)
                        No change in performance detected.
Found 4 outliers among 50 measurements (8.00%)
  4 (8.00%) high mild

Tested this against my PDF test suite and also in interactive mode.

@LaurenzV LaurenzV requested a review from b0nes164 May 10, 2026 09:05
Copy link
Copy Markdown
Member

@b0nes164 b0nes164 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo one comment!

Comment on lines +840 to +844
let row_bottom_x = if row_bottom_y == line_bottom_y {
line_bottom_x
} else {
p0_x + (row_bottom_y - p0_y) * x_slope
};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe once you do this you don't need the min/max clamping to catch NaN's anymore, because this will actually quite gracefully catch the perfectly horizontal case? (And since a perfectly horizontal line is always contained to a single row, no other guards are neccessary.)

Can you add a TODO for this?

@LaurenzV LaurenzV force-pushed the laurenz/tile_opt_1 branch from 93ecc2c to b88a6e6 Compare May 14, 2026 06:39
Base automatically changed from laurenz/tile_opt_1 to main May 14, 2026 12:24
@LaurenzV LaurenzV force-pushed the laurenz/tile_opt_2 branch from 705bba6 to aa80dde Compare May 14, 2026 17:09
@LaurenzV LaurenzV requested a review from b0nes164 May 14, 2026 20:33
@b0nes164
Copy link
Copy Markdown
Member

Are the failures on ubuntu flakes? I'm rerunning the jobs.

@LaurenzV LaurenzV force-pushed the laurenz/tile_opt_2 branch from 90e73b8 to c0e316e Compare May 15, 2026 19:46
@LaurenzV LaurenzV added this pull request to the merge queue May 15, 2026
Merged via the queue into main with commit b81500f May 15, 2026
17 checks passed
@LaurenzV LaurenzV deleted the laurenz/tile_opt_2 branch May 15, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants