[Sheriff] Revert "WebXR raw camera: revise the API to match the lates…

…t explainer" This reverts commit ff6bb4ad7287908e5ab2c3a6963c2bfbc30bebeb. Reason for revert: Supposendly causes memory leaks at WebKit Linux Leak, first run: https://ci.chromium.org/ui/p/chromium/builders/ci/WebKit%20Linux%20Leak/25321/overview Original change's description: > WebXR raw camera: revise the API to match the latest explainer > > Change-Id: Ib9e5ff94ccfdad03f3307a55383b36cf942a4a92 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2808840 > Commit-Queue: Piotr Bialecki <bialpio@chromium.org> > Reviewed-by: Daniel Cheng <dcheng@chromium.org> > Reviewed-by: Alexander Cooper <alcooper@chromium.org> > Cr-Commit-Position: refs/heads/master@{#872195} Owners-Override +1 Change-Id: Ic05a15dccd3499940c73d05391fc5a60348e679a No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2825368 Auto-Submit: Anatoliy Potapchuk <apotapchuk@chromium.org> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Anatoliy Potapchuk <apotapchuk@google.com> Reviewed-by: Anqing Zhao <anqing@chromium.org> Owners-Override: Anatoliy Potapchuk <apotapchuk@google.com> Cr-Commit-Position: refs/heads/master@{#872326} GitOrigin-RevId: 3653a271c334425656c8cafc45c0e02e0d603ca9
mjfroman · Apr 14, 2021 · 53f383c · 53f383c
1 parent 55bb3e1
commit 53f383c
Show file tree

Hide file tree

Showing 20 changed files with 29 additions and 189 deletions.
diff --git a/blink/renderer/bindings/generated_in_modules.gni b/blink/renderer/bindings/generated_in_modules.gni
@@ -2314,8 +2314,6 @@ generated_interface_sources_in_modules = [
   "$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_xr_anchor_set.h",
   "$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_xr_bounded_reference_space.cc",
   "$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_xr_bounded_reference_space.h",
-  "$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_xr_camera.cc",
-  "$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_xr_camera.h",
   "$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_xr_cpu_depth_information.cc",
   "$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_xr_cpu_depth_information.h",
   "$root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_xr_depth_information.cc",

diff --git a/blink/renderer/bindings/idl_in_modules.gni b/blink/renderer/bindings/idl_in_modules.gni
@@ -1064,7 +1064,6 @@ static_idl_files_in_modules = get_path_info(
           "//third_party/blink/renderer/modules/xr/xr_anchor.idl",
           "//third_party/blink/renderer/modules/xr/xr_anchor_set.idl",
           "//third_party/blink/renderer/modules/xr/xr_bounded_reference_space.idl",
-          "//third_party/blink/renderer/modules/xr/xr_camera.idl",
           "//third_party/blink/renderer/modules/xr/xr_cpu_depth_information.idl",
           "//third_party/blink/renderer/modules/xr/xr_depth_information.idl",
           "//third_party/blink/renderer/modules/xr/xr_depth_state_init.idl",

diff --git a/blink/renderer/modules/xr/BUILD.gn b/blink/renderer/modules/xr/BUILD.gn
@@ -15,8 +15,6 @@ blink_modules_sources("xr") {
     "xr_anchor_set.h",
     "xr_bounded_reference_space.cc",
     "xr_bounded_reference_space.h",
-    "xr_camera.cc",
-    "xr_camera.h",
     "xr_canvas_input_provider.cc",
     "xr_canvas_input_provider.h",
     "xr_cpu_depth_information.cc",

diff --git a/blink/renderer/modules/xr/idls.gni b/blink/renderer/modules/xr/idls.gni
@@ -8,7 +8,6 @@ modules_idl_files = [
   "xr_anchor.idl",
   "xr_anchor_set.idl",
   "xr_bounded_reference_space.idl",
-  "xr_camera.idl",
   "xr_cpu_depth_information.idl",
   "xr_depth_information.idl",
   "xr_dom_overlay_state.idl",

diff --git a/blink/renderer/modules/xr/xr_camera.cc b/blink/renderer/modules/xr/xr_camera.cc
diff --git a/blink/renderer/modules/xr/xr_camera.h b/blink/renderer/modules/xr/xr_camera.h
diff --git a/blink/renderer/modules/xr/xr_camera.idl b/blink/renderer/modules/xr/xr_camera.idl
diff --git a/blink/renderer/modules/xr/xr_session.cc b/blink/renderer/modules/xr/xr_session.cc
@@ -32,7 +32,6 @@
 #include "third_party/blink/renderer/modules/xr/type_converters.h"
 #include "third_party/blink/renderer/modules/xr/xr_anchor_set.h"
 #include "third_party/blink/renderer/modules/xr/xr_bounded_reference_space.h"
-#include "third_party/blink/renderer/modules/xr/xr_camera.h"
 #include "third_party/blink/renderer/modules/xr/xr_canvas_input_provider.h"
 #include "third_party/blink/renderer/modules/xr/xr_cube_map.h"
 #include "third_party/blink/renderer/modules/xr/xr_depth_information.h"
@@ -245,7 +244,6 @@ constexpr char XRSession::kNoSpaceSpecified[];
 constexpr char XRSession::kAnchorsFeatureNotSupported[];
 constexpr char XRSession::kPlanesFeatureNotSupported[];
 constexpr char XRSession::kDepthSensingFeatureNotSupported[];
-constexpr char XRSession::kRawCameraAccessFeatureNotSupported[];
 
 class XRSession::XRSessionResizeObserverDelegate final
     : public ResizeObserver::Delegate {
@@ -1760,16 +1758,6 @@ void XRSession::UpdateWorldUnderstandingStateForFrame(
     if (world_light_probe_ && light_data) {
       world_light_probe_->ProcessLightEstimationData(light_data, timestamp);
     }
-
-    camera_image_size_ = base::nullopt;
-    if (frame_data->camera_image_size.has_value()) {
-      DCHECK(frame_data->camera_image_buffer_holder.has_value());
-
-      // Let's store the camera image size. The texture ID will be filled out on
-      // the XRWebGLLayer by the session once the frame starts
-      // (in XRSession::OnFrame()).
-      camera_image_size_ = frame_data->camera_image_size;
-    }
   } else {
     plane_manager_->ProcessPlaneInformation(nullptr, timestamp);
     ProcessAnchorsData(nullptr, timestamp);
@@ -1784,8 +1772,6 @@ void XRSession::UpdateWorldUnderstandingStateForFrame(
     if (world_light_probe_) {
       world_light_probe_->ProcessLightEstimationData(nullptr, timestamp);
     }
-
-    camera_image_size_ = base::nullopt;
   }
 }
 

diff --git a/blink/renderer/modules/xr/xr_session.h b/blink/renderer/modules/xr/xr_session.h
@@ -86,8 +86,6 @@ class XRSession final
       "Plane detection feature is not supported by the session.";
   static constexpr char kDepthSensingFeatureNotSupported[] =
       "Depth sensing feature is not supported by the session.";
-  static constexpr char kRawCameraAccessFeatureNotSupported[] =
-      "Raw camera access feature is not supported by the session.";
   // Runs all the video.requestVideoFrameCallback() callbacks associated with
   // one HTMLVideoElement. |double| is the |high_res_now_ms|, derived from
   // MonotonicTimeToZeroBasedDocumentTime(|current_frame_time|), to be passed as
@@ -140,7 +138,6 @@ class XRSession final
   XRAnchorSet* TrackedAnchors() const;
 
   bool immersive() const;
-  device::mojom::blink::XRSessionMode mode() const { return mode_; }
 
   DEFINE_ATTRIBUTE_EVENT_LISTENER(end, kEnd)
   DEFINE_ATTRIBUTE_EVENT_LISTENER(select, kSelect)
@@ -379,10 +376,6 @@ class XRSession final
   HeapVector<Member<XRImageTrackingResult>> ImageTrackingResults(
       ExceptionState&);
 
-  const base::Optional<gfx::Size>& CameraImageSize() const {
-    return camera_image_size_;
-  }
-
  private:
   class XRSessionResizeObserverDelegate;
 
@@ -552,10 +545,6 @@ class XRSession final
   Member<XRPlaneManager> plane_manager_;
   Member<XRDepthManager> depth_manager_;
 
-  // Populated iff the raw camera feature has been enabled and the session
-  // received a frame from the device that contained the camera image.
-  base::Optional<gfx::Size> camera_image_size_;
-
   uint32_t view_parameters_id_ = 0;
   HeapVector<Member<XRViewData>> views_;
   Vector<device::mojom::blink::VREyeParametersPtr> pending_view_parameters_;

diff --git a/blink/renderer/modules/xr/xr_view.cc b/blink/renderer/modules/xr/xr_view.cc
@@ -5,9 +5,7 @@
 #include "third_party/blink/renderer/modules/xr/xr_view.h"
 
 #include "base/numerics/ranges.h"
-#include "third_party/blink/renderer/modules/xr/xr_camera.h"
 #include "third_party/blink/renderer/modules/xr/xr_frame.h"
-#include "third_party/blink/renderer/modules/xr/xr_session.h"
 #include "third_party/blink/renderer/modules/xr/xr_utils.h"
 #include "third_party/blink/renderer/platform/geometry/float_point_3d.h"
 
@@ -166,30 +164,6 @@ void XRView::requestViewportScale(base::Optional<double> scale) {
   view_data_->requestViewportScale(scale);
 }
 
-XRCamera* XRView::camera() const {
-  const bool camera_access_enabled = frame_->session()->IsFeatureEnabled(
-      device::mojom::XRSessionFeature::CAMERA_ACCESS);
-  const bool is_immersive_ar_session =
-      frame_->session()->mode() ==
-      device::mojom::blink::XRSessionMode::kImmersiveAr;
-
-  if (camera_access_enabled && is_immersive_ar_session) {
-    // The feature is enabled and we're in immersive-ar session, so let's return
-    // a camera object if the camera image was received in the current frame.
-    // Note: currently our only implementation of AR sessions is provided by
-    // ARCore device, which should *not* return a frame data with camera image
-    // that is not set in case the raw camera access is enabled, so we could
-    // DCHECK that the camera image size has value. Since there may be other AR
-    // devices that implement raw camera access via a different mechanism that's
-    // not neccessarily frame-aligned, a DCHECK here would affect them.
-    if (frame_->session()->CameraImageSize().has_value()) {
-      return MakeGarbageCollected<XRCamera>(frame_);
-    }
-  }
-
-  return nullptr;
-}
-
 void XRView::Trace(Visitor* visitor) const {
   visitor->Trace(frame_);
   visitor->Trace(projection_matrix_);

diff --git a/blink/renderer/modules/xr/xr_view.h b/blink/renderer/modules/xr/xr_view.h
@@ -17,7 +17,6 @@
 
 namespace blink {
 
-class XRCamera;
 class XRFrame;
 class XRSession;
 class XRViewData;
@@ -38,7 +37,6 @@ class MODULES_EXPORT XRView final : public ScriptWrappable {
   XRSession* session() const;
   DOMFloat32Array* projectionMatrix() const;
   XRRigidTransform* transform() const;
-  XRCamera* camera() const;
 
   // isFirstPersonObserver is only true for views that composed with a video
   // feed that is not directly displayed on the viewer device. Primarily this is

diff --git a/blink/renderer/modules/xr/xr_view.idl b/blink/renderer/modules/xr/xr_view.idl
@@ -24,7 +24,4 @@ enum XREye {
   readonly attribute boolean isFirstPersonObserver;
 
   [RuntimeEnabled=WebXRViewportScale] void requestViewportScale(double? scale);
-
-  [RuntimeEnabled=WebXRCameraAccess, SameObject]
-  readonly attribute XRCamera? camera;
 };
diff --git a/blink/renderer/modules/xr/xr_viewer_pose.cc b/blink/renderer/modules/xr/xr_viewer_pose.cc
@@ -19,16 +19,23 @@ XRViewerPose::XRViewerPose(XRFrame* frame,
 
   const HeapVector<Member<XRViewData>>& view_data = frame->session()->views();
 
+  bool camera_access_enabled = frame->session()->IsFeatureEnabled(
+      device::mojom::XRSessionFeature::CAMERA_ACCESS);
+
   // Snapshot the session's current views.
   for (XRViewData* view : view_data) {
     view->UpdatePoseMatrix(transform_->TransformMatrix());
     XRView* xr_view = MakeGarbageCollected<XRView>(frame, view);
     views_.push_back(xr_view);
+    if (camera_access_enabled) {
+      camera_views_.push_back(xr_view);
+    }
   }
 }
 
 void XRViewerPose::Trace(Visitor* visitor) const {
   visitor->Trace(views_);
+  visitor->Trace(camera_views_);
   XRPose::Trace(visitor);
 }
 

diff --git a/blink/renderer/modules/xr/xr_viewer_pose.h b/blink/renderer/modules/xr/xr_viewer_pose.h
@@ -24,11 +24,15 @@ class XRViewerPose final : public XRPose {
   ~XRViewerPose() override = default;
 
   const HeapVector<Member<XRView>>& views() const { return views_; }
+  const HeapVector<Member<XRView>>& cameraViews() const {
+    return camera_views_;
+  }
 
   void Trace(Visitor*) const override;
 
  private:
   HeapVector<Member<XRView>> views_;
+  HeapVector<Member<XRView>> camera_views_;
 };
 
 }  // namespace blink

diff --git a/blink/renderer/modules/xr/xr_viewer_pose.idl b/blink/renderer/modules/xr/xr_viewer_pose.idl
@@ -9,4 +9,5 @@
     RuntimeEnabled=WebXR
 ] interface XRViewerPose : XRPose {
   [SameObject, SaveSameObject] readonly attribute FrozenArray<XRView> views;
+  [RuntimeEnabled=WebXRCameraAccess, SameObject, SaveSameObject] readonly attribute FrozenArray<XRView> cameraViews;
 };
diff --git a/blink/renderer/modules/xr/xr_webgl_binding.cc b/blink/renderer/modules/xr/xr_webgl_binding.cc
@@ -7,7 +7,6 @@
 #include "third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.h"
 #include "third_party/blink/renderer/modules/webgl/webgl_texture.h"
 #include "third_party/blink/renderer/modules/webgl/webgl_unowned_texture.h"
-#include "third_party/blink/renderer/modules/xr/xr_camera.h"
 #include "third_party/blink/renderer/modules/xr/xr_cube_map.h"
 #include "third_party/blink/renderer/modules/xr/xr_frame.h"
 #include "third_party/blink/renderer/modules/xr/xr_light_probe.h"
@@ -147,58 +146,29 @@ WebGLTexture* XRWebGLBinding::getReflectionCubeMap(
   return texture;
 }
 
-WebGLTexture* XRWebGLBinding::getCameraImage(XRCamera* camera,
-                                             ExceptionState& exception_state) {
-  XRFrame* frame = camera->Frame();
-  DCHECK(frame);
-
-  XRSession* session = frame->session();
-  DCHECK(session);
-
-  if (!session->IsFeatureEnabled(
-          device::mojom::XRSessionFeature::CAMERA_ACCESS)) {
-    DVLOG(2) << __func__ << ": raw camera access is not enabled on a session";
-    exception_state.ThrowDOMException(
-        DOMExceptionCode::kNotSupportedError,
-        XRSession::kRawCameraAccessFeatureNotSupported);
-    return nullptr;
-  }
-
+WebGLTexture* XRWebGLBinding::getCameraImage(XRFrame* frame, XRView* view) {
+  // Verify that frame is currently active.
   if (!frame->IsActive()) {
-    DVLOG(2) << __func__ << ": frame is not active";
-    exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
-                                      XRFrame::kInactiveFrame);
     return nullptr;
   }
 
-  if (!frame->IsAnimationFrame()) {
-    DVLOG(2) << __func__ << ": frame is not animating";
-    exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
-                                      XRFrame::kNonAnimationFrame);
-    return nullptr;
-  }
-
-  if (session_ != session) {
-    exception_state.ThrowDOMException(
-        DOMExceptionCode::kInvalidStateError,
-        "Camera comes from a different session than this binding");
+  if (frame != view->frame()) {
     return nullptr;
   }
 
-  XRWebGLLayer* base_layer = session->renderState()->baseLayer();
+  XRWebGLLayer* base_layer = view->session()->renderState()->baseLayer();
   DCHECK(base_layer);
 
   base::Optional<gpu::MailboxHolder> camera_image_mailbox_holder =
       base_layer->CameraImageMailboxHolder();
 
   if (!camera_image_mailbox_holder) {
-    DVLOG(3) << __func__ << ": camera image mailbox holder is not set";
     return nullptr;
   }
 
   GLuint texture_id = base_layer->CameraImageTextureId();
 
-  // This resource is owned by the XRWebGLLayer, and is freed in OnFrameEnd();
+  // This resource is owned by the renderer, and is freed OnFrameEnd();
   WebGLUnownedTexture* texture = MakeGarbageCollected<WebGLUnownedTexture>(
       webgl_context_, texture_id, GL_TEXTURE_2D);
   return texture;

diff --git a/blink/renderer/modules/xr/xr_webgl_binding.h b/blink/renderer/modules/xr/xr_webgl_binding.h
@@ -15,7 +15,7 @@ namespace blink {
 class ExceptionState;
 class WebGLRenderingContextBase;
 class WebGLTexture;
-class XRCamera;
+class XRFrame;
 class XRLightProbe;
 class XRSession;
 class XRView;
@@ -35,9 +35,7 @@ class XRWebGLBinding final : public ScriptWrappable {
   XRSession* session() const { return session_; }
 
   WebGLTexture* getReflectionCubeMap(XRLightProbe*, ExceptionState&);
-
-  WebGLTexture* getCameraImage(XRCamera* camera,
-                               ExceptionState& exception_state);
+  WebGLTexture* getCameraImage(XRFrame*, XRView*);
 
   XRWebGLDepthInformation* getDepthInformation(XRView* view,
                                                ExceptionState& exception_state);