Add groove and ridge border support. #2465
Conversation
|
Critic review: https://critic.hoppipolla.co.uk/r/1597 This is an external review system which you may optionally use for the code review of your pull request. In order to help critic track your changes, please do not make in-place history rewrites (e.g. via |
| fn get_scaled_bounds(&self, | ||
| bounds : &Rect<Au>, | ||
| border : SideOffsets2D<f32>, | ||
| shrink_factor : f32) -> (Point2D<f32>, Point2D<f32>, Point2D<f32>, Point2D<f32>) { |
There was a problem hiding this comment.
nit: style in this file is to not line up the colons.
|
This is excellent, probably the cleanest implementation of border drawing I've ever seen in a browser engine. :) Is it possible to add a reftest, perhaps using an image as the reference? (See |
|
Thanks for the review Pat! |
Don't line up :'s in render_context.rs (style)
|
Added ref test and style changes. |
…port Significant cleanup of border support, adds inset & outset border code. Border rendering matches Chrome's border rendering.
|
Sry I must have been on something when adding the groove and ridge borders! The square & short end structures + code weren't needed. I've cleaned up the code + added inset/outset support @ bjwbell@2bb5c8b Include in this pull request or do a new one? |
Cleanup whitespace + move functions for easier reading.
|
Added double border support (with ref test) |
|
You can include it in this PR. |
| fn get_scaled_bounds(&self, | ||
| bounds: &Rect<Au>, | ||
| border: SideOffsets2D<f32>, | ||
| shrink_factor: f32) -> (Point2D<f32>, Point2D<f32>, Point2D<f32>, Point2D<f32>) { |
There was a problem hiding this comment.
Can you return a Rect<f32> here instead? At the least this 4-tuple should be turned into a struct so that the fields are self-documenting.
|
Looks pretty good modulo a few style nits, thanks! |
| border: SideOffsets2D<f32>, | ||
| color: Color, | ||
| fn draw_border_path(&self, | ||
| bounds: (Point2D<f32>, Point2D<f32>, Point2D<f32>, Point2D<f32>), |
There was a problem hiding this comment.
Could you make this a Rect or at least a struct? Four tuple elements aren't that self documenting.
There was a problem hiding this comment.
I'll make it a Rect, my excuse for not already doing it is lazyness.
|
Looks good modulo nits. 🤘 |
Also includes other minor code cleanup.
|
Added in the review comments. Hopefully no more nits ;) |
Matches Chrome's rendering behavior for groove and ridge borders (I can attach comparison images, if wanted).
Matches Chrome's rendering behavior for groove and ridge borders (I can attach comparison images, if wanted).