Skip to content

Commit

Permalink
Refactor WebXR Graphics Delegates classes
Browse files Browse the repository at this point in the history
While working to implement the Cardboard Overlay, I've hit some road
blocks to just re-using the existing GraphicsDelegateWin; most likely
because we do not use command-buffer based gl on Android. Ultimately
it seems that we'll need to implement a new GraphicsDelegateAndroid to
implement the Overlay for both Cardboard and OpenXR. Further, some of
the methods in GraphicsDelegateWin weren't actually used the way that
they claimed to be anymore. Mainly that everything was called from the
same thread and thus initialization no longer needed to be split up.

The following refactors were made to enable eventually implementing a
GraphicsDelegateCardboard:

1) Simplify interface of GraphicsDelegateWin
2) Move methods that VrBrowserRendererThreadWin calls into
   GraphicsDelegate to facilitate creation of a GraphicsDelegateAndroid
3) Collapse BaseGraphicsDelegate into it's only remaining child
   (the GvrGraphicsDelegate)
   - GVR is deprecated and will be removed soon and this allowed
     explicit logic around the NOTREACHED methods to be better
     documented

Bug: 1429376
Change-Id: Id1f988a6c8869dbf563b2569cafb435fe9c92d3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4936854
Reviewed-by: Piotr Bialecki <bialpio@chromium.org>
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
Auto-Submit: Alexander Cooper <alcooper@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1212861}
  • Loading branch information
alcooper91 authored and Chromium LUCI CQ committed Oct 20, 2023
1 parent d091f16 commit 63f82db
Show file tree
Hide file tree
Showing 11 changed files with 184 additions and 185 deletions.
100 changes: 98 additions & 2 deletions chrome/browser/android/vr/gvr_graphics_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <algorithm>

#include "base/functional/bind.h"
#include "base/notreached.h"
#include "base/numerics/safe_conversions.h"
#include "base/ranges/algorithm.h"
#include "base/synchronization/waitable_event.h"
Expand Down Expand Up @@ -127,7 +128,11 @@ GvrGraphicsDelegate::GvrGraphicsDelegate(
webvr_acquire_time_(sliding_time_size),
webvr_submit_time_(sliding_time_size) {}

GvrGraphicsDelegate::~GvrGraphicsDelegate() = default;
GvrGraphicsDelegate::~GvrGraphicsDelegate() {
if (curr_context_id_ != kNone) {
contexts_[curr_context_id_]->ReleaseCurrent(surface_.get());
}
}

void GvrGraphicsDelegate::Init(
base::WaitableEvent* gl_surface_created_event,
Expand Down Expand Up @@ -173,7 +178,19 @@ void GvrGraphicsDelegate::InitializeGl(gfx::AcceleratedWidget window) {
return;
}

if (!BaseGraphicsDelegate::Initialize(surface)) {
surface_ = surface;
share_group_ = base::MakeRefCounted<gl::GLShareGroup>();
for (auto& context : contexts_) {
context = gl::init::CreateGLContext(share_group_.get(), surface_.get(),
gl::GLContextAttribs());
if (!context.get()) {
LOG(ERROR) << "gl::init::CreateGLContext failed";
browser_->ForceExitVr();
return;
}
}

if (!MakeContextCurrent(kMainContext)) {
browser_->ForceExitVr();
return;
}
Expand Down Expand Up @@ -536,4 +553,83 @@ void GvrGraphicsDelegate::RecordFrameTimeTraces() const {
webvr_submit_time_.GetAverage().InMilliseconds());
}

bool GvrGraphicsDelegate::RunInSkiaContext(base::OnceClosure callback) {
DCHECK_EQ(curr_context_id_, kMainContext);
if (!MakeContextCurrent(kSkiaContext)) {
return false;
}
std::move(callback).Run();
return MakeContextCurrent(kMainContext);
}

void GvrGraphicsDelegate::SwapSurfaceBuffers() {
TRACE_EVENT0("gpu", __func__);
DCHECK(surface_);
surface_->SwapBuffers(base::DoNothing(), gfx::FrameData());
}

bool GvrGraphicsDelegate::MakeContextCurrent(ContextId context_id) {
DCHECK(context_id > kNone && context_id < kNumContexts);
if (curr_context_id_ == context_id) {
return true;
}
auto& context = contexts_[context_id];
if (!context->MakeCurrent(surface_.get())) {
LOG(ERROR) << "gl::GLContext::MakeCurrent() failed";
return false;
}
curr_context_id_ = context_id;
return true;
}

void GvrGraphicsDelegate::SetXrViews(
const std::vector<device::mojom::XRViewPtr>& views) {
// Only called by VrBrowserRendererThreadWin which never creates this class.
NOTREACHED_NORETURN();
}

bool GvrGraphicsDelegate::PreRender() {
// Only called by VrBrowserRendererThreadWin which never creates this class.
NOTREACHED_NORETURN();
}

void GvrGraphicsDelegate::PostRender() {
// Only called by VrBrowserRendererThreadWin which never creates this class.
NOTREACHED_NORETURN();
}

mojo::PlatformHandle GvrGraphicsDelegate::GetTexture() {
// Only called by VrBrowserRendererThreadWin which never creates this class.
NOTREACHED_NORETURN();
}

const gpu::SyncToken& GvrGraphicsDelegate::GetSyncToken() {
// Only called by VrBrowserRendererThreadWin which never creates this class.
NOTREACHED_NORETURN();
}

gfx::RectF GvrGraphicsDelegate::GetLeft() {
// Only called by VrBrowserRendererThreadWin which never creates this class.
NOTREACHED_NORETURN();
}

gfx::RectF GvrGraphicsDelegate::GetRight() {
// Only called by VrBrowserRendererThreadWin which never creates this class.
NOTREACHED_NORETURN();
}

void GvrGraphicsDelegate::ResetMemoryBuffer() {
// Only called by VrBrowserRendererThreadWin which never creates this class.
NOTREACHED_NORETURN();
}

bool GvrGraphicsDelegate::BindContext() {
// Only called by VrBrowserRendererThreadWin which never creates this class.
NOTREACHED_NORETURN();
}

void GvrGraphicsDelegate::ClearContext() {
// Only called by VrBrowserRendererThreadWin which never creates this class.
NOTREACHED_NORETURN();
}
} // namespace vr
31 changes: 29 additions & 2 deletions chrome/browser/android/vr/gvr_graphics_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@
#include "base/cancelable_callback.h"
#include "base/containers/queue.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/scoped_refptr.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/vr/base_graphics_delegate.h"
#include "chrome/browser/vr/graphics_delegate.h"
#include "chrome/browser/vr/render_info.h"
#include "device/vr/util/sliding_average.h"
#include "third_party/gvr-android-sdk/src/libraries/headers/vr/gvr/capi/include/gvr.h"
Expand All @@ -37,6 +38,8 @@ class GpuFence;
}

namespace gl {
class GLContext;
class GLShareGroup;
class GLSurface;
class SurfaceTexture;
} // namespace gl
Expand Down Expand Up @@ -70,7 +73,7 @@ struct Viewport {
}
};

class GvrGraphicsDelegate : public BaseGraphicsDelegate {
class GvrGraphicsDelegate : public GraphicsDelegate {
public:
using WebXrTokenSignaledCallback =
base::OnceCallback<void(std::unique_ptr<gfx::GpuFence>)>;
Expand Down Expand Up @@ -115,6 +118,8 @@ class GvrGraphicsDelegate : public BaseGraphicsDelegate {
gfx::Size webxr_surface_size() const { return webxr_surface_size_; }

private:
enum ContextId { kNone = -1, kMainContext, kSkiaContext, kNumContexts };

// GraphicsDelegate overrides.
FovRectangles GetRecommendedFovs() override;
float GetZNear() override;
Expand All @@ -127,6 +132,20 @@ class GvrGraphicsDelegate : public BaseGraphicsDelegate {
void PrepareBufferForBrowserUi() override;
void OnFinishedDrawingBuffer() override;
void GetWebXrDrawParams(int* texture_id, Transform* uv_transform) override;
bool RunInSkiaContext(base::OnceClosure callback) override;
// The following GraphicsDelegate overrides are only called by
// vr_browser_renderer_thread_win and thus are not used here. They will be
// relevant for GraphicsDelegateAndroid.
void SetXrViews(const std::vector<device::mojom::XRViewPtr>& views) override;
bool PreRender() override;
void PostRender() override;
mojo::PlatformHandle GetTexture() override;
const gpu::SyncToken& GetSyncToken() override;
gfx::RectF GetLeft() override;
gfx::RectF GetRight() override;
void ResetMemoryBuffer() override;
bool BindContext() override;
void ClearContext() override;
// End GraphicsDelegate overrides.

void UIBoundsChanged(int width, int height);
Expand All @@ -142,6 +161,14 @@ class GvrGraphicsDelegate : public BaseGraphicsDelegate {

void WebVrWaitForServerFence();

void SwapSurfaceBuffers();

bool MakeContextCurrent(ContextId context_id);

scoped_refptr<gl::GLShareGroup> share_group_;
scoped_refptr<gl::GLContext> contexts_[kNumContexts];
ContextId curr_context_id_ = kNone;

raw_ptr<device::WebXrPresentationState> webxr_;

// samplerExternalOES texture data for WebVR content image.
Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/vr/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,6 @@ component("vr_common") {
}

if (!use_command_buffer) {
sources += [
"base_graphics_delegate.cc",
"base_graphics_delegate.h",
]
deps += [ "//ui/gl/init" ]
}
}

Expand Down
68 changes: 0 additions & 68 deletions chrome/browser/vr/base_graphics_delegate.cc

This file was deleted.

48 changes: 0 additions & 48 deletions chrome/browser/vr/base_graphics_delegate.h

This file was deleted.

16 changes: 15 additions & 1 deletion chrome/browser/vr/browser_renderer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "chrome/browser/vr/browser_renderer.h"

#include "base/memory/raw_ptr.h"
#include "base/notreached.h"
#include "chrome/browser/vr/graphics_delegate.h"
#include "chrome/browser/vr/input_delegate.h"
#include "chrome/browser/vr/input_event.h"
Expand Down Expand Up @@ -95,12 +96,25 @@ class MockGraphicsDelegate : public GraphicsDelegate {
using_buffer_ = false;
}
void GetWebXrDrawParams(int*, Transform*) override {}
bool Initialize(const scoped_refptr<gl::GLSurface>&) override { return true; }
bool RunInSkiaContext(base::OnceClosure callback) override {
std::move(callback).Run();
return true;
}

// TODO(https://crbug.com/1493735): Provide implementations during refactor
// as needed.
void SetXrViews(const std::vector<device::mojom::XRViewPtr>& views) override {
}
bool PreRender() override { return true; }
void PostRender() override {}
mojo::PlatformHandle GetTexture() override { NOTREACHED_NORETURN(); }
const gpu::SyncToken& GetSyncToken() override { NOTREACHED_NORETURN(); }
gfx::RectF GetLeft() override { return {}; }
gfx::RectF GetRight() override { return {}; }
void ResetMemoryBuffer() override {}
bool BindContext() override { return true; }
void ClearContext() override {}

private:
void UseBuffer() {
CHECK(!using_buffer_);
Expand Down

0 comments on commit 63f82db

Please sign in to comment.