Skip to content

Commit a8b0bae

Browse files
seam0s-devseam0s-dev
committed
Fix stroke artifacts
1 parent 7a9cf59 commit a8b0bae

3 files changed

Lines changed: 33 additions & 40 deletions

File tree

‎editor/src/messages/portfolio/document/overlays/utility_types_native.rs‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -428,14 +428,14 @@ impl OverlayContext {
428428

429429
/// Fills the shape's fill region with a pattern of the given color. Assumes `color` is in gamma space.
430430
/// Used by the fill tool to show the area to be filled.
431-
pub fn fill_overlay(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, transform: DAffine2, color: &Color, stroke: Option<Stroke>) {
432-
self.internal().fill_overlay(subpaths, transform, color, stroke);
431+
pub fn fill_overlay(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, is_closed_on_all: bool, transform: DAffine2, color: &Color, stroke: Option<Stroke>) {
432+
self.internal().fill_overlay(subpaths, is_closed_on_all, transform, color, stroke);
433433
}
434434

435435
/// Fills the shape's fill region with a pattern of the given color. Assumes `color` is in gamma space.
436436
/// https://www.w3schools.com/tags/canvas_globalcompositeoperation.asp
437-
pub fn stroke_overlay(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, transform: DAffine2, color: &Color, stroke: Option<Stroke>) {
438-
self.internal().stroke_overlay(subpaths, transform, color, stroke);
437+
pub fn stroke_overlay(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, is_closed_on_all: bool, transform: DAffine2, color: &Color, stroke: Option<Stroke>) {
438+
self.internal().stroke_overlay(subpaths, is_closed_on_all, transform, color, stroke);
439439
}
440440

441441
pub fn text(&self, text: &str, font_color: &str, background_color: Option<&str>, transform: DAffine2, padding: f64, pivot: [Pivot; 2]) {
@@ -1002,7 +1002,7 @@ impl OverlayContextInternal {
10021002
path.push(bezier.as_path_el());
10031003
}
10041004

1005-
fn path_from_subpaths(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, transform: DAffine2) -> BezPath {
1005+
fn path_from_subpaths(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, auto_close: bool, transform: DAffine2) -> BezPath {
10061006
let mut path = BezPath::new();
10071007

10081008
for subpath in subpaths {
@@ -1043,7 +1043,7 @@ impl OverlayContextInternal {
10431043
}
10441044
}
10451045

1046-
if subpath.closed() {
1046+
if subpath.closed() && auto_close {
10471047
path.close_path();
10481048
}
10491049
}
@@ -1065,7 +1065,7 @@ impl OverlayContextInternal {
10651065
}
10661066

10671067
if !subpaths.is_empty() {
1068-
let path = self.path_from_subpaths(subpaths.iter(), transform);
1068+
let path = self.path_from_subpaths(subpaths.iter(), true, transform);
10691069
let color = color.unwrap_or(COLOR_OVERLAY_BLUE);
10701070

10711071
self.scene.stroke(&kurbo::Stroke::new(1.), self.get_transform(), Self::parse_color(color), None, &path);
@@ -1115,20 +1115,20 @@ impl OverlayContextInternal {
11151115
/// Fills the area inside the path. Assumes `color` is in gamma space.
11161116
/// Used by the Pen tool to show the path being closed.
11171117
fn fill_path(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, transform: DAffine2, color: &str) {
1118-
let path = self.path_from_subpaths(subpaths, transform);
1118+
let path = self.path_from_subpaths(subpaths, true, transform);
11191119

11201120
self.scene.fill(peniko::Fill::NonZero, self.get_transform(), Self::parse_color(color), None, &path);
11211121
}
11221122

11231123
/// Fills the shape's fill region with a pattern of the given color. Assumes `color` is in gamma space.
11241124
/// Used by the fill tool to show the area to be filled.
1125-
fn fill_overlay(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, transform: DAffine2, color: &Color, stroke: Option<Stroke>) {
1125+
fn fill_overlay(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, is_closed_on_all: bool, transform: DAffine2, color: &Color, stroke: Option<Stroke>) {
11261126
if let Some(stroke) = stroke {
11271127
let has_real_stroke = stroke.weight() > 0. && stroke.transform.matrix2.determinant() != 0.;
11281128
let applied_stroke_transform = if has_real_stroke { stroke.transform } else { transform };
11291129
let element_transform = if has_real_stroke { transform * stroke.transform.inverse() } else { DAffine2::IDENTITY };
11301130

1131-
let path = self.path_from_subpaths(subpaths, applied_stroke_transform);
1131+
let path = self.path_from_subpaths(subpaths, false, applied_stroke_transform);
11321132
let brush = peniko::Brush::Image(self.fill_canvas_pattern_image(color));
11331133

11341134
let do_fill = |scene: &mut Scene| {
@@ -1141,15 +1141,14 @@ impl OverlayContextInternal {
11411141
if let Some(scale) = stroke_scale {
11421142
stroke.width *= scale;
11431143
}
1144-
let path_bbox = path.bounding_box().inflate(stroke.width, stroke.width);
1144+
let path_bbox = path.bounding_box().inflate(stroke.width * 1.5, stroke.width * 1.5);
11451145

11461146
scene.push_layer(peniko::Fill::NonZero, BlendMode::new(peniko::Mix::Normal, compose_mode), 1.0, element_transform, &path_bbox);
11471147
scene.stroke(&stroke, element_transform, &brush, Some(element_transform.inverse()), &path);
11481148
scene.pop_layer();
11491149
};
11501150

11511151
// For layers with open subpaths, stroke align is ignored and set to default
1152-
let is_closed_on_all = true;
11531152
let stroke_align = if is_closed_on_all { stroke.align } else { StrokeAlign::Center };
11541153

11551154
match (stroke_align, stroke.paint_order) {
@@ -1173,21 +1172,21 @@ impl OverlayContextInternal {
11731172
}
11741173
}
11751174
} else {
1176-
let path = self.path_from_subpaths(subpaths, transform);
1175+
let path = self.path_from_subpaths(subpaths, false, transform);
11771176
let brush = peniko::Brush::Image(self.fill_canvas_pattern_image(color));
11781177
self.scene.fill(peniko::Fill::NonZero, Affine::IDENTITY, &brush, None, &path);
11791178
}
11801179
}
11811180

11821181
/// Fills the shape's fill region with a pattern of the given color. Assumes `color` is in gamma space.
11831182
/// https://www.w3schools.com/tags/canvas_globalcompositeoperation.asp
1184-
pub fn stroke_overlay(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, transform: DAffine2, color: &Color, stroke: Option<Stroke>) {
1183+
pub fn stroke_overlay(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, is_closed_on_all: bool, transform: DAffine2, color: &Color, stroke: Option<Stroke>) {
11851184
if let Some(stroke) = stroke {
11861185
let has_real_stroke = stroke.weight() > 0. && stroke.transform.matrix2.determinant() != 0.;
11871186
let applied_stroke_transform = if has_real_stroke { stroke.transform } else { transform };
11881187
let element_transform = if has_real_stroke { transform * stroke.transform.inverse() } else { DAffine2::IDENTITY };
11891188

1190-
let path = self.path_from_subpaths(subpaths, applied_stroke_transform);
1189+
let path = self.path_from_subpaths(subpaths, false, applied_stroke_transform);
11911190
let brush = peniko::Brush::Image(self.fill_canvas_pattern_image(color));
11921191

11931192
let do_stroke = |scene: &mut Scene, stroke_scale: Option<f64>| {
@@ -1201,7 +1200,7 @@ impl OverlayContextInternal {
12011200
};
12021201
let composite_fill_out = |scene: &mut Scene, compose_mode: peniko::Compose, stroke_scale: Option<f64>| {
12031202
let element_transform = Affine::new(element_transform.to_cols_array());
1204-
let stroke_width = stroke.weight() * stroke_scale.map_or(1.0, |scale| scale);
1203+
let stroke_width = stroke.weight() * stroke_scale.map_or(1.0, |scale| scale) * 1.5;
12051204
let path_bbox = path.bounding_box().inflate(stroke_width, stroke_width);
12061205

12071206
scene.push_layer(peniko::Fill::NonZero, BlendMode::new(peniko::Mix::Normal, compose_mode), 1.0, element_transform, &path_bbox);
@@ -1210,7 +1209,6 @@ impl OverlayContextInternal {
12101209
};
12111210

12121211
// For layers with open subpaths, stroke align is ignored and set to default
1213-
let is_closed_on_all = true;
12141212
let stroke_align = if is_closed_on_all { stroke.align } else { StrokeAlign::Center };
12151213

12161214
match (stroke_align, stroke.paint_order) {

‎editor/src/messages/portfolio/document/overlays/utility_types_web.rs‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -955,9 +955,7 @@ impl OverlayContext {
955955
self.end_dpi_aware_transform();
956956
}
957957

958-
pub fn draw_path_from_subpaths(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, stroke_transform: DAffine2) -> bool {
959-
// Subpaths on a layer is considered "closed" only if all subpaths are closed.
960-
let mut is_closed_on_all = true;
958+
pub fn draw_path_from_subpaths(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, auto_close: bool, stroke_transform: DAffine2) {
961959
self.render_context.begin_path();
962960
for subpath in subpaths {
963961
let subpath = subpath.borrow().clone();
@@ -998,13 +996,10 @@ impl OverlayContext {
998996
}
999997
}
1000998

1001-
if subpath.closed() {
999+
if subpath.closed() && auto_close {
10021000
self.render_context.close_path();
1003-
} else {
1004-
is_closed_on_all = false;
10051001
}
10061002
}
1007-
return is_closed_on_all;
10081003
}
10091004

10101005
/// Used by the Select tool to outline a path or a free point when selected or hovered.
@@ -1021,8 +1016,7 @@ impl OverlayContext {
10211016
});
10221017

10231018
if !subpaths.is_empty() {
1024-
// TODO: Modify stroke_transform to take note of this
1025-
self.draw_path_from_subpaths(subpaths.iter(), transform);
1019+
self.draw_path_from_subpaths(subpaths.iter(), true, transform);
10261020

10271021
let color = color.unwrap_or(COLOR_OVERLAY_BLUE);
10281022
self.render_context.set_stroke_style_str(color);
@@ -1082,15 +1076,15 @@ impl OverlayContext {
10821076

10831077
/// Used by the Pen tool to show the path being closed.
10841078
pub fn fill_path(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, transform: DAffine2, color: &str) {
1085-
self.draw_path_from_subpaths(subpaths, transform);
1079+
self.draw_path_from_subpaths(subpaths, true, transform);
10861080

10871081
self.render_context.set_fill_style_str(color);
10881082
self.render_context.fill();
10891083
}
10901084

10911085
/// Fills the shape's fill region with a pattern of the given color. Assumes `color` is in gamma space.
10921086
/// https://www.w3schools.com/tags/canvas_globalcompositeoperation.asp
1093-
pub fn fill_overlay(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, transform: DAffine2, color: &Color, stroke: Option<Stroke>) {
1087+
pub fn fill_overlay(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, is_closed_on_all: bool, transform: DAffine2, color: &Color, stroke: Option<Stroke>) {
10941088
// Render for elements with fill
10951089
// Render for elements with fill only
10961090
// Render for elements with fill and stroke
@@ -1107,9 +1101,9 @@ impl OverlayContext {
11071101

11081102
let [a, b, c, d, e, f] = element_transform.to_cols_array();
11091103
self.render_context.transform(a, b, c, d, e, f).expect("element_transform should be set to render stroke properly");
1104+
self.draw_path_from_subpaths(subpaths, false, applied_stroke_transform);
11101105

11111106
// For layers with open subpaths, stroke align is ignored and set to default
1112-
let is_closed_on_all = self.draw_path_from_subpaths(subpaths, applied_stroke_transform);
11131107
let stroke_align = if is_closed_on_all { stroke.align } else { StrokeAlign::Center };
11141108

11151109
let do_fill = || {
@@ -1148,6 +1142,7 @@ impl OverlayContext {
11481142
}
11491143
}
11501144
} else {
1145+
self.draw_path_from_subpaths(subpaths, false, transform);
11511146
self.render_context.set_fill_style_canvas_pattern(&self.fill_canvas_pattern(color));
11521147
self.render_context.fill();
11531148
}
@@ -1159,7 +1154,7 @@ impl OverlayContext {
11591154
/// Fills the shape's stroke region with a pattern of the given color. Assumes `color` is in gamma space.
11601155
/// WARN: Don't use source-in, destination-atop, destination-in, copy
11611156
/// on the main canvas as it will erase the existing overlays
1162-
pub fn stroke_overlay(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, layer_to_viewport: DAffine2, color: &Color, stroke: Option<Stroke>) {
1157+
pub fn stroke_overlay(&mut self, subpaths: impl Iterator<Item = impl Borrow<Subpath<PointId>>>, is_closed_on_all: bool, transform: DAffine2, color: &Color, stroke: Option<Stroke>) {
11631158
// Render for elements with stroke
11641159
//----StrokeAlign
11651160
// Render for elements with stroke only
@@ -1171,14 +1166,14 @@ impl OverlayContext {
11711166

11721167
if let Some(stroke) = stroke {
11731168
let has_real_stroke = stroke.weight() > 0. && stroke.transform.matrix2.determinant() != 0.;
1174-
let applied_stroke_transform = if has_real_stroke { stroke.transform } else { layer_to_viewport };
1175-
let element_transform = if has_real_stroke { layer_to_viewport * stroke.transform.inverse() } else { DAffine2::IDENTITY };
1169+
let applied_stroke_transform = if has_real_stroke { stroke.transform } else { transform };
1170+
let element_transform = if has_real_stroke { transform * stroke.transform.inverse() } else { DAffine2::IDENTITY };
11761171

11771172
let [a, b, c, d, e, f] = element_transform.to_cols_array();
11781173
self.render_context.transform(a, b, c, d, e, f).expect("element_transform should be set to render stroke properly");
1174+
self.draw_path_from_subpaths(subpaths, false, applied_stroke_transform);
11791175

11801176
// For layers with open subpaths, stroke align is ignored and set to default
1181-
let is_closed_on_all = self.draw_path_from_subpaths(subpaths, applied_stroke_transform);
11821177
let stroke_align = if is_closed_on_all { stroke.align } else { StrokeAlign::Center };
11831178

11841179
let do_stroke = |stroke_weight: f64| {

‎editor/src/messages/tool/tool_messages/fill_tool.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,8 @@ pub fn near_to_subpath(mouse_pos: DVec2, subpath: Subpath<PointId>, is_closed_on
9090
let mouse_point = kurbo::Point::new(mouse_pos.x, mouse_pos.y);
9191
for seg in subpath_bezpath.segments() {
9292
let nearest = seg.nearest(mouse_point, 0.01);
93-
let is_inside_seg = if is_closed_on_all {
94-
// Inside/outside detection for closed bezpaths
95-
subpath_bezpath.contains(mouse_point)
96-
} else {
97-
// Inside/outside detection for open bezpaths
93+
let is_inside_seg = {
94+
// Inside/outside detection for bezpaths
9895
let tangent = seg.tangent_at(nearest.t);
9996
let normal = kurbo::Vec2::new(-tangent.y, tangent.x);
10097
let dir = (seg.eval(nearest.t) - mouse_point).normalize();
@@ -115,6 +112,7 @@ pub fn near_to_subpath(mouse_pos: DVec2, subpath: Subpath<PointId>, is_closed_on
115112
max_stroke_distance = -1.0;
116113
}
117114
(StrokeAlign::Center, PaintOrder::StrokeAbove) => {}
115+
// TODO: Fix logic detection for open subpaths on (StrokeAlign::Inside, PaintOrder::StrokeBelow)
118116
(StrokeAlign::Center, PaintOrder::StrokeBelow) => {
119117
if is_inside_seg {
120118
max_stroke_distance = -1.0;
@@ -192,6 +190,7 @@ impl Fsm for FillToolFsmState {
192190
let stroke = vector_data.style.stroke();
193191

194192
let mut subpaths = vector_data.stroke_bezier_paths();
193+
// Subpaths on a layer is considered "closed" only if all subpaths are closed.
195194
let is_closed_on_all = subpaths.all(|subpath| subpath.closed);
196195
subpaths = vector_data.stroke_bezier_paths();
197196
let near_to_stroke = subpaths.any(|subpath| near_to_subpath(input.mouse.position, subpath, is_closed_on_all, stroke.clone(), document.metadata().transform_to_viewport(layer)));
@@ -203,9 +202,9 @@ impl Fsm for FillToolFsmState {
203202
subpaths = vector_data.stroke_bezier_paths();
204203
let layer_to_viewport = document.metadata().transform_to_viewport(layer);
205204
if stroke_exists_and_visible && near_to_stroke {
206-
overlay_context.stroke_overlay(subpaths, layer_to_viewport, &preview_color, stroke);
205+
overlay_context.stroke_overlay(subpaths, is_closed_on_all, layer_to_viewport, &preview_color, stroke);
207206
} else if fill_exists_and_visible {
208-
overlay_context.fill_overlay(subpaths, layer_to_viewport, &preview_color, stroke);
207+
overlay_context.fill_overlay(subpaths, is_closed_on_all, layer_to_viewport, &preview_color, stroke);
209208
}
210209
}
211210

@@ -246,6 +245,7 @@ impl Fsm for FillToolFsmState {
246245
let stroke = vector_data.style.stroke();
247246

248247
let mut subpaths = vector_data.stroke_bezier_paths();
248+
// Subpaths on a layer is considered "closed" only if all subpaths are closed.
249249
let is_closed_on_all = subpaths.all(|subpath| subpath.closed);
250250
subpaths = vector_data.stroke_bezier_paths();
251251
let near_to_stroke = subpaths.any(|subpath| near_to_subpath(input.mouse.position, subpath, is_closed_on_all, stroke.clone(), document.metadata().transform_to_viewport(layer)));

0 commit comments

Comments
 (0)