757758 - Honor hardware screen rotation

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

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

Some devices have a default rotation that we need to respect.  Patch coming up.

Comment 1

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

Attachment: Honor hardware screen rotation

Comment 2

[Michael Wu [:mwu]]

Comment on attachment 626352 [details] [diff] [review]
Honor hardware screen rotation

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

::: widget/gonk/nsWindow.cpp
@@ +572,5 @@
>  nsScreenGonk::nsScreenGonk(void *nativeScreen)
>  {
> +    char propValue[PROPERTY_VALUE_MAX];
> +    property_get("ro.sf.hwrotation", propValue, "0");
> +    sPhysicalScreenRotation = atoi(propValue) / 90;

If possible, do this in the constructor for nsWindow. That would seem less magical to me than relying on nsScreenGonk getting created before the first nsWindow.

@@ +638,5 @@
>          return NS_OK;
>  
>      sScreenRotation = aRotation;
>      sRotationMatrix.Reset();
> +    switch ((aRotation + sPhysicalScreenRotation) % (nsIScreen::ROTATION_270_DEG + 1)) {

nsIScreen::ROTATION_270_DEG + 1 is a little bizarre looking.. oh well.

Comment 3

[Michael Vines [:m1] [:evilmachines]]

(In reply to Michael Wu [:mwu] from comment #2)
> @@ +638,5 @@
> >          return NS_OK;
> >  
> >      sScreenRotation = aRotation;
> >      sRotationMatrix.Reset();
> > +    switch ((aRotation + sPhysicalScreenRotation) % (nsIScreen::ROTATION_270_DEG + 1)) {
> 
> nsIScreen::ROTATION_270_DEG + 1 is a little bizarre looking.. oh well.

Yeah.   A couple other options:
1)  "4":  IMO more difficult to grok at first glance.  ie, WTF is a 4?
2) "360 / 90": still 4 but makes it clear how that magic number was derived and matches the "atoi(propValue) / 90" elsewhere in the file, so a search for "90" will pick up both lines.
3) Add a new nsIScreen constant for the "nsIScreen::ROTATION_270_DEG + 1".  Seems like overkill.

I'm kindof liking #2 right now.

Comment 4

[Michael Wu [:mwu]]

Sure #2 sounds good. If there's more people who want to use this constant, #3 might be better but #2 sounds fine for now.

Comment 5

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

Attachment: Honor hardware screen rotation, v2

Requesting re-review to ensure nits were picked.

Comment 6

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/b9179adbbdb1

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/b9179adbbdb1