Increase device orientation event frequency to 60 Hz. Change constants

Increase device orientation event frequency to 60 Hz to match requestAnimationFrame rate on most hardware. Change constants to microseconds for increased precision.

BUG=413327
Committed: https://crrev.com/150650db7028bb7d738436f73071bc7127918390
Cr-Commit-Position: refs/heads/master@{#297332}

[jdarpinian, 2014-09-25 00:55:15]

jdarpinian@chromium.org changed reviewers:
+ timvolodine@chromium.org

[jdarpinian, 2014-09-25 00:55:15]

With this change, http://vr.chromeexperiments.com works much better. However,
it's not perfect: we miss frames often (several times per second). Increasing
the frequency to 120 Hz drastically reduces missed frames, but I'm not sure if
we want to go that far.

[timvolodine, 2014-09-25 21:30:51]

as an additional supporting argument maybe also add in the description that this
frequency would match requestAnimationFrame frequency (if I remember correctly).
some nits below.

https://codereview.chromium.org/604483003/diff/20001/content/browser/device_s...
File content/browser/device_sensors/inertial_sensor_consts.h (right):

https://codereview.chromium.org/604483003/diff/20001/content/browser/device_s...
content/browser/device_sensors/inertial_sensor_consts.h:22: const int
kInertialSensorIntervalMicroseconds = 1000000 / 60;
it not very clear what the interval means. could you use
Time::kMicrosecondsPerSecond instead of 1000000? also it may make sense to
introduce
kInertialSensorSamplingFrequencyHz = 60;
kInertialSensorIntervalMicroseconds = Time::kMicrosecondsPerSecond /
kInertialSensorSamplingFrequencyHz;

https://codereview.chromium.org/604483003/diff/20001/content/browser/device_s...
content/browser/device_sensors/inertial_sensor_consts.h:25: const int
kLightSensorIntervalMicroseconds = 1000000 / 5;
same as above

https://codereview.chromium.org/604483003/diff/20001/content/renderer/device_...
File content/renderer/device_sensors/device_light_event_pump.cc (right):

https://codereview.chromium.org/604483003/diff/20001/content/renderer/device_...
content/renderer/device_sensors/device_light_event_pump.cc:13: const int
kDefaultLightPumpDelayMicroseconds = 1000000 / 5;
same here

https://codereview.chromium.org/604483003/diff/20001/content/renderer/device_...
File content/renderer/device_sensors/device_sensor_event_pump.h (right):

https://codereview.chromium.org/604483003/diff/20001/content/renderer/device_...
content/renderer/device_sensors/device_sensor_event_pump.h:19: static const int
kDefaultPumpDelayMicroseconds = 1000000 / 60;
same here

[jdarpinian, 2014-09-25 22:36:19]

https://codereview.chromium.org/604483003/diff/20001/content/browser/device_s...
File content/browser/device_sensors/inertial_sensor_consts.h (right):

https://codereview.chromium.org/604483003/diff/20001/content/browser/device_s...
content/browser/device_sensors/inertial_sensor_consts.h:22: const int
kInertialSensorIntervalMicroseconds = 1000000 / 60;
On 2014/09/25 21:30:51, timvolodine wrote:
> it not very clear what the interval means. could you use
> Time::kMicrosecondsPerSecond instead of 1000000? also it may make sense to
> introduce
> kInertialSensorSamplingFrequencyHz = 60;
> kInertialSensorIntervalMicroseconds = Time::kMicrosecondsPerSecond /
> kInertialSensorSamplingFrequencyHz;

Done.

https://codereview.chromium.org/604483003/diff/20001/content/browser/device_s...
content/browser/device_sensors/inertial_sensor_consts.h:25: const int
kLightSensorIntervalMicroseconds = 1000000 / 5;
On 2014/09/25 21:30:51, timvolodine wrote:
> same as above

Done.

https://codereview.chromium.org/604483003/diff/20001/content/renderer/device_...
File content/renderer/device_sensors/device_light_event_pump.cc (right):

https://codereview.chromium.org/604483003/diff/20001/content/renderer/device_...
content/renderer/device_sensors/device_light_event_pump.cc:13: const int
kDefaultLightPumpDelayMicroseconds = 1000000 / 5;
On 2014/09/25 21:30:51, timvolodine wrote:
> same here

Done.

https://codereview.chromium.org/604483003/diff/20001/content/renderer/device_...
File content/renderer/device_sensors/device_sensor_event_pump.h (right):

https://codereview.chromium.org/604483003/diff/20001/content/renderer/device_...
content/renderer/device_sensors/device_sensor_event_pump.h:19: static const int
kDefaultPumpDelayMicroseconds = 1000000 / 60;
On 2014/09/25 21:30:51, timvolodine wrote:
> same here

Done.

[timvolodine, 2014-09-26 13:09:16]

https://codereview.chromium.org/604483003/diff/40001/content/browser/device_s...
File content/browser/device_sensors/inertial_sensor_consts.h (right):

https://codereview.chromium.org/604483003/diff/40001/content/browser/device_s...
content/browser/device_sensors/inertial_sensor_consts.h:27:
base::Time::kMicrosecondsPerSecond / kInertialSensorSamplingRateHz;
nit: maybe insert whiteline here, otherwise looks very dense.

https://codereview.chromium.org/604483003/diff/40001/content/browser/device_s...
File content/browser/device_sensors/sensor_manager_android.cc (right):

https://codereview.chromium.org/604483003/diff/40001/content/browser/device_s...
content/browser/device_sensors/sensor_manager_android.cc:153: int
rate_in_microseconds = (event_type == kTypeLight)
int64?

https://codereview.chromium.org/604483003/diff/40001/content/browser/device_s...
content/browser/device_sensors/sensor_manager_android.cc:160:
rate_in_microseconds);
also if you are using int64, the java interface should be a long.

https://codereview.chromium.org/604483003/diff/40001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/DeviceSensors.java
(right):

https://codereview.chromium.org/604483003/diff/40001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/DeviceSensors.java:102:
public boolean start(long nativePtr, int eventType, int rateInMicroseconds) {
long, also below

https://codereview.chromium.org/604483003/diff/40001/content/renderer/device_...
File content/renderer/device_sensors/device_sensor_event_pump.h (right):

https://codereview.chromium.org/604483003/diff/40001/content/renderer/device_...
content/renderer/device_sensors/device_sensor_event_pump.h:93: int
pump_delay_microseconds_;
int64?

[jdarpinian, 2014-09-26 23:40:30]

I had changed the constants to int64 to match time::kMicrosecondsPerSecond, but
now that I look at it the actual underlying Android API takes a 32-bit integer,
so int makes more sense. I changed them all back to int.

On 2014/09/26 13:09:16, timvolodine wrote:
>
https://codereview.chromium.org/604483003/diff/40001/content/browser/device_s...
> File content/browser/device_sensors/inertial_sensor_consts.h (right):
> 
>
https://codereview.chromium.org/604483003/diff/40001/content/browser/device_s...
> content/browser/device_sensors/inertial_sensor_consts.h:27:
> base::Time::kMicrosecondsPerSecond / kInertialSensorSamplingRateHz;
> nit: maybe insert whiteline here, otherwise looks very dense.
> 
>
https://codereview.chromium.org/604483003/diff/40001/content/browser/device_s...
> File content/browser/device_sensors/sensor_manager_android.cc (right):
> 
>
https://codereview.chromium.org/604483003/diff/40001/content/browser/device_s...
> content/browser/device_sensors/sensor_manager_android.cc:153: int
> rate_in_microseconds = (event_type == kTypeLight)
> int64?
> 
>
https://codereview.chromium.org/604483003/diff/40001/content/browser/device_s...
> content/browser/device_sensors/sensor_manager_android.cc:160:
> rate_in_microseconds);
> also if you are using int64, the java interface should be a long.
> 
>
https://codereview.chromium.org/604483003/diff/40001/content/public/android/j...
> File
>
content/public/android/java/src/org/chromium/content/browser/DeviceSensors.java
> (right):
> 
>
https://codereview.chromium.org/604483003/diff/40001/content/public/android/j...
>
content/public/android/java/src/org/chromium/content/browser/DeviceSensors.java:102:
> public boolean start(long nativePtr, int eventType, int rateInMicroseconds) {
> long, also below
> 
>
https://codereview.chromium.org/604483003/diff/40001/content/renderer/device_...
> File content/renderer/device_sensors/device_sensor_event_pump.h (right):
> 
>
https://codereview.chromium.org/604483003/diff/40001/content/renderer/device_...
> content/renderer/device_sensors/device_sensor_event_pump.h:93: int
> pump_delay_microseconds_;
> int64?

[jdarpinian, 2014-09-26 23:40:49]

https://codereview.chromium.org/604483003/diff/40001/content/browser/device_s...
File content/browser/device_sensors/inertial_sensor_consts.h (right):

https://codereview.chromium.org/604483003/diff/40001/content/browser/device_s...
content/browser/device_sensors/inertial_sensor_consts.h:27:
base::Time::kMicrosecondsPerSecond / kInertialSensorSamplingRateHz;
On 2014/09/26 13:09:16, timvolodine wrote:
> nit: maybe insert whiteline here, otherwise looks very dense.

Done.

[timvolodine, 2014-09-29 15:54:45]

I see, lgtm with nit.

https://codereview.chromium.org/604483003/diff/60001/content/browser/device_s...
File content/browser/device_sensors/inertial_sensor_consts.h (right):

https://codereview.chromium.org/604483003/diff/60001/content/browser/device_s...
content/browser/device_sensors/inertial_sensor_consts.h:8: #include
"base/basictypes.h"
is this needed?

[jdarpinian, 2014-09-29 21:08:44]

The CQ bit was checked by jdarpinian@chromium.org

[jdarpinian, 2014-09-29 21:08:53]

https://codereview.chromium.org/604483003/diff/60001/content/browser/device_s...
File content/browser/device_sensors/inertial_sensor_consts.h (right):

https://codereview.chromium.org/604483003/diff/60001/content/browser/device_s...
content/browser/device_sensors/inertial_sensor_consts.h:8: #include
"base/basictypes.h"
On 2014/09/29 15:54:45, timvolodine wrote:
> is this needed?

Not anymore. Removed.

[commit-bot: I haz the power, 2014-09-29 21:09:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/604483003/80001

[commit-bot: I haz the power, 2014-09-29 21:14:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-29 21:14:57]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/62076)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/66931)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[jdarpinian, 2014-09-30 00:59:03]

The CQ bit was checked by jdarpinian@chromium.org

[commit-bot: I haz the power, 2014-09-30 00:59:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/604483003/100001

[commit-bot: I haz the power, 2014-09-30 01:36:59]

Committed patchset #6 (id:100001) as fcd7bd62a550791705af1dc45d3a7abaf88ebc99

[commit-bot: I haz the power, 2014-09-30 01:37:39]

Patchset 6 (id:??) landed as
https://crrev.com/150650db7028bb7d738436f73071bc7127918390
Cr-Commit-Position: refs/heads/master@{#297332}