Skip to content

LibGfx+LibPDF: Add sample options to fill_path API (and other tidy ups) #25963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

MacDue
Copy link
Member

@MacDue MacDue commented May 26, 2025

No description provided.

MacDue added 4 commits May 26, 2025 20:57
This allows selecting one of the implemented sample modes (e.g.
Sample32xAA or Sample8xNoAA).
This updates LibPDF (and the aliased_fill test) to use the 2-bit NoAA
sample for drawing triangles via the painter API. To make switching
sample kinds easy down the line, the "Gfx::SampleAA" and
"Gfx::SampleNoAA" aliases have been added for the default
antialiased/aliased sample modes.
No need for this to be in the header (and could add to build times).
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label May 26, 2025
Copy link
Contributor

@nico nico left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

According to hyperfine 'Build/lagom/bin/pdf Tests/LibPDF/shade-gouraud-free.pdf --render-bench --render-repeats 50', a bit over 3% faster.

Before:

Benchmark 1: Build/lagom/bin/pdf Tests/LibPDF/shade-gouraud-free.pdf --render-bench --render-repeats 50
  Time (mean ± σ):     620.8 ms ±   2.4 ms    [User: 609.0 ms, System: 10.2 ms]
  Range (min … max):   616.8 ms … 624.7 ms    10 runs

After:

Benchmark 1: Build/lagom/bin/pdf Tests/LibPDF/shade-gouraud-free.pdf --render-bench --render-repeats 50
  Time (mean ± σ):     600.4 ms ±   3.6 ms    [User: 589.1 ms, System: 9.9 ms]
  Range (min … max):   595.2 ms … 607.1 ms    10 runs

One tiny nit:

@@ -436,5 +436,6 @@ template class EdgeFlagPathRasterizer<Sample8xAA>;
template class EdgeFlagPathRasterizer<Sample16xAA>;
template class EdgeFlagPathRasterizer<Sample32xAA>;
template class EdgeFlagPathRasterizer<Sample8xNoAA>;
template class EdgeFlagPathRasterizer<Sample2xNoAA>;
Copy link
Contributor

@nico nico May 26, 2025

Choose a reason for hiding this comment

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

nit: put above the 8xNoAA one for nicer sorting

@@ -50,6 +50,21 @@ struct AA {
}
};

// 2-bit sample (mainly for use with NoAA).
Copy link
Contributor

Choose a reason for hiding this comment

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

(here too)

@@ -247,10 +262,12 @@ using Sample8xAA = Detail::Sample8x<Detail::AA>;
using Sample16xAA = Detail::Sample16x<Detail::AA>;
using Sample32xAA = Detail::Sample32x<Detail::AA>;
using Sample8xNoAA = Detail::Sample8x<Detail::NoAA>;
using Sample2xNoAA = Detail::Sample2x<Detail::NoAA>;
Copy link
Contributor

Choose a reason for hiding this comment

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

(here too)


extern template class EdgeFlagPathRasterizer<Sample8xAA>;
extern template class EdgeFlagPathRasterizer<Sample16xAA>;
extern template class EdgeFlagPathRasterizer<Sample32xAA>;
extern template class EdgeFlagPathRasterizer<Sample8xNoAA>;
extern template class EdgeFlagPathRasterizer<Sample2xNoAA>;
Copy link
Contributor

Choose a reason for hiding this comment

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

(here too)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 pr-needs-review PR needs review from a maintainer or community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants