796722 - [system] screen rotation with 180 degree will delay

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[GH to BZ]

[GitHub issue by johnshih on 2012-09-25T02:43:36Z, https://github.com/mozilla-b2g/gaia/issues/5118]
## Environment :
Otoro phone, build 2012-09-24
Build info: 
* "gaia" revision= 028b9eb35bc0fbbf2d31a0b766319b94997c8eb1
* "gecko" revision= 7de377e67a367f389b44fd3c0d1d6fd3f08f71f0

## Repro :
1. Launch the app that can rotate the screen (e.g. browser app)
2. turn the phone up-side-down (rotate 180 degree directly)

## Expected:
* The screen will rotate 180 degree

## Actual:
* It will delay for a while

## Note:
* If you touch the screen during the delay, it will rotate immediately
* And there is no problem with rotate 90 degree

Comment 1

[GH to BZ]

[GitHub comment by autonome on 2012-09-28T05:39:06Z]
Yep, ~7 seconds. And yep, if you touch the screen it *immediately* rotates 180. Good find John :)

I'm not going to hold the release since the use-case is not really strong and there's a workaround.

Comment 2

[Peter Chang[:pchang]]

Found OnDraw event trigger more times under normal condtion.

[Normal]
nsScreenGonk setRotation 3
BasicShadowLayerManager::BeginTransactionWithTarget 0x44636320 mTargetRotation 0
nsPressShell::Paint aFlags 0x00000081
BasicShadowLayerManager::BeginTransactionWithTarget 0x43aaf660 mTargetRotation 3
nsPressShell::Paint aFlags 0x00000081
BasicShadowLayerManager::BeginTransactionWithTarget 0x43aaf660 mTargetRotation 3
nsPressShell::Paint aFlags 0x00000081
nsPressShell::Paint call updatePaintCountForPaintedPresShells aFlags 0x00000081

[Abnormal]
nsScreenGonk setRotation 3
BasicShadowLayerManager::BeginTransactionWithTarget 0x43aaf660 mTargetRotation 3
nsPressShell::Paint aFlags 0x00000081
nsPressShell::Paint call updatePaintCountForPaintedPresShells aFlags 0x00000081

Checking the difference between above sequences.

Comment 3

[Peter Chang[:pchang]]

Attachment: patch to fix screen not update when rotate 180 very quickly

Comment 4

[Peter Chang[:pchang]]

[Root cause]
In above repo steps, the rotation of 180 degree may not cause any new painting events happened but only rotation event happened. In current shadowlayer flow, it will skip screen update if no new painting events.

[Solution]
Added rotation event detection before screen update skip

Comment 5

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

Comment on attachment 678246 [details] [diff] [review]
patch to fix screen not update when rotate 180 very quickly

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

great!

::: gfx/layers/ipc/ShadowLayers.cpp
@@ +46,5 @@
>    {
>      mOpen = true;
>      mTargetBounds = aTargetBounds;
> +    if (aRotation != mTargetRotation)
> +        mRotationChanged = true;

{}

Comment 6

[Peter Chang[:pchang]]

Attachment: patch to fix screen not update when rotate 180 very quickly-v2

version 2 -- add {} in line 49

Comment 7

[David Scravaglieri [:scravag]]

Not bloking for shipping

Comment 8

[Peter Chang[:pchang]]

Attachment: v2 and sync with latest code

Comment 9

[Peter Chang[:pchang]]

https://tbpl.mozilla.org/?tree=Try&rev=e47a84461164

Comment 10

[Peter Chang[:pchang]]

checkin-needed

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/78a9c289f6f1

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/78a9c289f6f1

Comment 13

[Wayne Chang [:wchang]]

Fix has been ready for a while - the bug looks ugly when the rotation time is inconsistent or changes instantly on touch. Noming for leo+.

Comment 15

[Wayne Chang [:wchang]]

Triage - leo+ per partner agreement

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/8fee987844f9

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/8fee987844f9

Comment 18

[Vincent Lin[:vilin]]

Attachment: Apply WillPaintWindow/PanitWindow/DidPaintWindow call flow in nsWindow::DoDraw

Modification for test only.
I also append a property setting "debug.nsWindow.paint" in full_xxx.mk for run-timely verifying the change.

Comment 19

[Vincent Lin[:vilin]]

Hi Roc,

I think part of patches in Bug 826817 need to be committed in B2G.

Bug 826817 - Make all platforms always fire WillPaintWindow/DidPaintWindow

I've locally had a experiment. The performance is still poor on B2G unless WillPanitWindow/PanitWindow/DidPanitWindow call flow is applied to nsWindow::DoDraw.

Please check the "git diff" in attachment.

Comment 20

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

That's the right idea. But you should re-get 'listener' after each call --- otherwise there's no point in checking it in separate if statements.

Comment 21

[Wayne Chang [:wchang]]

Leo+ supersedes HD+, this is already landed on hd+

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #17)
> https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/8fee987844f9