Editorial changes #1

kenchris · 2017-03-06T14:27:41Z

Some minor changes and the start for discussion

kenchris · 2017-03-06T14:28:32Z

index.bs

-    let sensor = new DeviceOrientationSensor();
-    let matrix = new Float32Array(16);
+    const sensor = new DeviceOrientationSensor();
+    const mat4 = new Float32Array(16);


using mat4 to indicate it is a 4x4 (ie 16) array.

const is used when no reassignment occurs

does it allow to modify the array's content?

yes :-) but you cannot reassign, ie do mat4 = true that is the only thing it prevents... otherwise you need to use Object.freeze(mat4)

kenchris · 2017-03-06T14:28:47Z

index.bs

-Orientation sensor is a [=high-level=] sensor which is created through [=sensor-fusion=]
-of [=low-level=] motion sensors (accelerometer, gyroscope, magnetometer).
+The device orientation sensor is a [=high-level=] sensor which is created through [=sensor-fusion=]
+of the [=low-level=] motion sensors (accelerometer, gyroscope). The type of fusion (complementary


This is device orientation so lets drop magnetometer

Could you explain more? the underlying rotation vector is using all three

That is the compass. DeviceOrientation is normally what is called "Game rotation vector". So I guess you want to support both modes? (ie. game and compass) using the same sensor?

(update: as tobie stated, device orientation can mean both with old spec which is why everyone is confused - I guess it traditionally means a compass like sensor, but we want something else here)

I am trying to understand the different use case for

geomagnetic rotation vector

rotation vector (which uses magnetometer)

Also at least the former, you want to be able to easily adjust to true north (Android has an API for this)

Also these both seems more related to a compass sensor than a device orientation sensor

Actually for games, it is quite important that it is allowed to drift so that it can be used for controls:

A game rotation vector sensor is similar to a rotation vector sensor but not using the geomagnetic field. Therefore the Y axis doesn't point north but instead to some other reference. That reference is allowed to drift by the same order of magnitude as the gyroscope drifts around the Z axis.

As said, cannot we introduce also RelativeOrientationSensor : DeviceOrientationSensor // ignores magnetometer wrapping TYPE_GAME_ROTATION_VECTOR ? and let DeviceOrientationSensor wrap TYPE_ROTATION_VECTOR

I am not sure what is best. One is relative to the device orientation itself, and one is relative to the earth.

It seems better to have two separate objects for that to avoid confusion.

Now as both MS and Android calls the one relative to earth for Device Orientation Sensor, we should probably keep that name, but both could potentially inherit from one Orientation Sensor.

I think that makes sense, as we may have other devices in the future relative to other things. Like with "device" we usually mean the device where the code is running. With an external headset, that might not be the device actually running the code, so I could imagine a

GamepadSensor extends Relative Orientation Sensor
HeadsetSensor extends Orientation Sensor.

Actually looking at the above, I now see that the name "device" is probably quite bad, because it is not clear what it refers to, so just calling it Orientation Sensor may make sense.

I guess we could have

Orientation Sensor (relative to the world / earth)

Relative Orientation Sensor (relative to the device itself) extends the above.

Then in the future we might have external sensors inheriting those.

The fact that these very important conversations are happening within review comments of a pull request is a pretty good sign that this approach to spec writing isn't an adequate one.

The role of the editor is to gather consensus from the WG and document it in a spec. It is absolutely clear here that there is no such consensus. And it's really bothering that these conversations are buried in here.

I strongly suggest discussing and gathering higher-level consensus around these issues before diving in editorial work. Addressing the open issue on this topic, such as the email I penned on the mailing list in defence of writing a single motion sensor spec, seems like the right way to proceed.

@kenchris please feel free to open issues for your concerns and we'll continue there

I will update my patch here and then I will see if Tobie and I can write some general introduction to motion sensors.

kenchris · 2017-03-06T14:28:56Z

index.bs

-y-axis points north and z-axis points up and is perpendicular to the plane made up of x and y axis.
+of the device in a three dimensional Cartesian coordinate system (x, y, z), where the x-axis points
+east, the y-axis points north and the z-axis points upwards and is perpendicular to the plane made 
+up of x and y axis. The values are no necessarily normalized.


Are these normalized or not?

I think unit quaternion means that they are normalized

Right, let me update

What normalization means should be defined somewhere.

yes, the vector has a length of one... but that definition probably differs a bit for quaternions as only the 3 latter parts represents the vector the the first part the scalar. I will dig up a definition somewhere

Oh, its vector normalization. (That rings a distant bell somewhere.)

https://en.wikipedia.org/wiki/Quaternion#Unit_quaternion

I meant it needs a dfn in the spec.

Yeah, @alexshalamov is now working on that

kenchris · 2017-03-06T14:29:23Z

index.bs

@@ -85,8 +88,8 @@ The DeviceOrientationSensor Interface {#deviceorientationsensor-interface}
 <pre class="idl">
  [Constructor(optional SensorOptions sensorOptions)]
  interface DeviceOrientationSensor : Sensor {
-    void populateQuaternion(Float32Array array);
-    void populateMatrix(Float32Array array);
+    void populateQuaternion(Float32Array buffer);


I think that buffer makes it more clear that we write to these... we could use targetBuffer or similar

What do we do with the timestamps, here?

We need to expose those somehow and tie them to the correct typed array.

don't we still have the inherited timestamp attribute?

What when we have > 1 reading between animation frames?

same behavior as with all the existing sensors: we read data right before a new frame is rendered.

But when this data is buffered, and you get > 1 sample delivered on the animation frame, you'll need a different timestamp for each of the buffered data.

So what's the plan to handle that?

Are you suggesting different TypeArray organisations depending on whether the data is buffered or not? That seems like a rather bad idea.

data is always the same and it's corresponding to the timestamp, we just don't read from shared buffer in the middle of frame painting

the proposed API only populates the latest reading (same as in other sensors) it is not meant to collect series of previous readings.

Yes. But this is also something we want to do, no? So we need a consistent approach here, else we'll end up with different data representations depending on how the data is collected.

kenchris · 2017-03-06T14:29:41Z

index.bs

        "{{SyntaxError!!exception}}" {{DOMException}} and abort these steps.
-    1.  If |array| parameter is {{Float32Array}} with size other than four, [=throw=] an
+    1.  If |buffer| parameter is of type {{Float32Array}} with a size other than four, [=throw=] a


or should we use typeof ? @anssiko ?

"of type" is good in prose (and is proper English). It is clear the behaviour mirrors that of the "typeof" operator defined in ES.

This is WebIDL, so type is correct (typeof wouldn't make any sense here).

kenchris · 2017-03-06T14:30:15Z

index.bs

        "{{SyntaxError!!exception}}" {{DOMException}} and abort these steps.
    1.  If the value of sensor_instance|.[=[[state]]=] is not <a enum-value>"activated"</a>,
-        [=throw=] an "{{InvalidStateError!!exception}}" {{DOMException}} and abort these steps.
+        [=throw=] a "{{InvalidStateError!!exception}}" {{DOMException}} and abort these steps.


Mistake of mine here.

kenchris · 2017-03-06T14:30:37Z

index.bs

@@ -97,16 +100,16 @@ To <dfn>Construct a DeviceOrientationSensor Object</dfn> the user agent must inv

 <div algorithm="to populate quaternion">
 The {{DeviceOrientationSensor/populateQuaternion()}} method must run these steps or their [=equivalent=]:
-    1.  If |array| parameter is not {{Float32Array}}, [=throw=] an
+    1.  If |buffer| parameter is not of type {{Float32Array}}, [=throw=] an


Should be "a Syntax..."

You can drop "parameter" here. It's redundant with |x| notation.

kenchris · 2017-03-06T14:30:57Z

index.bs

-    1. A web-based game uses the device's orientation relative to the Earth's surface as user input. For example, simulation of Ball-in-a-maze puzzle.
-    1. A WebVR API polyfill implementation tracks the headset's (or mobile device's for mobile VR) orientation.
-    1. A mapping web application orientates the 2D map with the orientation of the device.
+    1. A compass monitor's the device's orientation relative to true north and aligns the compass heading in a three-dimensional space.


We can drop the first here..

Generally, I think made-up use cases that aren't rooted in real problems faced by real developers in real life provide a false sense of security. I recommend avoiding them.

kenchris · 2017-03-06T14:31:06Z

index.bs

-    1. A WebVR API polyfill implementation tracks the headset's (or mobile device's for mobile VR) orientation.
-    1. A mapping web application orientates the 2D map with the orientation of the device.
+    1. A compass monitor's the device's orientation relative to true north and aligns the compass heading in a three-dimensional space.
+    1. A game uses the device's orientation relative to the ground as user input. For example, a ball-in-a-maze puzzle.


shouldn't this say 2. ?

bikeshed is autoincrementing

Ah cool :-) makes sense then

Markdown auto-increments. :)

kenchris commented Mar 6, 2017

View reviewed changes

kenchris force-pushed the gh-pages branch from 0ea7a31 to 318d429 Compare March 7, 2017 12:13

kenchris closed this Mar 7, 2017

kenchris force-pushed the gh-pages branch from 318d429 to d66089e Compare March 7, 2017 13:10

kenchris reopened this Mar 7, 2017

kenchris changed the title ~~Minor editorial changes~~ Editorial changes Mar 7, 2017

kenchris closed this Mar 7, 2017

kenchris force-pushed the gh-pages branch from d66089e to f0a8580 Compare March 7, 2017 14:25

Editorial changes #1

Editorial changes #1

Conversation

kenchris commented Mar 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kenchris Mar 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kenchris Mar 6, 2017 • edited Loading

Choose a reason for hiding this comment

kenchris Mar 6, 2017 • edited Loading

Choose a reason for hiding this comment

kenchris Mar 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobie Mar 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobie Mar 6, 2017 • edited Loading

Choose a reason for hiding this comment

kenchris Mar 6, 2017 •

edited

Loading

kenchris Mar 6, 2017 •

edited

Loading

kenchris Mar 6, 2017 •

edited

Loading

kenchris Mar 6, 2017 •

edited

Loading

tobie Mar 6, 2017 •

edited

Loading

tobie Mar 6, 2017 •

edited

Loading