479.42 KB, image/png
47 bytes, text/x-phabricator-request
|Details | Review|
47 bytes, text/x-phabricator-request
|Details | Review|
Created by Bug 1509244 Comment 3. When I scrolled down https://italic.com/, product image was not rendered. When I contined scrolling up and down, product image was rendered as flickering. I saw the problem on Win10, mac and linux.
see also bug 1508127
(In reply to Sotaro Ikeda [:sotaro] from comment #1) > Created attachment 9027818 [details] > image during causing the probolem > > Image was not rendered when I scrolled the page to the bottom. I saw the > image during disabling WebRender and chrome browser. mozregression --good 2017-08-10 --bad 2017-10-10 --pref gfx.webrender.enabled:true gfx.webrendest.enabled:true gfx.webrender.layers-free:true gfx.webrender.blob-images:true image.mem.shared:true layout.display-list.retain:false -a https://italic.com/ > 5:04.37 INFO: Last good revision: ca7d18dbacbf103d74a3213d8d08a7c3e4def9a2 (2017-09-21) > 5:04.37 INFO: First bad revision: 3d72fdb0e561ea59d9e5850c3e71367dbb8a7148 (2017-09-22) > 5:04.37 INFO: Pushlog: > https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ca7d18dbacbf103d74a3213d8d08a7c3e4def9a2&tochange=3d72fdb0e561ea59d9e5850c3e71367dbb8a7148 > [...] > 7:21.00 WARNING: Skipping build 97282b0c985a: Unable to find build info using the taskcluster route 'gecko.v2.mozilla-central.revision.97282b0c985aa1778ced171514d1ae61945c634c.firefox.win64-opt' > 7:21.02 INFO: There are no build artifacts on inbound for these changesets (they are probably too old). The flickering handbag has a different range.
status-firefox63: --- → disabled
status-firefox64: --- → disabled
status-firefox65: --- → affected
status-firefox-esr60: --- → disabled
OS: Unspecified → All
Priority: -- → P3
So bug 1510306 and this one are likely the same issue. The author used: https://codepen.io/shshaw/full/LVKEdv to generate an SVG image. When the internal raster image finishes decoding, it invalidates the image and regenerates the surface. We seem to be oscillating between the original surface (which was just transparent) and the new surface (with the actual JPEG decoded/included).
When the internal raster image finishes decoding, we are supposed to invalidate the owning vector image. This was not happening consistently with WebRender, because the logic assumed a Draw call would come in if we needed another invalidation. I fixed that issue. However there seems to be another case where we don't receive the invalidation at all when the raster image finishes decoding. There does not seem to be an observer anymore for the SVG element. This is strange. Still investigating.
It seems that NS_FRAME_DESCENDANT_NEEDS_PAINT is still set on the root frame of the SVG document from the first inner image's frame update, so the subsequent notifications from the second inner image's frame update are not passed along to the VectorImage listener to cause us to recreate the surface.
We actually end up redrawing these large SVGs way more than we need to. I added a counter to track how many times we redraw the surfaces between calls to GetImageContainerAtSize (which resets the counter): [AO] VectorImage::GetImageContainerAtSize -- 0x7fa238a28bb0 size 1036x691 invalidations 8 [AO] VectorImage::GetImageContainerAtSize -- 0x7fa238a28bb0 size 1036x691 invalidations 4 [AO] VectorImage::GetImageContainerAtSize -- 0x7fa232ec3530 size 1036x1254 invalidations 12 [AO] VectorImage::GetImageContainerAtSize -- 0x7fa232ec3600 size 1036x860 invalidations 4 [AO] VectorImage::GetImageContainerAtSize -- 0x7fa232ec3600 size 1036x860 invalidations 8 [AO] VectorImage::GetImageContainerAtSize -- 0x7fa231afd8b0 size 1036x1037 invalidations 12 [AO] VectorImage::GetImageContainerAtSize -- 0x7fa231afdb20 size 1036x630 invalidations 4 [AO] VectorImage::GetImageContainerAtSize -- 0x7fa231afdb20 size 1036x630 invalidations 8 [AO] VectorImage::GetImageContainerAtSize -- 0x7fa231afdcc0 size 1036x1036 invalidations 4 [AO] VectorImage::GetImageContainerAtSize -- 0x7fa231afdcc0 size 1036x1037 invalidations 8 In an ideal world, each of these accesses would have only caused us to draw once, or 10 redraws. Instead we redraw 8+4+12+4+8+12+4+8+4+8 = 72 times. That is a lot of extra wasted work, especially when it is done on the main thread. (Ignore the off by 1 pixel accesses. Bug 1453747 should address that.)
status-firefox65: affected → fix-optional
status-firefox66: --- → affected
Got to the bottom of the remaining not rendered bug. We cannot update the image container directly from VectorImage::InvalidateObserversOnNextRefreshDriverTick if it is non-animated, because the layout tree state may be inconsistent. If I defer it the problem goes away.
If a vector image has an image container, it is unlikely the caller will call VectorImage::Draw (and thus Show indirectly) to display the image. As such, WebRender was missing subsequent invalidations and not regenerating the rasterized surface as expected. Thus we now resume honoring the invalidations if we updated the image container.
Originally we would invalidate image containers synchronously when SVGRootRenderingObserver::OnRenderingChange was called. However at this point in time, the layout tree is in the middle of updating its own state, and triggering a paint will be problematic. Animated vector images did not suffer from this problem because they would defer to the next refresh tick, but non-animated do not receive refresh tick events. As such, we should just defer the invalidation to immediately after the layout tree has finished updating itself. Depends on D15667
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/325d06d3ba73 Part 1. Ensure we honor invalidations if using vector image containers. r=tnikkel https://hg.mozilla.org/integration/mozilla-inbound/rev/b709624ce8fa Part 2. Do not invalidate vector image containers synchronously. r=tnikkel
Status: NEW → RESOLVED
Last Resolved: 2 months ago
status-firefox66: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.