Attachment 418143 Details for Bug 220732 – Patch

[patch] Patch

bug-220732-20210122151640.patch (text/plain), 14.71 KB, created by Imanol Fernandez on 2021-01-22 06:16:42 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Imanol Fernandez

Created: 2021-01-22 06:16:42 PST

Size: 14.71 KB

patch

obsolete

>Subversion Revision: 271741
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index a0e18570f7d25251fb16639e8b5a7c3d595804f1..4879f0eed0636588d7aa797c02eef9d2ceb17da0 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,4 +1,31 @@
>-2021-01-22  Youenn Fablet  <youenn@apple.com>
>+2021-01-22  Imanol Fernandez  <ifernandez@igalia.com>
>+
>+        Complete WebXRRigidTransform implementation
>+        https://bugs.webkit.org/show_bug.cgi?id=220732
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        - Implement XRRigidTransform.inverse().
>+        - Expose raw matrix to be used in other WebXR math calculations (e.g pose composition).
>+        - Correctly lazily initialize and reuse the Float32Array matrixData.
>+
>+        Tested by the WebXR platform tests.
>+
>+        * Modules/webxr/WebXRRigidTransform.cpp:
>+        (WebCore::normalizeQuaternion):
>+        (WebCore::WebXRRigidTransform::create):
>+        (WebCore::WebXRRigidTransform::WebXRRigidTransform):
>+        (WebCore::m_orientation):
>+        (WebCore::m_rawTransform):
>+        (WebCore::WebXRRigidTransform::matrix):
>+        (WebCore::WebXRRigidTransform::inverse):
>+        (WebCore::WebXRRigidTransform::rawTransform const):
>+        * Modules/webxr/WebXRRigidTransform.h:
>+        * platform/graphics/transforms/TransformationMatrix.cpp:
>+        (WebCore::TransformationMatrix::fromQuaternion):
>+        * platform/graphics/transforms/TransformationMatrix.h:
>+
>+2021-01-22  Youenn Fablet  <youenn@apple.com> 
> 
>         Validate ItemHandles when decoding them in GPUProcess
>         https://bugs.webkit.org/show_bug.cgi?id=220710
>diff --git a/Source/WebCore/Modules/webxr/WebXRRigidTransform.cpp b/Source/WebCore/Modules/webxr/WebXRRigidTransform.cpp
>index d3fdcfb90e725b179780054ba6e7ba5464ff989d..70ba44d5be7c068e79d0307ad440a094947ce850 100644
>--- a/Source/WebCore/Modules/webxr/WebXRRigidTransform.cpp
>+++ b/Source/WebCore/Modules/webxr/WebXRRigidTransform.cpp
>@@ -35,6 +35,18 @@
> 
> namespace WebCore {
> 
>+static bool normalizeQuaternion(DOMPointInit& q)
>+{
>+    const double length = std::sqrt(q.x * q.x + q.y * q.y + q.z * q.z + q.w * q.w);
>+    if (WTF::areEssentiallyEqual<double>(length, 0))
>+        return false;
>+    q.x /= length;
>+    q.y /= length;
>+    q.z /= length;
>+    q.w /= length;
>+    return true;
>+}
>+
> WTF_MAKE_ISO_ALLOCATED_IMPL(WebXRRigidTransform);
> 
> Ref<WebXRRigidTransform> WebXRRigidTransform::create()
>@@ -42,6 +54,12 @@ Ref<WebXRRigidTransform> WebXRRigidTransform::create()
>     return adoptRef(*new WebXRRigidTransform({ }, { }));
> }
> 
>+Ref<WebXRRigidTransform> WebXRRigidTransform::create(const TransformationMatrix& transform)
>+{
>+    return adoptRef(*new WebXRRigidTransform(transform));
>+}
>+
>+
> ExceptionOr<Ref<WebXRRigidTransform>> WebXRRigidTransform::create(const DOMPointInit& position, const DOMPointInit& orientation)
> {
>     // The XRRigidTransform(position, orientation) constructor MUST perform the following steps when invoked:
>@@ -58,17 +76,8 @@ ExceptionOr<Ref<WebXRRigidTransform>> WebXRRigidTransform::create(const DOMPoint
>     //   6. Else initialize transformâs orientationâs x value to orientationâs x dictionary member, y value to orientationâs y dictionary member, z value to orientationâs z dictionary member and w value to orientationâs w dictionary member.
>     //   7. Normalize x, y, z, and w components of transformâs orientation.
>     DOMPointInit orientationInit { orientation.x, orientation.y, orientation.z, orientation.w };
>-    {
>-        double length = std::sqrt(orientationInit.x * orientationInit.x + orientationInit.y * orientationInit.y
>-            + orientationInit.z * orientationInit.z + orientationInit.w * orientationInit.w);
>-        if (WTF::areEssentiallyEqual<double>(length, 0))
>-            return Exception { InvalidStateError };
>-
>-        orientationInit= {
>-            orientationInit.x / length, orientationInit.y / length,
>-            orientationInit.z / length, orientationInit.w / length,
>-        };
>-    }
>+    if (!normalizeQuaternion(orientationInit))
>+        return Exception { InvalidStateError };
> 
>     //   8. Return transform.
>     return adoptRef(*new WebXRRigidTransform(positionInit, orientationInit));
>@@ -78,10 +87,31 @@ WebXRRigidTransform::WebXRRigidTransform(const DOMPointInit& position, const DOM
>     : m_position(DOMPointReadOnly::create(position))
>     , m_orientation(DOMPointReadOnly::create(orientation))
> {
>-    // FIXME: implement properly, per spec.
>-    TransformationMatrix matrix;
>-    auto matrixData = matrix.toColumnMajorFloatArray();
>-    m_matrix = Float32Array::create(matrixData.data(), matrixData.size());
>+    TransformationMatrix translation;
>+    translation.translate3d(position.x, position.y, position.z);
>+    auto rotation = TransformationMatrix::fromQuaternion(orientation.x, orientation.y, orientation.z, orientation.w);
>+    m_rawTransform = translation * rotation;
>+}
>+
>+WebXRRigidTransform::WebXRRigidTransform(const TransformationMatrix& transform)
>+    : m_position(DOMPointReadOnly::create({ }))
>+    , m_orientation(DOMPointReadOnly::create({ }))
>+    , m_rawTransform(transform)
>+{
>+    if (transform.isIdentity()) {
>+        // TransformationMatrix::decompose returns a empty quaternion instead of unit quaternion for Identity.
>+        // WebXR tests expect a unit quaternion for this case.
>+        return;
>+    }
>+
>+    TransformationMatrix::Decomposed4Type decomp = { };
>+    transform.decompose4(decomp);
>+
>+    m_position = DOMPointReadOnly::create(decomp.translateX, decomp.translateY, decomp.translateZ, 1.0f);
>+
>+    DOMPointInit orientationInit { -decomp.quaternionX, -decomp.quaternionY, -decomp.quaternionZ, decomp.quaternionW };
>+    normalizeQuaternion(orientationInit);
>+    m_orientation = DOMPointReadOnly::create(orientationInit);
> }
> 
> WebXRRigidTransform::~WebXRRigidTransform() = default;
>@@ -96,17 +126,42 @@ const DOMPointReadOnly& WebXRRigidTransform::orientation() const
>     return m_orientation;
> }
> 
>-Ref<Float32Array>  WebXRRigidTransform::matrix() const
>+const Float32Array& WebXRRigidTransform::matrix()
>+{
>+    // If the operation IsDetachedBuffer on internal matrix is false, return transformâs internal matrix.
>+    if (m_matrix && !m_matrix->isDetached())
>+        return *m_matrix;
>+
>+    // Lazily create matrix Float32Array.
>+    auto matrixData = m_rawTransform.toColumnMajorFloatArray();
>+    m_matrix = Float32Array::create(matrixData.data(), matrixData.size());
>+
>+    return *m_matrix;
>+}
>+
>+const WebXRRigidTransform& WebXRRigidTransform::inverse()
> {
>-    auto matrix = Float32Array::create(16);
>-    matrix->zeroFill();
>-    return matrix;
>+    // The inverse of a inverse object should return the original object.
>+    if (m_parentInverse)
>+        return *m_parentInverse;
>+
>+    // Inverse should always return the same object.
>+    if (m_inverse)
>+        return *m_inverse;
>+    
>+    auto inverseTransform = m_rawTransform.inverse();
>+    ASSERT(!!inverseTransform);
>+
>+    m_inverse = WebXRRigidTransform::create(*inverseTransform);
>+    // The inverse of a inverse object should return the original object.
>+    m_inverse->m_parentInverse = makeWeakPtr(this);
>+
>+    return *m_inverse;
> }
> 
>-Ref<WebXRRigidTransform> WebXRRigidTransform::inverse() const
>+const TransformationMatrix& WebXRRigidTransform::rawTransform() const
> {
>-    // FIXME: implement properly.
>-    return WebXRRigidTransform::create();
>+    return m_rawTransform;
> }
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/Modules/webxr/WebXRRigidTransform.h b/Source/WebCore/Modules/webxr/WebXRRigidTransform.h
>index d7359467770d08d6bd5ca9a23e706c16182fef2a..78cb27e7762a992f03d46eae55098bc125db2867 100644
>--- a/Source/WebCore/Modules/webxr/WebXRRigidTransform.h
>+++ b/Source/WebCore/Modules/webxr/WebXRRigidTransform.h
>@@ -28,33 +28,41 @@
> #if ENABLE(WEBXR)
> 
> #include "ExceptionOr.h"
>+#include "TransformationMatrix.h"
> #include <JavaScriptCore/Float32Array.h>
> #include <wtf/IsoMalloc.h>
> #include <wtf/RefCounted.h>
>+#include <wtf/WeakPtr.h>
> 
> namespace WebCore {
> 
> struct DOMPointInit;
> class DOMPointReadOnly;
> 
>-class WebXRRigidTransform : public RefCounted<WebXRRigidTransform> {
>+class WebXRRigidTransform : public RefCounted<WebXRRigidTransform>, public CanMakeWeakPtr<WebXRRigidTransform> {
>     WTF_MAKE_ISO_ALLOCATED_EXPORT(WebXRRigidTransform, WEBCORE_EXPORT);
> public:
>     static Ref<WebXRRigidTransform> create();
>+    static Ref<WebXRRigidTransform> create(const TransformationMatrix&);
>     WEBCORE_EXPORT static ExceptionOr<Ref<WebXRRigidTransform>> create(const DOMPointInit&, const DOMPointInit&);
>     WEBCORE_EXPORT ~WebXRRigidTransform();
> 
>     const DOMPointReadOnly& position() const;
>     const DOMPointReadOnly& orientation() const;
>-    Ref<Float32Array> matrix() const;
>-    Ref<WebXRRigidTransform> inverse() const;
>+    const Float32Array& matrix();
>+    const WebXRRigidTransform& inverse();
>+    const TransformationMatrix& rawTransform() const;
> 
> private:
>     WebXRRigidTransform(const DOMPointInit&, const DOMPointInit&);
>+    WebXRRigidTransform(const TransformationMatrix&);
> 
>     Ref<DOMPointReadOnly> m_position;
>     Ref<DOMPointReadOnly> m_orientation;
>+    TransformationMatrix m_rawTransform;
>     RefPtr<Float32Array> m_matrix;
>+    RefPtr<WebXRRigidTransform> m_inverse;
>+    WeakPtr<WebXRRigidTransform> m_parentInverse;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp b/Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp
>index 1a3b02670ead0c2cb07e49eb115b29eb89be4194..847824b98ef71512f0561c08a21319434fa9b36f 100644
>--- a/Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp
>+++ b/Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp
>@@ -580,6 +580,26 @@ TransformationMatrix::TransformationMatrix(const AffineTransform& t)
>     setMatrix(t.a(), t.b(), t.c(), t.d(), t.e(), t.f());
> }
> 
>+
>+// TODO: Once https://bugs.webkit.org/show_bug.cgi?id=220856 is addressed we can reuse this function in TransformationMatrix::recompose4().
>+TransformationMatrix TransformationMatrix::fromQuaternion(double qx, double qy, double qz, double qw)
>+{
>+    const double xx = qx * qx;
>+    const double yy = qy * qy;
>+    const double zz = qz * qz;
>+    const double xz = qx * qz;
>+    const double xy = qx * qy; 
>+    const double yz = qy * qz;
>+    const double xw = qw * qx;
>+    const double yw = qw * qy;
>+    const double zw = qw * qz;
>+
>+    return TransformationMatrix(1 - 2 * (yy + zz), 2 * (xy + zw), 2 * (xz - yw), 0,
>+        2 * (xy - zw), 1 - 2 * (xx + zz), 2 * (yz + xw), 0,
>+        2 * (xz + yw), 2 * (yz - xw), 1 - 2 * (xx + yy), 0,
>+        0, 0, 0, 1);
>+}
>+
> TransformationMatrix& TransformationMatrix::scale(double s)
> {
>     return scaleNonUniform(s, s);
>diff --git a/Source/WebCore/platform/graphics/transforms/TransformationMatrix.h b/Source/WebCore/platform/graphics/transforms/TransformationMatrix.h
>index cdd52bca42f0906789ad4aec1daa9f29bc64dc3b..600e7d0b1ce26a7c2421d58561cc574b04a17b29 100644
>--- a/Source/WebCore/platform/graphics/transforms/TransformationMatrix.h
>+++ b/Source/WebCore/platform/graphics/transforms/TransformationMatrix.h
>@@ -119,6 +119,8 @@ public:
> 
>     WEBCORE_EXPORT TransformationMatrix(const AffineTransform&);
> 
>+    static TransformationMatrix fromQuaternion(double qx, double qy, double qz, double qw);
>+
>     static const TransformationMatrix identity;
> 
>     void setMatrix(double a, double b, double c, double d, double e, double f)
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index a5f4aef97c0204d62f9ed82493b8eceb8a955ed6..5247ebd5cc467dc947730f990865bac34a0681cb 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2021-01-22  Imanol Fernandez  <ifernandez@igalia.com>
>+
>+        Complete WebXRRigidTransform implementation
>+        https://bugs.webkit.org/show_bug.cgi?id=220732
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Enable XRRigidTransform WebXR tests.
>+
>+        * platform/wpe/TestExpectations:
>+
> 2021-01-21  Rob Buis  <rbuis@igalia.com>
> 
>         Handle shapeMargin becoming NaN
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index c681fa54bf1eb48bdcbd8d8f3129914f892fe770..bc7232e0bb55b76900298cbb6a986267acb27534 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,15 @@
>+2021-01-22  Imanol Fernandez  <ifernandez@igalia.com>
>+
>+        Complete WebXRRigidTransform implementation
>+        https://bugs.webkit.org/show_bug.cgi?id=220732
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Enable XRRigidTransform WebXR tests.
>+
>+        * web-platform-tests/webxr/xrRigidTransform_inverse.https-expected.txt: Added.
>+        * web-platform-tests/webxr/xrRigidTransform_matrix.https-expected.txt: Added.
>+
> 2021-01-21  Devin Rousso  <drousso@apple.com>
> 
>         [Apple Pay] use the first item in `shippingOptions` even when it's not `selected`
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/webxr/xrRigidTransform_inverse.https-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/webxr/xrRigidTransform_inverse.https-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..f47d23f4a662cd9e218f78b65129f46cdc722084
>--- /dev/null
>+++ b/LayoutTests/imported/w3c/web-platform-tests/webxr/xrRigidTransform_inverse.https-expected.txt
>@@ -0,0 +1,3 @@
>+
>+PASS XRRigidTransform inverse works
>+
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/webxr/xrRigidTransform_matrix.https-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/webxr/xrRigidTransform_matrix.https-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..9de2b7b0131886029a728834f1aca571ddb2e18a
>--- /dev/null
>+++ b/LayoutTests/imported/w3c/web-platform-tests/webxr/xrRigidTransform_matrix.https-expected.txt
>@@ -0,0 +1,3 @@
>+
>+PASS XRRigidTransform matrix works
>+
>diff --git a/LayoutTests/platform/wpe/TestExpectations b/LayoutTests/platform/wpe/TestExpectations
>index ea3f587917aac3e69ade48157f56f12d758b3478..dbc9ff2974717fe09dc3bf7375e1275b286edc24 100644
>--- a/LayoutTests/platform/wpe/TestExpectations
>+++ b/LayoutTests/platform/wpe/TestExpectations
>@@ -640,6 +640,8 @@ webkit.org/b/209859 imported/w3c/web-platform-tests/webxr/xrWebGLLayer_framebuff
> imported/w3c/web-platform-tests/webxr/navigator_xr_sameObject.https.html [ Pass ]
> imported/w3c/web-platform-tests/webxr/xrFrame_session_sameObject.https.html [ Pass ]
> imported/w3c/web-platform-tests/webxr/xrRigidTransform_constructor.https.html [ Pass ]
>+imported/w3c/web-platform-tests/webxr/xrRigidTransform_inverse.https.html [ Pass ]
>+imported/w3c/web-platform-tests/webxr/xrRigidTransform_matrix.https.html [ Pass ]
> imported/w3c/web-platform-tests/webxr/xrSession_prevent_multiple_exclusive.https.html [ Pass ]
> imported/w3c/web-platform-tests/webxr/render_state_vertical_fov_immersive.https.html [ Pass ]
> imported/w3c/web-platform-tests/webxr/render_state_update.https.html [ Pass ]

Actions: View | Formatted Diff | Diff

Attachments on bug 220732: 418139 | 418143 | 418267 | 418273 | 418277 | 418379 | 418519