New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Webrender #21725

Merged
merged 4 commits into from Oct 12, 2018

Conversation

Projects
None yet
9 participants
@pyfisch
Contributor

pyfisch commented Sep 15, 2018

New version is
9156a4465f6ad715a0206cdd9a7e9a6f0385fbd6


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because just updating WR

This change is Reviewable

@highfive

This comment has been minimized.

highfive commented Sep 15, 2018

Heads up! This PR modifies the following files:

  • @emilio: components/layout/display_list/webrender_helpers.rs, components/layout/display_list/builder.rs
@highfive

This comment has been minimized.

highfive commented Sep 15, 2018

warning Warning warning

  • These commits modify layout code, but no tests are modified. Please consider adding a test!
@pyfisch

This comment has been minimized.

Contributor

pyfisch commented Sep 15, 2018

@bors-servo try=wpt

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 15, 2018

⌛️ Trying commit 27f89c1 with merge a734a7a...

bors-servo added a commit that referenced this pull request Sep 15, 2018

Auto merge of #21725 - pyfisch:update-wr, r=<try>
Update Webrender

New version is
9156a4465f6ad715a0206cdd9a7e9a6f0385fbd6

---

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes do not require tests because just updating WR

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21725)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 15, 2018

💔 Test failed - linux-rel-css

@pyfisch

This comment has been minimized.

Contributor

pyfisch commented Sep 15, 2018

wpt:

{"status": "FAIL", "group": "default", "message": "/css/CSS2/floats-clear/clear-on-parent-and-child.html b303f7b1e7a369ab78edcb4d8ed954c77e39e37a\n/css/reference/ref-filled-green-200px-square.html 180f8d33743fd8d6be409074c105cdd53ad49312\nTesting b303f7b1e7a369ab78edcb4d8ed954c77e39e37a == 180f8d33743fd8d6be409074c105cdd53ad49312", "stack": null, "subtest": null, "test": "/css/CSS2/floats-clear/clear-on-parent-and-child.html", "line": 3584, "action": "test_result", "expected": "PASS"}
{"status": "FAIL", "group": "default", "message": "/css/css-backgrounds/scroll-positioned-multiple-background-images.html fb94a58677dd69d36c745d5f12071de87f1279c6\n/css/css-backgrounds/reference/60x60-green-background.html 7800904ac377ba85eb0edc9a895a0ddd214ec001\nTesting fb94a58677dd69d36c745d5f12071de87f1279c6 == 7800904ac377ba85eb0edc9a895a0ddd214ec001", "stack": null, "subtest": null, "test": "/css/css-backgrounds/scroll-positioned-multiple-background-images.html", "line": 23441, "action": "test_result", "expected": "PASS"}
{"status": "FAIL", "group": "default", "message": "/css/css-transforms/matrix/svg-matrix-061.html 4e6ec2a4e7504e5c4bb739eb4fad793251d3180f\n/css/css-transforms/matrix/reference/svg-matrix-four-color-ref.html 91434ac6ea51711fb229c7a0a460632227c659c1\nTesting 4e6ec2a4e7504e5c4bb739eb4fad793251d3180f == 91434ac6ea51711fb229c7a0a460632227c659c1", "stack": null, "subtest": null, "test": "/css/css-transforms/matrix/svg-matrix-061.html", "line": 30853, "action": "test_result", "expected": "PASS"}
{"status": "FAIL", "group": "default", "message": "/css/css-transforms/transform-table-006.html c436b96bd5d8760d073b745c257968843c2f3c22\n/css/css-transforms/transform-blank-ref.html b81be0af214ed7113846c7cd94d9d9d57fe9efe8\nTesting c436b96bd5d8760d073b745c257968843c2f3c22 == b81be0af214ed7113846c7cd94d9d9d57fe9efe8", "stack": null, "subtest": null, "test": "/css/css-transforms/transform-table-006.html", "line": 30993, "action": "test_result", "expected": "PASS"}
{"status": "PASS", "group": "default", "message": null, "stack": null, "subtest": null, "test": "/_mozilla/css/border_black_ridge_b.html", "line": 33997, "action": "test_result", "expected": "FAIL"}
{"status": "PASS", "group": "default", "message": null, "stack": null, "subtest": null, "test": "/_mozilla/css/border_black_ridge_a.html", "line": 48498, "action": "test_result", "expected": "FAIL"}
{"status": "FAIL", "group": "default", "message": "assert_equals: Should hit the scrollbox contents expected \"DIV\" but got \"HTML\"", "stack": "@http://web-platform.test:8000/css/cssom-view/elementFromPoint-dynamic-anon-box.html:39:3\nTest.prototype.step@http://web-platform.test:8000/resources/testharness.js:1558:20\ntest@http://web-platform.test:8000/resources/testharness.js:544:9\n@http://web-platform.test:8000/css/cssom-view/elementFromPoint-dynamic-anon-box.html:38:1\n", "subtest": "Link should be clickable after hiding a scrollbox with an anonymous table inside", "test": "/css/cssom-view/elementFromPoint-dynamic-anon-box.html", "line": 75877, "action": "test_result", "expected": "PASS"}
{"status": "FAIL", "group": "default", "message": "assert_equals: elementFromPoint didn't return the correct element expected \"targetDiv\" but got \"\"", "stack": "@http://web-platform.test:8000/css/cssom-view/elementFromPoint-001.html:30:13\nTest.prototype.step@http://web-platform.test:8000/resources/testharness.js:1558:20\ntest@http://web-platform.test:8000/resources/testharness.js:544:9\n@http://web-platform.test:8000/css/cssom-view/elementFromPoint-001.html:29:9\n", "subtest": "CSSOM View - 5 - extensions to the Document interface", "test": "/css/cssom-view/elementFromPoint-001.html", "line": 137495, "action": "test_result", "expected": "PASS"}

css:

{"status": "PASS", "group": "default", "message": null, "stack": null, "subtest": null, "test": "/css/CSS2/borders/groove-default.html", "line": 9063, "action": "test_result", "expected": "FAIL"}
{"status": "FAIL", "group": "default", "message": "/css/css-backgrounds/background-attachment-local/attachment-local-positioning-2.html eaeebb2839df1cf7055b4ee85b03c121243febe6\n/css/css-backgrounds/background-attachment-local/attachment-local-positioning-2-ref.html 5a2ec2bcffef32babf5ed006e575c95b40f17461\nTesting eaeebb2839df1cf7055b4ee85b03c121243febe6 == 5a2ec2bcffef32babf5ed006e575c95b40f17461", "stack": null, "subtest": null, "test": "/css/css-backgrounds/background-attachment-local/attachment-local-positioning-2.html", "line": 19284, "action": "test_result", "expected": "PASS"}
{"status": "FAIL", "group": "default", "message": "/css/css-backgrounds/background-attachment-local/attachment-scroll-positioning-1.html cda19fcbd5b8404f0ae13d72de40dbca9bb04146\n/css/css-backgrounds/background-attachment-local/attachment-scroll-positioning-1-ref.html b1fa7ca2fdf4212c15aee8b20a8e35a687ba3735\nTesting cda19fcbd5b8404f0ae13d72de40dbca9bb04146 == b1fa7ca2fdf4212c15aee8b20a8e35a687ba3735", "stack": null, "subtest": null, "test": "/css/css-backgrounds/background-attachment-local/attachment-scroll-positioning-1.html", "line": 19808, "action": "test_result", "expected": "PASS"}
{"status": "PASS", "group": "default", "message": null, "stack": null, "subtest": null, "test": "/css/compositing/mix-blend-mode/mix-blend-mode-animation.html", "line": 20086, "action": "test_result", "expected": "FAIL"}
{"status": "FAIL", "group": "default", "message": "/css/compositing/mix-blend-mode/mix-blend-mode-blended-with-transform-and-perspective.html 5ad2bac6c31d0d3298124dd51067ed9b080d2165\n/css/compositing/mix-blend-mode/reference/mix-blend-mode-blended-with-transform-and-perspective-ref.html 147b375c8f1e8d0fec76c36919c7bf7a7ec18b60\nTesting 5ad2bac6c31d0d3298124dd51067ed9b080d2165 == 147b375c8f1e8d0fec76c36919c7bf7a7ec18b60", "stack": null, "subtest": null, "test": "/css/compositing/mix-blend-mode/mix-blend-mode-blended-with-transform-and-perspective.html", "line": 22174, "action": "test_result", "expected": "PASS"}
{"status": "PASS", "group": "default", "message": null, "stack": null, "subtest": null, "test": "/css/CSS2/borders/ridge-default.html", "line": 30251, "action": "test_result", "expected": "FAIL"}
{"status": "FAIL", "group": "default", "message": "/css/compositing/mix-blend-mode/mix-blend-mode-paragraph.html f6cb58abcc1d2274b393d5d49c274f70ea9bfc8b\n/css/compositing/mix-blend-mode/reference/mix-blend-mode-paragraph-ref.html 737b4ae888e9d0b5d4a98f8e1bcb0cb7b7e8002b\nTesting f6cb58abcc1d2274b393d5d49c274f70ea9bfc8b == 737b4ae888e9d0b5d4a98f8e1bcb0cb7b7e8002b", "stack": null, "subtest": null, "test": "/css/compositing/mix-blend-mode/mix-blend-mode-paragraph.html", "line": 31281, "action": "test_result", "expected": "PASS"}
  • /css/CSS2/borders/groove-default.html, border_black_ridge_a.html, ridge_default.html and border_black_ridge_b.html pass after servo/webrender#3010
@pyfisch

This comment has been minimized.

Contributor

pyfisch commented Sep 16, 2018

The coordinate systems were changed in some way in the new WebRender as seen in these two tests.

  • /css/css-backgrounds/scroll-positioned-multiple-background-images.html
  • /css/compositing/mix-blend-mode/mix-blend-mode-blended-with-transform-and-perspective.html

@kvark Can you give me hint what was changed and how the DL from servo needs to be changed?

@emilio

This comment has been minimized.

Member

emilio commented Sep 16, 2018

cc @gw3583, since he's likely to be around earlier. See the comment on top ^.

@kvark

This comment has been minimized.

Member

kvark commented Sep 17, 2018

@pyfisch
WR changes: servo/webrender@38f3b57...9156a44

Looking through this list, there isn't anything striking as an API semantics change. Most (if not all) the things are just fixing the internal processing (basically, bugs) and optimizing the data. Given that there isn't a lot of changes in this PR, it should be easy to get a narrower regression range.

@pyfisch

This comment has been minimized.

Contributor

pyfisch commented Sep 17, 2018

Thanks @kvark for having a look.

Debugging notes:

  • no regression on a16e311b
  • no regression on 9399766
  • no regressions on 5fa5c46e
  • one regression at 69dae1fe
    • /css/css-transforms/transform-table-006.html
  • at c89f16a2 tests fail
  • at 3fa5eb8a tests fail
  • at 5e1f9b9a these tests fail:
    • /css/css-backgrounds/background-attachment-local/attachment-scroll-positioning-1.html
    • /css/CSS2/floats-clear/clear-on-parent-and-child.html
    • /css/css-backgrounds/background-attachment-local/attachment-local-positioning-2.html
    • /css/css-backgrounds/scroll-positioned-multiple-background-images.html
    • /css/css-transforms/transform-table-006.html
@kvark

This comment has been minimized.

Member

kvark commented Sep 17, 2018

Thanks @pyfisch ! This is still a fairly large window of 53 commits.
Also, do you happen to have the reflog (or a link to it) with those reftest errors?

@pyfisch

This comment has been minimized.

Contributor

pyfisch commented Sep 17, 2018

This is still a fairly large window of 53 commits.

I am narrowing it down further. Basically a binary search for the first commit where a test fails.
Now the window is only 34 commits (turns out I suck at finding the middle element)

I am not sure what you mean by "reflog". I run this command to test:

#!/bin/bash

./mach test-wpt --include \
tests/wpt/web-platform-tests/css/CSS2/floats-clear/clear-on-parent-and-child.html \
tests/wpt/web-platform-tests/css/css-backgrounds/scroll-positioned-multiple-background-images.html \
tests/wpt/web-platform-tests/css/css-transforms/matrix/svg-matrix-061.html \
tests/wpt/web-platform-tests/css/css-transforms/transform-table-006.html \
\
tests/wpt/web-platform-tests/css/css-backgrounds/background-attachment-local/attachment-local-positioning-2.html \
tests/wpt/web-platform-tests/css/css-backgrounds/background-attachment-local/attachment-scroll-positioning-1.html \
tests/wpt/web-platform-tests/css/compositing/mix-blend-mode/mix-blend-mode-blended-with-transform-and-perspective.html \
tests/wpt/web-platform-tests/css/compositing/mix-blend-mode/mix-blend-mode-paragraph.html
@jdm

This comment has been minimized.

Member

jdm commented Sep 17, 2018

@pyfisch If you use --log-raw logfile, the resulting log can be uploaded to http://hoppipolla.co.uk/410/reftest-analyser-structured.xhtml and screenshots of the differences can be produced.

@pyfisch

This comment has been minimized.

@kvark kvark referenced this pull request Sep 17, 2018

Open

Run Servo with gfx #173

1 of 2 tasks complete
@kvark

This comment has been minimized.

Member

kvark commented Sep 18, 2018

@pyfisch thank you! We are already within 3 PRs of finding the exact breaking change. If you can't bisect it once (or twice?) more, I'll get to it tomorrow. We'll need to have the corresponding reftests locally on WR side anyway so that it doesn't happen again.

@pyfisch

This comment has been minimized.

Contributor

pyfisch commented Sep 18, 2018

In servo/webrender@5fa5c46...69dae1f the test /css/css-transforms/transform-table-006.html regresses.
In servo/webrender@69dae1f...c89f16a four more tests fail.

There is at least one more commit where a regression is introduced as after all changes are applied 8 tests fail, but at servo/webrender@69dae1f...c89f16a only 5 do fail.

@kvark

This comment has been minimized.

Member

kvark commented Sep 18, 2018

Thank you! I filed servo/webrender#3077 and servo/webrender#3078 on WR side to investigate.

@nical

This comment has been minimized.

Contributor

nical commented Sep 18, 2018

The change in #2992 enforces scene building to happen on a dedicated thread.
All transactions that go through the scene builder thread preserve their ordering while the ones that don't use the scene builder thread can skip ahead and use the previous scene. That's how Gecko can keep scrolling and applying async animations while a scene is taking too long (as it can happen).

Most transactions (the ones that are created using Transaction::new()) by default use the scene builder. Some transactions by default don't use the scene builder (for example hit-testing stuff and api.set_window_parameters(..)) which is a bit confusing, sorry.

If async scene building caused a regression in servo it most likely means that some messages that were ordered before are not ordered anymore. One way to check is to make sure that all api calls that create a transaction have the use_scene_builder_thread flag set to true to avoid letting them skip ahead of async scene building, which should correspond to servo's behavior before async scene building.

@jdm

This comment has been minimized.

Member

jdm commented Oct 2, 2018

I think we should go ahead and accept the regressions that come with this WR update. We need it in order to fix #21820, which makes our windows nightlies completely unusable.

@jdm

This comment has been minimized.

Member

jdm commented Oct 12, 2018

@bors-servo retry

  • network issues
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 12, 2018

⌛️ Testing commit 4e5b3bc with merge e46f127...

bors-servo added a commit that referenced this pull request Oct 12, 2018

Auto merge of #21725 - pyfisch:update-wr, r=jdm
Update Webrender

New version is
9156a4465f6ad715a0206cdd9a7e9a6f0385fbd6

---

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes do not require tests because just updating WR

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21725)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 12, 2018

💔 Test failed - linux-rel-wpt

@jdm

This comment has been minimized.

Member

jdm commented Oct 12, 2018

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 12, 2018

⌛️ Testing commit 4e5b3bc with merge 78c2fcf...

bors-servo added a commit that referenced this pull request Oct 12, 2018

Auto merge of #21725 - pyfisch:update-wr, r=jdm
Update Webrender

New version is
9156a4465f6ad715a0206cdd9a7e9a6f0385fbd6

---

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes do not require tests because just updating WR

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21725)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 12, 2018

💔 Test failed - mac-rel-wpt2

@jdm

This comment has been minimized.

Member

jdm commented Oct 12, 2018

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 12, 2018

⌛️ Testing commit 4e5b3bc with merge 7e78edc...

bors-servo added a commit that referenced this pull request Oct 12, 2018

Auto merge of #21725 - pyfisch:update-wr, r=jdm
Update Webrender

New version is
9156a4465f6ad715a0206cdd9a7e9a6f0385fbd6

---

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes do not require tests because just updating WR

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21725)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 12, 2018

1 similar comment
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Oct 12, 2018

@bors-servo bors-servo merged commit 4e5b3bc into servo:master Oct 12, 2018

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

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Oct 18, 2018

xeonchen pushed a commit to xeonchen/gecko-cinnabar that referenced this pull request Oct 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment