Support multiple screen orientations in gonk backend

RESOLVED FIXED in mozilla13

Status

()

Core
Widget
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: cjones, Assigned: mwu)

Tracking

({dev-doc-complete})

Trunk
mozilla13
ARM
Gonk (Firefox OS)
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [not-fennec-11])

Attachments

(4 attachments, 7 obsolete attachments)

7.28 KB, patch
mwu
: review+
Details | Diff | Splinter Review
2.36 KB, patch
Details | Diff | Splinter Review
8.64 KB, patch
Details | Diff | Splinter Review
1.93 KB, patch
Details | Diff | Splinter Review
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.
Depends on: 672166
Created attachment 585118 [details] [diff] [review]
API for rotating screens
Attachment #585118 - Flags: superreview?(roc)
Attachment #585118 - Flags: superreview?(roc) → superreview+
Patches written, just need to test them.
Assignee: nobody → jones.chris.g
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.
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.
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.
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.
Assignee: jones.chris.g → nobody
(Assignee)

Comment 7

6 years ago
I'll take a look.
Assignee: nobody → mwu
(Assignee)

Updated

6 years ago
Status: NEW → ASSIGNED
Whiteboard: [not-fennec-11]
(Assignee)

Comment 8

6 years ago
Created attachment 596087 [details] [diff] [review]
Merge nsScreenManagerGonk.cpp into nsWindow.cpp

So, all I did was unbitrot this, so r=me. Whoo.
Attachment #591351 - Attachment is obsolete: true
Attachment #596087 - Flags: review+
(Assignee)

Comment 9

6 years ago
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.
Attachment #585118 - Attachment is obsolete: true
(Assignee)

Comment 10

6 years ago
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.
Attachment #591352 - Attachment is obsolete: true
Attachment #591355 - Attachment is obsolete: true
Attachment #596094 - Flags: review?(jones.chris.g)
(Assignee)

Comment 11

6 years ago
Created attachment 596095 [details] [diff] [review]
Rotation testing patch

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

Comment 12

6 years ago
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.
Attachment #596094 - Attachment is obsolete: true
Attachment #596094 - Flags: review?(jones.chris.g)
Attachment #596137 - Flags: review?(jones.chris.g)
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.
Attachment #596137 - Flags: review?(jones.chris.g) → review+
Blocks: 725247
(Assignee)

Comment 14

5 years ago
(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?
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.
And yeah let's do a static helper fn or something.  Sigh re: xpcom.
(Assignee)

Comment 17

5 years ago
Created attachment 596792 [details] [diff] [review]
Add rotation support to gonk backend, v2
Attachment #596137 - Attachment is obsolete: true
(Assignee)

Comment 18

5 years ago
Created attachment 596793 [details] [diff] [review]
Rotation testing patch, v2
Attachment #596095 - Attachment is obsolete: true
(Assignee)

Comment 19

5 years ago
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
Target Milestone: --- → mozilla13
\o/
https://hg.mozilla.org/mozilla-central/rev/3202141e00fb
https://hg.mozilla.org/mozilla-central/rev/7f89c92dea51
https://hg.mozilla.org/mozilla-central/rev/84725e191d21
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Keywords: dev-doc-needed
Documentation updated:

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

And mentioned on Firefox 13 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.