Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(736)

Issue 2413903002: Allow VRDisplay to manage it's own rAF callbacks (Closed)

Created:
4 years, 2 months ago by bajones
Modified:
4 years, 1 month ago
CC:
darktears, blink-reviews, blink-reviews-animation_chromium.org, blink-reviews-dom_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, Eric Willigers, haraken, rjwright, rwlbuis, shans, sof
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Allow VRDisplay to manage it's own rAF callbacks. This will be necessary when we begin firing VR rAFs at a different rate than window.rAF, but here is used to prevent misuse of submitFrame, which needs to happen in a VR-specific rAF in order to ensure a good experience across the board. BUG=389343 Committed: https://crrev.com/8dcae4a8c016b4b284cc550b23bc404a2c439d25 Cr-Commit-Position: refs/heads/master@{#427496}

Patch Set 1 #

Patch Set 2 : Fixing commit hook issues #

Patch Set 3 : A Few more tweaks #

Total comments: 3

Patch Set 4 : Address review feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+87 lines, -10 lines) Patch
M third_party/WebKit/Source/core/dom/ScriptedAnimationController.h View 1 2 chunks +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/vr/VRDisplay.h View 1 2 4 chunks +8 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/vr/VRDisplay.cpp View 1 2 3 8 chunks +77 lines, -9 lines 0 comments Download

Messages

Total messages: 21 (11 generated)
bajones
mkwst@: Could you take a look at the use of ScriptedAnimationController? Eventually we'll have a ...
4 years, 2 months ago (2016-10-14 22:10:15 UTC) #3
Mike West
I'm a bad reviewer for timing kinds of things. +skyostil@ who knows things about scheduling.
4 years, 2 months ago (2016-10-17 11:39:53 UTC) #5
Sami
This basically lgtm, although I had some questions mainly because I'm not very familiar with ...
4 years, 1 month ago (2016-10-24 10:12:24 UTC) #6
Mike West
RS LGTM. Once skyostil@ is happy with the shape of things, I'm happy too.
4 years, 1 month ago (2016-10-24 10:43:15 UTC) #7
bajones
> This basically lgtm, although I had some questions mainly because I'm not very > ...
4 years, 1 month ago (2016-10-24 22:13:01 UTC) #8
klausw
On 2016/10/24 22:13:01, bajones wrote: > https://codereview.chromium.org/2413903002/diff/40001/third_party/WebKit/Source/modules/vr/VRDisplay.cpp#newcode493 > > third_party/WebKit/Source/modules/vr/VRDisplay.cpp:493: // We need to make ...
4 years, 1 month ago (2016-10-24 23:00:16 UTC) #11
Sami
Thanks for the links! lgtm -- although I agree with Klaus that the special background ...
4 years, 1 month ago (2016-10-25 10:55:43 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2413903002/60001
4 years, 1 month ago (2016-10-25 21:25:02 UTC) #17
commit-bot: I haz the power
Committed patchset #4 (id:60001)
4 years, 1 month ago (2016-10-25 21:55:00 UTC) #19
commit-bot: I haz the power
4 years, 1 month ago (2016-10-25 21:57:27 UTC) #21
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/8dcae4a8c016b4b284cc550b23bc404a2c439d25
Cr-Commit-Position: refs/heads/master@{#427496}

Powered by Google App Engine
This is Rietveld 408576698