Change External Texture Crop Parameters

Removes the ExternalTextureVisibleRect type in favor of the existing
visibleOrigin/visibleRect parameters.

Bug: dawn:1622
Change-Id: Ifa661392b5541543c1445ce3e1e8e5e9db881be4
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/116124
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Brandon1 Jones <brandon1.jones@intel.com>

dawn.json

@@ -1389,16 +1389,6 @@
             {"value": 3, "name": "rotate 270 degrees"}
         ]
     },
-    "external texture visible rect" :{
-        "category": "structure",
-        "tags": ["dawn"],
-        "members": [
-            {"name": "x", "type": "float", "default": "0.0"},
-            {"name": "y", "type": "float", "default": "0.0"},
-            {"name": "width", "type": "float", "default": "1.0"},
-            {"name": "height", "type": "float", "default": "1.0"}
-        ]
-    },
     "external texture descriptor": {
         "category": "structure",
         "extensible": "in",
@@ -1419,8 +1409,7 @@
             {"name": "gamut conversion matrix", "type": "float", "annotation": "const*",
                 "length": 9},
             {"name": "flip y", "type": "bool", "default": "false"},
-            {"name": "rotation", "type": "external texture rotation", "default": "rotate 0 degrees"},
-            {"name": "visible rect", "type": "external texture visible rect"}
+            {"name": "rotation", "type": "external texture rotation", "default": "rotate 0 degrees"}
         ]
     },
     "feature name": {

src/dawn/native/ExternalTexture.cpp

@@ -99,26 +99,21 @@ MaybeError ValidateExternalTextureDescriptor(const DeviceBase* device,
         }
     }
 
-    // TODO(crbug.com/1316671): visible size width must have valid value after chromium side changes
-    // landed.
-    if (descriptor->visibleSize.width > 0) {
-        DAWN_INVALID_IF(descriptor->visibleSize.width == 0 || descriptor->visibleSize.height == 0,
-                        "VisibleSize %s have 0 on width or height.", &descriptor->visibleSize);
+    DAWN_INVALID_IF(descriptor->visibleSize.width == 0 || descriptor->visibleSize.height == 0,
+                    "VisibleSize %s have 0 on width or height.", &descriptor->visibleSize);
 
-        const Extent3D textureSize = descriptor->plane0->GetTexture()->GetSize();
-        DAWN_INVALID_IF(
-            descriptor->visibleSize.width > textureSize.width ||
-                descriptor->visibleSize.height > textureSize.height,
-            "VisibleSize %s is exceed the texture size, defined by Plane0 size (%u, %u).",
-            &descriptor->visibleSize, textureSize.width, textureSize.height);
-        DAWN_INVALID_IF(
-            descriptor->visibleOrigin.x > textureSize.width - descriptor->visibleSize.width ||
-                descriptor->visibleOrigin.y > textureSize.height - descriptor->visibleSize.height,
-            "VisibleRect[Origin: %s, Size: %s] is exceed the texture size, defined by "
-            "Plane0 size (%u, %u).",
-            &descriptor->visibleOrigin, &descriptor->visibleSize, textureSize.width,
-            textureSize.height);
-    }
+    const Extent3D textureSize = descriptor->plane0->GetTexture()->GetSize();
+    DAWN_INVALID_IF(descriptor->visibleSize.width > textureSize.width ||
+                        descriptor->visibleSize.height > textureSize.height,
+                    "VisibleSize %s is exceed the texture size, defined by Plane0 size (%u, %u).",
+                    &descriptor->visibleSize, textureSize.width, textureSize.height);
+    DAWN_INVALID_IF(
+        descriptor->visibleOrigin.x > textureSize.width - descriptor->visibleSize.width ||
+            descriptor->visibleOrigin.y > textureSize.height - descriptor->visibleSize.height,
+        "VisibleRect[Origin: %s, Size: %s] is exceed the texture size, defined by "
+        "Plane0 size (%u, %u).",
+        &descriptor->visibleOrigin, &descriptor->visibleSize, textureSize.width,
+        textureSize.height);
 
     return {};
 }
@@ -295,11 +290,23 @@ MaybeError ExternalTextureBase::Initialize(DeviceBase* device,
     // After translation, coordinates range from [0 .. 1] in both U and V.
     coordTransformMatrix = Translate(coordTransformMatrix, 0.5, 0.5);
 
+    // Calculate scale factors and offsets from the specified visibleSize.
+    ASSERT(descriptor->visibleSize.width > 0);
+    ASSERT(descriptor->visibleSize.height > 0);
+    uint32_t frameWidth = descriptor->plane0->GetTexture()->GetWidth();
+    uint32_t frameHeight = descriptor->plane0->GetTexture()->GetHeight();
+    float xScale =
+        static_cast<float>(descriptor->visibleSize.width) / static_cast<float>(frameWidth);
+    float yScale =
+        static_cast<float>(descriptor->visibleSize.height) / static_cast<float>(frameHeight);
+    float xOffset =
+        static_cast<float>(descriptor->visibleOrigin.x) / static_cast<float>(frameWidth);
+    float yOffset =
+        static_cast<float>(descriptor->visibleOrigin.y) / static_cast<float>(frameHeight);
+
     // Finally, scale and translate based on the visible rect. This applies cropping.
-    coordTransformMatrix =
-        Scale(coordTransformMatrix, descriptor->visibleRect.width, descriptor->visibleRect.height);
-    coordTransformMatrix =
-        Translate(coordTransformMatrix, descriptor->visibleRect.x, descriptor->visibleRect.y);
+    coordTransformMatrix = Scale(coordTransformMatrix, xScale, yScale);
+    coordTransformMatrix = Translate(coordTransformMatrix, xOffset, yOffset);
 
     // Transpose the mat2x3 into column vectors for use by WGSL.
     params.coordTransformMatrix[0] = coordTransformMatrix[0];

src/dawn/tests/end2end/ExternalTextureTests.cpp

@@ -118,6 +118,8 @@ TEST_P(ExternalTextureTests, CreateExternalTextureSuccess) {
     // Create an ExternalTextureDescriptor from the texture view
     wgpu::ExternalTextureDescriptor externalDesc = CreateDefaultExternalTextureDescriptor();
     externalDesc.plane0 = view;
+    externalDesc.visibleOrigin = {0, 0};
+    externalDesc.visibleSize = {kWidth, kHeight};
 
     // Import the external texture
     wgpu::ExternalTexture externalTexture = device.CreateExternalTexture(&externalDesc);
@@ -162,6 +164,8 @@ TEST_P(ExternalTextureTests, SampleExternalTexture) {
     // Create an ExternalTextureDescriptor from the texture view
     wgpu::ExternalTextureDescriptor externalDesc = CreateDefaultExternalTextureDescriptor();
     externalDesc.plane0 = externalView;
+    externalDesc.visibleOrigin = {0, 0};
+    externalDesc.visibleSize = {kWidth, kHeight};
 
     // Import the external texture
     wgpu::ExternalTexture externalTexture = device.CreateExternalTexture(&externalDesc);
@@ -255,6 +259,8 @@ TEST_P(ExternalTextureTests, SampleMultiplanarExternalTexture) {
         wgpu::ExternalTextureDescriptor externalDesc = CreateDefaultExternalTextureDescriptor();
         externalDesc.plane0 = externalViewPlane0;
         externalDesc.plane1 = externalViewPlane1;
+        externalDesc.visibleOrigin = {0, 0};
+        externalDesc.visibleSize = {kWidth, kHeight};
 
         // Import the external texture
         wgpu::ExternalTexture externalTexture = device.CreateExternalTexture(&externalDesc);
@@ -333,6 +339,8 @@ TEST_P(ExternalTextureTests, RotateAndOrFlipSinglePlane) {
         // Create an ExternalTextureDescriptor from the texture view
         wgpu::ExternalTextureDescriptor externalDesc = CreateDefaultExternalTextureDescriptor();
         externalDesc.plane0 = sourceTexture.CreateView();
+        externalDesc.visibleOrigin = {0, 0};
+        externalDesc.visibleSize = {kWidth, kHeight};
 
         // Import the external texture
         wgpu::ExternalTexture externalTexture = device.CreateExternalTexture(&externalDesc);
@@ -406,6 +414,8 @@ TEST_P(ExternalTextureTests, RotateAndOrFlipSinglePlane) {
         externalDesc.plane0 = sourceTexture.CreateView();
         externalDesc.rotation = exp.rotation;
         externalDesc.flipY = exp.flipY;
+        externalDesc.visibleOrigin = {0, 0};
+        externalDesc.visibleSize = {kWidth, kHeight};
 
         // Import the external texture
         wgpu::ExternalTexture externalTexture = device.CreateExternalTexture(&externalDesc);
@@ -516,6 +526,8 @@ TEST_P(ExternalTextureTests, RotateAndOrFlipMultiplanar) {
         wgpu::ExternalTextureDescriptor externalDesc = CreateDefaultExternalTextureDescriptor();
         externalDesc.plane0 = sourceTexturePlane0.CreateView();
         externalDesc.plane1 = sourceTexturePlane1.CreateView();
+        externalDesc.visibleOrigin = {0, 0};
+        externalDesc.visibleSize = {kWidth, kHeight};
 
         // Import the external texture
         wgpu::ExternalTexture externalTexture = device.CreateExternalTexture(&externalDesc);
@@ -589,6 +601,8 @@ TEST_P(ExternalTextureTests, RotateAndOrFlipMultiplanar) {
         externalDesc.plane1 = sourceTexturePlane1.CreateView();
         externalDesc.rotation = exp.rotation;
         externalDesc.flipY = exp.flipY;
+        externalDesc.visibleOrigin = {0, 0};
+        externalDesc.visibleSize = {kWidth, kHeight};
 
         // Import the external texture
         wgpu::ExternalTexture externalTexture = device.CreateExternalTexture(&externalDesc);
@@ -668,7 +682,8 @@ TEST_P(ExternalTextureTests, CropSinglePlane) {
                         wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::RenderAttachment);
 
     struct CropExpectation {
-        wgpu::ExternalTextureVisibleRect visibleRect;
+        wgpu::Origin2D visibleOrigin;
+        wgpu::Extent2D visibleSize;
         wgpu::ExternalTextureRotation rotation;
         utils::RGBA8 upperLeftColor;
         utils::RGBA8 upperRightColor;
@@ -677,55 +692,64 @@ TEST_P(ExternalTextureTests, CropSinglePlane) {
     };
 
     std::array<CropExpectation, 9> expectations = {{
-        {{0.0, 0.0, 1.0, 1.0},
+        {{0, 0},
+         {kWidth, kHeight},
          wgpu::ExternalTextureRotation::Rotate0Degrees,
          utils::RGBA8::kBlack,
          utils::RGBA8::kBlack,
          utils::RGBA8::kBlack,
          utils::RGBA8::kBlack},
-        {{0.25, 0.25, 0.25, 0.25},
+        {{kWidth / 4, kHeight / 4},
+         {kWidth / 4, kHeight / 4},
          wgpu::ExternalTextureRotation::Rotate0Degrees,
          utils::RGBA8::kGreen,
          utils::RGBA8::kGreen,
          utils::RGBA8::kGreen,
          utils::RGBA8::kGreen},
-        {{0.5, 0.25, 0.25, 0.25},
+        {{kWidth / 2, kHeight / 4},
+         {kWidth / 4, kHeight / 4},
          wgpu::ExternalTextureRotation::Rotate0Degrees,
          utils::RGBA8::kWhite,
          utils::RGBA8::kWhite,
          utils::RGBA8::kWhite,
          utils::RGBA8::kWhite},
-        {{0.25, 0.5, 0.25, 0.25},
+        {{kWidth / 4, kHeight / 2},
+         {kWidth / 4, kHeight / 4},
          wgpu::ExternalTextureRotation::Rotate0Degrees,
          utils::RGBA8::kRed,
          utils::RGBA8::kRed,
          utils::RGBA8::kRed,
          utils::RGBA8::kRed},
-        {{0.5, 0.5, 0.25, 0.25},
+        {{kWidth / 2, kHeight / 2},
+         {kWidth / 4, kHeight / 4},
          wgpu::ExternalTextureRotation::Rotate0Degrees,
          utils::RGBA8::kBlue,
          utils::RGBA8::kBlue,
          utils::RGBA8::kBlue,
          utils::RGBA8::kBlue},
-        {{0.25, 0.25, 0.5, 0.5},
+        {{kWidth / 4, kHeight / 4},
+         {kWidth / 2, kHeight / 2},
          wgpu::ExternalTextureRotation::Rotate0Degrees,
          utils::RGBA8::kGreen,
          utils::RGBA8::kWhite,
          utils::RGBA8::kRed,
          utils::RGBA8::kBlue},
-        {{0.25, 0.25, 0.5, 0.5},
+        {{kWidth / 4, kHeight / 4},
+         {kWidth / 2, kHeight / 2},
          wgpu::ExternalTextureRotation::Rotate90Degrees,
          utils::RGBA8::kRed,
          utils::RGBA8::kGreen,
          utils::RGBA8::kBlue,
          utils::RGBA8::kWhite},
-        {{0.25, 0.25, 0.5, 0.5},
+        {{kWidth / 4, kHeight / 4},
+         {kWidth / 2, kHeight / 2},
          wgpu::ExternalTextureRotation::Rotate180Degrees,
          utils::RGBA8::kBlue,
          utils::RGBA8::kRed,
          utils::RGBA8::kWhite,
          utils::RGBA8::kGreen},
-        {{0.25, 0.25, 0.5, 0.5},
+        {{kWidth / 4, kHeight / 4},
+         {kWidth / 2, kHeight / 2},
          wgpu::ExternalTextureRotation::Rotate270Degrees,
          utils::RGBA8::kWhite,
          utils::RGBA8::kBlue,
@@ -744,8 +768,9 @@ TEST_P(ExternalTextureTests, CropSinglePlane) {
         // Create an ExternalTextureDescriptor from the texture view
         wgpu::ExternalTextureDescriptor externalDesc = CreateDefaultExternalTextureDescriptor();
         externalDesc.plane0 = sourceTexture.CreateView();
-        externalDesc.visibleRect = exp.visibleRect;
         externalDesc.rotation = exp.rotation;
+        externalDesc.visibleOrigin = exp.visibleOrigin;
+        externalDesc.visibleSize = exp.visibleSize;
 
         // Import the external texture
         wgpu::ExternalTexture externalTexture = device.CreateExternalTexture(&externalDesc);
@@ -849,7 +874,8 @@ TEST_P(ExternalTextureTests, CropMultiplanar) {
                         wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::RenderAttachment);
 
     struct CropExpectation {
-        wgpu::ExternalTextureVisibleRect visibleRect;
+        wgpu::Origin2D visibleOrigin;
+        wgpu::Extent2D visibleSize;
         wgpu::ExternalTextureRotation rotation;
         utils::RGBA8 upperLeftColor;
         utils::RGBA8 upperRightColor;
@@ -858,55 +884,64 @@ TEST_P(ExternalTextureTests, CropMultiplanar) {
     };
 
     std::array<CropExpectation, 9> expectations = {{
-        {{0.0, 0.0, 1.0, 1.0},
+        {{0, 0},
+         {kWidth, kHeight},
          wgpu::ExternalTextureRotation::Rotate0Degrees,
          utils::RGBA8::kBlack,
          utils::RGBA8::kBlack,
          utils::RGBA8::kBlack,
          utils::RGBA8::kBlack},
-        {{0.25, 0.25, 0.25, 0.25},
+        {{kWidth / 4, kHeight / 4},
+         {kWidth / 4, kHeight / 4},
          wgpu::ExternalTextureRotation::Rotate0Degrees,
          utils::RGBA8::kGreen,
          utils::RGBA8::kGreen,
          utils::RGBA8::kGreen,
          utils::RGBA8::kGreen},
-        {{0.5, 0.25, 0.25, 0.25},
+        {{kWidth / 2, kHeight / 4},
+         {kWidth / 4, kHeight / 4},
          wgpu::ExternalTextureRotation::Rotate0Degrees,
          utils::RGBA8::kWhite,
          utils::RGBA8::kWhite,
          utils::RGBA8::kWhite,
          utils::RGBA8::kWhite},
-        {{0.25, 0.5, 0.25, 0.25},
+        {{kWidth / 4, kHeight / 2},
+         {kWidth / 4, kHeight / 4},
          wgpu::ExternalTextureRotation::Rotate0Degrees,
          utils::RGBA8::kRed,
          utils::RGBA8::kRed,
          utils::RGBA8::kRed,
          utils::RGBA8::kRed},
-        {{0.5, 0.5, 0.25, 0.25},
+        {{kWidth / 2, kHeight / 2},
+         {kWidth / 4, kHeight / 4},
          wgpu::ExternalTextureRotation::Rotate0Degrees,
          utils::RGBA8::kBlue,
          utils::RGBA8::kBlue,
          utils::RGBA8::kBlue,
          utils::RGBA8::kBlue},
-        {{0.25, 0.25, 0.5, 0.5},
+        {{kWidth / 4, kHeight / 4},
+         {kWidth / 2, kHeight / 2},
          wgpu::ExternalTextureRotation::Rotate0Degrees,
          utils::RGBA8::kGreen,
          utils::RGBA8::kWhite,
          utils::RGBA8::kRed,
          utils::RGBA8::kBlue},
-        {{0.25, 0.25, 0.5, 0.5},
+        {{kWidth / 4, kHeight / 4},
+         {kWidth / 2, kHeight / 2},
          wgpu::ExternalTextureRotation::Rotate90Degrees,
          utils::RGBA8::kRed,
          utils::RGBA8::kGreen,
          utils::RGBA8::kBlue,
          utils::RGBA8::kWhite},
-        {{0.25, 0.25, 0.5, 0.5},
+        {{kWidth / 4, kHeight / 4},
+         {kWidth / 2, kHeight / 2},
          wgpu::ExternalTextureRotation::Rotate180Degrees,
          utils::RGBA8::kBlue,
          utils::RGBA8::kRed,
          utils::RGBA8::kWhite,
          utils::RGBA8::kGreen},
-        {{0.25, 0.25, 0.5, 0.5},
+        {{kWidth / 4, kHeight / 4},
+         {kWidth / 2, kHeight / 2},
          wgpu::ExternalTextureRotation::Rotate270Degrees,
          utils::RGBA8::kWhite,
          utils::RGBA8::kBlue,
@@ -927,7 +962,8 @@ TEST_P(ExternalTextureTests, CropMultiplanar) {
         externalDesc.plane0 = sourceTexturePlane0.CreateView();
         externalDesc.plane1 = sourceTexturePlane1.CreateView();
         externalDesc.rotation = exp.rotation;
-        externalDesc.visibleRect = exp.visibleRect;
+        externalDesc.visibleOrigin = exp.visibleOrigin;
+        externalDesc.visibleSize = exp.visibleSize;
 
         // Import the external texture
         wgpu::ExternalTexture externalTexture = device.CreateExternalTexture(&externalDesc);

src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp

@@ -29,7 +29,7 @@ class BindGroupValidationTest : public ValidationTest {
                                 uint32_t layerCount) {
         wgpu::TextureDescriptor descriptor;
         descriptor.dimension = wgpu::TextureDimension::e2D;
-        descriptor.size = {16, 16, layerCount};
+        descriptor.size = {kWidth, kHeight, layerCount};
         descriptor.sampleCount = 1;
         descriptor.mipLevelCount = 1;
         descriptor.usage = usage;
@@ -75,6 +75,7 @@ class BindGroupValidationTest : public ValidationTest {
         desc.gamutConversionMatrix = mPlaceholderConstantArray.data();
         desc.srcTransferFunctionParameters = mPlaceholderConstantArray.data();
         desc.dstTransferFunctionParameters = mPlaceholderConstantArray.data();
+        desc.visibleSize = {kWidth, kHeight};
         return desc;
     }
 
@@ -96,6 +97,8 @@ class BindGroupValidationTest : public ValidationTest {
     static constexpr wgpu::TextureFormat kDefaultTextureFormat = wgpu::TextureFormat::RGBA8Unorm;
 
   private:
+    uint32_t kWidth = 16;
+    uint32_t kHeight = 16;
     wgpu::ExternalTexture mExternalTexture;
     std::array<float, 12> mPlaceholderConstantArray;
 };

src/dawn/tests/unittests/validation/ExternalTextureTests.cpp

@@ -47,6 +47,7 @@ class ExternalTextureTest : public ValidationTest {
         desc.gamutConversionMatrix = mPlaceholderConstantArray.data();
         desc.srcTransferFunctionParameters = mPlaceholderConstantArray.data();
         desc.dstTransferFunctionParameters = mPlaceholderConstantArray.data();
+        desc.visibleSize = {kWidth, kHeight};
 
         return desc;
     }

src/dawn/tests/unittests/validation/LabelTests.cpp

@@ -232,6 +232,7 @@ TEST_F(LabelTest, ExternalTexture) {
     descriptor.gamutConversionMatrix = mPlaceholderConstantArray.data();
     descriptor.srcTransferFunctionParameters = mPlaceholderConstantArray.data();
     descriptor.dstTransferFunctionParameters = mPlaceholderConstantArray.data();
+    descriptor.visibleSize = {1, 1};
 
     // The label should be empty if one was not set.
     {