960683 - OMTA Animations and Invalidation of Occluded Regions

Status: RESOLVED WORKSFORME

(Core :: Graphics, defect, P1)

Description

[Mason Chang [Inactive] [:mchang]]

Attachment: Video of Black Region after Keyboard Hides

In the homescreen app, the black background bleeds through for a bit as the keyboard animation occurs since we didn't paint the homescreen background yet. One potential solution is to change gecko to invalidate layers underneath an animation to pre render the newly visible region. See attached video.

Comment 1

[Andreas Gal :gal]

You probably want to split off the sub bugs as discussed in the thread.

Comment 2

[Benoit Girard (:BenWa)]

Can you try this again with will-change: transform?

Comment 3

[Mason Chang [Inactive] [:mchang]]

Attaching will-change transform to the homescreen background seems to speed up the repainting of the background, but we still get a blip of black.

Comment 4

[Andreas Gal :gal]

Why is this not retaining the background? Are we changing the visibility area async and race with that? (even though its painted)

Comment 5

[Mason Chang [Inactive] [:mchang]]

Attachment: Layer Tree of the homescreen w/ keyboard visible

Comment 6

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

Actually I don't understand why this is happening even without will-change.

When OMTA begins the animated element is wrapped in an nsDisplayTransform. nsDisplayTransform::ComputeVisibility treats everything as fully transparent for the purposes of determining occlusion; i.e. nothing under the transformed element will be occluded. So the layer transaction that sets the animation for OMTA should also ensure that layers under the animated element are properly updated to render correctly even if the animated element disappears entirely.

Maybe I misunderstood the explanation in the email thread or here.

Comment 7

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

(In reply to Mason Chang [:mchang] from comment #5)
> Created attachment 8361353 [details]
> Layer Tree of the homescreen w/ keyboard visible

I assume the homescreen layer subtree root is 0x456c7c00. All the layers there have height 256. I wonder why that is. I assume it must be because the height of the <iframe> that contains the homescreen is reduced while the keyboard is visible.

If that's true, then the problem is nothing to do with occlusion processing, but it's what Benoit said in email:
> The black flashing we're seeing is because different processes feed different transaction and we don't have a way of
> waiting for transaction from multiple processes. A possible solution to that may be to tell the compositor that it
> can't update the frame until it receive a transaction from all processes involved in the change

I.e. the system process resizes the homescreen <iframe> to the full screen and starts OMTA of the keyboard disappearing at the same time. There is nothing to ensure that the compositor gets a transaction updating the homescreen's layer subtree before we composite a frame where the keyboard has moved.

We could solve this by saying that a resize of a remote iframe means we delay sending any layer updates until the remote iframe's process has completed a layer transaction. But that would mean blocking system process rendering on subprocesses which would be bad. We kinda want to opt into the behavior here. Probably some way for content to explicitly wait for an <iframe> subprocess to complete a layer transaction.

Comment 8

[Andreas Gal :gal]

Why are we resizing the iframe?

Comment 9

[Benoit Girard (:BenWa)]

Imagine you click on a textbox at the bottom of the screen. When the keyboard comes above it the textbox will hide it. We solve this problem by resizing the page and letting the reflow adjust the page. If we didn't do this the keyboard would come over the field and it wouldn't be movable into view.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) (away January 20-24) from comment #7)
> We could solve this by saying that a resize of a remote iframe means we
> delay sending any layer updates until the remote iframe's process has
> completed a layer transaction. But that would mean blocking system process
> rendering on subprocesses which would be bad. We kinda want to opt into the
> behavior here. Probably some way for content to explicitly wait for an
> <iframe> subprocess to complete a layer transaction.

I think blocking the compositor is a big no-no. It's too much of a footgun to do here and there and will ruin APZC. The keyboard will already have will-change set so the animation will be ready on the compositor (and if it isn't we can make sure will-change is properly set). We ask the app to resize to fill the space under the keyboard. Then when the app fires mozAfterPaint we forward that to the keyboard and tell it to start hiding.

Comment 10

[Andreas Gal :gal]

Yeah the gaia fix sounds right for this.

Comment 11

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

(In reply to Benoit Girard (:BenWa) from comment #9)
> We ask the app to
> resize to fill the space under the keyboard. Then when the app fires
> mozAfterPaint we forward that to the keyboard and tell it to start hiding.

Sure, if we can do that without races. I'm afraid of the case where the app is doing a sequence of paints already and we ask it to resize. How can we tell when it has completed a transaction with the new size? I think we might need new API, e.g. to extend the paint notification event with information about the size of the viewport assumed by the transaction.

Comment 12

[Benoit Girard (:BenWa)]

We can do the modification in the app. Then we register rAF. When that fires we request a second rAF. After you make a change on the second rAF you know that the transaction with your original change has been completed (although not necessarily composited).

Comment 13

[Mason Chang [Inactive] [:mchang]]

Attachment: homescreen.patch

Gaia Patch which moves the wallpaper background into the homescreen app and out of the parent system app.

Comment 14

[Mason Chang [Inactive] [:mchang]]

Attachment: raf.patch

Patch that uses two window.requestAnimationFrame calls before we start the hide keyboard animation. Black background issue still shows up.

Comment 15

[Mason Chang [Inactive] [:mchang]]

Attachment: Layer Tree During Keyboard Hide Animation / Frame Resizing

Layer tree of the following actions:

1) we resize the frame to be full screen
2) wait 2 requestAnimationFrames 
3) start the keyboard hide animation.

Comment 16

[Mason Chang [Inactive] [:mchang]]

Brain dump before I keep digging and routes I tried:

1) I tried using MozAfterPaint. This didn't work and we still had the black background, because we paint often. In addition, MozAfterPaint is only available with a pref on, which is off by default for b2g. Not sure we want to always enable MozAfterPaint events because of the overhead.

2) Tried with requestAnimationFrames, still don't have a black background during the animation. Once the animation is over though, it snaps a bit faster to the homescreen image.

3) It works if we don't remove the homescreen background from the system app. Instead of painting the black background, we probably over paint the same background image on both the homescreen app + system app, so during the keyboard hide animation, we just see the homescreen from the parent app. It's a simple hack that works if we want to go that route.

4) Do something that isn't Gaia related and make Gecko Changes instead as discussed previously.

Comment 17

[Mason Chang [Inactive] [:mchang]]

Comment on attachment 8363415 [details] [diff] [review]
raf.patch

Hi Rudy,

Just wanted to see if you could please check to see if this was the right place to delaying the keyboard animation. What we'd like to do is delay the keyboard hide animation until we resized the homescreen app. Or if I'm missing some other critical piece to hiding the keyboard.

Thanks!

Comment 18

[Mason Chang [Inactive] [:mchang]]

Attachment: Layer Tree w/ Keyboard Visible on Homescreen

Comment 19

[Mason Chang [Inactive] [:mchang]]

Attachment: Layer Tree w/ Keyboard Visible on Homescreen

Comment 20

[Mason Chang [Inactive] [:mchang]]

Attachment: layerScope.mov

Layer Scope of keeping the keyboard open, searching, pushing home button and the keyboard stays open. See anything interesting?

I was thinking of another Gaia hack. Maybe we can resize the keyboard app only after we've resized the parent app, instead of only starting the animation after we've resized the parent app. Once the parent app has finished resizing, we can resize the keyboard app and leave everything else alone.

Comment 21

[Benoit Girard (:BenWa)]

(In reply to Mason Chang [:mchang] from comment #20)
> Created attachment 8364771 [details]
> layerScope.mov
> 
> Layer Scope of keeping the keyboard open, searching, pushing home button and
> the keyboard stays open. See anything interesting?

Did you hit home to 'hide' the keyboard and resize the background app at frame 210? That's when the background is ready. If we detect this case properly and then hide the keyboard there we shouldn't render black.

> 
> I was thinking of another Gaia hack. Maybe we can resize the keyboard app
> only after we've resized the parent app, instead of only starting the
> animation after we've resized the parent app.

We need to wait until the background app and the parent iframe have been properly resize before we start to hide the keyboard. That's what we're aiming for.

Looks like LayerScope doesn't grab the keyboard texture properly. Dan are you owning LayerScope? Would be nice to get that fixed.

Comment 22

[Mason Chang [Inactive] [:mchang]]

(In reply to Benoit Girard (:BenWa) from comment #21)

> Did you hit home to 'hide' the keyboard and resize the background app at
> frame 210? That's when the background is ready. If we detect this case
> properly and then hide the keyboard there we shouldn't render black.
> 

Yes, that's what I did. Should we be developing another API in gecko that shoots off an event when a frame has been resized and hook into that event in Gecko? I've been playing with Gaia and nothing quite seems to work.

Comment 23

[Mason Chang [Inactive] [:mchang]]

(In reply to Mason Chang [:mchang] from comment #22)

> Yes, that's what I did. Should we be developing another API in gecko that
> shoots off an event when a frame has been resized and hook into that event
> in Gaia? I've been playing with Gaia and nothing quite seems to work.

Typo, shoot off an event from Gecko and hook into it in Gaia.

Comment 24

[u480271]

(In reply to Benoit Girard (:BenWa) from comment #21)

> Looks like LayerScope doesn't grab the keyboard texture properly. Dan are
> you owning LayerScope? Would be nice to get that fixed.

I don't know if LayerScope has an owner, BenWa. People in Taipei are looking at adding improvements, but I guess I could be the owner. 

I'll take a look into why the keyboard textures isn't captured properly.

Comment 25

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 8363415 [details] [diff] [review]
raf.patch

Looks good to me for what you did to delay the keyboard hiding.

However, I'm not sure if it is a good idea to move the background image from system app to homescreen app. You may wanna flag :Alive if you want to do that.

Thanks.

Comment 26

[Mason Chang [Inactive] [:mchang]]

Attachment: transparent_keyboard.mov

I set a 5000 ms delay until we start the keyboard animation and made the keyboard transparent. We see there is quite a bit of time between painting the background and animating the hide. The problem is that using requestAnimationFrame occurs before we actually finish painting the background. 

Benoit suggested that we have the homescreen app itself send an event after the onresize event, but due to the Gaia security policy, this is not possible. This is by design that an app cannot send an event to the system app (https://groups.google.com/forum/#!msg/mozilla.dev.b2g/NBPA0EFVwHc/Q0BYbzbiXeQJ). 

I learned about forms.js, investigating if there might be a privileged Gecko event that we can send to the keyboard to start a hide after a resize.

Comment 27

[Mason Chang [Inactive] [:mchang]]

Also, with rocket bar, the search feature will no longer be in use and apps no longer resized as rocketbar is overlayed on top, so this would only help in v1.3.

Comment 28

[Mason Chang [Inactive] [:mchang]]

Attachment: WIP - animation.patch

Patch that works by doing an ugly ugly hack. Due to gaia security restrictions and by design, an app cannot send a message back to the system app. The hack around is to force the system app to listen to a setting. Then in the onresize event in the Homescreen app, we wait for 2 requestAnimationFrames and set mentioned setting to true. Then on the system app, we finally hide the keyboard. This seems like a terrible hack and since apps cannot talk to the gaia app, it looks like the Gaia route isn't going to work. 

Should we go the route mentioned by roc in comment 11? Or a modification of comment #7, where we send a gecko event up to the system app once a remote iframe has finished resizing and gecko has processed all the layer updates? Then we have the keyboard app listen to the gecko chrome event and start the keyboard hide animation then. Thanks!

Comment 29

[Benoit Girard (:BenWa)]

First can we confirm that there are no ways for the parent app to wait for an event from the child?

Comment 30

[Mason Chang [Inactive] [:mchang]]

Will try again with the inter-app communication protocol.

Comment 31

[Mason Chang [Inactive] [:mchang]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/15883

Patch that works with rocket bar + inter-app communication. Problem is that we now severely overdraw in the homescreen app when we move the background to the homescreen app. Follow in bug 960325.

We also slightly change the behavior of the 'return to homescreen' animation after rocketbar shows up. We no longer show the homescreen icons then hide the keyboard. We hide the keyboard then show all of the homescreen. See attached videos.

Comment 32

[Mason Chang [Inactive] [:mchang]]

Attachment: Original Hide Rocketbar Animation

Comment 33

[Mason Chang [Inactive] [:mchang]]

Attachment: updated.MOV

Updated Rocketbar Animation

Comment 34

[Kevin Grandon :kgrandon]

Regarding the rocketbar patch, have we ruled out that this is not the cause? https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window_manager.js#L254

It seems like we're just not setting it to be visible until the rocketbar is hidden, which is wrong when I think about it.

Comment 35

[Mason Chang [Inactive] [:mchang]]

Kevin landed a patch in gaia that never changes the visibility of rocketbar and the black animation no longer occurs. Kevin, can you say which bug landed, then I'll resolve this bug. Thanks!

Comment 36

[Kevin Grandon :kgrandon]

Bug 966975 fixes the rocketbar/homescreen rendering.

I'm assuming that this may still be a problem with other or third party apps though? I would assume that we should fix our keyboard events, to resize then hide the keyboard after the area is done painting, but maybe that's low priority.

Comment 37

[Mason Chang [Inactive] [:mchang]]

With the introduction of rocketbar and bug 966975 in rocketbar, we no longer see this issue when we move the background into the homescreen app. Resolving as worksforme.