Closed Bug 1373381 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Graphics: WebRender, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(9 files)

59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
59 bytes, text/x-review-board-request
mrobinson
: review+
jrmuizel
: review+
Details
59 bytes, text/x-review-board-request
kvark
: review+
jrmuizel
: review+
Details
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
59 bytes, text/x-review-board-request
mrobinson
: review+
Details
59 bytes, text/x-review-board-request
Gankra
: review+
Details
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
+++ 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.
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.
Depends on: 1374378
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 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 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 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 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 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 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 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 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 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.
Assignee: nobody → bugmail
Summary: Future webrender update bug → Update webrender to cset 1d6348023a4a4fdd89dce038640c5da906005acc (+ servo/webrender#1409)
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 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+
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.
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 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+
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
Depends on: 1376311
You need to log in before you can comment on or make changes to this bug.