Last Comment Bug 714416 - Support multiple screen orientations in gonk backend
: Support multiple screen orientations in gonk backend
Status: RESOLVED FIXED
[not-fennec-11]
: dev-doc-complete
Product: Core
Classification: Components
Component: Widget (show other bugs)
: Trunk
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla13
Assigned To: Michael Wu [:mwu]
:
:
Mentors:
Depends on: 672166
Blocks: 714414 725247
  Show dependency treegraph
 
Reported: 2011-12-30 20:53 PST by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2012-02-15 12:48 PST (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
API for rotating screens (3.10 KB, patch)
2011-12-31 00:30 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
roc: superreview+
Details | Diff | Splinter Review
Part 2: Merge nsScreenManagerGonk.cpp into nsWindow.cpp (7.44 KB, patch)
2012-01-24 19:09 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
Part 3: "Virtualize" the screen size, so that we can change it on rotation, WIP (3.77 KB, patch)
2012-01-24 19:10 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
Part 4: Implement screen rotation in gonk widget backend, WIP (12.43 KB, patch)
2012-01-24 19:12 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
Merge nsScreenManagerGonk.cpp into nsWindow.cpp (7.28 KB, patch)
2012-02-10 10:21 PST, Michael Wu [:mwu]
mwu.code: review+
Details | Diff | Splinter Review
API for rotating screens, v2 (2.36 KB, patch)
2012-02-10 10:23 PST, Michael Wu [:mwu]
no flags Details | Diff | Splinter Review
Add rotation support to gonk backend (GL only) (7.88 KB, patch)
2012-02-10 10:28 PST, Michael Wu [:mwu]
no flags Details | Diff | Splinter Review
Rotation testing patch (1.93 KB, patch)
2012-02-10 10:30 PST, Michael Wu [:mwu]
no flags Details | Diff | Splinter Review
Add rotation support to gonk backend (8.64 KB, patch)
2012-02-10 12:09 PST, Michael Wu [:mwu]
cjones.bugs: review+
Details | Diff | Splinter Review
Add rotation support to gonk backend, v2 (8.64 KB, patch)
2012-02-13 14:37 PST, Michael Wu [:mwu]
no flags Details | Diff | Splinter Review
Rotation testing patch, v2 (1.93 KB, patch)
2012-02-13 14:37 PST, Michael Wu [:mwu]
no flags Details | Diff | Splinter Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-30 20:53:39 PST
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 2011-12-31 00:30:22 PST
Created attachment 585118 [details] [diff] [review]
API for rotating screens
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-01-10 04:16:25 PST
Patches written, just need to test them.
Comment 3 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-01-24 19:09:28 PST
Created attachment 591351 [details] [diff] [review]
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 2012-01-24 19:10:30 PST
Created attachment 591352 [details] [diff] [review]
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 2012-01-24 19:12:33 PST
Created attachment 591355 [details] [diff] [review]
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 2012-01-24 19:14:03 PST
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] 2012-01-25 18:20:35 PST
I'll take a look.
Comment 8 Michael Wu [:mwu] 2012-02-10 10:21:06 PST
Created attachment 596087 [details] [diff] [review]
Merge nsScreenManagerGonk.cpp into nsWindow.cpp

So, all I did was unbitrot this, so r=me. Whoo.
Comment 9 Michael Wu [:mwu] 2012-02-10 10:23:30 PST
Created attachment 596089 [details] [diff] [review]
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] 2012-02-10 10:28:54 PST
Created attachment 596094 [details] [diff] [review]
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] 2012-02-10 10:30:22 PST
Created attachment 596095 [details] [diff] [review]
Rotation testing patch

I used this patch to test rotation. It makes the volume buttons rotate the screen.
Comment 12 Michael Wu [:mwu] 2012-02-10 12:09:44 PST
Created attachment 596137 [details] [diff] [review]
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 2012-02-10 16:01:40 PST
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] 2012-02-13 11:15:50 PST
(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 2012-02-13 11:38:56 PST
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 2012-02-13 11:39:24 PST
And yeah let's do a static helper fn or something.  Sigh re: xpcom.
Comment 17 Michael Wu [:mwu] 2012-02-13 14:37:26 PST
Created attachment 596792 [details] [diff] [review]
Add rotation support to gonk backend, v2
Comment 18 Michael Wu [:mwu] 2012-02-13 14:37:48 PST
Created attachment 596793 [details] [diff] [review]
Rotation testing patch, v2
Comment 20 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-02-13 15:16:36 PST
\o/
Comment 22 Eric Shepherd [:sheppy] 2012-02-15 12:48:45 PST
Documentation updated:

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

And mentioned on Firefox 13 for developers.

Note You need to log in before you can comment on or make changes to this bug.