Support changing the bounds of blob image visible rects.
Categories
(Core :: Graphics: WebRender, task, P3)
Tracking
()
People
(Reporter: nical, Assigned: nical)
References
Details
Attachments
(8 files, 6 obsolete files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
WebRrender-side changes needed for the blob-image recoordination work.
Assignee | ||
Comment 1•5 years ago
|
||
Assignee | ||
Comment 2•5 years ago
|
||
Assignee | ||
Comment 3•5 years ago
|
||
Updated•5 years ago
|
Assignee | ||
Comment 4•5 years ago
|
||
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 11•5 years ago
|
||
Quick note about the invalidation issue. I don't know how this works but it looks like in WebRenderCommandsBuilder.cpp we invalidate more than we need to. While scrolling a large SVG we hit this call to InvalidateRect: https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/gfx/layers/wr/WebRenderCommandBuilder.cpp#545 and I suspect that here (and in other places?) we don't want to use the full clipped image bounds and instead some thing that would be the intersection of the the clipped image bounds before and after scrolling.
Commenting out that line causes the blob to never update (because we don't update mValidRect at all during scrolling) so we get empty tiles past the initial visible area.
Comment 12•5 years ago
|
||
(In reply to Nicolas Silva [:nical] from comment #11)
Quick note about the invalidation issue. I don't know how this works but it looks like in WebRenderCommandsBuilder.cpp we invalidate more than we need to. While scrolling a large SVG we hit this call to InvalidateRect: https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/gfx/layers/wr/WebRenderCommandBuilder.cpp#545 and I suspect that here (and in other places?) we don't want to use the full clipped image bounds and instead some thing that would be the intersection of the the clipped image bounds before and after scrolling.
Commenting out that line causes the blob to never update (because we don't update mValidRect at all during scrolling) so we get empty tiles past the initial visible area.
Indeed. I'll investigate today.
Comment 13•5 years ago
|
||
So a piece that I forgot about that we'll need, is to make sure our invalid rect is intersected appropriately.
Today we intersect the invalid rect with the bounds of blob image image, but instead we need to intersect it with the intersection of the old bounds and the new bounds because that's only area that it makes sense to invalidate. Does that make sense? I'll try to put together a patch for it.
Comment 14•5 years ago
|
||
I haven't even tried to build this, but it should roughly do what we want here.
Some notable changes:
- mImageBounds changes to be the an un-unit-ed mLayerBounds instead of the visibleRect. This should reduce some unnecessary invalidation
- we don't intersect with mClippedGroupBounds. We shouldn't need to anymore because the visible area should already be tight
- add a mLastVisibleRect that tracks the previous visible rect
- we store a mPreservedRect which is the intersection between the current visiblerect and the last one
- all calls to InvalidateRect are intersected with mPreservedRect
Possible areas of trouble:
- new blob images don't have an invalid rect the size of the whole image. Instead it will be empty.
- obsolete assertions might trigger.
Updated•5 years ago
|
Assignee | ||
Comment 15•5 years ago
|
||
Updated•5 years ago
|
Comment 16•5 years ago
|
||
Latest try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ee4a038cc15eca090b38c862877344bc63a5f84
Comment 17•5 years ago
|
||
Pushed by nsilva@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/afa98bb10628 Fix a number of issues with the tile decomposition code. r=jrmuizel
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 18•5 years ago
|
||
Updated•5 years ago
|
Comment 19•5 years ago
|
||
bugherder |
Comment 20•5 years ago
|
||
Pushed by nsilva@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/548e4395ee1a Support changing the blob image visible rect in WebRender. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/214c815015a8 Wrench rawtest changes. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/ba19640abf33 Update reftests. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/14a4451b2781 Don't insert blob images with empty visible rects. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/8d4baf4d063a Put the blob dirty rect in the same space as the drawing commands and visible rect. r=jrmuizel
Comment 21•5 years ago
|
||
Backed out for assertion failure at WebRenderCommandBuilder.cpp
Push that started the failures https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=8d4baf4d063ac13cb7b0ef7e1457d43a91f8fbe5&selectedJob=266487618
Failure log https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=266487618&repo=autoland&lineNumber=26696
Backout: https://hg.mozilla.org/integration/autoland/rev/c2abef0bfc0b855a8f0b50e96dabe1f6a4973de2
***this failed on multiple test suites (crashtests, reftests, mochitests) both tier1 and tier2
Assignee | ||
Updated•5 years ago
|
Comment 22•5 years ago
|
||
Pushed by nsilva@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/32951c9cc186 Support changing the blob image visible rect in WebRender. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/775c0840f9cb Wrench rawtest changes. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/aac21319b917 Update reftests. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/aeeb928c954f Don't insert blob images with empty visible rects. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/e906f82ffa90 Put the blob dirty rect in the same space as the drawing commands and visible rect. r=jrmuizel
Assignee | ||
Comment 23•5 years ago
|
||
Comment 24•5 years ago
|
||
The patch could not be landed:
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. applying /tmp/tmpgkppmL layout/reftests/border-image/reftest.list Hunk #1 FAILED at 48. 1 out of 1 hunk FAILED -- saving rejects to file layout/reftests/border-image/reftest.list.rej abort: patch command failed: exited with status 256
Updated•5 years ago
|
Comment 25•5 years ago
|
||
Pushed by nsilva@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0aa2e90830bd Adjust a reftest. r=jnicol,aosmond
Comment 27•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/32951c9cc186
https://hg.mozilla.org/mozilla-central/rev/775c0840f9cb
https://hg.mozilla.org/mozilla-central/rev/aac21319b917
https://hg.mozilla.org/mozilla-central/rev/aeeb928c954f
https://hg.mozilla.org/mozilla-central/rev/e906f82ffa90
https://hg.mozilla.org/mozilla-central/rev/0aa2e90830bd
Assignee | ||
Updated•5 years ago
|
Description
•