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

Issue 2891033002: WebVR: flush before requesting sync token for mailbox (Closed)

Created:
3 years, 7 months ago by klausw
Modified:
3 years, 7 months ago
Reviewers:
*mthiesse, bajones
CC:
chromium-reviews, haraken, blink-reviews, feature-vr-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

WebVR: flush before requesting sync token for mailbox This is a speculative fix, but seems to mostly eliminate black screen flashes during VR presentation when the "WebVR experimental rendering optimization" flag is enabled. (It's off by default.) Since this flush can happen in parallel with the previous frame's rendering, move it ahead of the "wait for previous render to finish" wait. BUG=723962 Review-Url: https://codereview.chromium.org/2891033002 Cr-Commit-Position: refs/heads/master@{#473687} Committed: https://chromium.googlesource.com/chromium/src/+/83c3cf1e053cbeaa118ab891cb32fc5629a167a6

Patch Set 1 #

Total comments: 2

Patch Set 2 : Fix variable name mixup #

Patch Set 3 : Tweak trace events #

Patch Set 4 : Rebase to 2888313002/#ps40001 (ps3) #

Patch Set 5 : Rebase on origin/master #

Patch Set 6 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -5 lines) Patch
M third_party/WebKit/Source/modules/vr/VRDisplay.cpp View 1 2 3 4 5 chunks +25 lines, -5 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 37 (25 generated)
klausw
As noted in the description, this is a bit speculative, but it seemed to fix ...
3 years, 7 months ago (2017-05-18 05:55:22 UTC) #3
mthiesse
lgtm https://codereview.chromium.org/2891033002/diff/1/third_party/WebKit/Source/modules/vr/VRDisplay.cpp File third_party/WebKit/Source/modules/vr/VRDisplay.cpp (right): https://codereview.chromium.org/2891033002/diff/1/third_party/WebKit/Source/modules/vr/VRDisplay.cpp#newcode622 third_party/WebKit/Source/modules/vr/VRDisplay.cpp:622: auto token = static_image->GetSyncToken(); s/token/sync_token
3 years, 7 months ago (2017-05-18 15:33:53 UTC) #8
bajones
On 2017/05/18 15:33:53, mthiesse wrote: > lgtm > > https://codereview.chromium.org/2891033002/diff/1/third_party/WebKit/Source/modules/vr/VRDisplay.cpp > File third_party/WebKit/Source/modules/vr/VRDisplay.cpp (right): > ...
3 years, 7 months ago (2017-05-18 15:54:05 UTC) #9
klausw
On 2017/05/18 15:33:53, mthiesse wrote: > lgtm > > https://codereview.chromium.org/2891033002/diff/1/third_party/WebKit/Source/modules/vr/VRDisplay.cpp > File third_party/WebKit/Source/modules/vr/VRDisplay.cpp (right): > ...
3 years, 7 months ago (2017-05-18 17:46:27 UTC) #10
klausw
3 years, 7 months ago (2017-05-18 17:46:44 UTC) #11
klausw
https://codereview.chromium.org/2891033002/diff/1/third_party/WebKit/Source/modules/vr/VRDisplay.cpp File third_party/WebKit/Source/modules/vr/VRDisplay.cpp (right): https://codereview.chromium.org/2891033002/diff/1/third_party/WebKit/Source/modules/vr/VRDisplay.cpp#newcode622 third_party/WebKit/Source/modules/vr/VRDisplay.cpp:622: auto token = static_image->GetSyncToken(); On 2017/05/18 15:33:53, mthiesse wrote: ...
3 years, 7 months ago (2017-05-18 17:47:39 UTC) #12
commit-bot: I haz the power
This CL has an open dependency (Issue 2888313002 Patch 1). Please resolve the dependency and ...
3 years, 7 months ago (2017-05-18 17:49:11 UTC) #16
klausw
FYI, I made some minor edits to tweak the event tracing so that it focuses ...
3 years, 7 months ago (2017-05-18 17:59:45 UTC) #17
mthiesse
lgtm
3 years, 7 months ago (2017-05-18 18:02:17 UTC) #18
klausw
On 2017/05/18 18:02:17, mthiesse wrote: > lgtm I've rebased this change top of origin/master instead ...
3 years, 7 months ago (2017-05-19 23:33:28 UTC) #25
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/2891033002/100001
3 years, 7 months ago (2017-05-22 19:52:08 UTC) #34
commit-bot: I haz the power
3 years, 7 months ago (2017-05-22 20:57:57 UTC) #37
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/83c3cf1e053cbeaa118ab891cb32...

Powered by Google App Engine
This is Rietveld 408576698