Skip to content

GitLab

T
tangled
Commit 7f94d7ae authored by Alex Cooper's avatar Alex Cooper Committed by Commit Bot
Browse files
Options

Revert "Update XRInputSource Gamepad to comply with SameObject requirement" 

This reverts commit f9706cfd.

Reason for revert:963529

Original change's description:
> Update XRInputSource Gamepad to comply with SameObject requirement
>
> The WebXR spec requires that the Gamepad object on an XRInputSource be
> tagged as "SameObject."  The current implementation would allow for
> the Gamepad to be toggled between null and present on the same
> XRInputSource object as it currently is written.  This change exposes a
> method "NeedsReCreation" as well as static helper methods (and a deep-
> copy constructor) to allow for that re-creation to occur in a pattern
> which could be re-used by 958019 if the spec requires those changes.
> While this change adds an xr_browser_test to validate the blink code
> that destroys/re-creates the object, the work to add a WPT test for all
> of the same object properties is tracked by 960958.
>
> The exemption to audit_non_blink_usage was added because the xr module
> was currently utilizing device::Gamepad, but since it was previously
> wrapped in base::Optional, that usage was not flagged.
>
> Note that 955101 (currently tagged for 77) may relax/change this
> requirement, but this makes the code compliant with the spec for M76.
>
> Bug: 960978
> Change-Id: Ie36990f47e43c6df91af3fca06dd269a860a2a1e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1606421
> Commit-Queue: Alexander Cooper <alcooper@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Bill Orr <billorr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#659625}

TBR=dcheng@chromium.org,bajones@chromium.org,billorr@chromium.org,alcooper@chromium.org

Change-Id: Ia5b987d3e4e5746d94a65cd2e8786012345b8c5d
No-Presubmit: true
No-Try: true
Bug: 960978
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1613557
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
Reviewed-by: default avatarAlexander Cooper <alcooper@chromium.org>
Cr-Commit-Position: refs/heads/master@{#660016}
parent 999b1709
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 108 additions and 372 deletions
chrome/test/data/xr/e2e_test_files/html/test_webxr_input_same_object.html deleted 100644 → 0
View file @ 999b1709
<!doctype html>
<!--
A collection of helper functions and listeners to confirm the state of input
sources for the same object tests.
-->
<html>
<head>
<link rel="stylesheet" type="text/css" href="../resources/webxr_e2e.css">
</head>
<body>
<canvas id="webgl-canvas"></canvas>
<script src="../../../../../../third_party/blink/web_tests/resources/testharness.js"></script>
<script src="../resources/webxr_e2e.js"></script>
<script src="../resources/webxr_boilerplate.js"></script>
<script>
let inputChangeEvents = 0;
function onInputSourcesChange() {
inputChangeEvents++;
}
onSessionStartedCallback = function(session) {
if (session.mode === "immersive-vr") {
session.addEventListener('inputsourceschange', onInputSourcesChange, false);
}
}
function getCurrentInputSources() {
let currentSession = sessionInfos[sessionTypes.IMMERSIVE].currentSession;
return currentSession.getInputSources();
}
let cached_input_source = null;
function updateCachedInputSource(id) {
let input_sources = getCurrentInputSources();
assert_less_than(id, input_sources.length);
cached_input_source = input_sources[id];
}
function validateCachedSourcePresence(present) {
assert_not_equals(cached_input_source, null);
assert_not_equals(present, undefined);
let current_sources = getCurrentInputSources();
assert_equals(current_sources.includes(cached_input_source), present);
}
function validateInputSourceLength(length) {
assert_equals(getCurrentInputSources().length, length);
}
function validateCurrentAndCachedGamepadMatch() {
assert_not_equals(cached_input_source, null);
let current_sources = getCurrentInputSources();
let index = current_sources.indexOf(cached_input_source);
assert_not_equals(index, -1);
assert_equals(cached_input_source.gamepad, current_sources[index].gamepad);
}
</script>
</body>
</html>
third_party/blink/renderer/modules/xr/xr_input_source.cc
View file @ 7f94d7ae
......@@ -13,99 +13,6 @@
namespace blink {
namespace {
// TODO(https://crbug.com/962712): Switch to use typemapping instead.
XRInputSource::TargetRayMode MojomToBlinkTargetRayMode(
device::mojom::XRTargetRayMode target_ray_mode) {
switch (target_ray_mode) {
case device::mojom::XRTargetRayMode::GAZING:
return XRInputSource::TargetRayMode::kGaze;
case device::mojom::XRTargetRayMode::POINTING:
return XRInputSource::TargetRayMode::kTrackedPointer;
case device::mojom::XRTargetRayMode::TAPPING:
return XRInputSource::TargetRayMode::kScreen;
}
NOTREACHED();
}
// TODO(https://crbug.com/962712): Switch to use typemapping instead.
XRInputSource::Handedness MojomToBlinkHandedness(
device::mojom::XRHandedness handedness) {
switch (handedness) {
case device::mojom::XRHandedness::NONE:
return XRInputSource::Handedness::kHandNone;
case device::mojom::XRHandedness::LEFT:
return XRInputSource::Handedness::kHandLeft;
case device::mojom::XRHandedness::RIGHT:
return XRInputSource::Handedness::kHandRight;
}
NOTREACHED();
}
} // anonymous namespace
XRInputSource* XRInputSource::CreateOrUpdateFrom(
XRInputSource* other,
XRSession* session,
const device::mojom::blink::XRInputSourceStatePtr& state) {
if (!state)
return other;
XRInputSource* updated_source = other;
if (other && other->InvalidatesSameObject(state)) {
updated_source = MakeGarbageCollected<XRInputSource>(*other);
// Need to explicitly override any of the properties that could cause us to
// recreate the object.
// TODO(https://crbug.com/962724): Simplify this creation pattern
if (state->gamepad) {
updated_source->gamepad_ = MakeGarbageCollected<Gamepad>(
updated_source, 0, updated_source->base_timestamp_, TimeTicks::Now());
} else {
updated_source->gamepad_ = nullptr;
}
} else if (!other) {
updated_source = MakeGarbageCollected<XRInputSource>(session, state);
}
if (state->gamepad) {
updated_source->UpdateGamepad(*(state->gamepad));
}
// Update the input source's description if this state update includes them.
if (state->description) {
const device::mojom::blink::XRInputSourceDescriptionPtr& desc =
state->description;
updated_source->SetTargetRayMode(
MojomToBlinkTargetRayMode(desc->target_ray_mode));
updated_source->SetHandedness(MojomToBlinkHandedness(desc->handedness));
updated_source->SetEmulatedPosition(desc->emulated_position);
if (desc->pointer_offset && desc->pointer_offset->matrix.has_value()) {
const WTF::Vector<float>& m = desc->pointer_offset->matrix.value();
std::unique_ptr<TransformationMatrix> pointer_matrix =
std::make_unique<TransformationMatrix>(
m[0], m[1], m[2], m[3], m[4], m[5], m[6], m[7], m[8], m[9], m[10],
m[11], m[12], m[13], m[14], m[15]);
updated_source->SetPointerTransformMatrix(std::move(pointer_matrix));
}
}
if (state->grip && state->grip->matrix.has_value()) {
const Vector<float>& m = state->grip->matrix.value();
std::unique_ptr<TransformationMatrix> grip_matrix =
std::make_unique<TransformationMatrix>(
m[0], m[1], m[2], m[3], m[4], m[5], m[6], m[7], m[8], m[9], m[10],
m[11], m[12], m[13], m[14], m[15]);
updated_source->SetBasePoseMatrix(std::move(grip_matrix));
}
return updated_source;
}
XRInputSource::XRInputSource(XRSession* session, uint32_t source_id)
: session_(session),
source_id_(source_id),
......@@ -116,39 +23,6 @@ XRInputSource::XRInputSource(XRSession* session, uint32_t source_id)
SetHandedness(kHandNone);
}
XRInputSource::XRInputSource(
XRSession* session,
const device::mojom::blink::XRInputSourceStatePtr& state)
: XRInputSource(session, state->source_id) {
if (state->gamepad) {
gamepad_ = MakeGarbageCollected<Gamepad>(this, 0, base_timestamp_,
TimeTicks::Now());
}
}
// Deep copy because of the unique_ptrs
XRInputSource::XRInputSource(const XRInputSource& other)
: active_frame_id(other.active_frame_id),
primary_input_pressed(other.primary_input_pressed),
selection_cancelled(other.selection_cancelled),
session_(other.session_),
source_id_(other.source_id_),
target_ray_space_(other.target_ray_space_),
grip_space_(other.grip_space_),
gamepad_(other.gamepad_),
emulated_position_(other.emulated_position_),
base_timestamp_(other.base_timestamp_) {
// Since these setters also set strings, for convenience, setting them via
// their existing setters.
SetTargetRayMode(other.target_ray_mode_);
SetHandedness(other.handedness_);
base_pose_matrix_ =
std::make_unique<TransformationMatrix>(*(other.base_pose_matrix_.get()));
pointer_transform_matrix_ = std::make_unique<TransformationMatrix>(
*(other.pointer_transform_matrix_.get()));
}
XRSpace* XRInputSource::gripSpace() const {
if (target_ray_mode_ == kTrackedPointer) {
return grip_space_;
......@@ -165,20 +39,6 @@ Gamepad* XRInputSource::gamepad() const {
return gamepad_;
}
bool XRInputSource::InvalidatesSameObject(
const device::mojom::blink::XRInputSourceStatePtr& state) {
if ((state->gamepad && !gamepad_) || (!state->gamepad && gamepad_)) {
return true;
}
return false;
}
void XRInputSource::UpdateGamepad(const device::Gamepad& gamepad) {
DCHECK(gamepad_);
gamepad_->UpdateFromDeviceState(gamepad);
}
void XRInputSource::SetTargetRayMode(TargetRayMode target_ray_mode) {
if (target_ray_mode_ == target_ray_mode)
return;
......@@ -238,6 +98,23 @@ void XRInputSource::SetPointerTransformMatrix(
pointer_transform_matrix_ = std::move(pointer_transform_matrix);
}
// TODO(https://crbug.com/955101): Should Gamepad objects be updated in-place,
// or should a new object be created on every call?
void XRInputSource::SetGamepad(const base::Optional<device::Gamepad> gamepad) {
if (gamepad) {
if (!gamepad_) {
// TODO(https://crbug.com/955104): Is the Gamepad object creation time the
// correct time floor?
gamepad_ = MakeGarbageCollected<Gamepad>(this, 0, base_timestamp_,
TimeTicks::Now());
}
gamepad_->UpdateFromDeviceState(*gamepad);
} else {
gamepad_ = nullptr;
}
}
void XRInputSource::Trace(blink::Visitor* visitor) {
visitor->Trace(session_);
visitor->Trace(target_ray_space_);
......
third_party/blink/renderer/modules/xr/xr_input_source.h
View file @ 7f94d7ae
......@@ -5,7 +5,6 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_XR_XR_INPUT_SOURCE_H_
#define THIRD_PARTY_BLINK_RENDERER_MODULES_XR_XR_INPUT_SOURCE_H_
#include "device/vr/public/mojom/vr_service.mojom-blink.h"
#include "third_party/blink/renderer/modules/gamepad/gamepad.h"
#include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
#include "third_party/blink/renderer/platform/heap/handle.h"
......@@ -35,18 +34,9 @@ class XRInputSource : public ScriptWrappable, public Gamepad::Client {
kHandLeft = 1,
kHandRight = 2
};
enum TargetRayMode { kGaze = 1, kTrackedPointer = 2, kScreen = 3 };
static XRInputSource* CreateOrUpdateFrom(
XRInputSource* other /* may be null, input */,
XRSession* session,
const device::mojom::blink::XRInputSourceStatePtr& state);
XRInputSource(XRSession*, uint32_t source_id);
XRInputSource(XRSession*,
const device::mojom::blink::XRInputSourceStatePtr& state);
XRInputSource(const XRInputSource& other);
~XRInputSource() override = default;
XRSession* session() const { return session_; }
......@@ -61,7 +51,11 @@ class XRInputSource : public ScriptWrappable, public Gamepad::Client {
uint32_t source_id() const { return source_id_; }
void SetTargetRayMode(TargetRayMode);
void SetHandedness(Handedness);
void SetEmulatedPosition(bool emulated_position);
void SetBasePoseMatrix(std::unique_ptr<TransformationMatrix>);
void SetPointerTransformMatrix(std::unique_ptr<TransformationMatrix>);
void SetGamepad(const base::Optional<device::Gamepad>);
// Gamepad::Client
GamepadHapticActuator* GetVibrationActuatorForGamepad(
......@@ -82,17 +76,6 @@ class XRInputSource : public ScriptWrappable, public Gamepad::Client {
friend class XRGripSpace;
friend class XRTargetRaySpace;
void SetHandedness(Handedness);
void SetEmulatedPosition(bool emulated_position);
void SetBasePoseMatrix(std::unique_ptr<TransformationMatrix>);
// Use to check if the updates that would/should be made by a given
// XRInputSourceState would invalidate any SameObject properties guaranteed
// by the idl, and thus require the xr_input_source to be recreated.
bool InvalidatesSameObject(
const device::mojom::blink::XRInputSourceStatePtr& state);
void UpdateGamepad(const device::Gamepad& gamepad);
const Member<XRSession> session_;
const uint32_t source_id_;
Member<XRTargetRaySpace> target_ray_space_;
......@@ -107,12 +90,10 @@ class XRInputSource : public ScriptWrappable, public Gamepad::Client {
bool emulated_position_ = false;
// TODO(crbug.com/945947): Revisit use of std::unique_ptr.
std::unique_ptr<TransformationMatrix> base_pose_matrix_;
// This is the transform to apply to the base_pose_matrix_ to get the pointer
// matrix. In most cases it should be static.
// TODO(crbug.com/945947): Revisit use of std::unique_ptr.
std::unique_ptr<TransformationMatrix> pointer_transform_matrix_;
// gamepad_ uses this to get relative timestamps.
......
third_party/blink/renderer/modules/xr/xr_input_source.idl
View file @ 7f94d7ae
......@@ -23,5 +23,7 @@ enum XRTargetRayMode {
readonly attribute XRTargetRayMode targetRayMode;
[SameObject] readonly attribute XRSpace targetRaySpace;
[SameObject] readonly attribute XRSpace? gripSpace;
[SameObject] readonly attribute Gamepad? gamepad;
// TODO(https://crbug.com/955101): This should be tagged [SameObject].
readonly attribute Gamepad? gamepad;
};
third_party/blink/renderer/modules/xr/xr_session.cc
View file @ 7f94d7ae
......@@ -762,34 +762,28 @@ void XRSession::OnInputStateChange(
int16_t frame_id,
const WTF::Vector<device::mojom::blink::XRInputSourceStatePtr>&
input_states) {
bool input_sources_changed = false;
bool devices_changed = false;
// Update any input sources with new state information. Any updated input
// sources are marked as active.
for (const auto& input_state : input_states) {
XRInputSource* stored_input_source =
input_sources_.at(input_state->source_id);
XRInputSource* input_source = XRInputSource::CreateOrUpdateFrom(
stored_input_source, this, input_state);
// Using pointer equality to determine if the pointer needs to be set.
if (stored_input_source != input_source) {
XRInputSource* input_source = input_sources_.at(input_state->source_id);
if (!input_source) {
input_source =
MakeGarbageCollected<XRInputSource>(this, input_state->source_id);
input_sources_.Set(input_state->source_id, input_source);
input_sources_changed = true;
devices_changed = true;
}
input_source->active_frame_id = frame_id;
UpdateSelectState(input_source, input_state);
UpdateInputSourceState(input_source, input_state);
}
// Remove any input sources that are inactive. Note that this is done in
// two passes because HeapHashMap makes no guarantees about iterators on
// removal.
// Remove any input sources that are inactive..
std::vector<uint32_t> inactive_sources;
for (const auto& input_source : input_sources_.Values()) {
if (input_source->active_frame_id != frame_id) {
inactive_sources.push_back(input_source->source_id());
input_sources_changed = true;
devices_changed = true;
}
}
......@@ -799,7 +793,7 @@ void XRSession::OnInputStateChange(
}
}
if (input_sources_changed) {
if (devices_changed) {
DispatchEvent(
*XRSessionEvent::Create(event_type_names::kInputsourceschange, this));
}
......@@ -875,12 +869,47 @@ void XRSession::OnPoseReset() {
}
}
void XRSession::UpdateSelectState(
void XRSession::UpdateInputSourceState(
XRInputSource* input_source,
const device::mojom::blink::XRInputSourceStatePtr& state) {
if (!input_source || !state)
return;
input_source->SetGamepad(state->gamepad);
// Update the input source's description if this state update
// includes them.
if (state->description) {
const device::mojom::blink::XRInputSourceDescriptionPtr& desc =
state->description;
input_source->SetTargetRayMode(
static_cast<XRInputSource::TargetRayMode>(desc->target_ray_mode));
input_source->SetHandedness(
static_cast<XRInputSource::Handedness>(desc->handedness));
input_source->SetEmulatedPosition(desc->emulated_position);
if (desc->pointer_offset && desc->pointer_offset->matrix.has_value()) {
const WTF::Vector<float>& m = desc->pointer_offset->matrix.value();
std::unique_ptr<TransformationMatrix> pointer_matrix =
std::make_unique<TransformationMatrix>(
m[0], m[1], m[2], m[3], m[4], m[5], m[6], m[7], m[8], m[9], m[10],
m[11], m[12], m[13], m[14], m[15]);
input_source->SetPointerTransformMatrix(std::move(pointer_matrix));
}
}
if (state->grip && state->grip->matrix.has_value()) {
const Vector<float>& m = state->grip->matrix.value();
std::unique_ptr<TransformationMatrix> grip_matrix =
std::make_unique<TransformationMatrix>(
m[0], m[1], m[2], m[3], m[4], m[5], m[6], m[7], m[8], m[9], m[10],
m[11], m[12], m[13], m[14], m[15]);
input_source->SetBasePoseMatrix(std::move(grip_matrix));
}
// Handle state change of the primary input, which may fire events
if (state->primary_input_clicked)
OnSelect(input_source);
......
third_party/blink/renderer/modules/xr/xr_session.h
View file @ 7f94d7ae
......@@ -192,8 +192,9 @@ class XRSession final : public EventTargetWithInlineData,
void UpdateCanvasDimensions(Element*);
void ApplyPendingRenderState();
void UpdateSelectState(XRInputSource*,
const device::mojom::blink::XRInputSourceStatePtr&);
void UpdateInputSourceState(
XRInputSource*,
const device::mojom::blink::XRInputSourceStatePtr&);
XRInputSourceEvent* CreateInputSourceEvent(const AtomicString&,
XRInputSource*);
......
third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py
View file @ 7f94d7ae
......@@ -510,7 +510,6 @@ _CONFIG = [
'third_party/blink/renderer/modules/device_orientation/',
'third_party/blink/renderer/modules/gamepad/',
'third_party/blink/renderer/modules/sensor/',
'third_party/blink/renderer/modules/xr/',
],
'allowed': [
'base::subtle::Atomic32',
......
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