Skip to content
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

New "style" encoding to replace "linewidth" #389

Merged
merged 5 commits into from
Oct 27, 2023
Merged

Conversation

armansito
Copy link
Collaborator

@armansito armansito commented Oct 21, 2023

This PR replaces the existing floating-point "linewidth" stream with a new "style" stream based on the proposal in #303.

  • All pipelines up to flatten have been wired up to extract the fill style using the new encoding.
  • The encoding crate now encodes fills using the new scheme. Strokes still get converted to fills on the CPU but will be updated to optionally use the new encoding in follow-up changes.
  • Added a new test scene to specifically exercise even-odd vs non-zero fill rules.

Copy link
Collaborator

@dfrg dfrg left a comment

Choose a reason for hiding this comment

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

A few tiny style nits but otherwise lgtm, thanks!

/// ```
///
/// - miter_limit: u16 - The miter limit for a stroke, encoded in binary16 (half) floating
/// point representation. This field is on meaningful for the `Join::Miter`
Copy link
Collaborator

Choose a reason for hiding this comment

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

on -> only

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

pub const FLAGS_END_CAP_MASK: u32 = 0x0300_0000;
pub const MITER_LIMIT_MASK: u32 = 0xFFFF;

pub fn from_fill(fill: &Fill) -> Self {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it’d be slightly cleaner to take this parameter by value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

}

#[cfg(test)]
fn get_fill(&self) -> Option<Fill> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Small style nit: I know these are only for tests now but it seems likely they’ll become generally useful enough to be exposed at some point and the get_ prefix is generally elided for accessors.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

This will be used to pack the f64 miter_limit field in `kurbo::Stroke`
into a 16-bit encoding for GPU consumption.
Style encodes stroke vs fill style and their related parameters in an
8-byte data structure as described in vello#303. This data structure
will replace the existing "linewidth" stream.
Added a test scene specifically for fill rules, drawing a star shape
and intersecting arcs with opposite winding.
* The linewidth stream is now the "style" stream. This has been wired up
  all the way up to the `flatten` pipeline which uses the new encoding
  to extract the fill rule.

* Pipelines downstream of `flatten` still use the old linewidth scheme.
  They will be fixed up in a follow-up change.
@armansito armansito merged commit b5e3ae5 into main Oct 27, 2023
4 checks passed
@armansito armansito deleted the gpu-stroke-rework branch October 27, 2023 20:26
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