788411 - [Otoro] Lock screen arrow animation is not shown

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

STR:

1. Boot up the phone and observe lock screen

Exepected:

1. There should be a semi-transparent arrow travels from the handle to the icons, and iterates

Actual:

1. Nothing

Note:

* Arrow is done with CSS animation, see |@keyframes lockscreen-arrow-left| and -right in lockscreen.css
* Arrow is shown on SGS2 and B2G/Desktop

Comment 2

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

Thanks for filing this.  I noticed this problem when testing something unrelated on desktop-b2g.

Comment 3

[Andrew Overholt [:overholt]]

Seems like a usability-related blocker.

cjones, who should own this?

Comment 4

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

dz, if he's around.

Comment 5

[David Zbarsky (:dzbarsky)]

I don't have an otoro, and I can't reproduce this on the Nexus s.

Comment 6

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

Interesting.  Might be a gralloc issue.

Comment 7

[Kan-Ru Chen [:kanru] (UTC+9)]

I found that if we put some text in the div then the animation becomes visible.

Comment 8

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

We're allocating some 12x22 and 50x50 buffers.  Not sure which are the arrows.  Probably triggering a rounding bug somewhere.  Might also be an extreme version of the cache-flush bug, but I never see any pixels so I'd guess this is separate.

Comment 9

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

Attachment: Round gralloc buffer allocations up to a minimum of 64x64

This falls into the category of, I don't know why things break below 64x64 and I probably don't want to know.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 662812 [details] [diff] [review]
Round gralloc buffer allocations up to a minimum of 64x64

Review of attachment 662812 [details] [diff] [review]:
-----------------------------------------------------------------

How is this safe? Don't we need to adjust texture coordinates when fetching from these buffers?

Comment 11

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

Comment on attachment 662812 [details] [diff] [review]
Round gralloc buffer allocations up to a minimum of 64x64

It's not, major thinko.  I did the testing hack with the thebesbuffer directly.

So currently, on ThebesLayers are set up to handle this case properly.  That means we can either
 - add this support to all other layer types for all other compositors.  Phew!
 - make the thebesbuffer rect a minimum of 64x64 (or dimensions multiples of 64, which isn't a bad idea in general to avoid reallocs).  Somewhat complicated to handle along with resolution scaling.
 - bail out of the gralloc fastpath when the surface dims are < 64x64.  This would be fine as long as there weren't a ton of little buffers like this.

Kinda leaning towards the last one.  Let me know if you have a strong opinion.

Comment 12

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

(In reply to Chris Jones [:cjones] [:warhammer] from comment #11)
>  - bail out of the gralloc fastpath when the surface dims are < 64x64.  This
> would be fine as long as there weren't a ton of little buffers like this.
> 

Hm, this workaround has the disadvantage of not actually working.  (Even disabling gralloc in TextureImageEGL.)  I guess the GL impl doesn't like textures < 64x64.

Comment 13

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

Guys, this bug might be relevant to your interests.

Comment 14

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

Attachment: Gaia workaround

Nuclear option.  It would be extremely sad and embarrassing to have to go here, though.

Comment 15

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #14)
> Created attachment 662845 [details] [diff] [review]
> Gaia workaround
> 
> Nuclear option.  It would be extremely sad and embarrassing to have to go
> here, though.

That doesn't look too embarrassing to me. Do you want to check-in this in Gaia and continuing fix this bug by testing on an old Gaia?

Comment 16

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

Only the width needs to be <= 64.

Comment 17

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

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #15)
> (In reply to Chris Jones [:cjones] [:warhammer] from comment #14)
> > Created attachment 662845 [details] [diff] [review]
> > Gaia workaround
> > 
> > Nuclear option.  It would be extremely sad and embarrassing to have to go
> > here, though.
> 
> That doesn't look too embarrassing to me. Do you want to check-in this in
> Gaia and continuing fix this bug by testing on an old Gaia?

It's pretty bad.  I have another workaround/~fix for general web content (thebes layers); let's see how that one goes.  It'll still leave <canvas> and <video> below 64x64 in the lurch, but let's hope they're uncommon.

A vendor fix is also possible.

Comment 18

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

Attachment: Set a minimum allocation width for buffers that ThebesLayerBuffer

Doesn't help <canvas> or <video>, and doesn't make me very happy, but those are other issues to prioritize.  (Excepting me being happy.)

This is a fairly reasonably thing to do when allocating gfx surfaces, though likely redundant with most gfx allocators we'll see.

Maybe we can get a vendor fix.

Comment 19

[Michael Vines [:m1] [:evilmachines]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #8)
> Might also be an extreme version of the cache-flush bug, but I never
> see any pixels so I'd guess this is separate.

Globally disabling all pmem caching per https://bugzilla.mozilla.org/show_bug.cgi?id=784244#c42 does not bring the arrows back so definitely a separate issue.

Comment 20

[Diego Wilson [:diego]]

Interestingly enough, this works fine with Hardware composing (WIP)

Comment 21

[Diego Wilson [:diego]]

So my best guess is the problem is happening in the GPU composition phase

Comment 22

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

(In reply to Diego Wilson from comment #20)
> Interestingly enough, this works fine with Hardware composing (WIP)

Oh, interesting.  Thanks for checking.

I'm going to open a new bug to track a possible fix for canvas and video.  If that's hwcompositor, which should be a 99.9% workaround, that's good enough for me for v1.

Comment 23

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/8d579bf6e2de

Comment 24

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

Filed bug 792966.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8d579bf6e2de