github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-05-30 04:08:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Fix boolean issues on the wasm render

Browse Source 
* 🐛 Fix sharp angles in text-to-path due to wrong quad/conic degree elevation

* 🐛 Preserve even-odd fill type through Skia path conversions

* 🐛 Fix wrong quadratic-to-cubic degree elevation in push_bezier

* 🐛 Skip zero-length degenerate close segments in path_to_beziers

* 🐛 Replace BTreeMap for Vec for bool calculation

* 🐛 Fix even_odd missing when creating Path
This commit is contained in:
Alonso Torres 2026-05-26 15:11:52 +02:00 committed by GitHub
parent 75c61f9211
commit b609a964be
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 134 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

164
render-wasm/src/math/bools.rs
View File

@ -7,7 +7,7 @@ use bezier_rs::{Bezier, BezierHandles, ProjectionOptions, TValue};
use glam::DVec2;
use skia_safe as skia;
use std::cmp::Ordering;
use std::collections::{BTreeMap, HashMap};
use std::collections::HashMap;
const INTERSECT_THRESHOLD_SAME: f32 = 0.1;
const INTERSECT_THRESHOLD_DIFFERENT: f32 = 0.5;
@ -62,8 +62,14 @@ pub fn path_to_beziers(path: &Path) -> Vec<Bezier> {
Segment::Close => {
let (x1, y1) = prev?;
let (x2, y2) = start?;
let s = Bezier::from_linear_coordinates(x1, y1, x2, y2);
prev = Some((x2, y2));
// Skip degenerate zero-length close segment: path already returned
// to the start point via an explicit LineTo/CurveTo, so adding a
// zero-length linear bezier here would confuse intersection detection.
if (x1 - x2).abs() < 1e-6 && (y1 - y2).abs() < 1e-6 {
return None;
}
let s = Bezier::from_linear_coordinates(x1, y1, x2, y2);
Some(s)
}
})
@ -240,98 +246,49 @@ enum BezierSource {
B,
}
#[derive(Debug, Clone)]
struct BezierStart(BezierSource, DVec2);
type BezierPool = Vec<Option<(BezierSource, Bezier)>>;
impl PartialEq for BezierStart {
fn eq(&self, other: &Self) -> bool {
let x1 = self.1.x as f32;
let y1 = self.1.y as f32;
let x2 = other.1.x as f32;
let y2 = other.1.y as f32;
if self.0 == other.0 {
(x1 - x2).abs() <= INTERSECT_THRESHOLD_SAME
&& (y1 - y2).abs() <= INTERSECT_THRESHOLD_SAME
} else {
(x1 - x2).abs() <= INTERSECT_THRESHOLD_DIFFERENT
&& (y1 - y2).abs() <= INTERSECT_THRESHOLD_DIFFERENT
}
}
fn init_pool(beziers: &[(BezierSource, Bezier)]) -> BezierPool {
beziers.iter().copied().map(Some).collect()
}
impl Eq for BezierStart {}
impl PartialOrd for BezierStart {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
// Pop the first remaining entry from the pool (arbitrary start for a new subpath).
fn pop_first_from_pool(pool: &mut BezierPool) -> Option<(BezierSource, Bezier)> {
pool.iter_mut().find_map(|e| e.take())
}
impl Ord for BezierStart {
fn cmp(&self, other: &Self) -> Ordering {
let x1 = self.1.x as f32;
let y1 = self.1.y as f32;
let x2 = other.1.x as f32;
let y2 = other.1.y as f32;
// Find and remove the segment whose start point is closest to `end` within the
// appropriate threshold. Same-source segments use a tight threshold
// (INTERSECT_THRESHOLD_SAMEd) so we prefer staying on the same original path;
// cross-source segments use a wider threshold (INTERSECT_THRESHOLD_DIFFERENT)
// to allow switching paths at intersection points.
fn find_next_in_pool(
pool: &mut BezierPool,
end: DVec2,
source: BezierSource,
) -> Option<(BezierSource, Bezier)> {
let mut best_idx: Option<usize> = None;
let mut best_dist_sq = f64::MAX;
let (equal_x, equal_y) = if self.0 == other.0 {
(
(x1 - x2).abs() <= INTERSECT_THRESHOLD_SAME,
(y1 - y2).abs() <= INTERSECT_THRESHOLD_SAME,
)
} else {
(
(x1 - x2).abs() <= INTERSECT_THRESHOLD_DIFFERENT,
(y1 - y2).abs() <= INTERSECT_THRESHOLD_DIFFERENT,
)
for (i, entry) in pool.iter().enumerate() {
let Some((src, bezier)) = entry else {
continue;
};
if equal_x && equal_y {
Ordering::Equal
} else if equal_x && y1 > y2 || !equal_x && x1 > x2 {
Ordering::Greater
let threshold = if *src == source {
INTERSECT_THRESHOLD_SAME as f64
} else {
Ordering::Less
INTERSECT_THRESHOLD_DIFFERENT as f64
};
let dx = bezier.start.x - end.x;
let dy = bezier.start.y - end.y;
let dist_sq = dx * dx + dy * dy;
if dist_sq <= threshold * threshold && dist_sq < best_dist_sq {
best_dist_sq = dist_sq;
best_idx = Some(i);
}
}
}
type BM<'a> = BTreeMap<BezierStart, Vec<(BezierSource, Bezier)>>;
fn init_bm(beziers: &[(BezierSource, Bezier)]) -> BM<'_> {
let mut bm = BM::default();
for entry @ (source, bezier) in beziers.iter() {
let value = *entry;
let key = BezierStart(*source, bezier.start);
if let Some(v) = bm.get_mut(&key) {
v.push(value);
} else {
bm.insert(key, vec![value]);
}
}
bm
}
fn find_next(tree: &mut BM, key: BezierStart) -> Option<(BezierSource, Bezier)> {
let val = tree.get_mut(&key)?;
let first = val.pop()?;
if val.is_empty() {
tree.remove(&key);
}
Some(first)
}
fn pop_first(tree: &mut BM) -> Option<(BezierSource, Bezier)> {
let key = tree.keys().take(1).next()?.clone();
let val = tree.get_mut(&key)?;
let first = val.pop()?;
if val.is_empty() {
tree.remove(&key);
}
Some(first)
best_idx.and_then(|i| pool[i].take())
}
fn push_bezier(result: &mut Vec<Segment>, bezier: &Bezier) {
@ -340,10 +297,16 @@ fn push_bezier(result: &mut Vec<Segment>, bezier: &Bezier) {
result.push(Segment::LineTo((bezier.end.x as f32, bezier.end.y as f32)));
}
BezierHandles::Quadratic { handle } => {
let s = bezier.start;
let e = bezier.end;
let cp1x = s.x + (2.0 / 3.0) * (handle.x - s.x);
let cp1y = s.y + (2.0 / 3.0) * (handle.y - s.y);
let cp2x = e.x + (2.0 / 3.0) * (handle.x - e.x);
let cp2y = e.y + (2.0 / 3.0) * (handle.y - e.y);
result.push(Segment::CurveTo((
(handle.x as f32, handle.y as f32),
(handle.x as f32, handle.y as f32),
(bezier.end.x as f32, bezier.end.y as f32),
(cp1x as f32, cp1y as f32),
(cp2x as f32, cp2y as f32),
(e.x as f32, e.y as f32),
)));
}
BezierHandles::Cubic {
@ -361,23 +324,24 @@ fn push_bezier(result: &mut Vec<Segment>, bezier: &Bezier) {
fn beziers_to_segments(beziers: &[(BezierSource, Bezier)]) -> Vec<Segment> {
let mut result = Vec::new();
let mut pool = init_pool(beziers);
let mut bm = init_bm(beziers);
while let Some(bezier) = pop_first(&mut bm) {
let start = (bezier.1.start.x as f32, bezier.1.start.y as f32);
while let Some((mut cur_src, first_bezier)) = pop_first_from_pool(&mut pool) {
let start = (first_bezier.start.x as f32, first_bezier.start.y as f32);
result.push(Segment::MoveTo(start));
push_bezier(&mut result, &bezier.1);
let mut last_end = (bezier.1.end.x as f32, bezier.1.end.y as f32);
let mut next_p = BezierStart(bezier.0, bezier.1.end);
push_bezier(&mut result, &first_bezier);
let mut last_end = (first_bezier.end.x as f32, first_bezier.end.y as f32);
let mut cur_end = first_bezier.end;
loop {
let Some(next) = find_next(&mut bm, next_p) else {
let Some((next_src, next_bezier)) = find_next_in_pool(&mut pool, cur_end, cur_src)
else {
break;
};
push_bezier(&mut result, &next.1);
last_end = (next.1.end.x as f32, next.1.end.y as f32);
next_p = BezierStart(next.0, next.1.end);
push_bezier(&mut result, &next_bezier);
last_end = (next_bezier.end.x as f32, next_bezier.end.y as f32);
cur_end = next_bezier.end;
cur_src = next_src;
}
// Close the subpath if the last point is close to the start.
@ -417,6 +381,7 @@ pub fn bool_from_shapes(bool_type: BoolType, children_ids: &[Uuid], shapes: Shap
let (segs_a, segs_b) = split_segments(&current_path, &other_path);
let is_even_odd = current_path.is_even_odd() || other_path.is_even_odd();
let beziers = match bool_type {
BoolType::Union => union(&current_path, segs_a, &other_path, segs_b),
BoolType::Difference => difference(&current_path, segs_a, &other_path, segs_b),
@ -424,7 +389,7 @@ pub fn bool_from_shapes(bool_type: BoolType, children_ids: &[Uuid], shapes: Shap
BoolType::Exclusion => exclusion(segs_a, segs_b),
};
current_path = Path::new(beziers_to_segments(&beziers));
current_path = Path::new(beziers_to_segments(&beziers)).with_even_odd(is_even_odd);
}
current_path
@ -477,13 +442,14 @@ pub fn debug_render_bool_paths(
let (segs_a, segs_b) = split_segments(&current_path, &other_path);
let is_even_odd = current_path.is_even_odd() || other_path.is_even_odd();
let beziers = match bool_data.bool_type {
BoolType::Union => union(&current_path, segs_a, &other_path, segs_b),
BoolType::Difference => difference(&current_path, segs_a, &other_path, segs_b),
BoolType::Intersection => intersection(&current_path, segs_a, &other_path, segs_b),
BoolType::Exclusion => exclusion(segs_a, segs_b),
};
current_path = Path::new(beziers_to_segments(&beziers));
current_path = Path::new(beziers_to_segments(&beziers)).with_even_odd(is_even_odd);
if idx == 0 {
for b in &beziers {

69
render-wasm/src/shapes/paths.rs
View File

@ -68,34 +68,59 @@ impl Path {
pub fn from_skia_path(path: skia::Path) -> Self {
let verbs = path.verbs();
let points = path.points();
let fill_type = path.fill_type();
let mut segments = Vec::new();
let mut current_point = 0;
let mut last_point = skia::Point::new(0.0, 0.0);
for verb in verbs {
match verb {
skia::PathVerb::Move => {
let p = points[current_point];
segments.push(Segment::MoveTo((p.x, p.y)));
last_point = p;
current_point += 1;
}
skia::PathVerb::Line => {
let p = points[current_point];
segments.push(Segment::LineTo((p.x, p.y)));
last_point = p;
current_point += 1;
}
skia::PathVerb::Quad => {
let p1 = points[current_point];
let p2 = points[current_point + 1];
segments.push(Segment::CurveTo(((p1.x, p1.y), (p1.x, p1.y), (p2.x, p2.y))));
// Elevate quadratic to cubic: CP1 = P0 + 2/3*(Pctrl-P0), CP2 = P2 + 2/3*(Pctrl-P2)
let ctrl = points[current_point];
let end = points[current_point + 1];
let cp1x = last_point.x + (2.0 / 3.0) * (ctrl.x - last_point.x);
let cp1y = last_point.y + (2.0 / 3.0) * (ctrl.y - last_point.y);
let cp2x = end.x + (2.0 / 3.0) * (ctrl.x - end.x);
let cp2y = end.y + (2.0 / 3.0) * (ctrl.y - end.y);
segments.push(Segment::CurveTo((
(cp1x, cp1y),
(cp2x, cp2y),
(end.x, end.y),
)));
last_point = end;
current_point += 2;
}
skia::PathVerb::Conic => {
// TODO: There is no way currently to access the conic weight
// to transform this correctly
let p1 = points[current_point];
let p2 = points[current_point + 1];
segments.push(Segment::CurveTo(((p1.x, p1.y), (p1.x, p1.y), (p2.x, p2.y))));
// Approximate conic (rational quadratic) as cubic via degree elevation.
// This ignores the conic weight and treats it as a regular quadratic —
// accurate enough for the typical w≈1 font glyphs that use this path.
// For higher-fidelity conversion use from_skia_path_accurate instead.
let ctrl = points[current_point];
let end = points[current_point + 1];
let cp1x = last_point.x + (2.0 / 3.0) * (ctrl.x - last_point.x);
let cp1y = last_point.y + (2.0 / 3.0) * (ctrl.y - last_point.y);
let cp2x = end.x + (2.0 / 3.0) * (ctrl.x - end.x);
let cp2y = end.y + (2.0 / 3.0) * (ctrl.y - end.y);
segments.push(Segment::CurveTo((
(cp1x, cp1y),
(cp2x, cp2y),
(end.x, end.y),
)));
last_point = end;
current_point += 2;
}
skia::PathVerb::Cubic => {
@ -103,6 +128,7 @@ impl Path {
let p2 = points[current_point + 1];
let p3 = points[current_point + 2];
segments.push(Segment::CurveTo(((p1.x, p1.y), (p2.x, p2.y), (p3.x, p3.y))));
last_point = p3;
current_point += 3;
}
skia::PathVerb::Close => {
@ -111,16 +137,20 @@ impl Path {
}
}
Path::new(segments)
let mut result = Path::new(segments);
result.skia_path.set_fill_type(fill_type);
result
}
/// Like `from_skia_path` but properly converts conics to cubic beziers
/// (using Skia's conic-to-quad + quad-to-cubic elevation). Use this when
/// accurate curve conversion matters (e.g. stroke-to-path on circles).
/// accurate curve conversion matters (e.g. stroke-to-path on circles,
/// text glyph paths which contain many conic segments).
pub fn from_skia_path_accurate(path: skia::Path) -> Self {
let verbs = path.verbs();
let points = path.points();
let conic_weights = path.conic_weights();
let fill_type = path.fill_type();
let mut segments = Vec::new();
let mut current_point = 0;
@ -214,7 +244,9 @@ impl Path {
}
}
Path::new(segments)
let mut result = Path::new(segments);
result.skia_path.set_fill_type(fill_type);
result
}
pub fn to_skia_path(&self, svg_attrs: Option<&SvgAttrs>) -> skia::Path {
@ -231,6 +263,19 @@ impl Path {
self.skia_path.contains(p)
}
pub fn is_even_odd(&self) -> bool {
self.skia_path.fill_type() == skia::PathFillType::EvenOdd
}
// Builder method: set even-odd fill on this path and return it.
// Use as `Path::new(segments).with_even_odd(is_even_odd)`.
pub fn with_even_odd(mut self, is_even_odd: bool) -> Self {
if is_even_odd {
self.skia_path.set_fill_type(skia::PathFillType::EvenOdd);
}
self
}
pub fn is_open(&self) -> bool {
self.open
}

14
render-wasm/src/shapes/shape_to_path.rs
View File

@ -180,9 +180,13 @@ pub fn circle_segments(shape: &Shape) -> Vec<Segment> {
}
fn join_paths(path: Path, other: Path) -> Path {
// Propagate even-odd fill: if either input uses it, the joined result must too,
// so that Path::contains gives the correct answer during boolean operations
// (e.g. letters with holes like 'O' or 'A' use even-odd fill).
let is_even_odd = path.is_even_odd() || other.is_even_odd();
let mut segments = path.segments().clone();
segments.extend(other.segments().iter());
Path::new(segments)
Path::new(segments).with_even_odd(is_even_odd)
}
fn transform_segments(segments: Vec<Segment>, shape: &Shape) -> Vec<Segment> {
@ -233,9 +237,10 @@ impl ToPath for Shape {
result = join_paths(result, shape.to_path(shapes));
}
// Force closure of the group path
let is_even_odd = result.is_even_odd();
let mut segments = result.segments().clone();
segments.push(Segment::Close);
Path::new(segments)
Path::new(segments).with_even_odd(is_even_odd)
}
Type::Bool(bool_data) => bool_data.path.clone(),
@ -255,7 +260,12 @@ impl ToPath for Shape {
result = join_paths(result, Path::from_skia_path(path));
}
// Preserve even-odd fill through the transform step so that
// Path::contains works correctly in subsequent boolean operations
// (letters with interior holes need even-odd fill).
let is_even_odd = result.is_even_odd();
Path::new(transform_segments(result.segments().clone(), self))
.with_even_odd(is_even_odd)
}
}
}