902821 - Screen rotation will be lagged for a while.

Status: RESOLVED WONTFIX

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

Description

[Vincent Lin[:vilin]]

Attachment: fix.patch

Bug 796722 will not fix screen rotation issue completely. Open a new bug to fix it. The modification is referenced to the *PaintWindow calls flow in Bug 826817.

Comment 1

[Marco Chen [:mchen]]

Comment on attachment 787354 [details] [diff] [review]
fix.patch

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

Michael is the correct one to do the review and is the module owner.

::: widget/gonk/nsWindow.cpp
@@ +255,4 @@
>              AutoLayerManagerSetup setupLayerManager(
>                  gWindowToRedraw, ctx, mozilla::layers::BUFFER_NONE,
>                  ScreenRotation(EffectiveScreenRotation()));
> +	

Remove spaces.

@@ +255,5 @@
>              AutoLayerManagerSetup setupLayerManager(
>                  gWindowToRedraw, ctx, mozilla::layers::BUFFER_NONE,
>                  ScreenRotation(EffectiveScreenRotation()));
> +	
> +	    listener = gWindowToRedraw->GetWidgetListener();

Does it need to get listener again and again or just get it once is enough?

Comment 2

[Vincent Lin[:vilin]]

(In reply to Marco Chen [:mchen] from comment #1)
> Comment on attachment 787354 [details] [diff] [review]
> fix.patch
> 
> Review of attachment 787354 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Michael is the correct one to do the review and is the module owner.
> 
> ::: widget/gonk/nsWindow.cpp
> @@ +255,4 @@
> >              AutoLayerManagerSetup setupLayerManager(
> >                  gWindowToRedraw, ctx, mozilla::layers::BUFFER_NONE,
> >                  ScreenRotation(EffectiveScreenRotation()));
> > +	
> 
> Remove spaces.
> 
> @@ +255,5 @@
> >              AutoLayerManagerSetup setupLayerManager(
> >                  gWindowToRedraw, ctx, mozilla::layers::BUFFER_NONE,
> >                  ScreenRotation(EffectiveScreenRotation()));
> > +	
> > +	    listener = gWindowToRedraw->GetWidgetListener();
> 
> Does it need to get listener again and again or just get it once is enough?

Yes, it does.

Comment from here.
https://bugzilla.mozilla.org/show_bug.cgi?id=796722#c20

Comment 3

[Wayne Chang [:wchang]]

Spoken to vlin, 

bug 796722 will work on its own for master, but this bug is required for 796722 to have effect on v1.1.

leo+ing this as bug 796722 is leo+ and has been included on v1.1

Comment 4

[Michael Wu [:mwu]]

Comment on attachment 787354 [details] [diff] [review]
fix.patch

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

Is this a 1.1 only patch?

There's a bunch of tabs in this patch. Please replace them with spaces and make sure the style is consistent.

Comment 5

[Michael Wu [:mwu]]

Comment on attachment 787354 [details] [diff] [review]
fix.patch

Clearing review while waiting for a new patch and an answer about whether this is 1.1 only.

Comment 6

[Alex Keybl [:akeybl]]

(In reply to Wayne Chang [:wchang] from comment #3)
> leo+ing this as bug 796722 is leo+ and has been included on v1.1

This isn't automatically leo+, it all depends on current user experience. Are we sure it's not good enough already?

Comment 7

[Stephany Wilkes]

Flagging Patrick to comment on this rotation issue since this is HD. Please let me know if this should be a blocker for 1.1.

Comment 8

[Patryk Adamczyk [:patryk] UX]

Can I get more info? I am running 2013-08-22's build and the rotation on apps like the browser is OKAY, but could be faster. Its about 2 second delay before rotation, I'd expect it to be under 1 second. Is this what you mean?

Comment 9

[Vincent Lin[:vilin]]

(In reply to Patryk Adamczyk [:patryk] UX from comment #8)
> Can I get more info? I am running 2013-08-22's build and the rotation on
> apps like the browser is OKAY, but could be faster. Its about 2 second delay
> before rotation, I'd expect it to be under 1 second. Is this what you mean?

Yap~
You can flip screen between 0 and 180 orientation. This issue will be more visible and reproducible. And after orientation changes, sometimes user even have to touch the screen to trigger screen update.

Comment 10

[Vincent Lin[:vilin]]

Attachment: Remove tabs.

This patch is for 1.1 only.

Comment 11

[Dietrich Ayala (:dietrich)]

Leo has stopped accepting patches for their 1.1 build, so not going to block on this unless they explicitly ask for this to block the release.

Comment 12

[Michael Wu [:mwu]]

Comment on attachment 795234 [details] [diff] [review]
Remove tabs.

r=me for a straightforward backport of a fix.

This is probably worth landing on 1.1 if we can. If not for leo, then for other 1.1 updates.

Comment 13

[Preeti Raghunath(:Preeti)]

Dietrich, as part of our mega triage of 1.1 leo+ blockers, this bug had a vote to retain in 1.1

I'll wait for Patryk's testing and if worth while will move to a +

Comment 14

[Patryk Adamczyk [:patryk] UX]

I am going to need more information. 
+ how do I test this?
+ where is the repo?
+ how much faster is it (before / after)

From my experience on the leo device, I don't think the rotation performance is a blocker, but if it affects all 1.1 devices, we should push this into 1.1 HD and 1.2, 100%

Comment 15

[bhavana bajaj [:bajaj]]

(In reply to Patryk Adamczyk [:patryk] UX from comment #14)
> I am going to need more information. 
> + how do I test this?
> + where is the repo?
> + how much faster is it (before / after)

needinfo'ing vin to help with this info here.
> 
> From my experience on the leo device, I don't think the rotation performance
> is a blocker,

Agreed !

> but if it affects all 1.1 devices, we should push this into
> 1.1 HD and 1.2, 100%

This is close to resolution on 1.2 anyway, nevertheless flipping it back to koi?

Comment 16

[Michael Wu [:mwu]]

This is a backport of a fix that's already on 1.2, so no need to block on koi.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Can't land without appropriate blocking status. Please re-request checkin once it does.

Comment 18

[Michael Wu [:mwu]]

Not sure what the ni? here is for.

Comment 19

[Vincent Lin[:vilin]]

Please make sure the blocking status(Project Flags).

Comment 20

[Wayne Chang [:wchang]]

Let's hold this for now as we're all moving towards v1.2, unless a partner has a strong request on leo for this.

Setting state according to comment 16.