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

Unified Diff: third_party/WebKit/Source/modules/vr/VRDisplay.cpp

Issue 2384593002: Encode frame number in pixel data for pose sync (Closed)
Patch Set: Frame pose sync: fix oscillating off-by-one Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/modules/vr/VRDisplay.cpp
diff --git a/third_party/WebKit/Source/modules/vr/VRDisplay.cpp b/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
index b5ad05be54efe74136e4e23bea508c19a7bb62c2..867ffbcc09545f98240a7e3478915eeccc126e96 100644
--- a/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
+++ b/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
@@ -121,6 +121,10 @@ VRPose* VRDisplay::getPose()
void VRDisplay::updatePose()
{
if (m_canUpdateFramePose) {
+ // Increment pose counter. This must stay in sync with the
+ // frameNumStarted_ counter in device/vr/android/gvr/gvr_device.cc.
+ ++m_poseNum;
bajones 2016/10/02 20:55:23 This feels very fragile. Let's add a poseId to the
klausw 2016/10/02 21:25:50 I agree, that this would be much cleaner. I'll loo
klausw 2016/10/04 00:40:41 Done. I saw it has a timestamp already internally,
+ // LOG << "klausw:VRDisplay::updatePose " << m_poseNum;
m_framePose = controller()->getPose(m_displayId);
if (m_isPresenting)
m_canUpdateFramePose = false;
@@ -346,6 +350,32 @@ HeapVector<VRLayer> VRDisplay::getLayers()
void VRDisplay::submitFrame()
{
+ // Write the frame number for the pose used into a bottom left pixel block.
+ // It is read by chrome/browser/android/vr_shell/vr_shell.cc to associate
+ // the correct corresponding pose for submission.
+ auto ctx = toWebGLRenderingContextBase(m_layer.source()->renderingContext());
bajones 2016/10/02 20:55:23 Probably just want to cache this on calls to reque
klausw 2016/10/02 21:25:50 Sounds good.
klausw 2016/10/04 00:40:41 Done, saving the underlying GL context since that'
+ int frame = m_poseNum;
+ ctx->scissor(0, 0, 4, 4);
bajones 2016/10/02 20:55:23 why 4x4? we're only reading one pixel
klausw 2016/10/02 21:25:50 I need to investigate more. I had tried 2x2 at som
klausw 2016/10/04 00:40:41 I tested, smallest that worked was (0, 0, 1, 3). I
+ ctx->colorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
+ ctx->clearColor(
+ (frame & 255) / 255.0f,
+ ((frame >> 8) & 255) / 255.0f,
+ ((frame >> 16) & 255) / 255.0f,
+ 1.0f);
+ ctx->enable(GL_SCISSOR_TEST);
+ ctx->clear(GL_COLOR_BUFFER_BIT);
+
+ // TODO(klausw): restore the previous parameters here.
+ // WebGLRenderingContextBase's getParameter is based on ScriptState and
+ // ScriptValue, I don't know how those work. For now, just restore some
+ // presumably-sane values.
+ //
+ // Alternatively, use a GL context from ctx->drawingBuffer()->contextGL(),
+ // but drawingBuffer() is a protected method.
+ ctx->disable(GL_SCISSOR_TEST);
bajones 2016/10/02 20:55:23 This is definitely not going to fly, as it will br
klausw 2016/10/02 21:25:50 I know it's not acceptable, hence the TODO. I just
klausw 2016/10/03 16:50:17 Good news, WebGLRenderingContextBase has a public
klausw 2016/10/04 00:40:41 This seems to work now, though there's some slight
+ ctx->clearColor(0, 0, 0, 1.0f);
+ // LOG << "klausw:VRDisplay::submitFrame " << frameNum;
+
controller()->submitFrame(m_displayId, m_framePose.Clone());
m_canUpdateFramePose = true;
}

Powered by Google App Engine
This is Rietveld 408576698