In the FirefoxOS vertical homescreen you can change the layout of the page from 3 to 4 columns by either pinching, or going to settings. After changing the number of columns, if you scroll, you occasionally see the page enter "low-res", before becoming high resolution. The low res state does not match the full-res state (having a different number of columns and layout), and so the transition is extremely jarring. You see a flash of the page in the wrong state, before the icons res-in.

Milan - one issue I noticed while scrolling the vertical homescreen. Do you have any preference on how I report these, and is there anyone I should ni? I'm setting blocking to 2.0? but let me know if that isn't the right thing to do. Thanks!

No big preference. If we know it's related to low res-progressive, making it block bug 993473 is a good idea. If it's general homescreen issue - perhaps we need a meta bug on it. Now, we also have to be careful about 2.0+ in general, and not to use it for "this should be done". I'll clear up some stuff in the e-mail.

I am fairly sure that this is due to low-res tiling as I just started noticing this. Blocking bug 993473. Please let me know if there's any additional information I can provide.

I have run into this problem before, on Fennec. I think the low-res tile cache just doesn't get updated properly when contents change.

This is an overall regression and a correctness issue. We'll either fix it, or turn off low res tiling, but in either case, this symptom is a blocker.

Comment on attachment 8437956 [details] [diff] [review] Refactoring WIP I moved this out into bug 1023882.

AFAICT this should cause low-precision updates to continue happening as long as there is low-precision invalid areas. However it shouldn't interfere with high-precision updates because at the end of SharedFrameMetricsHelper::UpdateFromCompositorFrameMetrics there is an abort condition for when there's a more recent display-port in the pipeline. I didn't touch the Fennec code for this since this is a B2G bug but if it goes over well I'd like to make the same change there.

Comment on attachment 8438596 [details] [diff] [review] Update low-res tiles even when not at risk of checkerboarding Review of attachment 8438596 [details] [diff] [review]: ----------------------------------------------------------------- This will cause low precision rendering to always happen - do we want this? It's a considerable performance hit to always render low precision - we end up with situations where you would see low precision content, where previously you would have seen high precision content. On the other hand, you also have situations where you have low precision content where previously you would have nothing and also remove the situations where you see stale low precision content. I'm broadly in favour of this, but note that this will almost certainly negatively affect talos on Android. Note that the low precision rendering may be a bit screwy in b2g because AboutToCheckerboard is actually IsCheckerboardingRightNow - it would perform better with a Danger Zone(tm). All the same, r+ with the first comment addressed. ::: gfx/layers/client/TiledContentClient.cpp @@ -120,5 @@ > } > > SharedFrameMetricsHelper::SharedFrameMetricsHelper() > - : mLastProgressiveUpdateWasLowPrecision(false) > - , mProgressiveUpdateWasInDanger(false) You should remove these two member variables from SharedFrameMetricsHelper if they're no longer used. @@ +183,5 @@ > } > > // When not a low precision pass and the page is in danger of checker boarding > // abort update. > + if (!aLowPrecision && AboutToCheckerboard(contentMetrics, compositorMetrics)) { I wonder if we should have an AboutToCheckerboard where you pass in the displayport manually, so we can check if the low precision region is about to checkerboard too? This check seems a bit silly now that I look at it again.

We can fix this bug without landing all the patches for bug 1023882? Or is the dependency a hard one, in that we need to land the other bug as well? If so, please set 2.0? on the bug 1023882.

(In reply to Chris Lord [:cwiiis] from comment #9) > This will cause low precision rendering to always happen - do we want this? I think so, yes. > It's a considerable performance hit to always render low precision - we end > up with situations where you would see low precision content, where > previously you would have seen high precision content. Is this just because we might be in the middle of a low-precision paint that can't be pre-empted? We should be doing low-precision paints a tile at a time anyway so I can't imagine that this would be a huge problem. I might not understand all the intricacies of this though, please let me know if there's more I'm missing here. > On the other hand, you also have situations where you have low precision > content where previously you would have nothing and also remove the > situations where you see stale low precision content. I'm broadly in favour > of this, but note that this will almost certainly negatively affect talos on > Android. We could just leave it off on Android then :) > Note that the low precision rendering may be a bit screwy in b2g because > AboutToCheckerboard is actually IsCheckerboardingRightNow - it would perform > better with a Danger Zone(tm). That's true, I can make that change. > > SharedFrameMetricsHelper::SharedFrameMetricsHelper() > > - : mLastProgressiveUpdateWasLowPrecision(false) > > - , mProgressiveUpdateWasInDanger(false) > > You should remove these two member variables from SharedFrameMetricsHelper > if they're no longer used. Doh. Totally forgot :p > @@ +183,5 @@ > > // When not a low precision pass and the page is in danger of checker boarding > > // abort update. > > + if (!aLowPrecision && AboutToCheckerboard(contentMetrics, compositorMetrics)) { > > I wonder if we should have an AboutToCheckerboard where you pass in the > displayport manually, so we can check if the low precision region is about > to checkerboard too? This check seems a bit silly now that I look at it > again. So that we abort the low-precision paint if we're about to checkerboard out of the low-precision displayport, you mean? Yeah that sounds reasonable to do.

The dependency isn't really needed. It might be good to uplift if we're going to continue making changes in this code but I guess we can do that later.

I filed bug 1024126 for the AboutToCheckerboard change. I'll land this patch as-is for now (with the variables removed, of course), but I'll probably be fiddling with this code some more as I try to fix bug 1021085. https://hg.mozilla.org/integration/mozilla-inbound/rev/bc9031f59e1c

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/b548a30a0fe0 for Werror bustage from those variables: https://tbpl.mozilla.org/php/getParsedLog.php?id=41548937&tree=Mozilla-Inbound

Doh. I made the change and apparently forgot to regenerate the patch that I pushed to m-i. Fixed and relanded: https://hg.mozilla.org/integration/mozilla-inbound/rev/a461a267cf62

Backed out for being the likely cause of B2G fixed-1.html reftest permafail. Unfortunately, it landed during a time of pretty heavy coalescing on inbound, so it's not a complete certainty at this point. I'll update the bug here when it's confirmed or re-land if it wasn't. https://hg.mozilla.org/integration/mozilla-inbound/rev/e22480012f5a https://tbpl.mozilla.org/php/getParsedLog.php?id=41621436&tree=Mozilla-Inbound

Confirmed this was the cause of the failure in comment 16.

I am able to intermittently reproduce this problem on a local emulator build. Looks like there is an existing race condition that got tickled by this change to fail more often. The race condition has to deal with when the compositor thread gets around to processing the layers update transaction it receives and therefore when the compositor-side frame metrics get updated. If the compositor thread processes the transaction early then it gets the y=20 scroll offset earlier, and so the AboutToCheckerboard call returns false when the client side goes to do its progressive paint on the content. But if the compositor thread is slow then it doesn't get the y=20 scroll update soon enough and AboutToCheckerboard returns true. This results in the high-res content paint getting delayed until later. Not sure what the right fix is yet.

After some more investigation I believe this is an actual bug in the code, that can leave parts of the screen painted in low-res even in a stable state. I'll spin this off into a separate bug.

Flagging NO_UPLIFT for being suspected for causing the smoketest regression in bug 1028271.

I backed this out for causing the smoketest regression in bug 1028271. https://hg.mozilla.org/integration/mozilla-inbound/rev/0f07aa386616

FWIW I can't actually reproduce this bug any more. Either it was fixed by recent patches, or (more likely) having to go into the settings app to change the layout means the layer and tiles get thrown away and rebuilt. Previously I was able to pinch to change the layout on the vertical homescreen without going into settings, but that feature is no longer available.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #23) > I backed this out for causing the smoketest regression in bug 1028271. > > https://hg.mozilla.org/integration/mozilla-inbound/rev/0f07aa386616 Merge of backout: https://hg.mozilla.org/mozilla-central/rev/0f07aa386616

Kevin, are you still able to reproduce this bug? Please test on a build that has gecko >= 7b174d47f3cc (last inbound -> central merge as of this writing). I was unable to repro using the STR in comment 0 when I tried just now. If you can't repro then I'd like to close this as WFM.

I am currently unable to reproduce. I'm fine with closing as WFM for now.

Unblocking from 1015336 as this appears to work now, and this will help me track necessary uplifts.