Re: [screen-orientation] Review comments for ScreenOrientation API

On Wed, Apr 5, 2017 at 10:42 AM, Kenneth Rohde Christiansen <
kenneth.christiansen@gmail.com> wrote:

> Thanks for the review Alex,
>
> Let me give some quick comments - check below
>
> On Wed, Apr 5, 2017 at 9:08 AM Shalamov, Alexander <
> alexander.shalamov@intel.com> wrote:
>
>> Hi,
>>
>> During DAS meeting I’ve got an action point [1] to review
>> ScreenOrientation API [2].
>> No scope of the review was specified, so I went through concepts and
>> definitions
>> provided by the spec. If some of my comments are invalid, please
>> disregard them.
>>
>> Short summary:
>>
>> - Screen interface [3] defines physical properties of the output device,
>> yet,
>>   Screen.orientation provides document orientation / changes / locking.
>>
>
> I am not sure that is actually the case. Chrome on Android for instance
> doesn't actually rotate the document AFAIK, but Android rotates the whole
> screen, which means that the browser etc is all rotated. So while that is
> not the physical screen, it describes how the OS uses the screen, just like
> it does for resolution etc.
>

You are right, Android updates Screen orientation based on input from e.g.,
sensors. Maybe informative note would be sufficient in that case, that would
explain that when orientation is locked, orientation change events are not
sent, even when physical orientation of the screen might change and
developers
have to use DeviceOrientation API to check physical orientation of the
screen.


>
> If Android has an OS screen lock, the browser content/document cannot lock
> either right
>
> I don't think there is an issue here, but maybe the spec text needs to be
> updated?
>

> I am not sure why it talks about document orientation, but I assume it is
> to allow some browser to implement this without actually rotating screen
> output from the host OS. I agree that this is confusing and that that could
> probably be handled in a different way.
>
>
>> - ScreenOrientation.angle provides rotation angle of the document, rather
>>   than the screen. Specification of the angle property as well as its
>>   implementation prone to fragmentation and could be improved.
>>
>
> Or with the above, it provides the angle of the OS output to the screen in
> comparison to the physical screen.
>
> Do you think it would be hard to change the spec text to make this all
> more clear?
>

>
>> - ScreenOrientation.lock locks document orientation, not the screen
>> orientation.
>>   Screen orientation changes are not emitted when document orientation is
>> locked.
>> - Might be nice to have event for default orientation changes (2-in-1
>> devices)
>>
>
> I guess that affects resolution as well
>

Sure, Screen properties would be changed, as well as natural orientation,
thus, ScreenOrientation.angle might need to be recalculated / synced.
Some laptops have two accelerometers, one in screen and one in keyboard.
Keyboard attached  => natural orientation landscape-primary, angle 0.
Keyboard detached => natural orientation portrait-primary, angle 0.


>
>
>> - Would be good to have information about how orientation type is
>> obtained
>>   (e.g., EDID + OS display info + optional sensors).
>>
>
> EDID = ?
>

Extended display identification data, provides information about
resolution, refresh
rates, regardless of physical rotation. Could be used to define natural
orientation
for connected displays.


>
>
>> - Would be nice to have an API for getting default (natural) orientation.
>> - Sync definition of orientation (e.g., device’s coordinate system) with
>> other
>>   specs (WebVR, DeviceMotion & Orientation, Sensors).
>>
>>
>> Detailed information:
>>
>> Screen interface in CSSOM view spec describes properties of the output
>> device as well as rendering surface properties. I think specification can
>> be
>> more explicit in defining what ScreenOrientation means in relation to
>> physical
>> orientation of output device’s screen. At the moment, it looks like:
>> ScreenOrientation == DocumentOrientation.
>>
>
> I agree
>
> Based on assumption that ScreenOrientation should provide physical
>> orientation
>> of the output device’s screen, here are my review comments.
>>
>>
>> 1. ScreenOrientation.angle
>>
>> As a developer, I think that the angle should represent physical screen
>> rotation
>> from device’s default orientation, taking into account device’s local
>> coordinate
>> system, for example, around Z axis that is pointing from device’s screen.
>> Therefore, positive angle value would denote counterclockwise rotation
>> from
>> default orientation.
>>
>> It was confusing to find out that ScreenOrientation.angle is not an angle
>> between
>> default orientation and current screen orientation. It’s the angle between
>> document orientation and default orientation of the device’s screen.
>>
>> Also, I think, specification is not taking into account use-cases when
>> device
>> could change its default orientation at runtime. For example, 2-in-1
>> might have
>> ‘landscape == default’ when keyboard attached and ‘portrait == default’
>> when
>> keyboard is detached.
>>
>> Maybe it is better to deprecate (rethink) ScreenOrientation.angle
>> property?
>> Even the spec states that angle and orientation enum can be out-of-sync,
>> and has
>> to be synced by web developers at runtime. For convertibles, developers
>> would
>> need to re-sync when default orientation changes. Wouldn’t it be better
>> to keep
>> enums and add informative notes for possible relations between rotation
>> angles
>> and orientation?
>>
>
> It should be easy to get the angle though. I already have used that to
> modify my view matrix, so that the compass in my compass demo stays in the
> same position, even when the browser UI is rotated.
>
> It might be better to get it as a rotation matrix or quaternion instead
>

I think 4 enums should be sufficient.


>
>
>>
>> I also did some quick testing of the API on different platforms, results
>> are
>> in [Test results] section. Results show that angle property in the
>> current form
>> is not that useful and introduces fragmentation between platforms.
>>
>>
>> 2. ScreenOrientation.type
>>
>> Mapping of primary / secondary values to physical orientation should be
>> done
>> by the UA based on specifics of the hardware. Same should be done for
>> landscape
>> and portrait definitions. In my opinion spec should not explicitly
>> specify that
>> the the orientation is calculated only from the screen properties. What
>> if the
>> device have square screen? The device can use its sensors to determine
>> default
>> orientation.
>>
>>
>> 3. ScreenOrientation.lock
>>
>> The orientation lock has side-effects in my opinion. Web developers can
>> lock
>> document orientation, device’s physical screen orientation lock cannot be
>> done
>> programmatically. I think, it would be better to separate these two
>> concepts.
>> For example, developers should be able to get default and current screen
>> orientation, as well as locked document orientation, so that web page can
>> adapt
>> and re-lock document orientation if needed.
>>
>
> App on Android at least, can change orientation (lock) and that changes
> the whole UI, ie notification tray, status bar etc. That already works
> today if I change orientation in the manifest.json (which builds upon
> Screen Orientation Lock).
>

Maybe few notes can be added to CSSOM-view spec, that Screen interface
provides information about rendering surface, rather than physical size of
output device.


>
>
>> 4. Screen rotation when device is parallel to the ground
>>
>> I’ve tested latest chromium canary and when the device is parallel to the
>> ground and rotated (around Z axis), screen orientation is not affected at
>> all.
>> Should it?
>>
>
> No, please don't :-) That would break many game use-case and also be super
> annoying as the phone is often more or less parallel to the ground when
> surfing in bed :-)
>

That was just an observation :)


>
>
>> 5. Test results (Chromium browser)
>>
>> ## macOS (laptop, default orientation)
>>
>>  - ScreenOrientation.type: landscape-primary
>>  - ScreenOrientation.angle:  0 deg.
>>
>> Output to external display rotated 90 degrees in settings,  monitor
>> rotated 90
>> degrees CW, rendering surface rotated 90 degrees CCW from default
>> orientation.
>>
>>  - ScreenOrientation.type: portrait-primary
>>  - ScreenOrientation.angle: 90 deg.
>>
>>
>> ## Windows 8.1 (laptop, default orientation)
>>
>>  - ScreenOrientation.type: landscape-primary
>>  - ScreenOrientation.angle: 0 deg.
>>
>> Output to external display rotated 90 degrees in settings, monitor
>> rotated 90
>> degrees CW, rendering surface rotated 90 degrees CCW from default
>> orientation.
>>
>>  - ScreenOrientation.type: portrait-primary
>>  - ScreenOrientation.angle: 270 deg.
>>
>>
>> ## Linux (desktop)
>>
>> Physical screen orientation does not matter, orientation is calculated
>> from
>> viewport size, thus, changing browser’s window size affects orientation.
>>
>> ## Android (PixelXL, default orientation)
>>
>>  - ScreenOrientation.type: portrait-primary
>>  - ScreenOrientation.angle: 0 deg.
>>
>> Device rotated 90 degrees CCW, content rotated 90 CW.
>>
>>  - ScreenOrientation.type: landscape-primary
>>  - ScreenOrientation.angle: 90 deg.
>>
>> When auto-rotate is disabled in settings, no notifications are delivered
>> through
>> the API, yet, physical orientation of the screen is changing.
>>
>> [1] - https://www.w3.org/2017/03/23-dap-minutes.html#action01
>> [2] - https://w3c.github.io/screen-orientation/
>> [3] - https://www.w3.org/TR/cssom-view-1/#the-screen-interface
>>
>> Best regards,
>> Alexander
>>
>