Use strongly typed geometry - Part 1

[nical]

Part one of a series of pull requests to convert untyped geometry into strongly typed geometry.
This PR adds LayerPixel, ParentLayerPixel and WorldPixel, as well as the corresponging Rect, Point and Size structures, and converts good chunk of the Rect that I could identify as LayerRect.

In order to avoid churn in the public API, I postponed converting most of the webrender_trait types, but this will be done as soon as the webrender internals are properly typed. As a result of the step-by-step approach, this PR adds some unpleasant LayerFoo::from_untyped and foo.to_untyped at places, but the end goal is to have almost none of them in the end. I hope to land things progressively, though, because this type of changeset is time-consuming to rebase.

The added units are inspired from gecko's units (Layer/ParentLayer) and some of the webrneder terminology (World). Don't hesitate to propose other names.

This needs a careful review, in particular for LayerPixel vs ParentLayerPixel.

r? @glennw @kvark @staktrace

---

This change is 

[kvark]

Looks great!
I'd only wish the device_pixel() func to be renamed.

[kvark]

these could probably be LayerLength

[kvark]

the name is confusing, should probably be called device_length

[kvark]

hmm, it's strange that you need from_untyped here, given that all the inputs are already typed by U

[nical]

rect_from_points_f takes and returns untyped stuff (at the moment that is. I had to draw a line and submit a PR or I'd end up converting the entire code base).

[kvark]

where are these used at the moment?

[kvark]

approving on the promise that the device_pixel will be renamed in the follow up

[nical]

I'm going to call it a day. I think that this is in a landable state modulo future review comments.

There are still a few places with untyped geometry, and also the public api, which should be converted in one go whenever we feel good about requiring a patch to servo.
With that, most of the remaining rough edges should go away, and there are a few other things that would look cleaner once added directly to euclid (for example TypedMatrix4D::from_untyped and to_untyped similar to what the other types have would have been helpful, and there's a whole bunch of code in util.rs that is waiting to be moved over as well).

I sincerely apologize for this antisocial changeset (for the reviewers and the ones who will need to rebase against it). I do feel pretty good about how it improves the clarity and robustness of the code if that's any consolation.

[bors-servo]

☔ The latest upstream changes (presumably #569) made this pull request unmergeable. Please resolve the merge conflicts.

[glennw]

@nical This generally looks good to me, thanks for doing it!

My only real concern is how we refer to LayerToParent transforms.

The code says things like ParentLayerPoint are in a stacking context's parent coordinate space, which makes sense. But then the LayerToParent terminology suggests that the stacking context transforms are relative to their parent stacking context, which isn't the case (they are calculated relative to the scroll layer world transform) internally.

Perhaps we could have LayerToParent transform type for the (eventual) public interface change (where the transforms are relative to the parent stacking context), and LayerToScrollLayer for the internal storage of the transforms after flatten?

[nical]

Let's see if I understand correctly (this parent coordinates thing was probably the most challenging part to begin with):

• Every stacking context gets its own layer, so when so we can use the terms stacking contexts and layers interchangeably when it comes to coordinate systems.
• There are scrollable layers and fixed layers.
• The parent of a layer actually refers to its parent scrollable layer (stacking context A could have a fixed parent stacking context B which has a scrollable stacking context C, and A.local_transform refers to the transform from A to C).

Took me a while to write the above because my understanding completely changed in the process of writing up what I previously thought and checking the code (which is good!).

So is it ok to rename all ParentLayerSize/Point/Rect into ScrollLayerSize/Point/Rect? Are actual Parent(StackingContext)Size/Point/Rect all in the public API and directly translated to layers coordinates, or are there currently some points in frame.rs or tiling.rs that are actually in parent stacking context coordinates (but currently written as ParentLayerFoo in my PR)? I think not but I'd rather be sure.

Also, sorry I had to squash the commits together in order to rebase them (which turned out to be a fair bit more challenging than I expected).

[nical]

My latest commit renames all instances of ParentLayerPixel into ScrollLayerPixel. I hope that it's what you meant, otherwise I'll adjust.
I am still a bit confused by the existence of Layer objects that may not be scrollable while they are referred to using a ScrollLayerId (that doesn't sound like much but that's the type of stuff I hang on to when trying to read the code, in addition to the comment for Layer::local_transform that did not mention parent vs parent-scrollable, so I am not sure I got it right).

[emilio]

@nical: Note that you need to update wr_replay so this can merge, otherwise the build will fail :)

[glennw]

@nical First, let me apologize profusely because there's a horrible piece of naming confusion in the current code, which needs to be fixed - there's two meanings of "layer" in the current code base. This is probably the source of most of your confusion. Let me try and explain the differences and how they fit together.

First, we have the concept of a stacking context - this of course contains display items, a transform, filters and various other parameters.

There is also the concept of a "scroll root" - these are locations in the display list where there is a scrollable region. These are called "scroll layers" and are what you find in layer.rs. There is typically one of these for the main page, one for each iframe, and one for each overflow: scroll region - there are typically many more stacking contexts than scroll layers.

Sometimes, a stacking context gets split into multiple parts - for example, if a stacking context has items that are part of more than one scroll root. This is where the confusion occurs - in the frame builder interface, these parts of stacking contexts are referred to as a "layer" - for example, the naming of "layer_store" and push_layer in the FrameBuilder struct. These are unrelated to the concept of a scroll layer in layer.rs, and should be renamed to something else.

So for now, let's refer to scroll layers (as per layer.rs) and stacking context layers (as in push_layer). Each of the stacking context layers have their display item coordinates in the local coordinate space of their parent stacking context layer. (This is what Servo provides, and it makes sense, it's the simplest way for the client code to reason about stacking context spaces). As you would expect, the client code also provides the transform matrix for each stacking context in the local coordinate space of the parent stacking context. A concise way of saying that is: the client code never concerns itself with scroll layers - everything is provided in the local coordinate space of the parent stacking context.

Internally, however, things change. As we flatten the display list, the transform matrix for each stacking context layer is (a) set to identity when we create a scroll layer and (b) concatenated with the parent stacking context otherwise. This has the effect that the stored transform for each stacking context layer is relative to the transform of the nearest scroll layer that this stacking context layer is part of.

The update process when scrolling then looks something like this:

1. Recurse through the scroll layer tree, concatenating the scroll layer transforms, including scroll offsets - this gets a complete world transform for each scroll layer.
2. Calculate each final stacking context layer transform by multiplying its scroll-layer-local-transform by the world transform of the scroll layer it is part of - these get sent to the GPU to transform display items with.

I think I've described that all correctly, although @mrobinson should check over this to make sure it matches his understanding. Again, I apologize for both the naming confusion and the wall of text. Feel free to ping me during the week to discuss on Vidyo or IRC!

[nical]

Thanks for the explanations! If I understand correctly, the current state of this pull request is correct, then. We don't actually store layer-to-parent transforms in the internal structures, since we concatenate them into layer-to-scroll-root transforms when we create the internal StackingContext and Layer structures as we do the flattening.
We should improve some of the naming here (Layer should probably be ScrollLayer, and some of the variable names could use stacking_context instead of layer), but I'll come back to that in a followup, this PR being already way too big for its own sake.

[mrobinson]

So I think the only thing that I would clarify is that scrolling areas / scrolling divs are not stacking contexts, since they aren't in CSS either. That mainly matters when we talk about the offset of the items inside the scrolling area/div. Their coordinates are relative to the origin of the parent stacking context, which may be a parent of the scrollable div or the scrollable div itself.

It may be worth changing this behavior, so that items inside of scrollable divs are relative to their parent scroll roots, but that has the potential to make Servo quite a bit more complicated.

[glennw]

Reviewed 8 of 15 files at r1, 4 of 7 files at r2, 3 of 3 files at r3, 12 of 12 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[glennw]

@bors-servo r+

[bors-servo]

📌 Commit c1618f7 has been approved by glennw

[bors-servo]

⌛ Testing commit c1618f7 with merge 4db55ba...

[bors-servo]

☀️ Test successful - status-travis