From eded0a1dcd99a9e1a4421992237fe14a1e27f957 Mon Sep 17 00:00:00 2001 From: Arman Uguray Date: Mon, 13 Nov 2023 13:57:36 -0800 Subject: [PATCH] Address review comments --- crates/encoding/src/path.rs | 76 ++++++++++++++++++++++--------------- shader/flatten.wgsl | 14 +++---- shader/shared/pathtag.wgsl | 6 +-- 3 files changed, 56 insertions(+), 40 deletions(-) diff --git a/crates/encoding/src/path.rs b/crates/encoding/src/path.rs index 6118ff414..ed9cd99e5 100644 --- a/crates/encoding/src/path.rs +++ b/crates/encoding/src/path.rs @@ -444,10 +444,10 @@ impl<'a> PathEncoder<'a> { /// segment tells the GPU stroker whether to draw a cap or a join based on the topology of the /// path: /// - /// 1. This marker segment is encoded as a `quad-to` for an open path and a `line-to` for a - /// closed path. An open path gets drawn with a start and end cap. A closed path gets drawn - /// with a single join in place of the caps where the subpath's start and end control points - /// meet. + /// 1. This marker segment is encoded as a `quad-to` (2 additional points) for an open path and + /// a `line-to` (1 additional point) for a closed path. An open path gets drawn with a start + /// and end cap. A closed path gets drawn with a single join in place of the caps where the + /// subpath's start and end control points meet. /// /// 2. The marker segment tells the GPU flattening stage how to render caps and joins while /// processing each path segment in parallel. All subpaths end with the marker segment which @@ -523,15 +523,13 @@ impl<'a> PathEncoder<'a> { self.move_to(self.first_point[0], self.first_point[1]); } if self.state == PathState::MoveTo { - let x0 = self.first_point[0]; - let y0 = self.first_point[1]; + let p0 = (self.first_point[0], self.first_point[1]); // Ensure that we don't end up with a zero-length start tangent. - const EPS: f32 = 1e-12; - if (x - x0).abs() < EPS && (y - y0).abs() < EPS { + let Some((x, y)) = start_tangent_for_curve(p0, (x, y), p0, p0) else { // Drop the segment if its length is zero - // TODO: do this for all not segments, not just start? + // TODO: do this for all not segments, not just start. return; - } + }; self.first_start_tangent_end = [x, y]; } let buf = [x, y]; @@ -552,17 +550,11 @@ impl<'a> PathEncoder<'a> { self.move_to(self.first_point[0], self.first_point[1]); } if self.state == PathState::MoveTo { - let x0 = self.first_point[0]; - let y0 = self.first_point[1]; + let p0 = (self.first_point[0], self.first_point[1]); // Ensure that we don't end up with a zero-length start tangent. - const EPS: f32 = 1e-12; - let (x, y) = if (x1 - x0).abs() > EPS || (y1 - y0).abs() > EPS { - (x1, y1) - } else if (x2 - x0).abs() > EPS || (y2 - y0).abs() > EPS { - (x2, y2) - } else { + let Some((x, y)) = start_tangent_for_curve(p0, (x1, y1), (x2, y2), p0) else { // Drop the segment if its length is zero - // TODO: do this for all not segments, not just start? + // TODO: do this for all not segments, not just start. return; }; self.first_start_tangent_end = [x, y]; @@ -585,19 +577,11 @@ impl<'a> PathEncoder<'a> { self.move_to(self.first_point[0], self.first_point[1]); } if self.state == PathState::MoveTo { - let x0 = self.first_point[0]; - let y0 = self.first_point[1]; + let p0 = (self.first_point[0], self.first_point[1]); // Ensure that we don't end up with a zero-length start tangent. - const EPS: f32 = 1e-12; - let (x, y) = if (x1 - x0).abs() > EPS || (y1 - y0).abs() > EPS { - (x1, y1) - } else if (x2 - x0).abs() > EPS || (y2 - y0).abs() > EPS { - (x2, y2) - } else if (x3 - x0).abs() > EPS || (y3 - y0).abs() > EPS { - (x3, y3) - } else { + let Some((x, y)) = start_tangent_for_curve(p0, (x1, y1), (x2, y2), (x3, y3)) else { // Drop the segment if its length is zero - // TODO: do this for all not segments, not just start? + // TODO: do this for all not segments, not just start. return; }; self.first_start_tangent_end = [x, y]; @@ -739,6 +723,38 @@ impl fello::scale::Pen for PathEncoder<'_> { } } +// Returns the end point of the start tangent of a curve starting at `(x0, y0)`, or `None` if the +// curve is degenerate / has zero-length. The inputs are a sequence of control points that can +// represent a line, a quadratic Bezier, or a cubic Bezier. Lines and quadratic Beziers can be +// passed to this function by simply setting the invalid control point degrees equal to `(x0, y0)`. +fn start_tangent_for_curve( + p0: (f32, f32), + p1: (f32, f32), + p2: (f32, f32), + p3: (f32, f32), +) -> Option<(f32, f32)> { + debug_assert!(!p0.0.is_nan()); + debug_assert!(!p0.1.is_nan()); + debug_assert!(!p1.0.is_nan()); + debug_assert!(!p1.1.is_nan()); + debug_assert!(!p2.0.is_nan()); + debug_assert!(!p2.1.is_nan()); + debug_assert!(!p3.0.is_nan()); + debug_assert!(!p3.1.is_nan()); + + const EPS: f32 = 1e-12; + let pt = if (p1.0 - p0.0).abs() > EPS || (p1.1 - p0.1).abs() > EPS { + p1 + } else if (p2.0 - p0.0).abs() > EPS || (p2.1 - p0.1).abs() > EPS { + p2 + } else if (p3.0 - p0.0).abs() > EPS || (p3.1 - p0.1).abs() > EPS { + p3 + } else { + return None; + }; + Some(pt) +} + #[cfg(test)] mod tests { use super::*; diff --git a/shader/flatten.wgsl b/shader/flatten.wgsl index 6c0fe674b..d22d89e35 100644 --- a/shader/flatten.wgsl +++ b/shader/flatten.wgsl @@ -153,7 +153,7 @@ fn flatten_cubic(cubic: Cubic) { // HACK: this increase subdivision count as function of the stroke width for shitty strokes. var tol = sqrt(REM_ACCURACY); - if cubic.flags == 1u { + if cubic.flags == CUBIC_IS_STROKE { tol *= min(1000., dot(cubic.stroke, cubic.stroke)); } let params = estimate_subdiv(qp0, qp1, qp2, tol); @@ -302,7 +302,7 @@ fn read_path_segment(tag: PathTagData, transform: Transform, is_stroke: bool) -> var seg_type = tag.tag_byte & PATH_TAG_SEG_TYPE; let pathseg_offset = tag.monoid.pathseg_offset; - let is_stroke_cap_marker = is_stroke && (tag.tag_byte & PATH_TAG_SUBPATH_END_BIT) != 0u; + let is_stroke_cap_marker = is_stroke && (tag.tag_byte & PATH_TAG_SUBPATH_END) != 0u; let is_open = seg_type == PATH_TAG_QUADTO; if (tag.tag_byte & PATH_TAG_F32) != 0u { @@ -331,7 +331,7 @@ fn read_path_segment(tag: PathTagData, transform: Transform, is_stroke: bool) -> // is ignored. // // This is encoded this way because encoding this as a lineto would require adding a moveto, - // which would terminate the subpath too early (by setting the SUBPATH_END_BIT on the + // which would terminate the subpath too early (by setting the SUBPATH_END on the // segment preceding the cap marker). This scheme is only used for strokes. p0 = transform_apply(transform, p1); p1 = transform_apply(transform, p2); @@ -374,7 +374,7 @@ fn read_neighboring_segment(ix: u32) -> NeighboringSegment { let pts = read_path_segment(tag, transform, true); let is_closed = (tag.tag_byte & PATH_TAG_SEG_TYPE) == PATH_TAG_LINETO; - let is_stroke_cap_marker = (tag.tag_byte & PATH_TAG_SUBPATH_END_BIT) != 0u; + let is_stroke_cap_marker = (tag.tag_byte & PATH_TAG_SUBPATH_END) != 0u; let do_join = !is_stroke_cap_marker || is_closed; let p0 = pts.p0; let tangent = cubic_start_tangent(pts.p0, pts.p1, pts.p2, pts.p3); @@ -397,7 +397,7 @@ fn main( let out = &path_bboxes[path_ix]; let style_flags = scene[config.style_base + style_ix]; // The fill bit is always set to 0 for strokes which represents a non-zero fill. - let draw_flags = select(DRAW_INFO_FLAGS_FILL_RULE_BIT, 0u, (style_flags & STYLE_FLAGS_FILL_BIT) == 0u); + let draw_flags = select(DRAW_INFO_FLAGS_FILL_RULE_BIT, 0u, (style_flags & STYLE_FLAGS_FILL) == 0u); if (tag.tag_byte & PATH_TAG_PATH) != 0u { (*out).draw_flags = draw_flags; (*out).trans_ix = trans_ix; @@ -405,7 +405,7 @@ fn main( // Decode path data let seg_type = tag.tag_byte & PATH_TAG_SEG_TYPE; if seg_type != 0u { - let is_stroke = (style_flags & STYLE_FLAGS_STYLE_BIT) != 0u; + let is_stroke = (style_flags & STYLE_FLAGS_STYLE) != 0u; let transform = read_transform(config.transform_base, trans_ix); let pts = read_path_segment(tag, transform, is_stroke); var bbox = vec4(min(pts.p0, pts.p1), max(pts.p0, pts.p1)); @@ -421,7 +421,7 @@ fn main( stroke = 0.5 * linewidth * vec2(length(transform.mat.xz), length(transform.mat.yw)); bbox += vec4(-stroke, stroke); let is_open = (tag.tag_byte & PATH_TAG_SEG_TYPE) != PATH_TAG_LINETO; - let is_stroke_cap_marker = (tag.tag_byte & PATH_TAG_SUBPATH_END_BIT) != 0u; + let is_stroke_cap_marker = (tag.tag_byte & PATH_TAG_SUBPATH_END) != 0u; if is_stroke_cap_marker { if is_open { let n = cubic_start_normal(pts.p0, pts.p1, pts.p2, pts.p3) * stroke; diff --git a/shader/shared/pathtag.wgsl b/shader/shared/pathtag.wgsl index 37f0235ed..86f2565ae 100644 --- a/shader/shared/pathtag.wgsl +++ b/shader/shared/pathtag.wgsl @@ -21,12 +21,12 @@ let PATH_TAG_TRANSFORM = 0x20u; let PATH_TAG_PATH = 0x10u; let PATH_TAG_STYLE = 0x40u; #endif -let PATH_TAG_SUBPATH_END_BIT = 4u; +let PATH_TAG_SUBPATH_END = 4u; // Size of the `Style` data structure in words let STYLE_SIZE_IN_WORDS: u32 = 2u; -let STYLE_FLAGS_STYLE_BIT: u32 = 0x80000000u; -let STYLE_FLAGS_FILL_BIT: u32 = 0x40000000u; +let STYLE_FLAGS_STYLE: u32 = 0x80000000u; +let STYLE_FLAGS_FILL: u32 = 0x40000000u; // TODO: Declare the remaining STYLE flags here.