Closed Bug 1535585 Opened 7 months ago Closed 7 months ago

Unintended black square appears while scrolling on page

Categories

(Core :: Graphics, defect, P1)

All
Windows 10
defect

Tracking

()

VERIFIED FIXED
mozilla68
Tracking Status
firefox66 + wontfix
firefox67 --- verified
firefox68 --- verified

People

(Reporter: tbabos, Assigned: mattwoodrow)

References

(Blocks 2 open bugs, Regression, )

Details

Attachments

(1 file)

Affected versions:
Nightly 67.0a1 - aarch64 Windows 10 x64
Beta 66 (ID: 0190313193404) aarch64 Windows 10 x64
Release Candidate 66.0(64bit)(20190314220958)

Affected platforms:
Laptop Lenovo Yoga - Windows 10 x64 - ARM specific

Steps to reproduce:

Actual:
Unintended black square blocks the view of the page while scrolling: https://streamable.com/6ieyy
On RC build the system might also freeze and lead to an OS restart

Expected:
There should be no black square displayed.

Summary: Unintended black square appears while scrolling on page → [ARM64] Unintended black square appears while scrolling on page

@reporter - Thanks for the bug report - Does Firefox 65 have the same problem?

Flags: needinfo?(timea.babos)

Hi Maire,

As far as I know there are no Release 65 aarch builds available. The only Release version we tested was the release candidate 66 as mentioned in affected versions. The aarch builds for Laptop Lenovo Yoga were tested only on Nightly, Beta and Release candidate. If there is a build I am missing, please let me know.

Flags: needinfo?(timea.babos)

Sotaro - could you investigate the cause of this bug and let us know how complicated it would be to fix? It would also be good to know if this happens on other platforms as well or just ARM64

Assignee: nobody → sotaro.ikeda.g
Flags: needinfo?(sotaro.ikeda.g)

It is ARM64 specific (with aarch build), could not reproduce it on basic desktop with Windows 10 x64.

I confirmed the problem on Lenovo Yoga. The problem happened even with BasicCompositor.

Flags: needinfo?(sotaro.ikeda.g)

Oh, I could reproduce the problem also with nightly on x64 windows 10 when I disabled WebRender.

Hardware: ARM64 → All

The problem happened depends on width of browser window. When the window was certain size, the problem happened.

Summary: [ARM64] Unintended black square appears while scrolling on page → Unintended black square appears while scrolling on page

Checked with mozregression. Bug 1501382 seems like a culprit.

./mach mozregression --good 2018-12-01 --bad 2018-12-25 --pref gfx.webrender.force-disabled:true -a https://blog.hootsuite.com/linkedin-video/

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0e8109eaef4b4427272e284b6dc9ef17a0045a0f&tochange=0d3d0fa680d8ed6dcf0875c57f8c54d4cebe19b4

Blocks: 1501382

:jbonisteel, the problem happened on x64 windows10 when WebRender was disabled. Can this bug have a priority?

Flags: needinfo?(jbonisteel)

:mattwoodrow, can you take a bug by comment 8?

Flags: needinfo?(matt.woodrow)
Priority: -- → P1
Flags: needinfo?(jbonisteel)
Assignee: sotaro.ikeda.g → matt.woodrow
Flags: needinfo?(matt.woodrow)

This looks to be a FrameLayerBuilder bug :(

The relevant bit of the client Layer tree is this:

ClientContainerLayer (0x162e81c00) [clip=(x=0, y=0, w=2172, h=1616)] [visible=< (x=0, y=0, w=2172, h=180); >] [opaqueContent] [isFixedPosition scrollId=3 sides=0x8 anchor=(0,0)] [presShellResolution=1]
  ClientTiledPaintedLayer (0x1761de800) [visible=< (x=0, y=0, w=2172, h=907); >] { hitregion=< (x=0, y=0, w=2172, h=180); >} [valid=< (x=0, y=0, w=2172, h=907); >]

We've computed that the container has a visible height of 180, but the inner layer has a visible height of 907. There's no clipping or overlapping content that would explain this difference.

The container has opaqueContent, and due to Layer::CanUseOpaqueSurface [1], we decide we can also use an opaque surface for the painted layer. That creates an opaque surface 907 pixels high, draws 180 pixels of real content, and leaves the rest blank (black) since it think it'll never be shown.

On the compositor side, we compute all visible regions for the new scroll position. The (buggy) information that FrameLayerBuilder used to determine that only 180 pixels were visible isn't available in the Layer tree representation, so the visible height for the container changes to 907, exposing the unpainted area.

[1] https://searchfox.org/mozilla-central/rev/2c912888e3b7ae4baf161d98d7a01434f31830aa/gfx/layers/Layers.cpp#265

The difference appears to be when transforming coordinates, and then rounding the result out to nearest pixels. If an empty app unit rect isn't at a pixel edge, then rounding out to pixels results in a 1x1 pixel result.

The other issue is that we compute bounds in multiple paths, and some of them handle this case and others don't. We have an assertion to detect divergence, but it's disabled within opacity:0 [1].

The path going through nsDisplayItem::GetBounds handles this already in nsLayoutUtils::RoundGfxRectToAppRect [2].

[1] https://searchfox.org/mozilla-central/rev/2c912888e3b7ae4baf161d98d7a01434f31830aa/layout/painting/FrameLayerBuilder.cpp#5884
[2] https://searchfox.org/mozilla-central/rev/2c912888e3b7ae4baf161d98d7a01434f31830aa/layout/base/nsLayoutUtils.cpp#2323

Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/218dd6982a69
Make sure empty app unit rectangles get converted to empty pixel rectangles. r=jnicol
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Is this something we can/should uplift to beta ?

Matt, since this is a P1, is that something that would make sense to uplift to beta or should it go with 68? Thanks

Flags: needinfo?(matt.woodrow)

Comment on attachment 9053422 [details]
Bug 1535585 - Make sure empty app unit rectangles get converted to empty pixel rectangles. r?jnicol

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Bug 1501382
  • User impact if declined: Rare cases of unpainted content shown, at least one reproducible test case.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Super simple change, similar to one made before (with no regressions), this just makes the other codepath match the logic.
  • String changes made/needed: None
Flags: needinfo?(matt.woodrow)
Attachment #9053422 - Flags: approval-mozilla-beta?

Comment on attachment 9053422 [details]
Bug 1535585 - Make sure empty app unit rectangles get converted to empty pixel rectangles. r?jnicol

Low risk, fixes a P1 regression, approved for 67 beta 9, thanks.

Attachment #9053422 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
No longer blocks: 1501382
Regressed by: 1501382

Verified - Fixed on latest Firefox Nightly 68.0a1 (2019-05-03) and Beta16 on the same Lenovo Yoga Laptop.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.