Update webrender to cset 1d6348023a4a4fdd89dce038640c5da906005acc (+ servo/webrender#1409)

RESOLVED FIXED in Firefox 56

Status

()

Core
Graphics: WebRender
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: kats, Assigned: kats)

Tracking

(Blocks: 1 bug)

unspecified
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [gfx-noted])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(9 attachments)

59 bytes, text/x-review-board-request
jrmuizel
: review+
Details | Review
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details | Review
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details | Review
59 bytes, text/x-review-board-request
mrobinson
: review+
jrmuizel
: review+
Details | Review
59 bytes, text/x-review-board-request
kvark
: review+
jrmuizel
: review+
Details | Review
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details | Review
59 bytes, text/x-review-board-request
mrobinson
: review+
Details | Review
59 bytes, text/x-review-board-request
Gankro
: review+
Details | Review
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details | Review
+++ This bug was initially created as a clone of Bug #1372608 +++

I'm filing this as a placeholder bug for the next webrender update. I may be running a cron script [1] that does try pushes with webrender update attempts, so that we can track build/test breakages introduced by webrender on a rolling basis. This bug will hold the try push links as well as dependencies filed for those breakages, so that we have a better idea going into the update of what needs fixing. I might abort the cron job because once things get too far out of sync it's hard to fully automate fixing all the breakages.

When we are ready to actually land the update, we can rename this bug and use it for the update, and then file a new bug for the next "future update".

[1] https://github.com/staktrace/moz-scripts/blob/master/try-latest-webrender.sh
WR @ 0b46c16f92a4b8bbee6e1d2457b7a82e316f7a80

https://treeherder.mozilla.org/#/jobs?repo=try&revision=73eb983ac8371910f7a4ab6317231f448bd472b6
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae997c9c84abc519f6a5caa7c29f7297d169a669

Some reftest failures in R3 and R7. Last good cset was 568c727b7375961d08f0d979bd5e78b88ecde72f so this is the regression range:

*   0b46c16 Auto merge of #1387 - mrobinson:fix-wrench-clip-and-scroll-for-stacking-contexts, r=kvark
|\
| * 5265218 Apply clip-and-scroll wrench property to stacking contexts
* 3080ac3 Auto merge of #1385 - staktrace:unused_import, r=kvark
* 96b926c Remove unused import

The failure is definitely not just fuzziness, it appears some items got repositioned.
The regression doesn't appear to be from the WR csets in comment 1. It's likely a combination of previous WR changes and stuff that changed in m-c. It's showing up in the try push in bug 1372608 comment 8 as well. I'll track the issue over in that bug.
WR @ 891670e5233650843d6189d1b72f202da3837ae8

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e21d94077ff838d33921c19ce41ef3af264dd19d
https://treeherder.mozilla.org/#/jobs?repo=try&revision=921d99aebdc0d041cb4143fdee33a656719061ed

Bustage. Regression range:

*   891670e Auto merge of #1376 - glennw:text-opt-dims, r=kvark
|\
| * f3e76c9 Optimize CPU time spent in building glyph instances.
*   2523990 Auto merge of #1388 - jrmuizel:device-size, r=glennw
|\
| * f710c3d Factor out DeviceSize construction
* ee94744 Auto merge of #1360 - Thinkofname:rgba-fix, r=kvark
* 264d53d Rename RGBA8 to BGRA8 to match what it actually does

The compiler error is actually a weird error that says "cc1plus: error: to generate dependencies you must specify either -M or -MM" on Linux. On Windows there's more useful messages about the renaming of RGBA8 to BGRA8 which probably needs a fixup.
Blocks: 1373861
Most of the reftest changes are benign (just need to adjust fuzziness numbers). However the R6 failure, layout/reftests/pixel-rounding/iframe-1.html, looks legit. There's a row of red pixels where there shouldn't be.

Regression range:

*   b0d2024 Auto merge of #1386 - mrobinson:new-clips-in-nested-display-lists, r=glennw
|\
| * 229cf3a Add support for new clips within nested display lists
*   9637503 Auto merge of #1397 - jrmuizel:descriptor-size, r=glennw
|\
| * d0fdef9 Remove unused display_list_items_size
* |   6c7b99d Auto merge of #1395 - MortimerGoro:sampler_float_mobile, r=glennw
|\ \
| * | 74cb194 Fix FLOAT texture sampling precision problems on some mobile GPUs
| |/
* |   c8ad0a8 Auto merge of #1375 - Gankro:revert3, r=glennw
|\ \
| |/
|/|
| * fc758aa Revert "Revert the snap rect changes"
*   97d320d Auto merge of #1393 - jrmuizel:more-fuzz, r=kvark
|\
| * c2d292a Add some more fuzz so that these tests pass for me.
* 0030d93 Auto merge of #1392 - servo:serdeup, r=glennw,SimonSapin
* dfeefbd Bump serde to 1.0
There's also a shader compilation failure on windows, presumably from servo/webrender#1375.

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: Link("cs_clip_image", "C:\\fakepath(1116,24-94): error X3067: \'_intersect_rect\': ambiguous function call\n\nWarning: D3D shader compilation failed with default flags. (vs_5_0)\n Retrying with skip validation\nC:\\fakepath(1116,24-94): error X3067: \'_intersect_rect\': ambiguous function call\n\nWarning: D3D shader compilation failed with skip validation flags. (vs_5_0)\n Retrying with skip optimization\nC:\\fakepath(1116,24-94): error X3067: \'_intersect_rect\': ambiguous function call\n\nWarning: D3D shader compilation failed with skip optimization flags. (vs_5_0)\n\n\n\nFailed to create D3D shaders.\n")', src\libcore\result.rs:859

We shouldn't land this update until this is fixed, because it's going to cause nightly users with WR enabled to crash.
(Assignee)

Updated

10 months ago
Depends on: 1374378
(Assignee)

Updated

10 months ago
Duplicate of this bug: 1372608
1d6348023a4a4fdd89dce038640c5da906005acc seems to be a reasonably good commit to update to, at least according to my try pushes from last night

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4c259f981395a339a88a2a2c56faa57a7e2e760e
https://treeherder.mozilla.org/#/jobs?repo=try&revision=24592734ce5d06c10d7d4ce29e8c297816fea2f0

I'll do another one on top of latest m-c and prep the patches.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 20

10 months ago
mozreview-review
Comment on attachment 8879556 [details]
Bug 1373381 - Update call to push_built_display_list due to API change in WR cset 4eea6ae.

https://reviewboard.mozilla.org/r/150864/#review155632
Attachment #8879556 - Flags: review+

Comment 21

10 months ago
mozreview-review
Comment on attachment 8879555 [details]
Bug 1373381 - Re-vendor third-party libraries.

https://reviewboard.mozilla.org/r/150862/#review155634
Attachment #8879555 - Flags: review?(jmuizelaar) → review+

Comment 22

10 months ago
mozreview-review
Comment on attachment 8879557 [details]
Bug 1373381 - Update RGBA8 to BGRA8 since it was renamed in WR cset 264d53d.

https://reviewboard.mozilla.org/r/150866/#review155636

::: gfx/webrender_bindings/WebRenderTypes.h:54
(Diff revision 1)
>  inline Maybe<WrImageFormat>
>  SurfaceFormatToWrImageFormat(gfx::SurfaceFormat aFormat) {
>    switch (aFormat) {
>      case gfx::SurfaceFormat::R8G8B8X8:
>        // TODO: use RGBA + opaque flag
> -      return Some(WrImageFormat::RGBA8);
> +      return Some(WrImageFormat::BGRA8);

Not your problem but this should really be an assert. Treating R8G8B8X8 the same as B8G8R8X8 won't work.
We can fix this after.
Attachment #8879557 - Flags: review+

Comment 23

10 months ago
mozreview-review
Comment on attachment 8879558 [details]
Bug 1373381 - Update for size field removal in WR cset d0fdef9.

https://reviewboard.mozilla.org/r/150868/#review155638
Attachment #8879558 - Flags: review?(jmuizelaar) → review+

Comment 24

10 months ago
mozreview-review
Comment on attachment 8879553 [details]
Bug 1373381 - Update webrender to cset 1d6348023a4a4fdd89dce038640c5da906005acc.

https://reviewboard.mozilla.org/r/150858/#review155640
Attachment #8879553 - Flags: review?(jmuizelaar) → review+

Comment 25

10 months ago
mozreview-review
Comment on attachment 8879554 [details]
Bug 1373381 - Update webrender_bindings version numbers and generated header.

https://reviewboard.mozilla.org/r/150860/#review155642
Attachment #8879554 - Flags: review?(jmuizelaar) → review+
Shoot. I just noticed that at some point the OS X debug build job went red. I thought it was unrelated but it seems to be pretty consistent. It came from this range:

*   b0d2024 Auto merge of #1386 - mrobinson:new-clips-in-nested-display-lists, r=glennw
|\
| * 229cf3a Add support for new clips within nested display lists
*   9637503 Auto merge of #1397 - jrmuizel:descriptor-size, r=glennw
|\
| * d0fdef9 Remove unused display_list_items_size
* |   6c7b99d Auto merge of #1395 - MortimerGoro:sampler_float_mobile, r=glennw
|\ \
| * | 74cb194 Fix FLOAT texture sampling precision problems on some mobile GPUs
| |/
* |   c8ad0a8 Auto merge of #1375 - Gankro:revert3, r=glennw
|\ \
| |/
|/|
| * fc758aa Revert "Revert the snap rect changes"
*   97d320d Auto merge of #1393 - jrmuizel:more-fuzz, r=kvark
|\
| * c2d292a Add some more fuzz so that these tests pass for me.
* 0030d93 Auto merge of #1392 - servo:serdeup, r=glennw,SimonSapin
* dfeefbd Bump serde to 1.0

and appears to be a missing symbol, _CTFontCopyDefaultCascadeListForLanguages. This range includes a bump of core-text from 4.0.0 to 5.0.0 and I'm guessing we're not linking all the things needed to make that work on debug.

Comment 27

10 months ago
mozreview-review
Comment on attachment 8879559 [details]
Bug 1373381 - Update for NestingIndex change in WR cset 229cf3a.

https://reviewboard.mozilla.org/r/150870/#review155650
Attachment #8879559 - Flags: review?(mrobinson) → review+

Comment 28

10 months ago
mozreview-review
Comment on attachment 8879556 [details]
Bug 1373381 - Update call to push_built_display_list due to API change in WR cset 4eea6ae.

https://reviewboard.mozilla.org/r/150864/#review155644
Attachment #8879556 - Flags: review?(mrobinson) → review+

Comment 29

10 months ago
mozreview-review
Comment on attachment 8879557 [details]
Bug 1373381 - Update RGBA8 to BGRA8 since it was renamed in WR cset 264d53d.

https://reviewboard.mozilla.org/r/150866/#review155652
Attachment #8879557 - Flags: review?(kvark) → review+
According to Apple docs this function was added in 10.8+ so that's probably why we're not seeing it on OS X 10.7 builders. I'm did a try push with core-text downgraded back to 4.0.0 (and core-graphics downgraded back to 0.7.0). This also adds back a copy of serde 0.9.15 which is not ideal but even so I get compile errors:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3dd5d5620963fe8e5358415b3383b72ae48c0b7e
The suggested fix is to add a feature to core-text-rs that excludes the 10.8+ code, and then use that feature when pulling in core-text from webrender. I'm testing the approach here:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=49d73970bf236da55a31c3297fc76c96d0216397

Getting it merged requires a PR to core-text-rs landed and published, followed by a PR to webrender. Which I might just cherry-pick into m-c because I'm getting tired of this runaround.
The try push worked, and the PR to core-text is at servo/core-text-rs#61 (already reviewed, but waiting on tests).
core-text PR is merged and v5.0.1 is on crates.io. I opened a PR for webrender at servo/webrender#1409 to pick up the updated core-text. However I'm just going to cherry-pick that commit into this patchset.
Comment hidden (mozreview-request)
(Assignee)

Updated

10 months ago
Assignee: nobody → bugmail
Summary: Future webrender update bug → Update webrender to cset 1d6348023a4a4fdd89dce038640c5da906005acc (+ servo/webrender#1409)

Comment 35

10 months ago
mozreview-review
Comment on attachment 8879641 [details]
Bug 1373381 - Cherry-pick servo/webrender#1409 to unbreak OS X debug build.

https://reviewboard.mozilla.org/r/150996/#review155776
Attachment #8879641 - Flags: review?(jmuizelaar) → review+

Comment 36

10 months ago
mozreview-review
Comment on attachment 8879560 [details]
Bug 1373381 - Update reftest annotations from changes in WR cset fc758aa.

https://reviewboard.mozilla.org/r/150872/#review155784

::: layout/reftests/w3c-css/submitted/images3/reftest.list:179
(Diff revision 1)
> -fails-if(!styloVsGecko) == object-position-png-001c.html object-position-png-001-ref.html # bug 1105150
> +fails-if(!styloVsGecko&&!webrender) == object-position-png-001c.html object-position-png-001-ref.html # bug 1105150
>  == object-position-png-001e.html object-position-png-001-ref.html
>  == object-position-png-001i.html object-position-png-001-ref.html
>  == object-position-png-001o.html object-position-png-001-ref.html
>  == object-position-png-001p.html object-position-png-001-ref.html
> -fails-if(!styloVsGecko) == object-position-png-002c.html object-position-png-002-ref.html # bug 1105150
> +fails-if(!styloVsGecko&&!webrender) == object-position-png-002c.html object-position-png-002-ref.html # bug 1105150

Is no whitespace between &&'s intended?

Otherwise r+
(Assignee)

Comment 37

10 months ago
mozreview-review-reply
Comment on attachment 8879557 [details]
Bug 1373381 - Update RGBA8 to BGRA8 since it was renamed in WR cset 264d53d.

https://reviewboard.mozilla.org/r/150866/#review155636

> Not your problem but this should really be an assert. Treating R8G8B8X8 the same as B8G8R8X8 won't work.
> We can fix this after.

I filed bug 1374729 for this.
(Assignee)

Comment 38

10 months ago
mozreview-review-reply
Comment on attachment 8879560 [details]
Bug 1373381 - Update reftest annotations from changes in WR cset fc758aa.

https://reviewboard.mozilla.org/r/150872/#review155784

> Is no whitespace between &&'s intended?
> 
> Otherwise r+

Yes, if you add whitespace inside one of the tokens the reftest list parser treats it as separate (invalid) tokens. So adding whitespace would actually break it.

Comment 39

10 months ago
mozreview-review
Comment on attachment 8879560 [details]
Bug 1373381 - Update reftest annotations from changes in WR cset fc758aa.

https://reviewboard.mozilla.org/r/150872/#review155806
Attachment #8879560 - Flags: review?(a.beingessner) → review+

Comment 41

10 months ago
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9d23ec0ef50a
Update webrender to cset 1d6348023a4a4fdd89dce038640c5da906005acc. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/41de8a2dbb94
Update webrender_bindings version numbers and generated header. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/30e895051542
Re-vendor third-party libraries. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/71560f69abe2
Update call to push_built_display_list due to API change in WR cset 4eea6ae. r=jrmuizel,mrobinson
https://hg.mozilla.org/integration/autoland/rev/fcfdb4036a06
Update RGBA8 to BGRA8 since it was renamed in WR cset 264d53d. r=jrmuizel,kvark
https://hg.mozilla.org/integration/autoland/rev/9d65d68d9e5c
Update for size field removal in WR cset d0fdef9. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/e6261af46466
Update for NestingIndex change in WR cset 229cf3a. r=mrobinson
https://hg.mozilla.org/integration/autoland/rev/294e356f81e8
Update reftest annotations from changes in WR cset fc758aa. r=Gankro
https://hg.mozilla.org/integration/autoland/rev/b1c3b719cf77
Cherry-pick servo/webrender#1409 to unbreak OS X debug build. r=jrmuizel
(Assignee)

Updated

10 months ago
Depends on: 1376311
You need to log in before you can comment on or make changes to this bug.