Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign up[WIP] Remove explicit clip ID from stacking context #3311
Conversation
|
Thinking about this more and looking at the failing reftests, this isn't going to work. With the current implementation we need a separate clip ID inside the SC: the client code typically pushes a reference frame on the C/S stack, then pushes a stacking context, and it expects all the inner items to still be clipped by the clip associated with the scroll ID of the reference frame (yeah... see #3251 (comment)). So we currently have separate clipping semantics for SC inner/outer context, and we shouldn't be hacking the C/S stack semantics to avoid that separate field (instead, we should just kill C/S stack entirely, which is done in #3251). |
Don't ignore the clip on the stack for reference frames and clips Similar to #3311, this is a prelude to #3251, with the main purpose of getting rid of Gecko's hack in https://searchfox.org/mozilla-central/rev/d850d799a0009f851b5535580e0a8b4bb2c591d7/gfx/layers/wr/ClipManager.cpp#224 It also makes sense for WR in isolation (for the current clip/stack model, at least): in those spots addressed, we used to ignore the clip ID on the stack, only to re-interpret (smuggle!) the scroll ID as a clip. Now we are actually using the clip ID more consistently. See also - #3251 (comment) Gecko try (that disables the hack in addition to having the WR change): https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=212038686&revision=c10e5d839e024ba451a6622fa611c3525457f3ff (Looks green!🎉 ) <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/3315) <!-- Reviewable:end -->
kvark commentedNov 14, 2018
•
edited by larsbergstrom
This is one of the preparatory changes towards #3251
With this in, we should be able to remove this (hacky) code path in Gecko:
https://searchfox.org/mozilla-central/rev/d850d799a0009f851b5535580e0a8b4bb2c591d7/gfx/layers/wr/ClipManager.cpp#224
Currently, we provide a clip ID explicitly when pushing a stacking context. This change makes us use the clip ID on the clip/scroll stack instead, like with every other item. The implementation comes with a few quirks, namely the introduction of "push_clip_id_override" and "push_scroll_id_override" into the API. These aren't conflicting with #3251 and will be removed by it (together with the whole clip/scroll stack).
WIP because: still working on the Gecko side
This change is