Closed
Bug 757758
Opened 13 years ago
Closed 13 years ago
Honor hardware screen rotation
Categories
(Core :: Widget, defect)
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: cjones, Assigned: cjones)
Details
Attachments
(1 file, 1 obsolete file)
3.46 KB,
patch
|
mwu
:
review+
|
Details | Diff | Splinter Review |
Some devices have a default rotation that we need to respect. Patch coming up.
Comment 2•13 years ago
|
||
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.
Attachment #626352 -
Flags: review?(mwu) → review+
Comment 3•13 years ago
|
||
(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•13 years ago
|
||
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.
Assignee | ||
Comment 5•13 years ago
|
||
Requesting re-review to ensure nits were picked.
Attachment #626352 -
Attachment is obsolete: true
Attachment #626552 -
Flags: review?(mwu)
Updated•13 years ago
|
Attachment #626552 -
Flags: review?(mwu) → review+
Assignee | ||
Updated•13 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 6•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9179adbbdb1
Keywords: checkin-needed
Updated•13 years ago
|
Flags: in-testsuite-
Target Milestone: --- → mozilla15
Comment 7•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b9179adbbdb1
Assignee: nobody → jones.chris.g
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•