799644 - Graphical corruption with opacity and canvas in Maps app

Status: VERIFIED FIXED

(Core :: Graphics: Layers, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

STR
 (1) Open Maps app
 (2) Press "Layers" button
 (3) Dismiss layers popover

This causes artifacts in the thebes-layer content that's overlayed on the canvas.  See below.

http://people.mozilla.com/~cjones/b2g-bugs/maps-invalidation.png

I think this is a grotty enough artifact that it should block, but a workaround in the Maps app is certainly on the table.  Would prefer a platform fix though.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I suspect that this isn't an invalidation problem per se, but more likely either (i) marking ThebesLayers as opaque when they shouldn't be, or (ii) a bug in front->back buffer copy.

Possibly related to bug 796831.

Kan-Ru, do you have cycles to investigate?

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I can only sporadically reproduce on desktop using the steps from comment 0.  It's also intermittent on device, but much easier to reproduce.  However, I found a better way

STR
 (1) Press the "grid" button in the top-right of the UI to pop up the "Welcome!" menu
 (2) Tap anywhere to dismiss it
 (3) GOTO step (1)

With each press, you'll see the faded-out background get darker and darker.  The UI supposed to look something like this

http://people.mozilla.com/~cjones/b2g-bugs/maps-one-tap.png

but if you tap enough times, you can make it look like this

http://people.mozilla.com/~cjones/b2g-bugs/maps-many-taps.png

Definitely something to do with not clearing or preserving opacity in an RGBA buffer.

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

The relevant shadow-layer subtree is

1865447232[1420850]:           OGLShadowContainerLayer (0x2d34678) [shadow-transform=[ 1 0; 0 1; 0 20; ]] [shadow-visible=< (x=0, y=0, w=320, h=460); >] [visible=< (x=0, y=0, w=320, h=460); >] [opaqueContent] [metrics={ viewport=(x=0.000000, y=0.000000, w=320.000000, h=460.000000) viewportScroll=(x=0.000000, y=0.000000) displayport=(x=0.000000, y=0.000000, w=0.000000, h=0.000000) scrollId=1 }]
1865447232[1420850]:             OGLShadowThebesLayer (0x24ef818) [shadow-visible=< (x=0, y=0, w=320, h=460); >] [visible=< (x=0, y=0, w=320, h=460); >] [opaqueContent] [valid=< (x=0, y=0, w=320, h=460); >]
1865447232[1420850]:             OGLShadowCanvasLayer (0x3b61228) [shadow-clip=(x=0, y=0, w=320, h=460)] [shadow-transform=[ 1 0; 0 1; 0 50; ]] [shadow-visible=< (x=0, y=0, w=320, h=410); >] [clip=(x=0, y=0, w=320, h=460)] [transform=[ 1 0; 0 1; 0 50; ]] [visible=< (x=0, y=0, w=320, h=410); >]
1865447232[1420850]:             OGLShadowThebesLayer (0x355ba88) [shadow-visible=< (x=0, y=0, w=320, h=460); >] [visible=< (x=0, y=0, w=320, h=460); >] [componentAlpha] [valid=< (x=0, y=0, w=320, h=460); >]

^^^ that layer will have an RGBA buffer since componentAlpha is enabled for it.  That's probably where the problem lies.  (We won't enable component alpha on the phone, but that layer should also end up with an RGBA buffer.)

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Even simpler STR

 (1) Tap "grid" button in top-right of UI to pop up the "Welcome!" menu
 (2) Tap to dismiss that popup
 (3) Tap "layers" button in bottom-right

After the animation finishes, a "ghost" of the "Welcome!" UI appears above the layers menu.

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Definitely a bug with shadow layers --- if I force maps in-process, the artifacts are gone.

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

For some reason, in the in-process case, the layers here are marked fixedPosition, but not when OOP ...

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

In the shadow-layers case, we're retaining a ThebesLayer that appears to be responsible for the artifacts across steps (1)-(3).  In the no-shadow-layers case, we're creating a different layer.  Hmm ...

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I think I know what's happening here.  Trying to write a patch but there are many distractions and it's a bit subtle.  Will have something up tonight.

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

What I think is happening is
 - we allocate RGBA back buffer b1
 - we paint b1, send to compositor
 - allocate RGBA back buffer b2
 - paint b2, swap for b1
 - ThebesLayerBuffer starts to paint b1, but it isn't sized properly so we Clear() it
 - TLB allocates buffer b3, paints, sends to compositor
 - we get back b2 and start to paint it.  It happens to be the right size and is RGBA.

However, TLB doesn't know that b2 wasn't cleared when b3 was cleared.  So there's old translucent pixels hanging around in the buffer.

I wrote a patch that /should/ have fixed this, but it didn't.  So I'm not sure my guess is right.

Need to add image tracing to analyze further.  Thank heavens I can repro on desktop!

Comment 10

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Some very basic image tracing tells me that this bug isn't caused by bugs in front->back copy; it seems to be caused by composite-active-layers-to-inactive layer.  (I don't see my front->back sync tracing get hit, but I think that's a bug in my tracing.)  There are two big relatively recent changes to this logic.  First is bug 788044, but on its earliest parent commit

https://hg.mozilla.org/integration/mozilla-inbound/rev/33087122ace7

I see the same bug.  (Amazingly the gaia build actually starts up on that rev!)  The other big change was bug 792663.  However, that work landed after 33087122ace7 so I don't think that's it either, but I'll double check.

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

The bug is also present on https://hg.mozilla.org/integration/mozilla-inbound/rev/ab3709368125 , but manifests itself a bit differently.  So I'm guessing this has been a bug "forever" :(.

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

It also wasn't bug 773192.  I can reproduce the same bug when forcing Maps in-process and enabling omtc.

Comment 13

[Donovan Preston [:fzzzy]]

Thanks for looking into this Chris... Sorry it hasn't been easier to fix :-)

Comment 14

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Need to clear areas of rgba buffers we're about to repaint

The answer was literally staring me in the face for 5 hours this afternoon, in the form of about 10 tabs with data:image/png captures, and it took me that long to put 2+2 together.  Need something stronger than /headdesk for this ...

Comment 15

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0923556aea0e
https://hg.mozilla.org/releases/mozilla-aurora/rev/d5accf7d85e9

Comment 21

[Robert Kaiser]

Is bug 799606 another dupe - or at least the B2G stuff I noted there?

Comment 22

[Joe Drew (not getting mail)]

I figured the fix would be something like this. Sorry I didn't get to bug 793840 sooner.

Comment 23

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

clee, I would like to request this bug as a P1 crit please; this really needs a fix soon because it affects a lot of apps, and places in gaia/b2g.

Comment 25

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I've been treating this as P1/critical and dropping other work to fix it.  A patch has already landed.  What more do you want?

Comment 26

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0923556aea0e

Comment 27

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

@cjones... oh awesome.... didn't realize it.  Can I get a chocolate hat too?  :)

Comment 28

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

;)

Comment 33

[John Shih]

verified

2012-11-09
gaia master : 18107c6961b8c3f559c82fc7be8ba411c4a67c28
mozilla-aurora(hg) : 6735d6d13751