Commit cd438ad1 authored by Piotr Bialecki's avatar Piotr Bialecki Committed by Chromium LUCI CQ

AR depth: address developer feedback for depth API, minor refactor

Refactor XRSession logic to handle depth information into a separate
XRDepthManager to reduce the XRSession's responsibilities.

Make the XRDepthInformation object usable only during XRFrame (the frame
needs to be both active and animating). This allows us to reduce the
amount of copies (but means that multiple instances of
XRDepthInformation will share the same underlying depth buffer).

Change-Id: I1f709bd440ab771d2b96ebc55a3e15d7e9f25fc3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2575899
Commit-Queue: Piotr Bialecki <bialpio@chromium.org>
Reviewed-by: default avatarAlexander Cooper <alcooper@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835925}
parent 46886050
......@@ -21,6 +21,8 @@ blink_modules_sources("xr") {
"xr_cube_map.h",
"xr_depth_information.cc",
"xr_depth_information.h",
"xr_depth_manager.cc",
"xr_depth_manager.h",
"xr_dom_overlay_state.cc",
"xr_dom_overlay_state.h",
"xr_frame.cc",
......
......@@ -9,6 +9,7 @@
#include "base/numerics/checked_math.h"
#include "device/vr/public/mojom/vr_service.mojom-blink.h"
#include "third_party/blink/renderer/core/dom/dom_exception.h"
#include "third_party/blink/renderer/modules/xr/xr_frame.h"
#include "third_party/blink/renderer/modules/xr/xr_rigid_transform.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/transforms/transformation_matrix.h"
......@@ -16,40 +17,56 @@
namespace {
constexpr char kOutOfBoundsAccess[] =
"Attempted to access data that is out-of-bounds.";
constexpr char kFrameInactive[] =
"XRDepthInformation members are only accessible when their XRFrame's "
"`active` boolean is `true`.";
constexpr char kFrameNotAnimated[] =
"XRDepthInformation members are only accessible when their XRFrame's "
"`animationFrame` boolean is `true`.";
}
namespace blink {
XRDepthInformation::XRDepthInformation(
const device::mojom::blink::XRDepthDataUpdated& depth_data)
: width_(depth_data.size.width()),
height_(depth_data.size.height()),
norm_texture_from_norm_view_(depth_data.norm_texture_from_norm_view) {
DVLOG(3) << __func__ << ": width_=" << width_ << ", height_=" << height_
const XRFrame* xr_frame,
const gfx::Size& size,
const gfx::Transform& norm_texture_from_norm_view,
DOMUint16Array* data)
: xr_frame_(xr_frame),
size_(size),
data_(data),
norm_texture_from_norm_view_(norm_texture_from_norm_view) {
DVLOG(3) << __func__ << ": size_=" << size_.ToString()
<< ", norm_texture_from_norm_view_="
<< norm_texture_from_norm_view_.ToString();
CHECK_EQ(base::CheckMul(2, width_, height_).ValueOrDie(),
depth_data.pixel_data.size());
base::span<const uint16_t> pixel_data = base::make_span(
reinterpret_cast<const uint16_t*>(depth_data.pixel_data.data()),
depth_data.pixel_data.size() / 2);
// Copy the underlying pixel data into DOMUint16Array:
data_ = DOMUint16Array::Create(pixel_data.data(), pixel_data.size());
CHECK_EQ(base::CheckMul(2, size_.width(), size_.height()).ValueOrDie(),
data_->byteLength());
}
DOMUint16Array* XRDepthInformation::data() const {
DOMUint16Array* XRDepthInformation::data(
ExceptionState& exception_state) const {
if (!ValidateFrame(exception_state)) {
return nullptr;
}
return data_;
}
uint32_t XRDepthInformation::width() const {
return width_;
uint32_t XRDepthInformation::width(ExceptionState& exception_state) const {
if (!ValidateFrame(exception_state)) {
return 0;
}
return size_.width();
}
uint32_t XRDepthInformation::height() const {
return height_;
uint32_t XRDepthInformation::height(ExceptionState& exception_state) const {
if (!ValidateFrame(exception_state)) {
return 0;
}
return size_.height();
}
float XRDepthInformation::getDepth(uint32_t column,
......@@ -57,19 +74,24 @@ float XRDepthInformation::getDepth(uint32_t column,
ExceptionState& exception_state) const {
DVLOG(3) << __func__ << ": column=" << column << ", row=" << row;
if (column >= width_) {
if (!ValidateFrame(exception_state)) {
return 0.0;
}
if (column >= static_cast<size_t>(size_.width())) {
exception_state.ThrowDOMException(DOMExceptionCode::kNotAllowedError,
kOutOfBoundsAccess);
return 0.0;
}
if (row >= height_) {
if (row >= static_cast<size_t>(size_.height())) {
exception_state.ThrowDOMException(DOMExceptionCode::kNotAllowedError,
kOutOfBoundsAccess);
return 0.0;
}
auto checked_index = base::CheckAdd(column, base::CheckMul(row, width_));
auto checked_index =
base::CheckAdd(column, base::CheckMul(row, size_.width()));
size_t index = checked_index.ValueOrDie();
// Data is stored in millimeters, convert to meters when accessing:
......@@ -80,12 +102,34 @@ float XRDepthInformation::getDepth(uint32_t column,
return result;
}
XRRigidTransform* XRDepthInformation::normTextureFromNormView() const {
XRRigidTransform* XRDepthInformation::normTextureFromNormView(
ExceptionState& exception_state) const {
if (!ValidateFrame(exception_state)) {
return nullptr;
}
return MakeGarbageCollected<XRRigidTransform>(
TransformationMatrix(norm_texture_from_norm_view_.matrix()));
}
bool XRDepthInformation::ValidateFrame(ExceptionState& exception_state) const {
if (!xr_frame_->IsActive()) {
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
kFrameInactive);
return false;
}
if (!xr_frame_->IsAnimationFrame()) {
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
kFrameNotAnimated);
return false;
}
return true;
}
void XRDepthInformation::Trace(Visitor* visitor) const {
visitor->Trace(xr_frame_);
visitor->Trace(data_);
ScriptWrappable::Trace(visitor);
}
......
......@@ -9,27 +9,32 @@
#include "third_party/blink/renderer/core/typed_arrays/dom_typed_array.h"
#include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
#include "third_party/blink/renderer/platform/wtf/forward.h"
#include "ui/gfx/geometry/size.h"
#include "ui/gfx/transform.h"
namespace blink {
class ExceptionState;
class XRFrame;
class XRRigidTransform;
class XRDepthInformation final : public ScriptWrappable {
DEFINE_WRAPPERTYPEINFO();
public:
explicit XRDepthInformation(
const device::mojom::blink::XRDepthDataUpdated& depth_data);
explicit XRDepthInformation(const XRFrame* xr_frame,
const gfx::Size& size,
const gfx::Transform& norm_texture_from_norm_view,
DOMUint16Array* data);
DOMUint16Array* data() const;
DOMUint16Array* data(ExceptionState& exception_state) const;
uint32_t width() const;
uint32_t width(ExceptionState& exception_state) const;
uint32_t height() const;
uint32_t height(ExceptionState& exception_state) const;
XRRigidTransform* normTextureFromNormView() const;
XRRigidTransform* normTextureFromNormView(
ExceptionState& exception_state) const;
float getDepth(uint32_t column,
uint32_t row,
......@@ -38,11 +43,18 @@ class XRDepthInformation final : public ScriptWrappable {
void Trace(Visitor* visitor) const override;
private:
uint32_t width_;
uint32_t height_;
const Member<const XRFrame> xr_frame_;
Member<DOMUint16Array> data_;
gfx::Transform norm_texture_from_norm_view_;
const gfx::Size size_;
const Member<DOMUint16Array> data_;
const gfx::Transform norm_texture_from_norm_view_;
// Helper to validate whether a frame is in a correct state. Should be invoked
// before every member access. If the validation returns `false`, it means the
// validation failed & an exception is going to be thrown and the rest of the
// member access code should not run.
bool ValidateFrame(ExceptionState& exception_state) const;
};
} // namespace blink
......
......@@ -7,13 +7,14 @@
Exposed=Window,
RuntimeEnabled=WebXRDepth
] interface XRDepthInformation {
[SameObject, MeasureAs=XRDepthInformationDataAttribute]
[RaisesException, SameObject, MeasureAs=XRDepthInformationDataAttribute]
readonly attribute Uint16Array data;
readonly attribute unsigned long width;
readonly attribute unsigned long height;
[RaisesException] readonly attribute unsigned long width;
[RaisesException] readonly attribute unsigned long height;
[SameObject] readonly attribute XRRigidTransform normTextureFromNormView;
[RaisesException, SameObject]
readonly attribute XRRigidTransform normTextureFromNormView;
[RaisesException, MeasureAs=XRDepthInformationGetDepth]
float getDepth(unsigned long column, unsigned long row);
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "third_party/blink/renderer/modules/xr/xr_depth_manager.h"
#include "base/trace_event/trace_event.h"
#include "third_party/blink/renderer/modules/xr/xr_depth_information.h"
#include "third_party/blink/renderer/modules/xr/xr_session.h"
namespace blink {
XRDepthManager::XRDepthManager(base::PassKey<XRSession> pass_key,
XRSession* session)
: session_(session) {}
XRDepthManager::~XRDepthManager() = default;
void XRDepthManager::ProcessDepthInformation(
device::mojom::blink::XRDepthDataPtr depth_data) {
DVLOG(3) << __func__ << ": depth_data valid? " << !!depth_data;
// Throw away old data, we won't need it anymore because we'll either replace
// it with new data, or no new data is available (& we don't want to keep the
// old data in that case as well).
depth_data_ = nullptr;
data_ = nullptr;
if (depth_data) {
DVLOG(3) << __func__ << ": depth_data->which()="
<< static_cast<uint32_t>(depth_data->which());
switch (depth_data->which()) {
case device::mojom::blink::XRDepthData::Tag::DATA_STILL_VALID:
// Stale depth buffer is still the most recent information we have.
// Current API shape is not well-suited to return data pertaining to
// older frames, so we just discard the data we previously got and will
// not set the new one.
break;
case device::mojom::blink::XRDepthData::Tag::UPDATED_DEPTH_DATA:
// We got new depth buffer - store the current depth data as a member.
depth_data_ = std::move(depth_data->get_updated_depth_data());
break;
}
}
}
XRDepthInformation* XRDepthManager::GetDepthInformation(
const XRFrame* xr_frame) {
if (!depth_data_) {
return nullptr;
}
EnsureData();
return MakeGarbageCollected<XRDepthInformation>(
xr_frame, depth_data_->size, depth_data_->norm_texture_from_norm_view,
data_);
}
void XRDepthManager::EnsureData() {
DCHECK(depth_data_);
if (data_) {
return;
}
base::span<const uint16_t> pixel_data = base::make_span(
reinterpret_cast<const uint16_t*>(depth_data_->pixel_data.data()),
depth_data_->pixel_data.size() / 2);
// Copy the underlying pixel data into DOMUint16Array:
data_ = DOMUint16Array::Create(pixel_data.data(), pixel_data.size());
}
void XRDepthManager::Trace(Visitor* visitor) const {
visitor->Trace(session_);
visitor->Trace(data_);
}
} // namespace blink
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_XR_XR_DEPTH_MANAGER_H_
#define THIRD_PARTY_BLINK_RENDERER_MODULES_XR_XR_DEPTH_MANAGER_H_
#include "base/types/pass_key.h"
#include "device/vr/public/mojom/vr_service.mojom-blink.h"
#include "third_party/blink/renderer/core/typed_arrays/dom_typed_array.h"
#include "third_party/blink/renderer/platform/heap/handle.h"
namespace blink {
class XRDepthInformation;
class XRFrame;
class XRSession;
// Helper class, used to separate the code related to depth buffer processing
// out of XRSession.
class XRDepthManager : public GarbageCollected<XRDepthManager> {
public:
explicit XRDepthManager(base::PassKey<XRSession> pass_key,
XRSession* session);
virtual ~XRDepthManager();
void ProcessDepthInformation(device::mojom::blink::XRDepthDataPtr depth_data);
XRDepthInformation* GetDepthInformation(const XRFrame* xr_frame);
void Trace(Visitor* visitor) const;
private:
Member<XRSession> session_;
// Current depth data buffer.
device::mojom::blink::XRDepthDataUpdatedPtr depth_data_;
// Cached version of the depth buffer data. If not null, contains the same
// information as |depth_data_.pixel_data| buffer.
Member<DOMUint16Array> data_;
void EnsureData();
};
} // namespace blink
#endif // THIRD_PARTY_BLINK_RENDERER_MODULES_XR_XR_DEPTH_MANAGER_H_
......@@ -46,11 +46,14 @@ const char kCannotObtainNativeOrigin[] =
} // namespace
XRFrame::XRFrame(XRSession* session) : session_(session) {}
XRFrame::XRFrame(XRSession* session, bool is_animation_frame)
: session_(session), is_animation_frame_(is_animation_frame) {}
XRViewerPose* XRFrame::getViewerPose(XRReferenceSpace* reference_space,
ExceptionState& exception_state) {
DVLOG(3) << __func__;
DVLOG(3) << __func__ << ": is_active_=" << is_active_
<< ", is_animation_frame_=" << is_animation_frame_;
if (!is_active_) {
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
kInactiveFrame);
......@@ -162,13 +165,19 @@ XRDepthInformation* XRFrame::getDepthInformation(
return nullptr;
}
if (!is_animation_frame_) {
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
kNonAnimationFrame);
return nullptr;
}
if (this != view->frame()) {
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
kInvalidView);
return nullptr;
}
return session_->GetDepthInformation();
return session_->GetDepthInformation(this);
}
// Return an XRPose that has a transform of basespace_from_space, while
......
......@@ -41,7 +41,7 @@ class XRFrame final : public ScriptWrappable {
DEFINE_WRAPPERTYPEINFO();
public:
explicit XRFrame(XRSession* session);
explicit XRFrame(XRSession* session, bool is_animation_frame = false);
XRSession* session() const { return session_; }
......@@ -60,9 +60,7 @@ class XRFrame final : public ScriptWrappable {
bool IsActive() const;
void SetAnimationFrame(bool is_animation_frame) {
is_animation_frame_ = is_animation_frame;
}
bool IsAnimationFrame() const { return is_animation_frame_; }
HeapVector<Member<XRHitTestResult>> getHitTestResults(
XRHitTestSource* hit_test_source,
......@@ -106,7 +104,7 @@ class XRFrame final : public ScriptWrappable {
// Only frames created by XRSession.requestAnimationFrame callbacks are
// animation frames. getViewerPose should only be called from JS on active
// animation frames.
bool is_animation_frame_ = false;
bool is_animation_frame_;
};
} // namespace blink
......
......@@ -6,6 +6,7 @@
#define THIRD_PARTY_BLINK_RENDERER_MODULES_XR_XR_PLANE_MANAGER_H_
#include "base/types/pass_key.h"
#include "device/vr/public/mojom/vr_service.mojom-blink.h"
#include "third_party/blink/renderer/modules/xr/xr_session.h"
#include "third_party/blink/renderer/platform/heap/handle.h"
......
......@@ -34,6 +34,7 @@
#include "third_party/blink/renderer/modules/xr/xr_bounded_reference_space.h"
#include "third_party/blink/renderer/modules/xr/xr_canvas_input_provider.h"
#include "third_party/blink/renderer/modules/xr/xr_depth_information.h"
#include "third_party/blink/renderer/modules/xr/xr_depth_manager.h"
#include "third_party/blink/renderer/modules/xr/xr_dom_overlay_state.h"
#include "third_party/blink/renderer/modules/xr/xr_frame.h"
#include "third_party/blink/renderer/modules/xr/xr_frame_provider.h"
......@@ -326,6 +327,10 @@ XRSession::XRSession(
plane_manager_(
MakeGarbageCollected<XRPlaneManager>(base::PassKey<XRSession>{},
this)),
depth_manager_(
MakeGarbageCollected<XRDepthManager>(base::PassKey<XRSession>{},
this)),
input_sources_(MakeGarbageCollected<XRInputSourceArray>()),
client_receiver_(this, xr->GetExecutionContext()),
input_receiver_(this, xr->GetExecutionContext()),
......@@ -1122,12 +1127,6 @@ void XRSession::ProcessAnchorsData(
<< " anchors that have not been updated";
}
void XRSession::ProcessPlaneInformation(
const device::mojom::blink::XRPlaneDetectionData* detected_planes_data,
double timestamp) {
plane_manager_->ProcessPlaneInformation(detected_planes_data, timestamp);
}
XRPlaneSet* XRSession::GetDetectedPlanes() const {
return plane_manager_->GetDetectedPlanes();
}
......@@ -1217,39 +1216,9 @@ void XRSession::ProcessHitTestData(
}
}
void XRSession::ProcessDepthData(
device::mojom::blink::XRDepthDataPtr depth_data) {
DVLOG(3) << __func__ << ": depth_data valid? " << !!depth_data;
if (depth_data) {
switch (depth_data->which()) {
case device::mojom::blink::XRDepthData::Tag::DATA_STILL_VALID:
// Stale depth buffer is still the most recent information we have.
// Current API shape is not well-suited to return data pertaining to
// older frames, so just discard what we have.
depth_data_ = nullptr;
break;
case device::mojom::blink::XRDepthData::Tag::UPDATED_DEPTH_DATA:
// Just store the current depth data as a member - we will need to
// construct instances of XRDepthInformation once the app requests them
// anyway.
depth_data_ = std::move(depth_data->get_updated_depth_data());
break;
}
} else {
// Device did not return new pixel data.
depth_data_ = nullptr;
}
}
XRDepthInformation* XRSession::GetDepthInformation() const {
DVLOG(2) << __func__;
if (!depth_data_) {
return nullptr;
}
return MakeGarbageCollected<XRDepthInformation>(*depth_data_);
XRDepthInformation* XRSession::GetDepthInformation(
const XRFrame* xr_frame) const {
return depth_manager_->GetDepthInformation(xr_frame);
}
ScriptPromise XRSession::requestLightProbe(ScriptState* script_state,
......@@ -1756,10 +1725,11 @@ void XRSession::UpdateWorldUnderstandingStateForFrame(
const device::mojom::blink::XRFrameDataPtr& frame_data) {
// Update objects that might change on per-frame basis.
if (frame_data) {
ProcessPlaneInformation(frame_data->detected_planes_data.get(), timestamp);
plane_manager_->ProcessPlaneInformation(
frame_data->detected_planes_data.get(), timestamp);
ProcessAnchorsData(frame_data->anchors_data.get(), timestamp);
ProcessHitTestData(frame_data->hit_test_subscription_results.get());
ProcessDepthData(std::move(frame_data->depth_data));
depth_manager_->ProcessDepthInformation(std::move(frame_data->depth_data));
ProcessTrackedImagesData(frame_data->tracked_images.get());
const device::mojom::blink::XRLightEstimationData* light_data =
......@@ -1768,10 +1738,10 @@ void XRSession::UpdateWorldUnderstandingStateForFrame(
world_light_probe_->ProcessLightEstimationData(light_data, timestamp);
}
} else {
ProcessPlaneInformation(nullptr, timestamp);
plane_manager_->ProcessPlaneInformation(nullptr, timestamp);
ProcessAnchorsData(nullptr, timestamp);
ProcessHitTestData(nullptr);
ProcessDepthData(nullptr);
depth_manager_->ProcessDepthInformation(nullptr);
ProcessTrackedImagesData(nullptr);
if (world_light_probe_) {
......@@ -1852,8 +1822,7 @@ void XRSession::OnFrame(
return;
}
XRFrame* presentation_frame = CreatePresentationFrame();
presentation_frame->SetAnimationFrame(true);
XRFrame* presentation_frame = CreatePresentationFrame(true);
// Make sure that any frame-bounded changed to the views array take effect.
if (update_views_next_frame_) {
......@@ -1941,10 +1910,11 @@ base::Optional<TransformationMatrix> XRSession::GetMojoFrom(
}
}
XRFrame* XRSession::CreatePresentationFrame() {
DVLOG(2) << __func__;
XRFrame* XRSession::CreatePresentationFrame(bool is_animation_frame) {
DVLOG(2) << __func__ << ": is_animation_frame=" << is_animation_frame;
XRFrame* presentation_frame = MakeGarbageCollected<XRFrame>(this);
XRFrame* presentation_frame =
MakeGarbageCollected<XRFrame>(this, is_animation_frame);
return presentation_frame;
}
......@@ -2309,6 +2279,7 @@ void XRSession::Trace(Visitor* visitor) const {
visitor->Trace(request_hit_test_source_promises_);
visitor->Trace(reference_spaces_);
visitor->Trace(plane_manager_);
visitor->Trace(depth_manager_);
visitor->Trace(anchor_ids_to_anchors_);
visitor->Trace(anchor_ids_to_pending_anchor_promises_);
visitor->Trace(prev_base_layer_);
......
......@@ -43,6 +43,7 @@ class XRAnchor;
class XRAnchorSet;
class XRCanvasInputProvider;
class XRDepthInformation;
class XRDepthManager;
class XRDOMOverlayState;
class XRHitTestOptionsInit;
class XRHitTestSource;
......@@ -336,15 +337,14 @@ class XRSession final
base::Optional<TransformationMatrix> GetMojoFrom(
device::mojom::blink::XRReferenceSpaceType space_type) const;
XRDepthInformation* GetDepthInformation() const;
XRDepthInformation* GetDepthInformation(const XRFrame* xr_frame) const;
XRPlaneSet* GetDetectedPlanes() const;
// Creates presentation frame based on current state of the session.
// State currently used in XRFrame creation is mojo_from_viewer_ and
// world_information_. The created XRFrame also stores a reference to this
// XRSession.
XRFrame* CreatePresentationFrame();
// The created XRFrame will store a reference to this XRSession and use it to
// get the latest information out of it.
XRFrame* CreatePresentationFrame(bool is_animation_frame = false);
// Updates the internal XRSession state that is relevant to creating
// presentation frames.
......@@ -433,18 +433,12 @@ class XRSession final
const device::mojom::blink::XRAnchorsData* tracked_anchors_data,
double timestamp);
void ProcessPlaneInformation(
const device::mojom::blink::XRPlaneDetectionData* detected_planes_data,
double timestamp);
void CleanUpUnusedHitTestSources();
void ProcessHitTestData(
const device::mojom::blink::XRHitTestSubscriptionResultsData*
hit_test_data);
void ProcessDepthData(device::mojom::blink::XRDepthDataPtr depth_data);
void ProcessTrackedImagesData(
const device::mojom::blink::XRTrackedImagesData*);
HeapVector<Member<XRImageTrackingResult>> frame_tracked_images_;
......@@ -541,6 +535,7 @@ class XRSession final
HashSet<uint64_t> hit_test_source_for_transient_input_ids_;
Member<XRPlaneManager> plane_manager_;
Member<XRDepthManager> depth_manager_;
uint32_t view_parameters_id_ = 0;
HeapVector<Member<XRViewData>> views_;
......@@ -577,9 +572,6 @@ class XRSession final
// Viewer pose in mojo space.
std::unique_ptr<TransformationMatrix> mojo_from_viewer_;
// Current depth data buffer.
device::mojom::blink::XRDepthDataUpdatedPtr depth_data_;
bool pending_frame_ = false;
bool resolving_frame_ = false;
bool update_views_next_frame_ = false;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment