Ryan VanderMeulen [:RyanVM] Reporter
	Description • 8 years ago

Only happens with the mouse wheel, so I assume it's APZ.

	Milan Sreckovic [:milan] (needinfo for best results)
	Comment 1 • 8 years ago

What's your graphics configuration?  You don't have an Nvidia card by any chance?

	Ryan VanderMeulen [:RyanVM] Reporter
	Comment 2 • 8 years ago

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

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 3 • 8 years ago

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?

	Ryan VanderMeulen [:RyanVM] Reporter
	Comment 4 • 8 years ago

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.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 5 • 8 years ago

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?

	Markus Stange [:mstange] Assignee
	Comment 6 • 8 years ago

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.

	Ryan VanderMeulen [:RyanVM] Reporter
	Comment 7 • 8 years ago

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.

	Ryan VanderMeulen [:RyanVM] Reporter
	Comment 8 • 8 years ago

Bug 1231538 that is :)

	Markus Stange [:mstange] Assignee
	Comment 9 • 8 years ago

Can you set layout.display-list.dump to true and attach the output to this bug?

	Ryan VanderMeulen [:RyanVM] Reporter
	Comment 10 • 8 years ago

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.

	Ryan VanderMeulen [:RyanVM] Reporter
	Comment 11 • 8 years ago

Where will the dump be saved to?

	Ryan VanderMeulen [:RyanVM] Reporter
	Comment 12 • 8 years ago

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.

	Markus Stange [:mstange] Assignee
	Comment 13 • 8 years ago

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?

	Ryan VanderMeulen [:RyanVM] Reporter
	Comment 14 • 8 years ago

4K resolution in case it matters.

	Markus Stange [:mstange] Assignee
	Comment 15 • 8 years ago

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.

	Bas Schouten (:bas.schouten)
	Comment 17 • 8 years ago

(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.

	Markus Stange [:mstange] Assignee
	Comment 18 • 8 years ago

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.

	Markus Stange [:mstange] Assignee
	Comment 19 • 8 years ago

Review commit: https://reviewboard.mozilla.org/r/39281/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39281/

	Markus Stange [:mstange] Assignee
	Comment 20 • 8 years ago

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.

	Markus Stange [:mstange] Assignee
	Comment 21 • 8 years ago

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.

	Ryan VanderMeulen [:RyanVM] Reporter
	Comment 22 • 8 years ago

It works!

	Markus Stange [:mstange] Assignee
	Comment 23 • 8 years ago

Yay!

	Bas Schouten (:bas.schouten)
	Comment 24 • 8 years ago

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.

	Markus Stange [:mstange] Assignee
	Comment 25 • 8 years ago

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.

	Pulsebot
	Comment 26 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b497ec84814

	Carsten Book [:Tomcat]
	Comment 27 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/4b497ec84814

	Markus Stange [:mstange] Assignee
	Comment 28 • 8 years ago

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

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 30 • 8 years ago

RyanVM, could you please verify the fix works as expected on a latest Nightly build? Thanks!

	Ryan VanderMeulen [:RyanVM] Reporter
	Comment 31 • 8 years ago

Yeah, working fine.

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 32 • 8 years ago

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.

	Carsten Book [:Tomcat]
	Comment 33 • 8 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-aurora/rev/7a0624f58e6f

	Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)
	Comment 34 • 8 years ago

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.

	Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)
	Comment 35 • 8 years ago

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.

	Markus Stange [:mstange] Assignee
	Comment 36 • 8 years ago

(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.

	Carsten Book [:Tomcat]
	Comment 37 • 8 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/4929e9426cc4