Remove concept of Layers from Servo #13848

mrobinson · 2016-10-20T07:52:50Z

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #__ (github issue number if applicable).

There are tests for these changes OR
These changes do not require tests because this PR should not change behavior.

Layers were a feature of the legacy drawing path. If we re-add them at
some point, it probably makes more sense to make them a product of
display list inspection.

This change also remove a bunch of dead painting code.

This change is

highfive · 2016-10-20T07:52:54Z

Heads up! This PR modifies the following files:

@asajeffrey: components/constellation/constellation.rs
@KiChjang: components/script_layout_interface/wrapper_traits.rs, components/script_traits/script_msg.rs, components/script_traits/script_msg.rs, components/script_traits/lib.rs, components/script_traits/lib.rs, components/script/script_thread.rs, components/script_layout_interface/message.rs, components/script/dom/window.rs, components/script_layout_interface/rpc.rs
@fitzgen: components/script_layout_interface/wrapper_traits.rs, components/script_traits/script_msg.rs, components/script_traits/script_msg.rs, components/script_traits/lib.rs, components/script_traits/lib.rs, components/script/script_thread.rs, components/script_layout_interface/message.rs, components/script/dom/window.rs, components/script_layout_interface/rpc.rs
@emilio: components/layout/display_list_builder.rs, components/layout/fragment.rs, components/layout/webrender_helpers.rs, components/layout/block.rs, components/layout/query.rs, components/layout/context.rs, components/layout/flow.rs

highfive · 2016-10-20T07:52:55Z

Warning

These commits modify gfx, layout, and script code, but no tests are modified. Please consider adding a test!

glennw · 2016-10-20T10:31:48Z

@bors-servo try

bors-servo · 2016-10-20T10:31:51Z

⌛ Trying commit ded8b6b with merge f6bcace...


        Auto merge of #13848 - mrobinson:remove-layers, r=<try>

Remove concept of Layers from Servo  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [x] These changes do not require tests because this PR should not change behavior.  Layers were a feature of the legacy drawing path. If we re-add them at some point, it probably makes more sense to make them a product of display list inspection. This change also remove a bunch of dead painting code.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13848)

bors-servo · 2016-10-20T11:30:51Z

💔 Test failed - linux-rel-css

mrobinson · 2016-10-20T11:33:23Z

The failure seems to be #13847.

emilio

The layout changes look good to me, modulo these two nits.

I'll let @glennw sign-off it it he wants though :)

emilio · 2016-10-20T11:41:02Z

components/layout/flow.rs

@@ -635,9 +625,6 @@ bitflags! {
    pub flags FlowFlags: u32 {
        // text align flags
        #[doc = "Whether this flow must have its own layer. Even if this flag is not set, it might"]


You'll also need to remove this line :P

Thanks for pointing this out! I've fixed it in the latest version of the branch.

emilio · 2016-10-20T11:41:02Z

components/layout/fragment.rs

+
+        // TODO(mrobinson): Determine if this is necessary, since blocks with
+        // transformations already create stacking contexts.
+        if self.style().get_effects().perspective != LengthOrNone::None {


Yeah, I think it's not necessary.

Do you mind if I remove this in a followup change, since I want to minimize the risk of changing behavior in this PR?

emilio · 2016-10-20T12:20:00Z

On Thu, Oct 20, 2016 at 05:00:10AM -0700, Martin Robinson wrote:

Do you mind if I remove this in a followup change, since I want to minimize the risk of changing behavior in this PR?

Sure, that's fine for me :)

mrobinson · 2016-10-20T12:58:07Z

@bors-servo try

bors-servo · 2016-10-20T12:58:13Z

⌛ Trying commit 15faf7c with merge 64424bb...


        Auto merge of #13848 - mrobinson:remove-layers, r=<try>

Remove concept of Layers from Servo  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [x] These changes do not require tests because this PR should not change behavior.  Layers were a feature of the legacy drawing path. If we re-add them at some point, it probably makes more sense to make them a product of display list inspection. This change also remove a bunch of dead painting code.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13848)

bors-servo · 2016-10-20T14:09:01Z

☀️ Test successful - arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev

metajack · 2016-10-20T14:11:08Z

@mrobinson Does this remove the dependency on rust-layers as well?

mrobinson · 2016-10-20T14:19:32Z

@metajack If I'm not mistaken, that dependency has already been removed.

mrobinson · 2016-10-20T14:20:12Z

I pushed a new version of the branch that also removed my old scrolling documentation which is doubly out-of-date now.

glennw

One question, otherwise r=me

glennw · 2016-10-20T23:13:51Z

components/layout/webrender_helpers.rs

@@ -298,9 +298,7 @@ impl WebRenderStackingContextConverter for StackingContext {
                                mut scroll_policy: ScrollPolicy,


This variable is never read with this change, so can be removed if intentional.

Whoops. I'll remove this.


        Remove concept of Layers from Servo

Layers were a feature of the legacy drawing path. If we re-add them at some point, it probably makes more sense to make them a product of display list inspection. This change also remove a bunch of dead painting code.

mrobinson · 2016-10-21T06:43:23Z

@bors-servo r=glennw

bors-servo · 2016-10-21T06:43:24Z

📌 Commit ccb7ab9 has been approved by glennw

bors-servo · 2016-10-21T06:43:26Z

⌛ Testing commit ccb7ab9 with merge bb271ef...


        Auto merge of #13848 - mrobinson:remove-layers, r=glennw

Remove concept of Layers from Servo  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable).  - [ ] There are tests for these changes OR - [x] These changes do not require tests because this PR should not change behavior.  Layers were a feature of the legacy drawing path. If we re-add them at some point, it probably makes more sense to make them a product of display list inspection. This change also remove a bunch of dead painting code.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13848)

bors-servo · 2016-10-21T07:35:14Z

☀️ Test successful - arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev

highfive assigned glennw Oct 20, 2016

highfive added the S-awaiting-review label Oct 20, 2016

bors-servo mentioned this pull request Oct 20, 2016

Avoid some unnecessary clones when handling cookies. #13854

Merged

highfive added the S-tests-failed label Oct 20, 2016

emilio approved these changes Oct 20, 2016

View changes

mrobinson force-pushed the mrobinson:remove-layers branch from ded8b6b to 15faf7c Oct 20, 2016

highfive removed the S-tests-failed label Oct 20, 2016

mrobinson force-pushed the mrobinson:remove-layers branch from 15faf7c to fe5a8a1 Oct 20, 2016

glennw reviewed Oct 20, 2016

View changes

Remove concept of Layers from Servo

Loading status checks…

ccb7ab9

Layers were a feature of the legacy drawing path. If we re-add them at some point, it probably makes more sense to make them a product of display list inspection. This change also remove a bunch of dead painting code.

mrobinson force-pushed the mrobinson:remove-layers branch from fe5a8a1 to ccb7ab9 Oct 21, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Oct 21, 2016

bors-servo merged commit ccb7ab9 into servo:master Oct 21, 2016
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Oct 21, 2016

mrobinson deleted the mrobinson:remove-layers branch Oct 21, 2016

stshine mentioned this pull request Nov 8, 2016

layout: Mark flex items properly during construction #14130

Merged

3 of 5 tasks complete

servo / servo

Remove concept of Layers from Servo #13848

Remove concept of Layers from Servo #13848

mrobinson commented Oct 20, 2016 •

edited by larsbergstrom

highfive commented Oct 20, 2016

highfive commented Oct 20, 2016

glennw commented Oct 20, 2016

bors-servo commented Oct 20, 2016

bors-servo commented Oct 20, 2016

mrobinson commented Oct 20, 2016

emilio left a comment •

edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

emilio commented Oct 20, 2016

mrobinson commented Oct 20, 2016

bors-servo commented Oct 20, 2016

bors-servo commented Oct 20, 2016

metajack commented Oct 20, 2016

mrobinson commented Oct 20, 2016

mrobinson commented Oct 20, 2016

glennw left a comment

This comment has been minimized.

This comment has been minimized.

mrobinson commented Oct 21, 2016

bors-servo commented Oct 21, 2016

bors-servo commented Oct 21, 2016

bors-servo commented Oct 21, 2016

		@@ -298,9 +298,7 @@ impl WebRenderStackingContextConverter for StackingContext {
		mut scroll_policy: ScrollPolicy,

servo / servo

Join GitHub today

Remove concept of Layers from Servo #13848

Remove concept of Layers from Servo #13848

Conversation

mrobinson commented Oct 20, 2016 • edited by larsbergstrom

highfive commented Oct 20, 2016

highfive commented Oct 20, 2016

glennw commented Oct 20, 2016

bors-servo commented Oct 20, 2016

bors-servo commented Oct 20, 2016

mrobinson commented Oct 20, 2016

emilio left a comment • edited

This comment has been minimized.

emilio Oct 20, 2016

This comment has been minimized.

mrobinson Oct 20, 2016

This comment has been minimized.

emilio Oct 20, 2016

This comment has been minimized.

mrobinson Oct 20, 2016

emilio commented Oct 20, 2016

mrobinson commented Oct 20, 2016

bors-servo commented Oct 20, 2016

bors-servo commented Oct 20, 2016

metajack commented Oct 20, 2016

mrobinson commented Oct 20, 2016

mrobinson commented Oct 20, 2016

glennw left a comment

This comment has been minimized.

glennw Oct 20, 2016

This comment has been minimized.

mrobinson Oct 21, 2016 • edited

mrobinson commented Oct 21, 2016

bors-servo commented Oct 21, 2016

bors-servo commented Oct 21, 2016

bors-servo commented Oct 21, 2016

mrobinson commented Oct 20, 2016 •

edited by larsbergstrom

emilio left a comment •

edited

mrobinson Oct 21, 2016 •

edited