WR update: preserve-3d support #16775

kvark · 2017-05-08T21:10:39Z

This is WR update to servo/webrender@d335555 having new features:

limited "preserve-3d" support (servo/webrender#1169, servo/webrender#1208)
rayon thread pool (servo/webrender#1202)
further border rendering improvements

Edit: the references to bincode serialization and border styles are removed from here, since they are already integrated into Servo.
Edit2: this is alternative/similar to #16801, based on @mrobinson code (see first commit).

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

Related to #9087
Note that I'm unlocking a few tests as well as changing some related to preserve-3d. The changes come from common sense and comparison to Chromium. I'm ready to discuss them on a individual basis.

There are tests for these changes OR
These changes do not require tests because _____

There is still an investigation to do with regards to the differences of preserve3d logic between Blink and Gecko.

This change is

highfive · 2017-05-08T21:10:45Z

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @wafflespeanut (or someone else) soon.

highfive · 2017-05-08T21:10:46Z

Heads up! This PR modifies the following files:

@bholley: components/style/properties/properties.mako.rs
@emilio: components/layout/display_list_builder.rs, components/layout/webrender_helpers.rs, components/style/properties/properties.mako.rs, components/layout/fragment.rs

highfive · 2017-05-08T21:10:47Z

Warning

This pull request modifies the contents of
tests/wpt/css-tests/, which are overwriten occasionally whenever the
directory is synced from upstream.

emilio · 2017-05-08T21:14:02Z

components/layout/display_list_builder.rs

@@ -2528,14 +2539,15 @@ impl InlineFlowDisplayListBuilding for InlineFlow {
                    fragment.stacking_context_id = fragment.stacking_context_id();

                    let current_stacking_context_id = state.current_stacking_context_id;
-                    let current_scroll_root_id = state.current_scroll_root_id;
+                    let stacking_context = fragment.create_stacking_context(
+                       fragment.stacking_context_id,


nit: indentation seems weird here.

emilio · 2017-05-08T21:14:02Z

components/style/properties/properties.mako.rs

-           !effects.clip.is_auto() {
-           effects.mix_blend_mode != mix_blend_mode::T::normal ||
+           !effects.clip.is_auto() ||
+           effects.mix_blend_mode != mix_blend_mode::T::normal {
            return transform_style::T::flat;


Oh, did this happen to do the right thing?

Well, no, i guess it didn't :)

Yeah, the old wording was really confusing. I think the old logic could be rewritten as

if effects.opacity < 1.0 || !effects.filter.is_empty() || !effects.clip.is_auto() { if effects.mix_blend_mode == mix_blend_mode::T::normal { return transform_style::T::flat; } }

My assumption was that it's just a refactoring bug, where someone mixed the || with { at the end of the lines.

emilio · 2017-05-08T21:14:02Z

tests/wpt/css-tests/css-transforms-1_dev/html/reference/transform3d-sorting-006-ref.htm

    <div style="height: 100px; width: 50px; background: red;
+      transform: translateY(-100px) rotateX(45deg) rotateY(45deg);
+      transform-origin: right"></div>


I'm not suer if we can upstream this changes right now. @jdm?

Actually, given these tests are in wpt, I don't think we should change them at all. I guess these pass on both Blink and Gecko?

I think I'll revert them and keep disabled then

If we have tests that differ between gecko/chromium, or fail in WR due to incomplete features - we can disable them in the test metadata, for now.

Yeah, these change will need manual upstreaming.

glennw · 2017-05-08T21:17:05Z

components/servo/lib.rs

@@ -182,6 +182,7 @@ impl<Window> Browser<Window> where Window: WindowMethods + 'static {
                enable_aa: opts.enable_text_antialiasing,
                enable_profiler: opts.webrender_stats,
                debug: opts.webrender_debug,
+                enable_batcher: false, //TEMP


We'll want to fix this up before any merging!

oops :) nicely spotted!


        WR Update

kvark · 2017-05-13T02:25:24Z

@emilio thanks for taking a look! Everything should be addressed now.

emilio

r=me with that.

Thanks!

emilio · 2017-05-14T01:05:41Z

tests/wpt/metadata-css/css-transforms-1_dev/html/transform3d-sorting-004.htm.ini

@@ -0,0 +1,3 @@
+[transform3d-sorting-004.htm]
+  type: reftest
+  disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1362543


Do we really need to disable these?

We can just mark them as failing with a link to that bug.

I tried multiple things, didn't find how to add comments to those ini files


        Using TransformStyle::Preserve3D

kvark · 2017-05-14T01:56:04Z

@emilio thanks!
@bors-servo r=emilio

bors-servo · 2017-05-14T01:56:04Z

📌 Commit 3a511ee has been approved by emilio

bors-servo · 2017-05-14T02:53:08Z

⌛ Testing commit 3a511ee with merge 06c5424...


        Auto merge of #16775 - kvark:preserve3d, r=emilio

@mrobinson

WR update: preserve-3d support  This is WR update to servo/webrender@d335555 having new features: - limited "preserve-3d" support (servo/webrender#1169, servo/webrender#1208) - rayon thread pool (servo/webrender#1202) - further border rendering improvements Edit: the references to bincode serialization and border styles are removed from here, since they are already integrated into Servo. Edit2: this is alternative/similar to #16801, based on @mrobinson code (see first commit). ---  - [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). Related to #9087 Note that I'm unlocking a few tests as well as changing some related to `preserve-3d`. The changes come from common sense and comparison to Chromium. I'm ready to discuss them on a individual basis.  - [x] There are tests for these changes OR - [ ] These changes do not require tests because _____ There is still an investigation to do with regards to the differences of preserve3d logic between Blink and Gecko.    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16775)

bors-servo · 2017-05-14T03:34:08Z

💔 Test failed - linux-rel-css

emilio · 2017-05-14T14:37:05Z

  ▶ PASS [expected FAIL] /compositing-1_dev/html/mix-blend-mode-intermediate-element-overflow-hidden-and-border-radius.htm

  ▶ PASS [expected FAIL] /css-backgrounds-3_dev/html4/border-bottom-right-radius-010.htm

  ▶ PASS [expected FAIL] /css-backgrounds-3_dev/html4/border-bottom-left-radius-010.htm

  ▶ PASS [expected FAIL] /css-backgrounds-3_dev/html4/border-top-left-radius-010.htm

  ▶ PASS [expected FAIL] /css-backgrounds-3_dev/html4/border-top-right-radius-010.htm

  ▶ PASS [expected FAIL] /css-transforms-1_dev/html/transform-3d-rotateY-stair-below-001.htm

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/transform-3d-rotateY-stair-above-001.htm
  └   → /css-transforms-1_dev/html/transform-3d-rotateY-stair-above-001.htm f5edea3f9bcf9f3852f2959341726fa2da8225e0
/css-transforms-1_dev/html/reference/transform-3d-rotateY-stair-above-ref-001.htm 22397e5af63eece9a8c29e2619e2ec9e356aa97e
Testing f5edea3f9bcf9f3852f2959341726fa2da8225e0 == 22397e5af63eece9a8c29e2619e2ec9e356aa97e

glennw · 2017-05-14T22:21:03Z

Superseded by #16860

highfive assigned wafflespeanut May 8, 2017

highfive added the S-awaiting-review label May 8, 2017

emilio reviewed May 8, 2017

View changes

kvark changed the title ~~WR update: preserve-3d, new borders, bincode serialization~~ WR update: preserve-3d support May 8, 2017

glennw reviewed May 8, 2017

View changes

kvark force-pushed the kvark:preserve3d branch from b45faee to d26f124 May 8, 2017

WR Update

6cd6da5

kvark force-pushed the kvark:preserve3d branch 4 times, most recently from 57f8581 to af21ea2 May 11, 2017

emilio approved these changes May 14, 2017

View changes

Using TransformStyle::Preserve3D

Loading status checks…

3a511ee

kvark force-pushed the kvark:preserve3d branch from af21ea2 to 3a511ee May 14, 2017

highfive assigned emilio and unassigned wafflespeanut May 14, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels May 14, 2017

highfive added S-tests-failed and removed S-awaiting-merge labels May 14, 2017

glennw closed this May 14, 2017

This was referenced May 14, 2017

Update WR (ellipse border-radius, preserve-3d, rayon thread pool). #16801

Closed

Update WR (preserve-3d, AA improvements) #16860

Merged

servo / servo

WR update: preserve-3d support #16775

WR update: preserve-3d support #16775

kvark commented May 8, 2017 •

edited

highfive commented May 8, 2017

highfive commented May 8, 2017

highfive commented May 8, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

kvark commented May 13, 2017

emilio left a comment

This comment has been minimized.

This comment has been minimized.

kvark commented May 14, 2017

bors-servo commented May 14, 2017

bors-servo commented May 14, 2017

bors-servo commented May 14, 2017

emilio commented May 14, 2017

glennw commented May 14, 2017 •

edited

servo / servo

Join GitHub today

WR update: preserve-3d support #16775

WR update: preserve-3d support #16775

Conversation

kvark commented May 8, 2017 • edited

highfive commented May 8, 2017

highfive commented May 8, 2017

highfive commented May 8, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

kvark commented May 13, 2017

emilio left a comment

This comment has been minimized.

This comment has been minimized.

kvark commented May 14, 2017

bors-servo commented May 14, 2017

bors-servo commented May 14, 2017

bors-servo commented May 14, 2017

emilio commented May 14, 2017

glennw commented May 14, 2017 • edited

kvark commented May 8, 2017 •

edited

glennw commented May 14, 2017 •

edited