Skip to content

Commit 1543d97

Browse files
authored
Fix 'Jitter Points' and 'Sample Polylines' working incorrectly with X or Y scale of 0 content (#3984)
* Fix NaN points produced by Sample Polylines on 0-scaled input * Fix Jitter Points inverse transform for zero-scale axes and stop resetting stroke transform * Remove a couple confusing Debug nodes * Fix edge case * Update demo art * Fix order change in Jitter Points causing different results from earlier * Fix bug in bisect tool * Break out functionality into helper functions
1 parent 79bf1ab commit 1543d97

File tree

6 files changed

+135
-67
lines changed

6 files changed

+135
-67
lines changed

demo-artwork/parametric-dunescape.graphite

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ fn document_node_definitions() -> HashMap<DefinitionIdentifier, DocumentNodeDefi
155155
// TODO: Auto-generate this from its proto node macro
156156
DocumentNodeDefinition {
157157
identifier: "Monitor",
158-
category: "Debug",
158+
category: "",
159159
node_template: NodeTemplate {
160160
document_node: DocumentNode {
161161
implementation: DocumentNodeImplementation::ProtoNode(memo::monitor::IDENTIFIER),
@@ -1530,7 +1530,7 @@ fn document_node_definitions() -> HashMap<DefinitionIdentifier, DocumentNodeDefi
15301530
},
15311531
DocumentNodeDefinition {
15321532
identifier: "Extract",
1533-
category: "Debug",
1533+
category: "",
15341534
node_template: NodeTemplate {
15351535
document_node: DocumentNode {
15361536
implementation: DocumentNodeImplementation::Extract,

editor/src/messages/tool/common_functionality/shape_editor.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1822,6 +1822,9 @@ impl ShapeState {
18221822
/// Find the `t` value along the path segment we have clicked upon, together with that segment ID.
18231823
fn closest_segment(&self, network_interface: &NodeNetworkInterface, layer: LayerNodeIdentifier, position: glam::DVec2, tolerance: f64) -> Option<ClosestSegment> {
18241824
let transform = network_interface.document_metadata().transform_to_viewport_if_feeds(layer, network_interface);
1825+
if transform.matrix2.determinant() == 0. {
1826+
return None;
1827+
}
18251828
let layer_pos = transform.inverse().transform_point2(position);
18261829

18271830
let tolerance = tolerance + 0.5;

node-graph/libraries/vector-types/src/vector/algorithms/bezpath_algorithms.rs

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -85,17 +85,18 @@ pub fn tangent_on_bezpath(bezpath: &BezPath, t_value: TValue, segments_length: O
8585
}
8686
}
8787

88-
pub fn sample_polyline_on_bezpath(
89-
bezpath: BezPath,
88+
/// Computes sample locations along a bezpath, returning parametric `(segment_index, t)` pairs and whether the path was closed.
89+
/// The `bezpath` is used for euclidean-to-parametric conversion, and `segments_length` provides pre-calculated world-space segment lengths.
90+
/// Callers can evaluate these locations on any bezpath with the same topology (e.g., an untransformed version).
91+
pub fn compute_sample_locations(
92+
bezpath: &BezPath,
9093
point_spacing_type: PointSpacingType,
9194
amount: f64,
9295
start_offset: f64,
9396
stop_offset: f64,
9497
adaptive_spacing: bool,
9598
segments_length: &[f64],
96-
) -> Option<BezPath> {
97-
let mut sample_bezpath = BezPath::new();
98-
99+
) -> Option<(Vec<(usize, f64)>, bool)> {
99100
let was_closed = matches!(bezpath.elements().last(), Some(PathEl::ClosePath));
100101

101102
// Calculate the total length of the collected segments.
@@ -142,7 +143,8 @@ pub fn sample_polyline_on_bezpath(
142143
let sample_count_usize = sample_count as usize;
143144
let max_i = if was_closed { sample_count_usize } else { sample_count_usize + 1 };
144145

145-
// Generate points along the path based on calculated intervals.
146+
// Generate sample locations along the path based on calculated intervals.
147+
let mut locations = Vec::with_capacity(max_i);
146148
let mut length_up_to_previous_segment = 0.;
147149
let mut next_segment_index = 0;
148150

@@ -167,20 +169,11 @@ pub fn sample_polyline_on_bezpath(
167169

168170
let segment = bezpath.get_seg(next_segment_index + 1).unwrap();
169171
let t = eval_pathseg_euclidean(segment, t, DEFAULT_ACCURACY);
170-
let point = segment.eval(t);
171-
172-
if sample_bezpath.elements().is_empty() {
173-
sample_bezpath.move_to(point)
174-
} else {
175-
sample_bezpath.line_to(point)
176-
}
177-
}
178172

179-
if was_closed {
180-
sample_bezpath.close_path();
173+
locations.push((next_segment_index, t));
181174
}
182175

183-
Some(sample_bezpath)
176+
Some((locations, was_closed))
184177
}
185178

186179
#[derive(Debug, Clone, Copy)]

node-graph/nodes/vector/src/vector_nodes.rs

Lines changed: 87 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use core_types::registry::types::{Angle, Length, Multiplier, Percentage, PixelLe
66
use core_types::table::{Table, TableRow, TableRowMut};
77
use core_types::transform::Footprint;
88
use core_types::{CloneVarArgs, Color, Context, Ctx, ExtractAll, OwnedContextImpl};
9-
use glam::{DAffine2, DVec2};
9+
use glam::{DAffine2, DMat2, DVec2};
1010
use graphic_types::Vector;
1111
use graphic_types::raster_types::{CPU, GPU, Raster};
1212
use graphic_types::{Graphic, IntoGraphicTable};
@@ -16,7 +16,7 @@ use rand::{Rng, SeedableRng};
1616
use std::collections::hash_map::DefaultHasher;
1717
use vector_types::subpath::{BezierHandles, ManipulatorGroup};
1818
use vector_types::vector::PointDomain;
19-
use vector_types::vector::algorithms::bezpath_algorithms::{self, TValue, eval_pathseg_euclidean, evaluate_bezpath, sample_polyline_on_bezpath, split_bezpath, tangent_on_bezpath};
19+
use vector_types::vector::algorithms::bezpath_algorithms::{self, TValue, eval_pathseg_euclidean, evaluate_bezpath, split_bezpath, tangent_on_bezpath};
2020
use vector_types::vector::algorithms::merge_by_distance::MergeByDistanceExt;
2121
use vector_types::vector::algorithms::offset_subpath::offset_bezpath;
2222
use vector_types::vector::algorithms::spline::{solve_spline_first_handle_closed, solve_spline_first_handle_open};
@@ -1355,11 +1355,12 @@ async fn sample_polyline(
13551355
// Keeps track of the index of the first segment of the next bezpath in order to get lengths of all segments.
13561356
let mut next_segment_index = 0;
13571357

1358-
for mut bezpath in bezpaths {
1359-
// Apply the tranformation to the current bezpath to calculate points after transformation.
1360-
bezpath.apply_affine(Affine::new(row.transform.to_cols_array()));
1358+
for local_bezpath in bezpaths {
1359+
// Apply the transform to compute sample locations in world space (for correct distance-based spacing)
1360+
let mut world_bezpath = local_bezpath.clone();
1361+
world_bezpath.apply_affine(Affine::new(row.transform.to_cols_array()));
13611362

1362-
let segment_count = bezpath.segments().count();
1363+
let segment_count = world_bezpath.segments().count();
13631364

13641365
// For the current bezpath we get its segment's length by calculating the start index and end index.
13651366
let current_bezpath_segments_length = &subpath_segment_lengths[next_segment_index..next_segment_index + segment_count];
@@ -1371,14 +1372,30 @@ async fn sample_polyline(
13711372
PointSpacingType::Separation => separation,
13721373
PointSpacingType::Quantity => quantity as f64,
13731374
};
1374-
let Some(mut sample_bezpath) = sample_polyline_on_bezpath(bezpath, spacing, amount, start_offset, stop_offset, adaptive_spacing, current_bezpath_segments_length) else {
1375+
1376+
// Compute sample locations using world-space distances, then evaluate positions on the untransformed bezpath.
1377+
// This avoids needing to invert the transform (which fails when the transform is singular, e.g. zero scale).
1378+
let Some((locations, was_closed)) =
1379+
bezpath_algorithms::compute_sample_locations(&world_bezpath, spacing, amount, start_offset, stop_offset, adaptive_spacing, current_bezpath_segments_length)
1380+
else {
13751381
continue;
13761382
};
13771383

1378-
// Reverse the transformation applied to the bezpath as the `result` already has the transformation set.
1379-
sample_bezpath.apply_affine(Affine::new(row.transform.to_cols_array()).inverse());
1384+
// Evaluate the sample locations on the untransformed bezpath and append the result
1385+
let mut sample_bezpath = BezPath::new();
1386+
for &(segment_index, t) in &locations {
1387+
let segment = local_bezpath.get_seg(segment_index + 1).unwrap();
1388+
let point = segment.eval(t);
13801389

1381-
// Append the bezpath (subpath) that connects generated points by lines.
1390+
if sample_bezpath.elements().is_empty() {
1391+
sample_bezpath.move_to(point);
1392+
} else {
1393+
sample_bezpath.line_to(point);
1394+
}
1395+
}
1396+
if was_closed {
1397+
sample_bezpath.close_path();
1398+
}
13821399
result.append_bezpath(sample_bezpath);
13831400
}
13841401

@@ -1879,6 +1896,59 @@ async fn spline(_: impl Ctx, content: Table<Vector>) -> Table<Vector> {
18791896
.collect()
18801897
}
18811898

1899+
/// Computes the inverse of a transform's linear (matrix2) part, handling singular transforms
1900+
/// (e.g. zero scale on one axis) by replacing the collapsed axis with a unit perpendicular
1901+
/// so offsets still apply there (visible if the transform is later replaced).
1902+
fn inverse_linear_or_repair(linear: DMat2) -> DMat2 {
1903+
if linear.determinant() != 0. {
1904+
return linear.inverse();
1905+
}
1906+
1907+
let col0 = linear.col(0);
1908+
let col1 = linear.col(1);
1909+
let col0_exists = col0.length_squared() > (f64::EPSILON * 1e3).powi(2);
1910+
let col1_exists = col1.length_squared() > (f64::EPSILON * 1e3).powi(2);
1911+
1912+
let repaired = match (col0_exists, col1_exists) {
1913+
(true, _) => DMat2::from_cols(col0, col0.perp().normalize()),
1914+
(false, true) => DMat2::from_cols(col1.perp().normalize(), col1),
1915+
(false, false) => DMat2::IDENTITY,
1916+
};
1917+
repaired.inverse()
1918+
}
1919+
1920+
/// Applies per-point displacement deltas to the point and handle positions of a vector element.
1921+
fn apply_point_deltas(element: &mut Vector, deltas: &[DVec2], transform: DAffine2) {
1922+
let mut already_applied = vec![false; element.point_domain.positions().len()];
1923+
1924+
for (handles, start, end) in element.segment_domain.handles_and_points_mut() {
1925+
let start_delta = deltas[*start];
1926+
let end_delta = deltas[*end];
1927+
1928+
if !already_applied[*start] {
1929+
let start_position = element.point_domain.positions()[*start];
1930+
element.point_domain.set_position(*start, start_position + start_delta);
1931+
already_applied[*start] = true;
1932+
}
1933+
if !already_applied[*end] {
1934+
let end_position = element.point_domain.positions()[*end];
1935+
element.point_domain.set_position(*end, end_position + end_delta);
1936+
already_applied[*end] = true;
1937+
}
1938+
1939+
match handles {
1940+
BezierHandles::Cubic { handle_start, handle_end } => {
1941+
*handle_start += start_delta;
1942+
*handle_end += end_delta;
1943+
}
1944+
BezierHandles::Quadratic { handle } => {
1945+
*handle = transform.transform_point2(*handle) + (start_delta + end_delta) / 2.;
1946+
}
1947+
BezierHandles::Linear => {}
1948+
}
1949+
}
1950+
}
1951+
18821952
/// Perturbs the positions of anchor points in vector geometry by random amounts and directions.
18831953
#[node_macro::node(category("Vector: Modifier"), path(core_types::vector))]
18841954
async fn jitter_points(
@@ -1899,11 +1969,9 @@ async fn jitter_points(
18991969
.into_iter()
19001970
.map(|mut row| {
19011971
let mut rng = rand::rngs::StdRng::seed_from_u64(seed.into());
1972+
let inverse_linear = inverse_linear_or_repair(row.transform.matrix2);
19021973

1903-
let transform = row.transform;
1904-
let inverse_transform = if transform.matrix2.determinant() != 0. { transform.inverse() } else { Default::default() };
1905-
1906-
let deltas = (0..row.element.point_domain.positions().len())
1974+
let deltas: Vec<_> = (0..row.element.point_domain.positions().len())
19071975
.map(|point_index| {
19081976
let normal = if along_normals {
19091977
row.element.segment_domain.point_tangent(point_index, row.element.point_domain.positions()).map(|t| t.perp())
@@ -1912,44 +1980,17 @@ async fn jitter_points(
19121980
};
19131981

19141982
let offset = if let Some(normal) = normal {
1915-
(rng.random::<f64>() * 2. - 1.) * normal
1983+
normal * (rng.random::<f64>() * 2. - 1.)
19161984
} else {
1917-
rng.random::<f64>() * DVec2::from_angle(rng.random::<f64>() * TAU)
1985+
DVec2::from_angle(rng.random::<f64>() * TAU) * rng.random::<f64>()
19181986
};
19191987

1920-
inverse_transform.transform_vector2(offset * amount)
1988+
inverse_linear * (offset * amount)
19211989
})
1922-
.collect::<Vec<_>>();
1923-
let mut already_applied = vec![false; row.element.point_domain.positions().len()];
1924-
1925-
for (handles, start, end) in row.element.segment_domain.handles_and_points_mut() {
1926-
let start_delta = deltas[*start];
1927-
let end_delta = deltas[*end];
1928-
1929-
if !already_applied[*start] {
1930-
let start_position = row.element.point_domain.positions()[*start];
1931-
row.element.point_domain.set_position(*start, start_position + start_delta);
1932-
already_applied[*start] = true;
1933-
}
1934-
if !already_applied[*end] {
1935-
let end_position = row.element.point_domain.positions()[*end];
1936-
row.element.point_domain.set_position(*end, end_position + end_delta);
1937-
already_applied[*end] = true;
1938-
}
1990+
.collect();
19391991

1940-
match handles {
1941-
BezierHandles::Cubic { handle_start, handle_end } => {
1942-
*handle_start += start_delta;
1943-
*handle_end += end_delta;
1944-
}
1945-
BezierHandles::Quadratic { handle } => {
1946-
*handle = row.transform.transform_point2(*handle) + (start_delta + end_delta) / 2.;
1947-
}
1948-
BezierHandles::Linear => {}
1949-
}
1950-
}
1992+
apply_point_deltas(&mut row.element, &deltas, row.transform);
19511993

1952-
row.element.style.set_stroke_transform(DAffine2::IDENTITY);
19531994
row
19541995
})
19551996
.collect()

website/static/js/page/contributor-guide/bisect-tool.js

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,36 @@ document.addEventListener("DOMContentLoaded", () => {
214214
commits = fetched;
215215
}
216216

217+
async function extendCommitsForward() {
218+
if (commits.length === 0) return false;
219+
220+
const newest = commits[commits.length - 1];
221+
const since = new Date(newest.date.getTime() + 1000).toISOString();
222+
223+
let /** @type {any[]} */ allRaw = [];
224+
let page = 1;
225+
226+
while (true) {
227+
const raw = await fetchCommitList(since, undefined, page);
228+
if (!raw || raw.length === 0) break;
229+
allRaw = allRaw.concat(raw);
230+
if (raw.length < 100) break;
231+
page++;
232+
}
233+
234+
if (allRaw.length === 0) return false;
235+
236+
let fetched = parseCommits(allRaw);
237+
fetched.reverse();
238+
239+
const existingShas = new Set(commits.map((c) => c.sha));
240+
fetched = fetched.filter((c) => !existingShas.has(c.sha));
241+
if (fetched.length === 0) return false;
242+
243+
commits = [...commits, ...fetched];
244+
return true;
245+
}
246+
217247
async function extendCommitsBackward() {
218248
if (commits.length === 0) return false;
219249

@@ -340,8 +370,9 @@ document.addEventListener("DOMContentLoaded", () => {
340370
badIndex = currentIndex;
341371
boundarySearching = true;
342372
} else {
343-
// Absent at starting commit. The newest commit should have it (user assumes master has it).
373+
// Absent at starting commit, so the issue was introduced more recently. Extend the commit list forward (towards HEAD) before narrowing.
344374
goodIndex = currentIndex;
375+
await extendCommitsForward();
345376
badIndex = commits.length - 1;
346377
bisectPhase = "binary";
347378
}

0 commit comments

Comments
 (0)