1527320 - Use Moz2D point/rect types instead of webrender's in Gecko code

Status: NEW

(Core :: Graphics: WebRender, enhancement, P3)

Description

[Nicolas Silva [:nical]]

Filing this for later. I want to tidy up Gecko's webrender API wrappers so that they take Moz2D math types (for example gfx::LayoutDeviceRect) instead of the ones generated in the bindings (gfx::wr::LayoutRect). The only place where we would interact with the webrender math types would be (give or take) WebRenderBindings.cpp near the ffi boundary.

All point and rect kinds have two types in C++ representing exactly the same things and having to juggle between the two in layout code does not spark joy.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

I've generally been pushing in the opposite direction. i.e. directly exposing the generated types in WebRenderAPI. e.g. bug 1509182. Pushing the gecko types down into WebRenderAPI will make it have a mix of Gecko type and generated types. How bad this is will probably depend on the actual details.

I've also wanted to move in the direction of having more of WebRenderAPI be generated instead of having to manually write wrappers around the rust functions.

Comment 2

[Nicolas Silva [:nical]]

How bad this is will probably depend on the actual details.

I'm curious about what downsides of using Moz2D types for points and rects you have in mind.

I don't mind using both wr::ClipId and gfx::LayoutIntRect in the same APIs but having two (assignment-) incompatible types that represent a rectangle in layout space is, in my opinion, not great (it's gfxPoint vs gfx::Point all over again with wr::ToLayoutRect spread all over the DL building code, etc.).

Moz2D's point types are proper math types with methods and are used everywhere, there's no way we'll use the generated wr math types in all of our layout code for example.

If what you are after is a clean dependency graph, we could extract Moz2D's math types. The WebRender wrappers already depend on ipdl, though.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

(In reply to Nicolas Silva [:nical] from comment #2)

How bad this is will probably depend on the actual details.

I'm curious about what downsides of using Moz2D types for points and rects you have in mind.

I don't have anything in mind, other than if a method takes a list of points or rects than it's usually better for the caller to do conversion instead of the callee.

I don't mind using both wr::ClipId and gfx::LayoutIntRect in the same APIs but having two (assignment-) incompatible types that represent a rectangle in layout space is, in my opinion, not great (it's gfxPoint vs gfx::Point all over again with wr::ToLayoutRect spread all over the DL building code, etc.).

Moz2D's point types are proper math types with methods and are used everywhere, there's no way we'll use the generated wr math types in all of our layout code for example.

I agree and I agree with sentiment that we should be doing the conversion in as few places as possible. I just want to make sure we keep our wrappers as thin as possible so we don't accidentally do extra work in them.

If what you are after is a clean dependency graph, we could extract Moz2D's math types. The WebRender wrappers already depend on ipdl, though.

This wasn't something I was worried about.