Closed
Bug 1362115
Opened 8 years ago
Closed 7 years ago
[meta] Turn on gfx.webrender.blob-images (blob images) by default
Categories
(Core :: Graphics: WebRender, enhancement, P3)
Core
Graphics: WebRender
Tracking
()
RESOLVED
FIXED
mozilla60
Tracking | Status | |
---|---|---|
firefox56 | --- | unaffected |
firefox57 | --- | unaffected |
firefox60 | --- | fixed |
People
(Reporter: jrmuizel, Assigned: Gankra)
References
(Depends on 1 open bug)
Details
(Keywords: meta, Whiteboard: [gfx-noted])
Attachments
(3 files, 1 obsolete file)
No description provided.
Reporter | ||
Comment 1•8 years ago
|
||
The latest reftest run https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e5e0be482d78f961829bff13bb92b24dfa74ea5&selectedJob=96747316 with fixes for bug 1362117, and bug 1362221. I have looked at any of the failures yet, so they're all free for the taking.
Updated•8 years ago
|
Whiteboard: [gfx-noted]
Reporter | ||
Comment 2•7 years ago
|
||
Reporter | ||
Comment 3•7 years ago
|
||
Updated•7 years ago
|
Priority: -- → P3
Updated•7 years ago
|
Blocks: stage-wr-nightly
Updated•7 years ago
|
Priority: P3 → P2
Whiteboard: [gfx-noted] → [wr-mvp] [gfx-noted]
Updated•7 years ago
|
status-firefox56:
--- → unaffected
status-firefox57:
--- → unaffected
Updated•7 years ago
|
Keywords: meta
Priority: P2 → --
Summary: Turn on BlobImage by default → [meta] Turn on BlobImage by default
Whiteboard: [wr-mvp] [gfx-noted] → [gfx-noted]
Comment hidden (obsolete) |
Comment 5•7 years ago
|
||
Accidentally cancelled that push. Here's another one: https://treeherder.mozilla.org/#/jobs?repo=try&revision=46899918a8f44a16db912f53c65b16733b88fd6e
Comment 6•7 years ago
|
||
Looks like we're consistently leaking a handful of NativeFontResourceFontconfig and UnscaledFontFontconfig instances, causing all the debug jobs to fail. Based on the way those objects are allocated and used I'm guessing that we're not deleting all the font stuff during teardown, which leaves some UnscaledFontFontconfig lying around in sFontDataTable. Those objects in turn keep a reference to the NativeFontResourceFontconfig and so those get leaked too.
Other than that there's a lot of reftest failures. And what look like OOMs in the crashtest run.
Comment 7•7 years ago
|
||
Another month, another try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb4927bfc33f1d3df334eeabe4835d6541c0e85e
Updated•7 years ago
|
Priority: -- → P3
Comment 8•7 years ago
|
||
Comment 9•7 years ago
|
||
Here's a push Lee did with the fix from bug 1431211: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0427966c52e94b61ed74e1b4cf899dc7818ead34&group_state=expanded
The R7/R8 failures can be dealt with by annotating, but there's also a crashtest failure now which looks like an OOM. I've retriggered to see if it's consistent.
Comment 10•7 years ago
|
||
The crashtest failure seems pretty consistent, and it's not happening on m-c/inbound/autoland now, so it's almost certainly a result of turning on blob images. We'll need to investigate that.
Comment 11•7 years ago
|
||
Making the title more searchable
Summary: [meta] Turn on BlobImage by default → [meta] Turn on gfx.webrender.blob-images (blob images) by default
Comment 12•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → a.beingessner
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 16•7 years ago
|
||
The last issue was a crashtest that produces an empty video with a massive size. Webrender blobs this, and because it doesn't know the blob is actually empty it tries to produce tiles. However this ends up requiring 1,000,000,000 tiles which makes us run out of memory and die producing tile metadata.
The fix I take here is to just consider empty videos to have no content (which matches the logic in BuildLayer).
The core issue should still be fixed in webrender, and an issue has been filed, but we can proceed with CI without it.
try build with no crashes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9ddf9e53d5c22bcef5d736121abf799217f8ecae&selectedJob=159943294
try build with adjusted test expectations (pending as of this writing): https://treeherder.mozilla.org/#/jobs?repo=try&revision=03e86a0f36ff5e7ecd366f10e22b4a0cb3bc837c
Comment 17•7 years ago
|
||
mozreview-review |
Comment on attachment 8947611 [details]
Bug 1362115 - don't emit a blob-image for contentless videos.
https://reviewboard.mozilla.org/r/217300/#review223092
Attachment #8947611 -
Flags: review?(bugmail) → review+
Comment 18•7 years ago
|
||
mozreview-review |
Comment on attachment 8947612 [details]
Bug 1362115 - turn on blob-images by default with webrender.
https://reviewboard.mozilla.org/r/217302/#review223094
Looks like somebody added https://searchfox.org/mozilla-central/rev/eeb7190f9ad6f1a846cd6df09986325b3f2c3117/browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm#24 which probably needs updating as well.
Attachment #8947612 -
Flags: review?(bugmail)
Comment 19•7 years ago
|
||
mozreview-review |
Comment on attachment 8947613 [details]
Bug 1362115 - adjust reftest expectations for blob-images.
https://reviewboard.mozilla.org/r/217304/#review223098
Squash this into the previous patch
Attachment #8947613 -
Flags: review?(bugmail) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8947613 -
Attachment is obsolete: true
Assignee | ||
Comment 22•7 years ago
|
||
squashed and edited that new file.
Had a syntax error (deleted a ==) so trying again: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a66b076a817672788dc041d070d1180c57032b6b
Comment 23•7 years ago
|
||
mozreview-review |
Comment on attachment 8947612 [details]
Bug 1362115 - turn on blob-images by default with webrender.
https://reviewboard.mozilla.org/r/217302/#review223108
Attachment #8947612 -
Flags: review?(bugmail) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 24•7 years ago
|
||
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/71eb4d997c0d
don't emit a blob-image for contentless videos. r=kats
https://hg.mozilla.org/integration/autoland/rev/761a65991c7f
turn on blob-images by default with webrender. r=kats
Keywords: checkin-needed
Comment 25•7 years ago
|
||
Backed out 2 changesets (bug 1362115) for frequent crashtest failures in gfx/tests/crashtests/1317403-1.html
https://treeherder.mozilla.org/logviewer.html#?job_id=160002392&repo=autoland&lineNumber=46053
https://hg.mozilla.org/integration/autoland/rev/6c8eb870b45f80661450511f45d2c14d3d1b2aec
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=761a65991c7f58cbb81981cccd686c93068897b2
Flags: needinfo?(a.beingessner)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 29•7 years ago
|
||
New changes:
* skip-if'ing the problematic test temporarily
* fixed some FFI code that was newly crashing
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f0636dc46dbc053f4b5324204accd120099afc37
Flags: needinfo?(a.beingessner)
Comment 30•7 years ago
|
||
mozreview-review |
Comment on attachment 8948447 [details]
Bug 1362115 - properly handle empty slices in FFI bindings.
https://reviewboard.mozilla.org/r/217888/#review223668
Looks like there was a lot of discussion about this code when it was added in bug 1347641 and although using as_ptr was mentioned it looks like it got buried under mountains of other FFI/safety discussion. You may want to read it over to make sure we're not accidentally reverting an intentional change though. I didn't really understand a lot of it.
Attachment #8948447 -
Flags: review?(bugmail) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 32•7 years ago
|
||
Notes on slice thing: confirmed it's just a bug, my fix is correct.
Notes on new push: it seems crashes in graphics are misattributed to future tests because the crash-test runner doesn't block on rendering. The new skip-if is the correct test (isolated with binary search). The issue is a fairly large mask being animated, so that if the frame occurs at some times it is skewed massively (xscale of 57).
This is sufficiently degenerate that we can proceed while skipping the test. I have filed https://bugzilla.mozilla.org/show_bug.cgi?id=1435896 for it.
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=930bb57dd24b42b6692e72b728f3844b49f8e588
Comment 34•7 years ago
|
||
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/87b04696a381
don't emit a blob-image for contentless videos. r=kats
https://hg.mozilla.org/integration/autoland/rev/cae458b810a8
properly handle empty slices in FFI bindings. r=kats
https://hg.mozilla.org/integration/autoland/rev/b2804c6bec4c
turn on blob-images by default with webrender. r=kats
Keywords: checkin-needed
Comment 35•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/87b04696a381
https://hg.mozilla.org/mozilla-central/rev/cae458b810a8
https://hg.mozilla.org/mozilla-central/rev/b2804c6bec4c
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•