WR update #22973

paulrouget · 2019-03-05T16:10:25Z

@emilio can I ask you to look at the "WR udpate: layout" commit? There are a few changes I'm not sure about (should_inflate, the new filters_data and cache_tiles).

Fix: #22993

This change is

highfive · 2019-03-05T16:10:29Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/Cargo.toml, components/webdriver_server/Cargo.toml
@jgraham: components/webdriver_server/Cargo.toml
@edunham: servo-tidy.toml
@KiChjang: components/script/Cargo.toml, components/net_traits/Cargo.toml
@emilio: components/layout/Cargo.toml, components/style/Cargo.toml, components/layout/display_list/webrender_helpers.rs, components/layout/display_list/builder.rs

highfive · 2019-03-05T16:10:32Z

Warning

These commits modify unsafe code. Please review it carefully!

jdm · 2019-03-05T16:41:57Z

If we remove the use of the icon_loading feature (and replace it with code from https://github.com/tomaka/winit/pull/799/files) then can we skip the winit issue?

bors-servo · 2019-03-05T21:27:44Z

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

emilio · 2019-03-05T22:44:50Z

components/layout/display_list/builder.rs

@@ -2012,6 +2012,7 @@ impl Fragment {
                        offset: LayoutVector2D::new(shadow.horizontal.px(), shadow.vertical.px()),
                        color: self.style.resolve_color(shadow.color).to_layout(),
                        blur_radius: shadow.blur.px(),
+                        should_inflate: false,


This should be true (Gecko inflates shadow bounds already to account for the blurs, Servo does not).

emilio · 2019-03-05T22:44:50Z

components/layout/display_list/webrender_helpers.rs

@@ -233,7 +243,9 @@ impl WebRenderDisplayItemConverter for DisplayItem {
                    stacking_context.transform_style,
                    stacking_context.mix_blend_mode,
                    &stacking_context.filters,
+                    &[],


This seems fine, it's only used for component transfer iirc.

emilio · 2019-03-05T22:44:50Z

components/layout/display_list/webrender_helpers.rs

                    RasterSpace::Screen,
+                    /* cache_tiles = */ false,


You should probably set this to true only for the root stacking context. @gw3583 can probably confirm that.

I would probably just leave it as false unconditionally, for now. That will preserve existing behavior. Enabling picture caching could be done as a separate step / follow up.

paulrouget · 2019-03-06T13:34:00Z

@gw3583 or @emilio: can you look at the font API commit? Some changes were necessary for Windows.

paulrouget · 2019-03-06T13:39:14Z

@jdm:

If we remove the use of the icon_loading feature (and replace it with code from https://github.com/tomaka/winit/pull/799/files) then can we skip the winit issue?

Yes, this would have been easier. But at that point I've updated servo and skia to the new winit/glutin versions, so I'd rather go with it.

We can remove the dependency on the the icon_loading feature later.

paulrouget · 2019-03-07T14:21:25Z

The rand update is making things difficult on Android. I'm going to do what @jdm suggested instead.

paulrouget · 2019-03-07T16:02:01Z

r? @emilio or @gw3583

emilio

LGTM

emilio · 2019-03-08T10:29:37Z

components/gfx/platform/windows/font_template.rs

+            let face = font.create_font_face();
+            let files = face.get_files();
+            if files.len() >= 1 {
+                if let Some(path) = files[0].get_font_file_path() {


This could be written more clearly like:

if self.bytes.is_some() { return None; } let font = ... let face = ... let files = ... let path = files.iter().next()?.get_font_file_path()?; Some(NativeFontHandle { .. })

emilio · 2019-03-08T10:29:37Z

components/layout/display_list/webrender_helpers.rs

@@ -209,12 +211,20 @@ impl WebRenderDisplayItemConverter for DisplayItem {
                                stacking_context.perspective.is_some()
                        );


I'd remove the assertion since it's now a release assertion effectively right below.

atouchet · 2019-03-10T16:21:58Z

winit 0.19/glutin 0.20 are now out.

paulrouget · 2019-03-11T09:42:50Z

@bors-servo r=emilio

bors-servo · 2019-03-11T09:42:51Z

📌 Commit 9adaa15 has been approved by emilio


        Auto merge of #22973 - paulrouget:wrupdate, r=<try>

@emilio

WR update ~Need rust-windowing/winit#803 @emilio can I ask you to look at the *"WR udpate: layout"* commit? There are a few changes I'm not sure about (should_inflate, the new filters_data and cache_tiles). Fix: #22993  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22973)

bors-servo · 2019-03-22T11:21:55Z

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
State: approved= try=True

paulrouget · 2019-03-22T11:24:09Z

I'll go ahead and land this.

I had to mark 3 tests as FAIL, but the only one that's actually worrying is the mix-blend test on background behavior. Hope to fix that soon. See servo/webrender#3594

paulrouget · 2019-03-22T11:27:33Z

@bors-servo r=emilio

bors-servo · 2019-03-22T11:27:34Z

📌 Commit f1f7779 has been approved by emilio


        Auto merge of #22973 - paulrouget:wrupdate, r=emilio

@emilio

WR update ~Need rust-windowing/winit#803 @emilio can I ask you to look at the *"WR udpate: layout"* commit? There are a few changes I'm not sure about (should_inflate, the new filters_data and cache_tiles). Fix: #22993  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22973)

bors-servo · 2019-03-22T11:27:38Z

⌛ Testing commit f1f7779 with merge ef9a4a1...

bors-servo · 2019-03-22T12:46:25Z

💔 Test failed - mac-rel-wpt4

jdm · 2019-03-22T12:48:12Z

@bors-servo retry

bors-servo · 2019-03-22T12:48:18Z

⌛ Testing commit f1f7779 with merge 525484a...


        Auto merge of #22973 - paulrouget:wrupdate, r=emilio

@emilio

WR update ~Need rust-windowing/winit#803 @emilio can I ask you to look at the *"WR udpate: layout"* commit? There are a few changes I'm not sure about (should_inflate, the new filters_data and cache_tiles). Fix: #22993  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22973)

bors-servo · 2019-03-22T14:09:19Z

☀️ Test successful - android-mac, arm32, arm64, linux-rel-css, linux-rel-wpt, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, magicleap, status-taskcluster
Approved by: emilio
Pushing 525484a to master...


        Bug 1519718 - Revert the mix-blend rewrite r=jrmuizel

https://phabricator.services.mozilla.com/D20608 Caused problems in Servo servo/servo#22973 and MotionMark - https://bugzilla.mozilla.org/show_bug.cgi?id=1519718 Differential Revision: https://phabricator.services.mozilla.com/D23318 UltraBlame original commit: 84bc38dc1fdeb810790f1b7987df59377c027ae9


        Bug 1519718 - Revert the mix-blend rewrite r=jrmuizel

https://phabricator.services.mozilla.com/D20608 Caused problems in Servo servo/servo#22973 and MotionMark - https://bugzilla.mozilla.org/show_bug.cgi?id=1519718 Differential Revision: https://phabricator.services.mozilla.com/D23318 UltraBlame original commit: 84bc38dc1fdeb810790f1b7987df59377c027ae9


        Bug 1519718 - Revert the mix-blend rewrite r=jrmuizel

https://phabricator.services.mozilla.com/D20608 Caused problems in Servo servo/servo#22973 and MotionMark - https://bugzilla.mozilla.org/show_bug.cgi?id=1519718 Differential Revision: https://phabricator.services.mozilla.com/D23318 UltraBlame original commit: 84bc38dc1fdeb810790f1b7987df59377c027ae9

highfive assigned nox Mar 5, 2019

highfive added the S-awaiting-review label Mar 5, 2019

highfive added the S-needs-rebase label Mar 5, 2019

emilio reviewed Mar 5, 2019

View changes

paulrouget force-pushed the paulrouget:wrupdate branch 2 times, most recently from 9cf87d3 to a40570c Mar 6, 2019

paulrouget mentioned this pull request Mar 7, 2019

Do not use Glutin/Winit's icon_loading feature #22993

Closed

paulrouget force-pushed the paulrouget:wrupdate branch from a40570c to b7b5c2a Mar 7, 2019

paulrouget force-pushed the paulrouget:wrupdate branch 3 times, most recently from 1a7a15f to 721e68a Mar 7, 2019

highfive assigned emilio and unassigned nox Mar 7, 2019

emilio approved these changes Mar 8, 2019

View changes

paulrouget force-pushed the paulrouget:wrupdate branch 2 times, most recently from 1e05b4a to 9adaa15 Mar 11, 2019

highfive added S-awaiting-merge and removed S-awaiting-review S-needs-rebase labels Mar 11, 2019

highfive added S-awaiting-merge and removed S-awaiting-review S-needs-rebase S-tests-failed labels Mar 22, 2019

highfive added S-tests-failed and removed S-awaiting-merge labels Mar 22, 2019

highfive added S-awaiting-merge and removed S-tests-failed labels Mar 22, 2019

highfive removed the S-awaiting-merge label Mar 22, 2019

This was referenced Mar 22, 2019

Shutdown player when HTMLMediaElement is dropped #22944

Merged

Add various state tracking to RTCPeerConnection, add .close() #22974

Merged

WIP initial reshuffling of opts parsing API for #23009 #23061

Closed

atouchet mentioned this pull request Mar 22, 2019

Use a streaming base64 encoder in canvas.toDataURL #22147

Closed

paulrouget mentioned this pull request Mar 25, 2019

Fix bleeding in FxR by adding Padding support to ServoSurface #23002

Merged

servo / servo

WR update #22973

WR update #22973

paulrouget commented Mar 5, 2019 •

edited

highfive commented Mar 5, 2019

highfive commented Mar 5, 2019

jdm commented Mar 5, 2019

bors-servo commented Mar 5, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

paulrouget commented Mar 6, 2019

paulrouget commented Mar 6, 2019

paulrouget commented Mar 7, 2019 •

edited

paulrouget commented Mar 7, 2019

emilio left a comment

This comment has been minimized.

This comment has been minimized.

atouchet commented Mar 10, 2019

paulrouget commented Mar 11, 2019

bors-servo commented Mar 11, 2019

bors-servo commented Mar 22, 2019

paulrouget commented Mar 22, 2019

paulrouget commented Mar 22, 2019

bors-servo commented Mar 22, 2019

bors-servo commented Mar 22, 2019

bors-servo commented Mar 22, 2019

jdm commented Mar 22, 2019

bors-servo commented Mar 22, 2019

bors-servo commented Mar 22, 2019

		@@ -209,12 +211,20 @@ impl WebRenderDisplayItemConverter for DisplayItem {
		stacking_context.perspective.is_some()
		);

servo / servo

Join GitHub today

WR update #22973

WR update #22973

Conversation

paulrouget commented Mar 5, 2019 • edited

highfive commented Mar 5, 2019

highfive commented Mar 5, 2019

jdm commented Mar 5, 2019

bors-servo commented Mar 5, 2019

This comment has been minimized.

emilio Mar 5, 2019

This comment has been minimized.

emilio Mar 5, 2019

This comment has been minimized.

emilio Mar 5, 2019

This comment has been minimized.

gw3583 Mar 5, 2019

paulrouget commented Mar 6, 2019

paulrouget commented Mar 6, 2019

paulrouget commented Mar 7, 2019 • edited

paulrouget commented Mar 7, 2019

emilio left a comment

This comment has been minimized.

emilio Mar 8, 2019

This comment has been minimized.

emilio Mar 8, 2019

atouchet commented Mar 10, 2019

paulrouget commented Mar 11, 2019

bors-servo commented Mar 11, 2019

bors-servo commented Mar 22, 2019

paulrouget commented Mar 22, 2019

paulrouget commented Mar 22, 2019

bors-servo commented Mar 22, 2019

bors-servo commented Mar 22, 2019

bors-servo commented Mar 22, 2019

jdm commented Mar 22, 2019

bors-servo commented Mar 22, 2019

bors-servo commented Mar 22, 2019

paulrouget commented Mar 5, 2019 •

edited

paulrouget commented Mar 7, 2019 •

edited