Closed Bug 1251894 Opened 4 years ago Closed 4 years ago

Artifacting around edges of Twitter feed when scrolling with mouse wheel

Categories

(Core :: Panning and Zooming, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla48
Tracking Status
firefox46 --- fixed
firefox47 --- fixed
firefox48 --- verified

People

(Reporter: RyanVM, Assigned: mstange)

References

()

Details

(Keywords: regression, Whiteboard: [gfx-noted])

Attachments

(5 files)

Attached image screenshot
Only happens with the mouse wheel, so I assume it's APZ.
What's your graphics configuration?  You don't have an Nvidia card by any chance?
Optimus.

Graphics
--------
Adapter Description: Intel(R) HD Graphics P530
Adapter Description (GPU #2): NVIDIA Quadro M1000M
Adapter Drivers: igdumdim64 igd10iumd64 igd10iumd64 igd12umd64 igdumdim32 igd10iumd32 igd10iumd32 igd12umd32
Adapter Drivers (GPU #2): nvd3dumx,nvwgf2umx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um,nvwgf2um
Adapter RAM: Unknown
Adapter RAM (GPU #2): 2048
Asynchronous Pan/Zoom: wheel input enabled; touch input enabled
Device ID: 0x191d
Device ID (GPU #2): 0x13b1
Direct2D Enabled: true
DirectWrite Enabled: true (10.0.10586.0)
Driver Date: 2-2-2016
Driver Date (GPU #2): 2-8-2016
Driver Version: 20.19.15.4380
Driver Version (GPU #2): 10.18.13.6191
GPU #2 Active: false
GPU Accelerated Windows: 1/1 Direct3D 11 (OMTC)
Subsys ID: 06d91028
Subsys ID (GPU #2): 06d91028
Supports Hardware H264 Decoding: Yes
Vendor ID: 0x8086
Vendor ID (GPU #2): 0x10de
WebGL Renderer: Google Inc. -- ANGLE (Intel(R) HD Graphics P530 Direct3D11 vs_5_0 ps_5_0)
windowLayerManagerRemote: true
AzureCanvasAccelerated: 0
AzureCanvasBackend: direct2d 1.1
AzureContentBackend: direct2d 1.1
AzureFallbackCanvasBackend: cairo
(#0): CP+[GFX1-]: [D2D1.1] 4CreateBitmap failure Size(13,24287) Code: 0x80070057 format 0
I'm unable to reproduce on my windows machine. Can you confirm it doesn't happen with APZ disabled?

(In reply to Ryan VanderMeulen [:RyanVM] from comment #2)
> (#0): CP+[GFX1-]: [D2D1.1] 4CreateBitmap failure Size(13,24287) Code:
> 0x80070057 format 0

I wonder if this is related?
Doesn't reproduce when using keyboard scrolling or with APZ preffed off. I find it easiest to reproduce by scrolling all the way to the bottom than back up and down.
I still can't repro :(

Markus, can you apply your psychic debugging powers to the attached screenshot and speculate as to what's going wrong here?
Flags: needinfo?(mstange)
Ryan, can you record a video of this happening? I wonder if we're accumulating errors on pixels that we touch in multiple frames without resetting them. (Though I don't know how this might happen; we should always be resetting pixels that we end up presenting in a frame.)
I think the foreground layer is a component alpha layer, and the bad pixels are what we blend against when compositing that layer.
Flags: needinfo?(mstange) → needinfo?(ryanvm)
Whiteboard: [gfx-noted]
Regression range: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c525c5164cdde373a636d525e53c77f52d8fd693&tochange=c99e8abb5e00eaa439a3be7864df0e4135326349

I'm guessing regression from bug c99e8abb5e00. I can bisect locally if you think it'll help.
Flags: needinfo?(ryanvm) → needinfo?(matt.woodrow)
Keywords: regression
Bug 1231538 that is :)
Can you set layout.display-list.dump to true and attach the output to this bug?
Flags: needinfo?(ryanvm)
Attached video video capture
As requested, here's a video capture.

While bisecting this with mozregression, I should also point out that I occasionally got a site background that didn't reproduce (a black & white picture of someone in an Eakin #20 jersey). But the soccer-related backgrounds reproduced consistently if I started scrolling after the video at the top started autoplaying.
Flags: needinfo?(ryanvm)
Where will the dump be saved to?
Attached file layers dump
OK, I launched a debug build with the pref set and with the twitter page loaded immediately on startup. Here's the resulting dump after scrolling around a bit with confirmed artifacting and then closing the browser afterwards.
This is wild. Thanks.

Bas, we're hitting the warning "Could not copy render target - source rect out of bounds" here:
https://dxr.mozilla.org/mozilla-central/rev/af7c0cb0798f5425d5d344cbaf0ac0ecb1a72a86/gfx/layers/d3d11/CompositorD3D11.cpp#572

Is that bad? Does the video give you any other ideas?
Flags: needinfo?(bas)
4K resolution in case it matters.
In case it gives us any other ideas, could you get one more log, please? This time with layers.dump-host-layers set to true (and layout.display-list.dump to false). The layers.dump-host-layers pref only works in debug builds or in --enable-dump-painting builds.
Flags: needinfo?(ryanvm)
Attached file host-layers dump
Flags: needinfo?(ryanvm)
(In reply to Markus Stange [:mstange] from comment #13)
> This is wild. Thanks.
> 
> Bas, we're hitting the warning "Could not copy render target - source rect
> out of bounds" here:
> https://dxr.mozilla.org/mozilla-central/rev/
> af7c0cb0798f5425d5d344cbaf0ac0ecb1a72a86/gfx/layers/d3d11/CompositorD3D11.
> cpp#572
> 
> Is that bad? Does the video give you any other ideas?

It basically means that LayerManagerComposite is incorrectly calculating the region to pass to that function. In the past we'd silently fail but we've noticed that makes some display drivers crash so we explicitly ignore it now.
Flags: needinfo?(bas)
Ok, so here's what I got:

> ContainerLayerComposite (0x247bf4de400) [shadow-clip=(x=0, y=0, w=3840, h=1938)] [shadow-visible=< (x=0, y=0, w=3840, h=1938); >] [clip=(x=0, y=0, w=3840, h=1938)] [effective-transform=[ 1 0; 0 1; 0 155; ]] [visible=< (x=0, y=0, w=3840, h=1938); >] [componentAlpha] [isFixedPosition scrollId=2 sides=0xf anchor=(1920,969)]
>   PaintedLayerComposite (0x247bf4df800) [shadow-visible=< (x=0, y=0, w=3840, h=1938); >] [effective-transform=[ 1 0; 0 1; 0 155; ]] [bounds=(x=0, y=0, w=3840, h=1938)] [visible=< (x=0, y=0, w=3840, h=1938); >] { hitregion=< (x=0, y=0, w=3840, h=1938); > dispatchtocontentregion=< (x=3806, y=0, w=34, h=1938); >} [valid=< (x=0, y=0, w=3840, h=1938); >]
>     ContentHost (0x247bb887190) [buffer-rect=(x=0, y=0, w=3840, h=1938)] [buffer-rotation=(0,0)]
>       TextureHost (0x247bf4ca1f0) [size=(w=3840, h=1938)] [format=SurfaceFormat::B8G8R8A8] [flags=NoFlags]
>   ContainerLayerComposite (0x247bf4de800) [shadow-transform=[ 1 0; 0 1; 0 88.8846; ]] [shadow-visible=< (x=3806, y=100, w=34, h=425); >] [effective-transform=[ 1 0; 0 1; 0 256.514; ]] [visible=< (x=3806, y=100, w=34, h=425); >] [vscrollbar=3]
>     PaintedLayerComposite (0x247bf4dec00) [shadow-transform=[ 1 0; 0 1; 3806 100; ]] [shadow-visible=< (x=0, y=0, w=34, h=425); >] [transform=[ 1 0; 0 1; 3806 100; ]] [effective-transform=[ 1 0; 0 1; 3806 357; ]] [bounds=(x=0, y=0, w=34, h=425)] [visible=< (x=0, y=0, w=34, h=425); >] { hitregion=< (x=0, y=0, w=34, h=425); > dispatchtocontentregion=< (x=0, y=0, w=34, h=425); >} [opaqueContent] [valid=< (x=0, y=0, w=34, h=425); >]
>       ContentHost (0x247bf4bf7f0) [buffer-rect=(x=0, y=0, w=64, h=425)] [buffer-rotation=(0,0)]
>         TextureHost (0x247bf4ca280) [size=(w=64, h=425)] [format=SurfaceFormat::B8G8R8X8] [flags=NoFlags]
>   PaintedLayerComposite (0x247b628d800) [shadow-clip=(x=0, y=0, w=3806, h=1938)] [shadow-transform=[ 1 0; 0 1; 0 -673.433; ]] [transform=[ 1 0; 0 1; 0 -288; ]] [effective-transform=[ 1 0; 0 1; 0 -573; ]] [not visible] { hitregion=< (x=0, y=0, w=3806, h=8204); >} [opaqueContent] [metrics0={ [cb=(x=0.000000, y=0.000000, w=3806.000000, h=1938.000000)] [sr=(x=0.000000, y=0.000000, w=1903.000000, h=4101.750000)] [s=(0,144)] [dp=(x=0.000000, y=-144.000000, w=1903.000000, h=3456.000000)] [cdp=(x=0.000000, y=0.000000, w=0.000000, h=0.000000)] [color=rgba(0, 0, 0, 0.549020)] [scrollId=3] [scrollParent=2] [clip=(x=0, y=0, w=3806, h=1938)] [z=2] }]
>   ContainerLayerComposite (0x247b0319400) [shadow-clip=(x=0, y=0, w=3806, h=1938)] [shadow-transform=[ 1 0; 0 1; 1263 -553.433; ]] [shadow-visible=< (x=-62, y=-5, w=1418, h=6797); >] [transform=[ 1 0; 0 1; 1263 -168; ]] [effective-transform=[ 1 0; 0 1; 1263 -453; ]] [visible=< (x=-62, y=-5, w=1418, h=6797); >] [componentAlpha] [metrics0={ [cb=(x=0.000000, y=0.000000, w=3806.000000, h=1938.000000)] [sr=(x=0.000000, y=0.000000, w=1903.000000, h=4101.750000)] [s=(0,144)] [dp=(x=0.000000, y=-144.000000, w=1903.000000, h=3456.000000)] [cdp=(x=0.000000, y=0.000000, w=0.000000, h=0.000000)] [color=rgba(0, 0, 0, 0.549020)] [scrollId=3] [scrollParent=2] [clip=(x=0, y=0, w=3806, h=1938)] [z=2] }] [usesTmpSurf]
>     PaintedLayerComposite (0x247b0aab000) [shadow-visible=< (x=-62, y=-5, w=1417, h=6797); >] [bounds=(x=-62, y=-6, w=1418, h=7950)] [visible=< (x=-62, y=-5, w=1417, h=6797); >] { hitregion=< (x=1322, y=-5, w=32, h=5); (x=0, y=0, w=1284, h=52); (x=1322, y=0, w=32, h=52); (x=0, y=52, w=1284, h=248); (x=-160, y=300, w=1600, h=244); (x=0, y=544, w=1284, h=7540); >} [componentAlpha] [valid=< (x=-62, y=-5, w=1417, h=6797); >]
>       ContentHost (0x247afc2d060) [buffer-rect=(x=-62, y=-5, w=1417, h=6797)] [buffer-rotation=(0,0)]
>         TextureHost (0x247afe4ffb0) [size=(w=1417, h=6797)] [format=SurfaceFormat::B8G8R8X8] [flags=]

We have a container layer for position:fixed, with four children. The first two are the scrollbar, the third is an event region, and the fourth is a ContainerLayer for a CSS transform on #permalink-overlay-dialog. This ContainerLayer is scrolled, [usesTmpSurf] and [componentAlpha]. I think this is the problematic layer.
Whenever we composite, we need to copy the background behind this ContainerLayer into the layer's intermediate surface. If that doesn't work, we'll composite garbage, exactly in the locations that are shown in the video.
So what we should be looking into is the warning I pasted in in comment 13. I think fixing that will fix the garbage.
There are more problems to solve here, but this patch might be an acceptable workaround for now.
The ContainerLayer requires an intermediate surface because its transform is fractional. We should stop APZ from setting a fractional transform on it.
And we should find out why PostProcessLayers doesn't clip the ContainerLayer's visible region to its clip rect (transformed into its own space). Once that's working correctly, the copy rect should be almost completely contained in the source rect, except for an 1px border due to the fractional transform.
Ryan, can you test a build from the try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b306f835b5a once the one you want has finished?
I wrote this patch blindly, I hope it compiles.
It works!
Yay!
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Flags: needinfo?(matt.woodrow)
Comment on attachment 8729180 [details]
MozReview Request: Bug 1251894 - In CompositorD3D11::CreateTexture, copy as much as the render target allows. r?Bas

https://reviewboard.mozilla.org/r/39281/#review35999

I'm not a huge fan of this, as I'd rather see us not call this function with invalid sizes. I feel it's the responsibility of callers to prove they know what they're doing and not do this. However I see the practical value in fixing this now, please add an ASSERT (or something else that's annoying in debug builds) so that this will be fixed ASAP.
Attachment #8729180 - Flags: review?(bas) → review+
See Also: → 1255607
I promised Bas to fix the caller soon, and I don't see much value of pain driven development, so I'm going to land it with the warning as an NS_WARNING.
https://hg.mozilla.org/mozilla-central/rev/4b497ec84814
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment on attachment 8729180 [details]
MozReview Request: Bug 1251894 - In CompositorD3D11::CreateTexture, copy as much as the render target allows. r?Bas

Approval Request Comment
[Feature/regressing bug #]: APZ
[User impact if declined]: strange artifacts / smearing / flashing during scrolling on some pages on Windows with the D3D11 compositor
[Describe test coverage new/current, TreeHerder]: none
[Risks and why]: low, this fix was specifically designed to be low impact
[String/UUID change made/needed]: none
Attachment #8729180 - Flags: approval-mozilla-beta?
Attachment #8729180 - Flags: approval-mozilla-aurora?
Duplicate of this bug: 1236593
RyanVM, could you please verify the fix works as expected on a latest Nightly build? Thanks!
Flags: needinfo?(ryanvm)
Yeah, working fine.
Status: RESOLVED → VERIFIED
Flags: needinfo?(ryanvm)
Comment on attachment 8729180 [details]
MozReview Request: Bug 1251894 - In CompositorD3D11::CreateTexture, copy as much as the render target allows. r?Bas

The fix was verified, Aurora47+. Will let Liz decide about the beta uplift.
Attachment #8729180 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Will this be ok on 46 even if APZ is not enabled? I'm assuming so but thought it good to check since we are likely disabling APZ before 46 goes to release.
Flags: needinfo?(mstange)
Comment on attachment 8729180 [details]
MozReview Request: Bug 1251894 - In CompositorD3D11::CreateTexture, copy as much as the render target allows. r?Bas

Since we are turning on apz for a couple of weeks in beta, let's uplift this.
This should land in time for beta 4.
Attachment #8729180 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #34)
> Will this be ok on 46 even if APZ is not enabled?

Yes, this patch does not have any bad effects if APZ is off.
Flags: needinfo?(mstange)
You need to log in before you can comment on or make changes to this bug.