WIP: Support the clip-path property #211 #250
Conversation
There was a problem hiding this comment.
@gautam1168 Firstly, no worries on the delay. This is open source, you can do thing when you like.
A couple of notes:
- Kurbo has
SvgArc, which seems to have the same parameterization as Stylo. And then a built-in function to convert that to it's regularArcrepresentation. - It also have this BezPath::from_svg function to parse an string SVG path into a
BezPath. I don't think you can use that directly, but I think you could base your conversion code on it's source (https://docs.rs/kurbo/latest/src/kurbo/svg.rs.html#105)
| let vert = match pos.vertical.unpack() { | ||
| Unpacked::Length(l) => l.px() as f64, | ||
| Unpacked::Percentage(p) => { | ||
| box_height * p.to_percentage() as f64 / 2.0 | ||
| }, | ||
| Unpacked::Calc(calc) => { | ||
| calc.resolve(CSSPixelLength::new(box_height as f32)).px() as f64 / 2.0 | ||
| } | ||
| }; |
There was a problem hiding this comment.
I think this can just be pos.vertical.resolve(CSSPixelLength::new(box_height as f32 / 2.0)) (see https://docs.rs/stylo/latest/style/values/computed/length_percentage/struct.LengthPercentage.html#method.resolve)
There was a problem hiding this comment.
But what is the / 2.0 for? Are you missing a scale factor conversion?
There was a problem hiding this comment.
Yes this is one of those things I would look into deeper. But the (/2.0) is there because I was thinking the circle's radius would be half the box size. But I haven't thought too hard about this yet. I have also ignored the by_to parameter for all the cases right now just to get a compilable/runnable first draft.
I will checkout the svgArc and replace Arc with SvgArc. Hopefully that will simplify the code a lot and get rid of a bunch of convertions.
| ShapeBox::BorderBox => &cx.frame.border_box_path(), | ||
| ShapeBox::PaddingBox => &cx.frame.padding_box_path(), | ||
| ShapeBox::ContentBox => &cx.frame.content_box_path(), | ||
| ShapeBox::MarginBox => &cx.frame.border_box_path(), |
There was a problem hiding this comment.
This should be margin box. If you don't want to implement yet, then a TODO comment would be fine.
| ShapeBox::MarginBox => ( | ||
| cx.frame.border_box.width(), | ||
| cx.frame.border_box.height(), | ||
| ), |
There was a problem hiding this comment.
Same here, can we leave a comment that margin box is TODO (just makes it clear to anyone reading the code why this was set to border box)
| // Kurbo doesn't support rounded rect with elliptical radii, | ||
| // so we ignore the y axes right now. |
|
Hey thanks for taking the time to go over this. Ill work on the things you have mentioned here. |
|
Hey @nicoburns I have suddenly become aware of very serious health issues. These have been bothering me for a while but I have finally reached the breaking point it seems. I may not be able to complete this PR in any reasonable timeframe. I will work on it if my health permits but if you want to close this one or let someone else work on it that would be fine. Sorry, for the inconvenience caused here. I will make up for it if I get healthy again. Best regards |
2 participants
Issue
The code is completely work in progress and not intended to be considered for merging yet. Im opening this pull request so that a maintainer can check that I am not going off in a direction that will be unmergeable. Mainly, I want to show how much the code change would be. It felt like I might be writing too much for a first issue.
To run this just run the
outlineexample. It will show a screen with some clip paths: