mirror of
https://github.com/storytold/photocraft.git
synced 2026-11-13 05:33:14 +00:00
Address CodeRabbit review: soft shape frame from the hard render, feather claim
A soft shape's transform frame, snap rectangle and Properties X/Y/W/H came from the path plus the full stroke width on every side, so a centred or inside stroke grew the frame when Density or Feather was set. The frame is now the pixels the same shape renders at full Density without Feather (edge strips only, clipped to the canvas), identical to the hard shape's for every stroke alignment. feather_matches_photoshops_ramp now enforces the 1-level claim the scorecard makes (measured max deviation 1 at 8, 16 and 32 bit). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
5bf2898f4e
commit
e5f4f8e260
5 files changed
+84
-13
No files matched your search
@@ -1144,9 +1144,7 @@ pub fn relayout_damage(a: &Document, b: &Document) -> Rect {
|
||||
}
|
||||
|
||||
/// The bounding box of a shape layer's path and stroke, ignoring its Density and Feather: its
|
||||
/// path control bounds plus stroke width. It is the frame Free Transform and the Move tool's
|
||||
/// transform controls show for a soft shape, whose rendering reaches past it (in Photoshop a
|
||||
/// shape's transform frame follows its path whatever its mask Density and Feather).
|
||||
/// path control bounds plus stroke width (a conservative reach for any alignment, cap and join).
|
||||
pub fn hard_shape_bounds(sh: &photocraft_doc::ShapeLayer) -> Option<Rect> {
|
||||
if sh.fill.is_none() && sh.stroke.as_ref().is_none_or(|s| s.width <= 0.0 || s.opacity <= 0.0) {
|
||||
return None;
|
||||
@@ -1157,6 +1155,35 @@ pub fn hard_shape_bounds(sh: &photocraft_doc::ShapeLayer) -> Option<Rect> {
|
||||
(!r.is_empty()).then_some(r)
|
||||
}
|
||||
|
||||
/// The frame Free Transform, the Move tool's transform controls, snapping and Properties show for
|
||||
/// a soft shape (Density below 100 % or a Feather), whose rendering reaches past its path: the
|
||||
/// pixels the same shape renders in `format` over `canvas` at full Density without Feather, so
|
||||
/// the frame is exactly the hard shape's whatever the stroke's alignment, cap and join (in
|
||||
/// Photoshop a shape's transform frame follows its path and stroke whatever its mask Density and
|
||||
/// Feather, #5846). An inverted path's frame is its own outline, not the whole canvas. Renders
|
||||
/// only the strips along the edges of the rasterizers' (1 px conservative) bounds.
|
||||
pub fn soft_shape_frame(sh: &photocraft_doc::ShapeLayer, format: photocraft_color::PixelFormat, canvas: Rect) -> Option<Rect> {
|
||||
let mut path = sh.path.clone();
|
||||
path.inverted = false;
|
||||
let hard = photocraft_doc::ShapeLayer { density: 1.0, feather: 0.0, path, fill: sh.fill.clone(), stroke: sh.stroke.clone(), ..Default::default() };
|
||||
let shape = photocraft_vector::CompiledShape::new(&hard, photocraft_vector::DEFAULT_TOLERANCE, canvas);
|
||||
let mut r = shape.bounds()?.intersect(&canvas);
|
||||
let painted = |strip: Rect| !shape.render(format, strip).content_bounds().is_empty();
|
||||
while !r.is_empty() && !painted(Rect::new(r.x0, r.y0, r.x1, r.y0 + 1)) {
|
||||
r.y0 += 1;
|
||||
}
|
||||
while !r.is_empty() && !painted(Rect::new(r.x0, r.y1 - 1, r.x1, r.y1)) {
|
||||
r.y1 -= 1;
|
||||
}
|
||||
while !r.is_empty() && !painted(Rect::new(r.x0, r.y0, r.x0 + 1, r.y1)) {
|
||||
r.x0 += 1;
|
||||
}
|
||||
while !r.is_empty() && !painted(Rect::new(r.x1 - 1, r.y0, r.x1, r.y1)) {
|
||||
r.x1 -= 1;
|
||||
}
|
||||
(!r.is_empty()).then_some(r)
|
||||
}
|
||||
|
||||
/// The bounding box of a shape layer's vector content: its path control bounds plus stroke width
|
||||
/// (and the reach of a soft fill).
|
||||
pub fn shape_bounds(sh: &photocraft_doc::ShapeLayer) -> Option<Rect> {
|
||||
|
||||
@@ -58,8 +58,9 @@ fn feather_matches_photoshops_ramp() {
|
||||
let r = row(&shape_doc(depth, 1.0, 10.0, None), 100);
|
||||
let ours: Vec<i32> = (0..15).map(|i| r[30 + 3 * i]).collect();
|
||||
let dev = ours.iter().zip(PS_FEATHER_10).map(|(a, b)| (a - i32::from(b)).abs()).max().unwrap_or(0);
|
||||
// Same Gaussian (sigma = feather) as a vector mask: within 3 levels of Photoshop.
|
||||
assert!(dev <= 3, "{depth:?}: ours {ours:?} vs Photoshop {PS_FEATHER_10:?} (max {dev})");
|
||||
// Same Gaussian (sigma = feather) as a vector mask: within 1 level of Photoshop (as
|
||||
// scorecard TOOL-214-12 claims; measured max deviation 1 at every depth).
|
||||
assert!(dev <= 1, "{depth:?}: ours {ours:?} vs Photoshop {PS_FEATHER_10:?} (max {dev})");
|
||||
eprintln!("{depth:?}: ours {ours:?}, Photoshop {PS_FEATHER_10:?}, max deviation {dev}");
|
||||
// The path runs between pixels 49 and 50: their mean is half way.
|
||||
assert!((r[49] + r[50] - 255).abs() <= 2, "{depth:?}: half way on the path: {} {}", r[49], r[50]);
|
||||
|
||||
@@ -95,8 +95,8 @@ fn rect_of(doc: &Document, l: &photocraft_doc::Layer) -> Option<[f64; 4]> {
|
||||
/// Bounds of a layer as a Snap To Layers target: all of its content, whatever the selection
|
||||
/// holds of it (#6290: clipped to the selection, every layer's edges and centre were the
|
||||
/// selection's, so a moved selection snapped back to where it started).
|
||||
fn target_rect_of(l: &photocraft_doc::Layer) -> Option<[f64; 4]> {
|
||||
snappable(l).then(|| crate::transform_cmds::whole_layer_bounds(l)).and_then(to_rect)
|
||||
fn target_rect_of(doc: &Document, l: &photocraft_doc::Layer) -> Option<[f64; 4]> {
|
||||
snappable(l).then(|| crate::transform_cmds::whole_layer_bounds(doc, l)).and_then(to_rect)
|
||||
}
|
||||
|
||||
fn snappable(l: &photocraft_doc::Layer) -> bool {
|
||||
@@ -132,7 +132,7 @@ impl SnapTargets {
|
||||
if !l.visible || exclude.contains(&l.id) {
|
||||
continue;
|
||||
}
|
||||
if let Some(r) = target_rect_of(l) {
|
||||
if let Some(r) = target_rect_of(doc, l) {
|
||||
t.add_rect(r, SnapKind::LayerEdge, SnapKind::LayerCenter);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -29,7 +29,7 @@ pub fn moves_whole(layer: &Layer) -> bool {
|
||||
/// content within the selection, or all of it for a layer that [`moves_whole`] (#2630).
|
||||
/// Content scans are cached per tile: snapping asks for every layer's bounds per Move drag.
|
||||
pub fn transform_bounds(doc: &Document, layer: &Layer) -> Rect {
|
||||
let content = whole_layer_bounds(layer);
|
||||
let content = whole_layer_bounds(doc, layer);
|
||||
match &doc.selection {
|
||||
Some(sel) if !moves_whole(layer) => content.intersect(&sel.content_bounds()),
|
||||
_ => content,
|
||||
@@ -54,12 +54,12 @@ fn framed_by_selection(doc: &Document, bounds: Rect) -> Rect {
|
||||
}
|
||||
|
||||
/// A group moves all its descendants, so its frame must not clip raster children to the selection.
|
||||
pub(crate) fn whole_layer_bounds(layer: &Layer) -> Rect {
|
||||
pub(crate) fn whole_layer_bounds(doc: &Document, layer: &Layer) -> Rect {
|
||||
let content = match &layer.content {
|
||||
LayerContent::Group(g) => g.children.iter().map(whole_layer_bounds).fold(Rect::EMPTY, |a, b| a.union(&b)),
|
||||
LayerContent::Group(g) => g.children.iter().map(|c| whole_layer_bounds(doc, c)).fold(Rect::EMPTY, |a, b| a.union(&b)),
|
||||
// A soft shape (Density below 100 % or a Feather) renders past its path, over the whole
|
||||
// canvas below 100 %: its frame follows the path and stroke, as in Photoshop (#5846).
|
||||
LayerContent::Shape(sh) if sh.has_soft_mask() => photocraft_compose::hard_shape_bounds(sh).unwrap_or(Rect::EMPTY),
|
||||
LayerContent::Shape(sh) if sh.has_soft_mask() => photocraft_compose::soft_shape_frame(sh, doc.pixel_format(), doc.bounds()).unwrap_or(Rect::EMPTY),
|
||||
_ => layer.surface().map_or(Rect::EMPTY, photocraft_compose::bounds::content_bounds),
|
||||
};
|
||||
if content.is_empty() { layer.mask.as_ref().map_or(Rect::EMPTY, |m| photocraft_compose::bounds::content_bounds(&m.surface)) } else { content }
|
||||
@@ -1539,4 +1539,32 @@ mod tests {
|
||||
assert_eq!((x0.round(), y0.round(), x1.round(), y1.round()), (-10.0, 30.0, 110.0, 70.0));
|
||||
assert_eq!((sh.density, sh.feather), (1.0, 20.0), "the transform keeps the soft settings");
|
||||
}
|
||||
|
||||
/// A stroked soft shape's frame is the hard shape's rendered frame for every stroke alignment:
|
||||
/// a centred stroke reaches half its width past the path, not its full width; both stop at
|
||||
/// the canvas edge.
|
||||
#[test]
|
||||
fn a_stroked_soft_shapes_frame_matches_the_hard_shapes() {
|
||||
for (align, reach, x) in [("center", 5, 50), ("inside", 0, 50), ("outside", 10, 50), ("center", 5, -20)] {
|
||||
let mut s = Session::new();
|
||||
s.execute("file.new", json!({"width": 200, "height": 200})).unwrap();
|
||||
let stroke = json!({"width": 10, "color": "#ff0000", "align": align, "join": "round"});
|
||||
let id = s.execute("shape.create", json!({"kind": "rect", "rect": [x, 50, 60, 40], "fill": "#000000", "stroke": stroke})).unwrap()["layer"]
|
||||
.as_u64()
|
||||
.unwrap();
|
||||
let id = photocraft_doc::LayerId(id);
|
||||
let boxes = |s: &Session| {
|
||||
let st = s.active().unwrap();
|
||||
let l = st.doc.layer(id).unwrap();
|
||||
(transform_bounds(&st.doc, l), transform_frame(&st.doc, l), crate::snap::layer_rect(&st.doc, id))
|
||||
};
|
||||
let hard = boxes(&s);
|
||||
// Hand-computed: the path is x..x + 60 x 50..90, the stroke reaches `reach` px past it.
|
||||
assert_eq!(hard.0, Rect::new((x - reach).max(0), 50 - reach, x + 60 + reach, 90 + reach), "{align} at {x}: the hard shape's frame");
|
||||
s.execute("layer.vectorMask.edit", json!({"density": 60, "feather": 8})).unwrap();
|
||||
assert_eq!(boxes(&s), hard, "{align} at {x}: Density and Feather leave the frame");
|
||||
s.execute("layer.vectorMask.edit", json!({"density": 100, "feather": 8})).unwrap();
|
||||
assert_eq!(boxes(&s), hard, "{align} at {x}: Feather alone leaves the frame");
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -2593,7 +2593,22 @@ impl PhotocraftApp {
|
||||
/// transform frame does.
|
||||
pub fn layer_frame_bounds(&mut self, layer: &photocraft_doc::Layer, surface: &photocraft_raster::Surface) -> photocraft_geom::Rect {
|
||||
match &layer.content {
|
||||
photocraft_doc::LayerContent::Shape(sh) if sh.has_soft_mask() => photocraft_compose::hard_shape_bounds(sh).unwrap_or(photocraft_geom::Rect::EMPTY),
|
||||
photocraft_doc::LayerContent::Shape(sh) if sh.has_soft_mask() => {
|
||||
// Cached per rendering, like the pixel bounds: the frame renders strips of the shape.
|
||||
let (key, rev) = (layer.id.0 | 1 << 63, surface_fingerprint(surface));
|
||||
if let Some((r, b)) = self.bounds_cache.get(&key)
|
||||
&& *r == rev
|
||||
{
|
||||
return *b;
|
||||
}
|
||||
let canvas = self.session.active().map_or_else(|| surface.content_bounds(), |st| st.doc.bounds());
|
||||
let b = photocraft_compose::soft_shape_frame(sh, surface.format(), canvas).unwrap_or(photocraft_geom::Rect::EMPTY);
|
||||
if self.bounds_cache.len() > 256 {
|
||||
self.bounds_cache.clear();
|
||||
}
|
||||
self.bounds_cache.insert(key, (rev, b));
|
||||
b
|
||||
}
|
||||
_ => self.cached_bounds(layer.id.0, surface),
|
||||
}
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user