Webrender: Remove BorderWidths in favor of SideOffsets2D

Created on 13 Sep 2018  路  6Comments  路  Source: servo/webrender

For both borders, margins (and others) information (usually lengths) needs to be stored for the four sides of a rect. This is done with the SideOffsets2D type in servo-layout. But in Webrender the BorderWidths struct is used for borders instead. This is a bit annoying as the field order is different left/top/right/bottom vs top/right/bottom/left, BorderWidths lacks convenience methods to do calculations or test stuff (border_widths.is_zero()?). Additionally servo-layout needs to convert SideOffsets2D into two different types with f32 as the value.

I understand that offset does not immediately convey width but I think the other benefits outweigh the more general name.

question

Most helpful comment

Yeah side offsets is used for a lot of stuff. In the CSS box model border, padding and margin are expressed as offsets. In addition even border color and border style are stored in an offset type. border-image-slice and border-image-outset (should be removed soon from WR) are processed as offsets and are sent in this way to webrender.

If we pick one here (and port all the missing bits and helpers from the other), what would be the best shared ancestor to expose it: euclid, webrender_api, or something else?

It would be best if SideOffsets2D is exposed by euclid as it currently is. Maybe webrender_api wants to reexport a LayoutOffsets alias for the type SideOffsets2D<f32, LayoutPixel> like it is done for other euclid types used in the API. This type would be used for the border widths.

All 6 comments

@pyfisch I agree that BorderWidths is cleaner, although SideOffsets2D is more universal/generic. Is it used for lots of other stuff in Servo? If we pick one here (and port all the missing bits and helpers from the other), what would be the best shared ancestor to expose it: euclid, webrender_api, or something else?

Yeah side offsets is used for a lot of stuff. In the CSS box model border, padding and margin are expressed as offsets. In addition even border color and border style are stored in an offset type. border-image-slice and border-image-outset (should be removed soon from WR) are processed as offsets and are sent in this way to webrender.

If we pick one here (and port all the missing bits and helpers from the other), what would be the best shared ancestor to expose it: euclid, webrender_api, or something else?

It would be best if SideOffsets2D is exposed by euclid as it currently is. Maybe webrender_api wants to reexport a LayoutOffsets alias for the type SideOffsets2D<f32, LayoutPixel> like it is done for other euclid types used in the API. This type would be used for the border widths.

Sounds feasible to me, calling @nical for the euclid part.

Sounds good to me. It looks like TypedSideOffsefs2D<T, Unit> already exists so all the parts should be already there on euclid's side, or did I miss something?

all the parts should be already there on euclid's side

Everything is there on euclid's side.

In theory NormalBorder could also be replaced by SideOffsets2D with BorderDetails::Normal having an additional field for the radii. But I think that this would be generalization for the sake of generalization.

Was this page helpful?
0 / 5 - 0 ratings