canvas: Store current path only in user space (#38098)

Before we stored path in user or device space and do conversions as
needed, but that complicated things.

So we got two options:
1. store path in device space: here we would need to add
`Option<Transform>` to each path command, and before each usage we need
to invert current transform (as current transform is already applied by
stroke/fill and we cannot just pass I, because we need scaling to make
lines thicker)
2. store path in user space and do transform whenever we change the
transform. There is no need to do inverse on uses.

I chose option 2. because it's less complicated and will probably
benefit performance (set transform typically called more rarely than
path building/fill/stroke).

In follow up PR I will move all of this to script, that's why
PathBuilderRef was not removed yet.

Testing: Existing WPT tests
work towards #38022
try run: https://github.com/sagudev/servo/actions/runs/16304221495

---------

Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
This commit is contained in:
sagudev 2025-07-16 09:30:20 +02:00 committed by GitHub
parent 2ea95c8813
commit 429b91da49
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -34,24 +34,6 @@ use crate::backend::{
// https://github.com/servo/webrender/blob/main/webrender/src/texture_cache.rs#L1475 // https://github.com/servo/webrender/blob/main/webrender/src/texture_cache.rs#L1475
const MIN_WR_IMAGE_SIZE: Size2D<u64> = Size2D::new(1, 1); const MIN_WR_IMAGE_SIZE: Size2D<u64> = Size2D::new(1, 1);
/// The canvas data stores a state machine for the current status of
/// the path data and any relevant transformations that are
/// applied to it. The Azure drawing API expects the path to be in
/// userspace. However, when a path is being built but the canvas'
/// transform changes, we choose to transform the path and perform
/// further operations to it in device space. When it's time to
/// draw the path, we convert it back to userspace and draw it
/// with the correct transform applied.
/// TODO: De-abstract now that Azure is removed?
enum PathState {
/// Path in user-space. If a transform has been applied but
/// but no further path operations have occurred, it is stored
/// in the optional field.
UserSpacePath(Path, Option<Transform2D<f32>>),
/// Path in device-space.
DeviceSpacePath(Path),
}
/// A wrapper around a stored PathBuilder and an optional transformation that should be /// A wrapper around a stored PathBuilder and an optional transformation that should be
/// applied to any points to ensure they are in the matching device space. /// applied to any points to ensure they are in the matching device space.
pub(crate) struct PathBuilderRef<'a> { pub(crate) struct PathBuilderRef<'a> {
@ -305,7 +287,8 @@ pub(crate) enum Filter {
pub(crate) struct CanvasData<'a, B: Backend> { pub(crate) struct CanvasData<'a, B: Backend> {
backend: B, backend: B,
drawtarget: B::DrawTarget, drawtarget: B::DrawTarget,
path_state: Option<PathState>, /// <https://html.spec.whatwg.org/multipage/#current-default-path>
path_state: Path,
state: CanvasPaintState<'a, B>, state: CanvasPaintState<'a, B>,
saved_states: Vec<CanvasPaintState<'a, B>>, saved_states: Vec<CanvasPaintState<'a, B>>,
compositor_api: CrossProcessCompositorApi, compositor_api: CrossProcessCompositorApi,
@ -329,7 +312,7 @@ impl<'a, B: Backend> CanvasData<'a, B> {
state: backend.new_paint_state(), state: backend.new_paint_state(),
backend, backend,
drawtarget: draw_target, drawtarget: draw_target,
path_state: None, path_state: Path::new(),
saved_states: vec![], saved_states: vec![],
compositor_api, compositor_api,
image_key, image_key,
@ -708,64 +691,19 @@ impl<'a, B: Backend> CanvasData<'a, B> {
pub(crate) fn begin_path(&mut self) { pub(crate) fn begin_path(&mut self) {
// Erase any traces of previous paths that existed before this. // Erase any traces of previous paths that existed before this.
self.path_state = None; self.path_state = Path::new();
} }
pub(crate) fn close_path(&mut self) { pub(crate) fn close_path(&mut self) {
self.path_builder().close(); self.path_builder().close();
} }
/// Turn the [`Self::path_state`] into a user-space path, returning `None` if the
/// path transformation matrix is uninvertible.
fn ensure_path(&mut self) -> Option<&Path> {
// If there's no record of any path yet, create a new path in user-space.
if self.path_state.is_none() {
self.path_state = Some(PathState::UserSpacePath(Path::new(), None));
}
// If a user-space path exists, create a device-space path based on it if
// any transform is present.
let new_state = match *self.path_state.as_ref().unwrap() {
PathState::UserSpacePath(ref path, Some(ref transform)) => {
let mut path = path.clone();
path.transform(transform.cast());
Some(path)
},
PathState::UserSpacePath(..) | PathState::DeviceSpacePath(..) => None,
};
if let Some(builder) = new_state {
self.path_state = Some(PathState::DeviceSpacePath(builder));
}
// If a device-space path is present, create a user-space path from its
// finished path by inverting the initial transformation.
let new_state = match *self.path_state.as_mut().unwrap() {
PathState::DeviceSpacePath(ref mut builder) => {
let inverse = self.drawtarget.get_transform().inverse()?;
let mut path = builder.clone();
path.transform(inverse.cast());
Some(path)
},
PathState::UserSpacePath(..) => None,
};
if let Some(path) = new_state {
self.path_state = Some(PathState::UserSpacePath(path, None));
}
match self.path_state.as_ref() {
Some(PathState::UserSpacePath(path, _)) => Some(path),
_ => unreachable!("Should have been able to successful build path or returned early."),
}
}
pub(crate) fn fill(&mut self) { pub(crate) fn fill(&mut self) {
if self.state.fill_style.is_zero_size_gradient() { if self.state.fill_style.is_zero_size_gradient() {
return; // Paint nothing if gradient size is zero. return; // Paint nothing if gradient size is zero.
} }
let Some(path) = self.ensure_path().cloned() else { let path = self.path_state.clone();
return; // Path is uninvertible.
};
self.maybe_bound_shape_with_pattern( self.maybe_bound_shape_with_pattern(
self.state.fill_style.clone(), self.state.fill_style.clone(),
@ -794,9 +732,7 @@ impl<'a, B: Backend> CanvasData<'a, B> {
return; // Paint nothing if gradient size is zero. return; // Paint nothing if gradient size is zero.
} }
let Some(path) = self.ensure_path().cloned() else { let path = self.path_state.clone();
return; // Path is uninvertible.
};
self.maybe_bound_shape_with_pattern( self.maybe_bound_shape_with_pattern(
self.state.stroke_style.clone(), self.state.stroke_style.clone(),
@ -832,11 +768,9 @@ impl<'a, B: Backend> CanvasData<'a, B> {
} }
pub(crate) fn clip(&mut self) { pub(crate) fn clip(&mut self) {
let Some(path) = self.ensure_path().cloned() else { let path = &self.path_state;
return; // Path is uninvertible.
};
self.drawtarget.push_clip(&path); self.drawtarget.push_clip(path);
} }
pub(crate) fn clip_path(&mut self, path: &Path) { pub(crate) fn clip_path(&mut self, path: &Path) {
@ -850,18 +784,9 @@ impl<'a, B: Backend> CanvasData<'a, B> {
fill_rule: FillRule, fill_rule: FillRule,
chan: IpcSender<bool>, chan: IpcSender<bool>,
) { ) {
self.ensure_path(); let mut path = self.path_state.clone();
let result = match self.path_state.as_ref() { path.transform(self.get_transform().cast());
Some(PathState::UserSpacePath(path, transform)) => { chan.send(path.is_point_in_path(x, y, fill_rule)).unwrap();
let target_transform = self.drawtarget.get_transform();
let path_transform = transform.as_ref().unwrap_or(&target_transform);
let mut path = path.clone();
path.transform(path_transform.cast());
path.is_point_in_path(x, y, fill_rule)
},
Some(_) | None => false,
};
chan.send(result).unwrap();
} }
pub(crate) fn move_to(&mut self, point: &Point2D<f32>) { pub(crate) fn move_to(&mut self, point: &Point2D<f32>) {
@ -873,57 +798,9 @@ impl<'a, B: Backend> CanvasData<'a, B> {
} }
fn path_builder(&mut self) -> PathBuilderRef { fn path_builder(&mut self) -> PathBuilderRef {
if self.path_state.is_none() { PathBuilderRef {
self.path_state = Some(PathState::UserSpacePath(Path::new(), None)); builder: &mut self.path_state,
} transform: Transform2D::identity(),
// Rust is not pleased by returning a reference to a builder in some branches
// and overwriting path_state in other ones. The following awkward use of duplicate
// matches works around the resulting borrow errors.
let new_state = {
match *self.path_state.as_mut().unwrap() {
PathState::DeviceSpacePath(_) => None,
PathState::UserSpacePath(ref path, Some(ref transform)) => {
let mut path = path.clone();
path.transform(transform.cast());
Some(PathState::DeviceSpacePath(path))
},
PathState::UserSpacePath(ref path, None) => {
Some(PathState::UserSpacePath(path.clone(), None))
},
}
};
match new_state {
// There's a new builder value that needs to be stored.
Some(state) => self.path_state = Some(state),
// There's an existing builder value that can be returned immediately.
None => match *self.path_state.as_mut().unwrap() {
PathState::UserSpacePath(ref mut builder, None) => {
return PathBuilderRef {
builder,
transform: Transform2D::identity(),
};
},
PathState::DeviceSpacePath(ref mut builder) => {
return PathBuilderRef {
builder,
transform: self.drawtarget.get_transform(),
};
},
_ => unreachable!(),
},
}
match *self.path_state.as_mut().unwrap() {
PathState::UserSpacePath(ref mut builder, None) => PathBuilderRef {
builder,
transform: Transform2D::identity(),
},
PathState::DeviceSpacePath(ref mut builder) => PathBuilderRef {
builder,
transform: self.drawtarget.get_transform(),
},
PathState::UserSpacePath(..) => unreachable!(),
} }
} }
@ -932,9 +809,6 @@ impl<'a, B: Backend> CanvasData<'a, B> {
} }
pub(crate) fn quadratic_curve_to(&mut self, cp: &Point2D<f32>, endpoint: &Point2D<f32>) { pub(crate) fn quadratic_curve_to(&mut self, cp: &Point2D<f32>, endpoint: &Point2D<f32>) {
if self.path_state.is_none() {
self.move_to(cp);
}
self.path_builder().quadratic_curve_to(cp, endpoint); self.path_builder().quadratic_curve_to(cp, endpoint);
} }
@ -944,9 +818,6 @@ impl<'a, B: Backend> CanvasData<'a, B> {
cp2: &Point2D<f32>, cp2: &Point2D<f32>,
endpoint: &Point2D<f32>, endpoint: &Point2D<f32>,
) { ) {
if self.path_state.is_none() {
self.move_to(cp1);
}
self.path_builder().bezier_curve_to(cp1, cp2, endpoint); self.path_builder().bezier_curve_to(cp1, cp2, endpoint);
} }
@ -1027,18 +898,12 @@ impl<'a, B: Backend> CanvasData<'a, B> {
} }
pub(crate) fn set_transform(&mut self, transform: &Transform2D<f32>) { pub(crate) fn set_transform(&mut self, transform: &Transform2D<f32>) {
// If there is an in-progress path, store the existing transformation required self.path_state.transform(self.get_transform().cast());
// to move between device and user space.
match self.path_state.as_mut() {
None | Some(PathState::DeviceSpacePath(..)) => (),
Some(PathState::UserSpacePath(_, transform)) => {
if transform.is_none() {
*transform = Some(self.drawtarget.get_transform());
}
},
}
self.state.transform = *transform; self.state.transform = *transform;
self.drawtarget.set_transform(transform) self.drawtarget.set_transform(transform);
if let Some(inverse) = transform.inverse() {
self.path_state.transform(inverse.cast());
}
} }
pub(crate) fn set_global_alpha(&mut self, alpha: f32) { pub(crate) fn set_global_alpha(&mut self, alpha: f32) {
@ -1061,7 +926,7 @@ impl<'a, B: Backend> CanvasData<'a, B> {
.create_drawtarget(Size2D::new(size.width, size.height)); .create_drawtarget(Size2D::new(size.width, size.height));
// Step 2. Empty the list of subpaths in context's current default path. // Step 2. Empty the list of subpaths in context's current default path.
self.path_state = None; self.path_state = Path::new();
// Step 3. Clear the context's drawing state stack. // Step 3. Clear the context's drawing state stack.
self.saved_states.clear(); self.saved_states.clear();