714416 - Support multiple screen orientations in gonk backend

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

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

The gonk widget backend is the source of truth about screen orientation on b2g.  The screen will have a default orientation decided by the display hardware.  Beyond that, we need to support the screen rotated by 90, 180, and 270 degrees.  (The orientation is portrait if width<height, landscape otherwise.  This is separate from rotation.)

When the rotation changes, we need to resize gecko's view of the screen and window(s).  There are some fancy things we can do here that should go in followups.

I have a pretty dumb prototype that implements rotation using transforms, but in some cases we may be able to use HW support for screen rotation.  Work for followups, if needed.

Comment 1

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

Attachment: API for rotating screens

Comment 2

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

Patches written, just need to test them.

Comment 3

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

Attachment: Part 2: Merge nsScreenManagerGonk.cpp into nsWindow.cpp

Having these in separate files is just getting in the way.  Would require grosser hacks to maintain separation for later patches.

Works fine.

Comment 4

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

Attachment: Part 3: "Virtualize" the screen size, so that we can change it on rotation, WIP

Fixes the hard-codings of gScreenSize that break code when we rotate the screen.  Should mostly work, might have missed a spot or two.

Comment 5

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

Attachment: Part 4: Implement screen rotation in gonk widget backend, WIP

This is very WIP.  It implements rotation for when we're using GL, but no impl for CPU.

There's some testing code in here that rotates the screen every 5 seconds off of a timer.  As can be seen, it doesn't work correctly: something is not being resized correctly after rotation.  Fixing that and implementing rotation of the memmapped framebuffer are the two TODOs left here.

Comment 6

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

Stepping down for the time being.  mwu, faramarz is going to contact you about the screen rotation work.  It'd be great if you could take this.  Should be a few hours' work to get this landable.

Comment 7

[Michael Wu [:mwu]]

I'll take a look.

Comment 8

[Michael Wu [:mwu]]

Attachment: Merge nsScreenManagerGonk.cpp into nsWindow.cpp

So, all I did was unbitrot this, so r=me. Whoo.

Comment 9

[Michael Wu [:mwu]]

Attachment: API for rotating screens, v2

This is basically an unbitrotted version of the original patch, except I changed the nsIScreen UUID out of paranoia. Probably not actually necessary though since we're adding things to the end of the interface.

Comment 10

[Michael Wu [:mwu]]

Attachment: Add rotation support to gonk backend (GL only)

This is a simplified version of the original part 3 and 4 patches. It only attempts to support GL - I want to cover the non-GL path in a follow up.

I had issues with this patch for a while until I fixed the resize code in the gonk backend to set mBounds correctly.

Comment 11

[Michael Wu [:mwu]]

Attachment: Rotation testing patch

I used this patch to test rotation. It makes the volume buttons rotate the screen.

Comment 12

[Michael Wu [:mwu]]

Attachment: Add rotation support to gonk backend

Well, nevermind. It turns out adding support for the non-GL case is trivial.

Comment 13

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

Comment on attachment 596137 [details] [diff] [review]
Add rotation support to gonk backend

>diff --git a/widget/gonk/nsWindow.cpp b/widget/gonk/nsWindow.cpp
>index d336b29..7c1cab1 100644
>--- a/widget/gonk/nsWindow.cpp
>+++ b/widget/gonk/nsWindow.cpp
>@@ -64,6 +64,9 @@ using namespace mozilla::layers;
> using namespace mozilla::widget;
> 
> nsIntRect gScreenBounds;
>+PRUint32 gScreenRotation;

Please, no more magical globals.  Make this file-static and use
nsIScreen::GetRotation() elsewhere please.

>@@ -127,6 +133,7 @@ nsWindow::DoDraw(void)
>         nsRefPtr<gfxASurface> backBuffer = Framebuffer::BackBuffer();
>         {
>             nsRefPtr<gfxContext> ctx = new gfxContext(backBuffer);
>+            ctx->SetMatrix(sRotationMatrix);

This is wrong.  Let's just not attempt to rotate for the fallback path
until we really try to support the non-GL coordinate space.

>             gfxUtils::PathFromRegion(ctx, event.region);
>             ctx->Clip();
> 
>@@ -137,7 +144,11 @@ nsWindow::DoDraw(void)
>         }
>         backBuffer->Flush();
> 
>-        Framebuffer::Present(event.region);
>+        // XXX Properly rotate the region for other orientations
>+        if (gScreenRotation == nsIScreen::ROTATION_0_DEG)
>+            Framebuffer::Present(event.region);

And here.

r=me with those changes.

Comment 14

[Michael Wu [:mwu]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #13)
> Comment on attachment 596137 [details] [diff] [review]
> Add rotation support to gonk backend
> 
> >diff --git a/widget/gonk/nsWindow.cpp b/widget/gonk/nsWindow.cpp
> >index d336b29..7c1cab1 100644
> >--- a/widget/gonk/nsWindow.cpp
> >+++ b/widget/gonk/nsWindow.cpp
> >@@ -64,6 +64,9 @@ using namespace mozilla::layers;
> > using namespace mozilla::widget;
> > 
> > nsIntRect gScreenBounds;
> >+PRUint32 gScreenRotation;
> 
> Please, no more magical globals.  Make this file-static and use
> nsIScreen::GetRotation() elsewhere please.
> 

This was attempted and along with adding some annoying XPCOM goop, it crashed. For some reason, the screen manager can't be obtained as early as the appshell wants. This can be placed in nsWindow to be less ugly.

> >@@ -127,6 +133,7 @@ nsWindow::DoDraw(void)
> >         nsRefPtr<gfxASurface> backBuffer = Framebuffer::BackBuffer();
> >         {
> >             nsRefPtr<gfxContext> ctx = new gfxContext(backBuffer);
> >+            ctx->SetMatrix(sRotationMatrix);
> 
> This is wrong.  Let's just not attempt to rotate for the fallback path
> until we really try to support the non-GL coordinate space.
> 

FWIW this has been tested a bit on the emulator and it seems to rotate and respond to events correctly. Do you really want this removed?

Comment 15

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

The coordinate space for the fallback surface is different so I'm not sure how the same transform is working.  If it does, *shrug* ok.

Comment 16

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

And yeah let's do a static helper fn or something.  Sigh re: xpcom.

Comment 17

[Michael Wu [:mwu]]

Attachment: Add rotation support to gonk backend, v2

Comment 18

[Michael Wu [:mwu]]

Attachment: Rotation testing patch, v2

Comment 19

[Michael Wu [:mwu]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3202141e00fb
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f89c92dea51
https://hg.mozilla.org/integration/mozilla-inbound/rev/84725e191d21

Comment 20

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

\o/

Comment 21

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/3202141e00fb
https://hg.mozilla.org/mozilla-central/rev/7f89c92dea51
https://hg.mozilla.org/mozilla-central/rev/84725e191d21

Comment 22

[Eric Shepherd [:sheppy]]

Documentation updated:

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIScreen

And mentioned on Firefox 13 for developers.