Loic Reporter
	Description • 8 years ago

STR:
1) Enable e10s
2) Open canvas https://bugzilla.mozilla.org/attachment.cgi?id=8727834
3) Go to Print Preview
4) Scroll down and up fastly with the mouse wheel

Result: flickering during drawing box-shadow around the canvas when scrolling.

Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=cbe9a2aea9541146ae0a11051ed32d31eaf4e428&tochange=5ddf0a252b086c7e8e468243c6251af5c0b1da57

Mason Chang — Bug 1155828 - Draw box-shadows using an approach inspired by border-image. r=mstange

	Mason Chang [Inactive] [:mchang]
	Comment 1 • 8 years ago

I can only reproduce this on Windows 7. If you set the preference "layers.acceleration.disabled" to true, restart firefox, does this still happen for you?

I can't reproduce this on Windows 10 or if I don't use d3d for content. The canvas preference doesn't seem to matter, which is odd since it's a canvas page.

	Mason Chang [Inactive] [:mchang]
	Comment 2 • 8 years ago

Even with deleting the box shadow code, I can still reproduce the flashing. The box shadow code just seems to make the condition where we flash happen much more often.

	Mason Chang [Inactive] [:mchang]
	Comment 4 • 8 years ago

If I disable APZ, I only see sometimes one initial flash but continually scrolling no longer flashes the content. With APZ, I can get pretty easily reproduce the flashing.

	Loic Reporter
	Comment 5 • 8 years ago

Yes, I noticed too that disabling HWA (but with e10s enabled) works fine.

	Mason Chang [Inactive] [:mchang]
	Comment 6 • 8 years ago

Something on nightly landed today where I can't reproduce the flashing anymore. I also went back to beta (45 w/ forced enabled e10s) and developer edition (46) and can't reproduce it there. Can you reproduce it on today's nightly (March 9th)?

	Loic Reporter
	Comment 7 • 8 years ago

Yes, it's reproducible with e10s disabled and HWA enabled: http://i.imgur.com/HSkLPQy.jpg

With e10s enabled, it's visible if you use the vertical scrollbar fastly.

	Mason Chang [Inactive] [:mchang]
	Comment 8 • 8 years ago

(In reply to Loic from comment #7)
> Yes, it's reproducible with e10s disabled and HWA enabled:
> http://i.imgur.com/HSkLPQy.jpg
> 
> With e10s enabled, it's visible if you use the vertical scrollbar fastly.

Is it significantly better than before? I used to be able to scroll and it'd happen every frame. Now it's once every couple of seconds and only if I use the vertical scrollbar. I can't reproduce it with the wheel anymore.

Could you still reproduce it on release w/ e10s disabled? I'd still get it only once on first load.

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

Based on IRC discussion with mchang, this isn't APZ related after all. At least not directly.

	Loic Reporter
	Comment 10 • 8 years ago

(In reply to Mason Chang [:mchang] from comment #8)
> (In reply to Loic from comment #7)
> > Yes, it's reproducible with e10s disabled and HWA enabled:
> > http://i.imgur.com/HSkLPQy.jpg
> > 
> > With e10s enabled, it's visible if you use the vertical scrollbar fastly.
> 
> Is it significantly better than before? I used to be able to scroll and it'd
> happen every frame. Now it's once every couple of seconds and only if I use
> the vertical scrollbar. I can't reproduce it with the wheel anymore.
> 
> Could you still reproduce it on release w/ e10s disabled? I'd still get it
> only once on first load.

Yes it's better with e10s enabled, and I need to scroll fastly with the vertical scrollbar to see the issue. But with e10s off, the issue is the same.

	Mason Chang [Inactive] [:mchang]
	Comment 11 • 8 years ago

It wasn't the image lib, but bug 1192910 fixed the issue locally for me. Part 2 of the patch, https://hg.mozilla.org/mozilla-central/rev/3e9a803d69ed makes it a lot harder for this to happen.

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

(In reply to Mason Chang [:mchang] from comment #11)
> It wasn't the image lib, but bug 1192910 fixed the issue locally for me.
> Part 2 of the patch, https://hg.mozilla.org/mozilla-central/rev/3e9a803d69ed
> makes it a lot harder for this to happen.

That's quite surprising to me. That code shouldn't be running in production. Can you stick a breakpoint somewhere in there and see if it's actually getting hit?

	Mason Chang [Inactive] [:mchang]
	Comment 13 • 8 years ago

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #12)
> (In reply to Mason Chang [:mchang] from comment #11)
> > It wasn't the image lib, but bug 1192910 fixed the issue locally for me.
> > Part 2 of the patch, https://hg.mozilla.org/mozilla-central/rev/3e9a803d69ed
> > makes it a lot harder for this to happen.
> 
> That's quite surprising to me. That code shouldn't be running in production.
> Can you stick a breakpoint somewhere in there and see if it's actually
> getting hit?

Yeah that code isn't running. I think somehow the patch probably just changed the timing enough to make the bug go away. I can't reproduce the bug either in debug builds / visual studio graphics analyzer either :/ builds.

	Jim Mathies [:jimm]
	Comment 14 • 8 years ago

e10s and non-e10s related issue.

	Mason Chang [Inactive] [:mchang]
	Comment 15 • 8 years ago

Probably a dupe of 1253386.

	Mason Chang [Inactive] [:mchang]
	Comment 16 • 8 years ago

Can you try gecko 40 and see if scrolling is super janky instead of flashing? - https://ftp.mozilla.org/pub/firefox/releases/40.0/win32/en-US/

	Mason Chang [Inactive] [:mchang]
	Comment 17 • 8 years ago

I've been trying to debug this but haven't gotten anywhere. On e10s, this was fixed by APZ by bug 1192910, because we stopped painting every frame. On non-e10s, we still paint all the time since we're doing synchronous scrolling. I think this is triggering the race condition in bug 1251257 but I can't reproduce it without e10s. With e10s and without bug 1192910, I can reproduce the flashing, but any debug information I try to get makes the bug go away. Debug builds and more than 1-2 printfs also make the bug go away. Any tooling with visual studio also makes the bug go away. There isn't anything special with having a print view and we still render everything with acceleration. There isn't anything in the display list dump that indicates special rendering as component alpha is only used for subpixel AA.

I highly suspect that the problem didn't appear before bug 1155828 because we took 100+ms to render the box shadow, so the race didn't happen and we'd just be too slow such that the flashing would be replaced by jank.

	Loic Reporter
	Comment 18 • 8 years ago

It's still happening for me with the latest Nightly and e10s, and no need to scroll fastly, just use the scrollbar with a very low medium speed.

I attached a screencast, sorry for the quality but I needed to use a dumbphone to catch the flashing when scrolling, because using online screencast tool with Java adds latency and the flashing doesn't appear.

	Loic Reporter
	Comment 19 • 8 years ago

And a screenshot.

	Mason Chang [Inactive] [:mchang]
	Comment 20 • 8 years ago

Yes, it is still happening on nightly. From comment 16, can you please download gecko 40 and see if instead you only get jank.

	Loic Reporter
	Comment 21 • 8 years ago

(In reply to Mason Chang [:mchang] from comment #20)
> Yes, it is still happening on nightly. From comment 16, can you please
> download gecko 40 and see if instead you only get jank.

It's not janky, 40 behaves like 45, no more, no less.

	Mason Chang [Inactive] [:mchang]
	Comment 22 • 8 years ago

(In reply to Loic from comment #21)
> (In reply to Mason Chang [:mchang] from comment #20)
> > Yes, it is still happening on nightly. From comment 16, can you please
> > download gecko 40 and see if instead you only get jank.
> 
> It's not janky, 40 behaves like 45, no more, no less.

IF this is the case, then bug 1155828 is not at fault here.

	Mason Chang [Inactive] [:mchang]
	Comment 23 • 8 years ago

You don't even need a canvas page to cause the flashing. It happens less frequently, but still happens.

	Mason Chang [Inactive] [:mchang]
	Comment 25 • 8 years ago

Can't reproduce w/ nvidia gpu

	Mason Chang [Inactive] [:mchang]
	Comment 26 • 8 years ago

Can you attach your about:support here please?

	Mason Chang [Inactive] [:mchang]
	Comment 28 • 8 years ago

Can you try one of these builds and see if it fixes the issue for you? Thanks!

https://archive.mozilla.org/pub/firefox/try-builds/mchang@mozilla.com-c0320bfa6a2f050aaf1f3d224e9c177daa372f52/try-win32/

	Mason Chang [Inactive] [:mchang]
	Comment 29 • 8 years ago

Also please try with and without e10s.

	Loic Reporter
	Comment 30 • 8 years ago

It seems to be good with this build, I tried both e10s and non-e10s, with slow and fast scrolling, no more graphics glitches in print preview.

	Loic Reporter
	Comment 31 • 8 years ago

NB: I tested with the canvas X cross in print preview.

	Mason Chang [Inactive] [:mchang]
	Comment 32 • 8 years ago

This patch backs out bug 1192910 and adds a keyed mutex to every shared texture instead of using a single global one as was added in bug 1088414. This is only for testing purposes, but it fixes the issue for both me and the reporter in comment 30.

	Mason Chang [Inactive] [:mchang]
	Comment 34 • 8 years ago

Forced single buffering [1] makes the flashing almost perma flash.

[1] https://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/ContentClient.cpp?from=ContentClient.cpp#95

	Mason Chang [Inactive] [:mchang]
	Comment 37 • 8 years ago

I actually really still don't have a strong clue about what's going on with this bug. I think it's either (a) a deep layout bug (b) race condition because we rely on a single texture to lock and hope that the GPU syncs all the textures across processes or (c) we already have an existing bug where the second page of the canvas isn't shown, so there might be some race condition where layout isn't painting the second page blank white sometimes.

For some of the suspicious things in the display list like the rgba(1,1,1,1) BackgroundColor or CanvasBackgroundColor, I actually instrumented DrawTarget::FillRect to see what color we were filling, and it wasn't black. Because this also happens in a super simple test case outside of canvas in attachment 8734472 [details], I'm leaning a lot more towards (b) rather than a layout bug. I think this only happens in print preview because we invalidate half of the screen every single scroll, which may cause the race to happen much more frequently.

	Mason Chang [Inactive] [:mchang]
	Comment 38 • 8 years ago

Can you please take a look at the display list dumps? I don't see anything that strikes me as odd, does anything intrigue you? Thanks!

	Matt Woodrow (:mattwoodrow)
	Comment 39 • 8 years ago

Nothing really stands out, but they are big dumps.

It seems like the problem is that changing the timing (by adding keyed mutex) might potentially be fixing other races.

I think the first step is to confirm that the rendering issue is within a single layer, not to do with compositing.

After that, can you get a moz2d recording of the broken behaviour (without forcing single buffering)?

That way you should be able to play with the timing of the playback to determine if it reproduces there or not and decide if it's a bug in the d2d command stream we're sending, or racing to get them completed.

	Mason Chang [Inactive] [:mchang]
	Comment 40 • 8 years ago

Can you try this test program to see if you have any flashing of any of the boxes? Thanks!

	Loic Reporter
	Comment 42 • 8 years ago

(In reply to Mason Chang [:mchang] from comment #40)
> Created attachment 8742970 [details]
> Texture sharing test case
> 
> Can you try this test program to see if you have any flashing of any of the
> boxes? Thanks!

	Loic Reporter
	Comment 43 • 8 years ago

I don't have any flashing for the boxes with the test program.

	Mason Chang [Inactive] [:mchang]
	Comment 44 • 8 years ago

Since this isn't due to texture sharing, I'm not really sure what's left to check on the graphics side. Someone from layout should probably investigate why this page constantly invalidates everything. (Probably have to disable APZ or backout bug 1192910 with APZ). Unassigned since I can't pin down anything with gfx atm.

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

OK, let's try layout.