Fix drawing tools to work in viewport space instead of document space (#2438)

* Stroke width in viewports

* Update rectangle tests
This commit is contained in:
James Lindsay 2025-03-15 11:22:50 +00:00 committed by GitHub
parent 5cdcc37379
commit ca5810c92a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 41 additions and 51 deletions

View file

@ -42,44 +42,45 @@ impl Resize {
self.layer.take();
return None;
}
Some(self.calculate_points_ignore_layer(document, input, center, lock_ratio))
Some(self.calculate_points_ignore_layer(document, input, center, lock_ratio, false))
}
/// Compute the drag start and end based on the current mouse position. Ignores the state of the layer.
/// If you want to only draw whilst a layer exists, use [`Resize::calculate_points`].
pub fn calculate_points_ignore_layer(&mut self, document: &DocumentMessageHandler, input: &InputPreprocessorMessageHandler, center: Key, lock_ratio: Key) -> [DVec2; 2] {
let start = self.drag_start;
pub fn calculate_points_ignore_layer(&mut self, document: &DocumentMessageHandler, input: &InputPreprocessorMessageHandler, center: Key, lock_ratio: Key, in_document: bool) -> [DVec2; 2] {
let start = self.viewport_drag_start(document);
let mouse = input.mouse.position;
let document_to_viewport = document.navigation_handler.calculate_offset_transform(input.viewport_bounds.center(), &document.document_ptz);
let document_mouse = document_to_viewport.inverse().transform_point2(mouse);
let mut document_points = [start, document_mouse];
let mut points_viewport = [start, mouse];
let ignore = if let Some(layer) = self.layer { vec![layer] } else { vec![] };
let ratio = input.keyboard.get(lock_ratio as usize);
let center = input.keyboard.get(center as usize);
let snap_data = SnapData::ignore(document, input, &ignore);
let config = SnapTypeConfiguration::default();
if ratio {
let size = document_points[1] - document_points[0];
let size = size.abs().max(size.abs().yx()) * size.signum();
document_points[1] = document_points[0] + size;
let end = document_points[1];
let viewport_size = points_viewport[1] - points_viewport[0];
let raw_size = if in_document { document_to_viewport.inverse() } else { DAffine2::IDENTITY }.transform_vector2(viewport_size);
let adjusted_size = raw_size.abs().max(raw_size.abs().yx()) * raw_size.signum();
let size = if in_document { document_to_viewport.transform_vector2(adjusted_size) } else { adjusted_size };
points_viewport[1] = points_viewport[0] + size;
let end_document = document_to_viewport.inverse().transform_point2(points_viewport[1]);
let constraint = SnapConstraint::Line {
origin: self.drag_start,
direction: end - self.drag_start,
direction: end_document - self.drag_start,
};
if center {
let snapped = self.snap_manager.constrained_snap(&snap_data, &SnapCandidatePoint::handle(end), constraint, config);
let far = SnapCandidatePoint::handle(2. * self.drag_start - end);
let snapped = self.snap_manager.constrained_snap(&snap_data, &SnapCandidatePoint::handle(end_document), constraint, config);
let far = SnapCandidatePoint::handle(2. * self.drag_start - end_document);
let snapped_far = self.snap_manager.constrained_snap(&snap_data, &far, constraint, config);
let best = if snapped_far.other_snap_better(&snapped) { snapped } else { snapped_far };
document_points[0] = best.snapped_point_document;
document_points[1] = self.drag_start * 2. - best.snapped_point_document;
points_viewport[0] = document_to_viewport.transform_point2(best.snapped_point_document);
points_viewport[1] = document_to_viewport.transform_point2(self.drag_start * 2. - best.snapped_point_document);
self.snap_manager.update_indicator(best);
} else {
let snapped = self.snap_manager.constrained_snap(&snap_data, &SnapCandidatePoint::handle(end), constraint, config);
document_points[1] = snapped.snapped_point_document;
let snapped = self.snap_manager.constrained_snap(&snap_data, &SnapCandidatePoint::handle(end_document), constraint, config);
points_viewport[1] = document_to_viewport.transform_point2(snapped.snapped_point_document);
self.snap_manager.update_indicator(snapped);
}
} else if center {
@ -87,28 +88,24 @@ impl Resize {
let opposite = 2. * self.drag_start - document_mouse;
let snapped_far = self.snap_manager.free_snap(&snap_data, &SnapCandidatePoint::handle(opposite), config);
let best = if snapped_far.other_snap_better(&snapped) { snapped } else { snapped_far };
document_points[0] = best.snapped_point_document;
document_points[1] = self.drag_start * 2. - best.snapped_point_document;
points_viewport[0] = document_to_viewport.transform_point2(best.snapped_point_document);
points_viewport[1] = document_to_viewport.transform_point2(self.drag_start * 2. - best.snapped_point_document);
self.snap_manager.update_indicator(best);
} else {
let snapped = self.snap_manager.free_snap(&snap_data, &SnapCandidatePoint::handle(document_mouse), config);
document_points[1] = snapped.snapped_point_document;
points_viewport[1] = document_to_viewport.transform_point2(snapped.snapped_point_document);
self.snap_manager.update_indicator(snapped);
}
document_points
points_viewport
}
pub fn calculate_transform(&mut self, document: &DocumentMessageHandler, input: &InputPreprocessorMessageHandler, center: Key, lock_ratio: Key, skip_rerender: bool) -> Option<Message> {
let viewport_points = self.calculate_points(document, input, center, lock_ratio).map(|points| {
let document_to_viewport = document.metadata().document_to_viewport;
[document_to_viewport.transform_point2(points[0]), document_to_viewport.transform_point2(points[1])]
})?;
let points_viewport = self.calculate_points(document, input, center, lock_ratio)?;
Some(
GraphOperationMessage::TransformSet {
layer: self.layer?,
transform: DAffine2::from_scale_angle_translation(viewport_points[1] - viewport_points[0], 0., viewport_points[0]),
transform: DAffine2::from_scale_angle_translation(points_viewport[1] - points_viewport[0], 0., points_viewport[0]),
transform_in: TransformIn::Viewport,
skip_rerender,
}

View file

@ -316,7 +316,9 @@ impl Fsm for ArtboardToolFsmState {
ArtboardToolFsmState::Dragging
}
(ArtboardToolFsmState::Drawing, ArtboardToolMessage::PointerMove { constrain_axis_or_aspect, center }) => {
let [start, end] = tool_data.draw.calculate_points_ignore_layer(document, input, center, constrain_axis_or_aspect);
let [start, end] = tool_data.draw.calculate_points_ignore_layer(document, input, center, constrain_axis_or_aspect, true);
let viewport_to_document = document.metadata().document_to_viewport.inverse();
let [start, end] = [start, end].map(|point| viewport_to_document.transform_point2(point));
if let Some(artboard) = tool_data.selected_artboard {
assert_ne!(artboard, LayerNodeIdentifier::ROOT_PARENT, "Selected artboard cannot be ROOT_PARENT");

View file

@ -236,7 +236,7 @@ impl Fsm for EllipseToolFsmState {
responses.add(GraphOperationMessage::TransformSet {
layer,
transform: DAffine2::from_translation((start + end) / 2.),
transform_in: TransformIn::Local,
transform_in: TransformIn::Viewport,
skip_rerender: false,
});
}
@ -400,17 +400,14 @@ mod test_ellipse {
let ellipse = get_ellipse(&mut editor).await;
assert_eq!(ellipse.len(), 1);
println!("{ellipse:?}");
// TODO: re-enable after https://github.com/GraphiteEditor/Graphite/issues/2370
// assert_eq!(ellipse[0].radius_x, 5.);
// assert_eq!(ellipse[0].radius_y, 5.);
assert_eq!(ellipse[0].radius_x, 5.);
assert_eq!(ellipse[0].radius_y, 5.);
// assert!(ellipse[0]
// .transform
// .abs_diff_eq(DAffine2::from_angle_translation(-f64::consts::FRAC_PI_4, DVec2::X * f64::consts::FRAC_1_SQRT_2 * 10.), 0.001));
float_eq!(ellipse[0].radius_x, 11. / core::f64::consts::SQRT_2 / 2.);
float_eq!(ellipse[0].radius_y, 11. / core::f64::consts::SQRT_2 / 2.);
assert!(ellipse[0].transform.abs_diff_eq(DAffine2::from_translation(DVec2::splat(11. / core::f64::consts::SQRT_2 / 2.)), 0.001));
assert!(
ellipse[0]
.transform
.abs_diff_eq(DAffine2::from_angle_translation(-f64::consts::FRAC_PI_4, DVec2::X * f64::consts::FRAC_1_SQRT_2 * 10.), 0.001)
);
}
#[tokio::test]
@ -427,13 +424,9 @@ mod test_ellipse {
let ellipse = get_ellipse(&mut editor).await;
assert_eq!(ellipse.len(), 1);
// TODO: re-enable after https://github.com/GraphiteEditor/Graphite/issues/2370
// assert_eq!(ellipse[0].radius_x, 10.);
// assert_eq!(ellipse[0].radius_y, 10.);
// assert!(ellipse[0].transform.abs_diff_eq(DAffine2::from_angle(-f64::consts::FRAC_PI_4), 0.001));
float_eq!(ellipse[0].radius_x, 11. / core::f64::consts::SQRT_2);
float_eq!(ellipse[0].radius_y, 11. / core::f64::consts::SQRT_2);
assert!(ellipse[0].transform.abs_diff_eq(DAffine2::IDENTITY, 0.001));
assert_eq!(ellipse[0].radius_x, 10.);
assert_eq!(ellipse[0].radius_y, 10.);
assert!(ellipse[0].transform.abs_diff_eq(DAffine2::from_angle(-f64::consts::FRAC_PI_4), 0.001));
}
#[tokio::test]

View file

@ -325,7 +325,7 @@ impl Fsm for PolygonToolFsmState {
responses.add(GraphOperationMessage::TransformSet {
layer,
transform: DAffine2::from_scale_angle_translation(scale, 0., (start + end) / 2.),
transform_in: TransformIn::Local,
transform_in: TransformIn::Viewport,
skip_rerender: false,
});
}

View file

@ -241,7 +241,7 @@ impl Fsm for RectangleToolFsmState {
responses.add(GraphOperationMessage::TransformSet {
layer,
transform: DAffine2::from_translation((start + end) / 2.),
transform_in: TransformIn::Local,
transform_in: TransformIn::Viewport,
skip_rerender: false,
});
}

View file

@ -593,9 +593,7 @@ impl Fsm for TextToolFsmState {
TextToolFsmState::Ready
}
(Self::Placing | TextToolFsmState::Dragging, TextToolMessage::PointerMove { center, lock_ratio }) => {
let document_points = tool_data.resize.calculate_points_ignore_layer(document, input, center, lock_ratio);
let document_to_viewport = document.metadata().document_to_viewport;
tool_data.cached_resize_bounds = [document_to_viewport.transform_point2(document_points[0]), document_to_viewport.transform_point2(document_points[1])];
tool_data.cached_resize_bounds = tool_data.resize.calculate_points_ignore_layer(document, input, center, lock_ratio, false);
responses.add(OverlaysMessage::Draw);