Support "length" in dawn.json could be a integer

Struct member defined in dawn.json could have a 'length' attribute
defines the length of the member if it is an array.

In previous, the 'length' could only be 'strlen' or other variable
which has 'uint32_t' types. It cannot support constant length.

CopyTextureForBrowserOptions transfers conversion parameters and
conversion matrix with a constant length. This CL adds this ability
to meet such requirements.

Bug: dawn:1140
Change-Id: I6eeabbc55cc3853fe15e33bdd44060b16f6096bf
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/72580
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Shaobo Yan <shaobo.yan@intel.com>

dawn.json

@@ -881,14 +881,12 @@
             {"name": "alpha op", "type": "alpha op", "default": "dont change", "tags": ["deprecated"]},
             {"name": "needs color space conversion", "type": "bool", "default": "false"},
             {"name": "src alpha mode", "type": "alpha mode", "default": "unpremultiplied"},
-            {"name": "transfer function parameters count", "type": "uint32_t", "default": "0"},
             {"name": "src transfer function parameters", "type": "float", "annotation": "const*",
-                     "length": "transfer function parameters count", "default": "nullptr"},
+                     "length": 7, "optional": true},
-            {"name": "conversion matrix elements count", "type": "uint32_t", "default": "0"},
             {"name": "conversion matrix", "type": "float", "annotation": "const*",
-                     "length": "conversion matrix elements count", "default": "nullptr"},
+                     "length": 9, "optional": true},
             {"name": "dst transfer function parameters", "type": "float", "annotation": "const*",
-                     "length": "transfer function parameters count", "default": "nullptr"},
+                     "length": 7, "optional": true},
             {"name": "dst alpha mode", "type": "alpha mode", "default": "unpremultiplied"}
         ]
     },

generator/dawn_json_generator.py

@@ -297,6 +297,10 @@ def linked_record_members(json_data, types):
                     assert False
             elif m['length'] == 'strlen':
                 member.length = 'strlen'
+            elif isinstance(m['length'], int):
+                assert m['length'] > 0
+                member.length = "constant"
+                member.constant_length = m['length']
             else:
                 member.length = members_by_name[m['length']]
 

src/dawn_native/CopyTextureForBrowserHelper.cpp

@@ -354,13 +354,6 @@ namespace dawn_native {
         DAWN_TRY(ValidateAlphaMode(options->dstAlphaMode));
 
         if (options->needsColorSpaceConversion) {
-            DAWN_INVALID_IF(options->transferFunctionParametersCount != 7u,
-                            "Invalid transfer"
-                            " function parameter count (%u).",
-                            options->transferFunctionParametersCount);
-            DAWN_INVALID_IF(options->conversionMatrixElementsCount != 9u,
-                            "Invalid conversion matrix elements count (%u).",
-                            options->conversionMatrixElementsCount);
             DAWN_INVALID_IF(options->srcTransferFunctionParameters == nullptr,
                             "srcTransferFunctionParameters is nullptr when doing color conversion");
             DAWN_INVALID_IF(options->conversionMatrix == nullptr,
@@ -368,7 +361,6 @@ namespace dawn_native {
             DAWN_INVALID_IF(options->dstTransferFunctionParameters == nullptr,
                             "dstTransferFunctionParameters is nullptr when doing color conversion");
         }
-
         return {};
     }
 

src/tests/end2end/CopyTextureForBrowserTests.cpp

@@ -916,9 +916,7 @@ class CopyTextureForBrowser_ColorSpace
         wgpu::CopyTextureForBrowserOptions options = {};
         options.needsColorSpaceConversion = srcColorSpace != dstColorSpace;
         options.srcAlphaMode = GetParam().mSrcAlphaMode;
-        options.transferFunctionParametersCount = 7;
         options.srcTransferFunctionParameters = srcColorSpaceInfo.gammaDecodingParams.data();
-        options.conversionMatrixElementsCount = 9;
         options.conversionMatrix = matrix.data();
         options.dstTransferFunctionParameters = dstColorSpaceInfo.gammaEncodingParams.data();
         options.dstAlphaMode = GetParam().mDstAlphaMode;

src/tests/unittests/validation/CopyTextureForBrowserTests.cpp

@@ -265,19 +265,17 @@ TEST_F(CopyTextureForBrowserTest, ColorSpaceConversion_ColorSpace) {
                         wgpu::TextureUsage::CopyDst | wgpu::TextureUsage::RenderAttachment);
 
     wgpu::CopyTextureForBrowserOptions options = {};
-    std::array<float, 7> srcTransferFunctionParameters = {};
-    std::array<float, 7> dstTransferFunctionParameters = {};
-    std::array<float, 9> conversionMatrix = {};
     options.needsColorSpaceConversion = true;
-    options.srcTransferFunctionParameters = srcTransferFunctionParameters.data();
-    options.dstTransferFunctionParameters = dstTransferFunctionParameters.data();
-    options.conversionMatrix = conversionMatrix.data();
-    options.conversionMatrixElementsCount = 9;
-    options.transferFunctionParametersCount = 7;
 
     // Valid cases
     {
         wgpu::CopyTextureForBrowserOptions validOptions = options;
+        std::array<float, 7> srcTransferFunctionParameters = {};
+        std::array<float, 7> dstTransferFunctionParameters = {};
+        std::array<float, 9> conversionMatrix = {};
+        validOptions.srcTransferFunctionParameters = srcTransferFunctionParameters.data();
+        validOptions.dstTransferFunctionParameters = dstTransferFunctionParameters.data();
+        validOptions.conversionMatrix = conversionMatrix.data();
         TestCopyTextureForBrowser(utils::Expectation::Success, source, 0, {0, 0, 0}, destination, 0,
                                   {0, 0, 0}, {4, 4, 1}, wgpu::TextureAspect::All, validOptions);
 
@@ -289,51 +287,51 @@ TEST_F(CopyTextureForBrowserTest, ColorSpaceConversion_ColorSpace) {
                                   noColorSpaceConversion);
     }
 
-    // Invalid cases: wrong transferFunctionParametersCount
-    {
-        // wrong: transferFunctionParametersCount must be 7
-        wgpu::CopyTextureForBrowserOptions invalidOptions = options;
-        invalidOptions.transferFunctionParametersCount = 6;
-        TestCopyTextureForBrowser(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0,
-                                  {0, 0, 0}, {4, 4, 1}, wgpu::TextureAspect::All, invalidOptions);
-    }
-
-    // Invalid cases: wrong conversionMatrixElementsCount
-    {
-        // wrong: conversionMatrixElementsCount
-        wgpu::CopyTextureForBrowserOptions invalidOptions = options;
-        invalidOptions.transferFunctionParametersCount = 10;
-        TestCopyTextureForBrowser(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0,
-                                  {0, 0, 0}, {4, 4, 1}, wgpu::TextureAspect::All, invalidOptions);
-    }
-
     // Invalid cases: srcTransferFunctionParameters, dstTransferFunctionParameters or
-    // conversionMatrix is nullptr
+    // conversionMatrix is nullptr or not set
     {
+        // not set srcTransferFunctionParameters
         wgpu::CopyTextureForBrowserOptions invalidOptions = options;
-        if (UsesWire()) {
+        std::array<float, 7> dstTransferFunctionParameters = {};
-            invalidOptions.transferFunctionParametersCount = 0;
+        std::array<float, 9> conversionMatrix = {};
-        }
+        invalidOptions.dstTransferFunctionParameters = dstTransferFunctionParameters.data();
+        invalidOptions.conversionMatrix = conversionMatrix.data();
+        TestCopyTextureForBrowser(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0,
+                                  {0, 0, 0}, {4, 4, 1}, wgpu::TextureAspect::All, invalidOptions);
+
+        // set to nullptr
         invalidOptions.srcTransferFunctionParameters = nullptr;
         TestCopyTextureForBrowser(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0,
                                   {0, 0, 0}, {4, 4, 1}, wgpu::TextureAspect::All, invalidOptions);
     }
 
     {
+        // not set dstTransferFunctionParameters
         wgpu::CopyTextureForBrowserOptions invalidOptions = options;
-        if (UsesWire()) {
+        std::array<float, 7> srcTransferFunctionParameters = {};
-            invalidOptions.transferFunctionParametersCount = 0;
+        std::array<float, 9> conversionMatrix = {};
-        }
+        invalidOptions.srcTransferFunctionParameters = srcTransferFunctionParameters.data();
+        invalidOptions.conversionMatrix = conversionMatrix.data();
+        TestCopyTextureForBrowser(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0,
+                                  {0, 0, 0}, {4, 4, 1}, wgpu::TextureAspect::All, invalidOptions);
+
+        // set to nullptr
         invalidOptions.dstTransferFunctionParameters = nullptr;
         TestCopyTextureForBrowser(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0,
                                   {0, 0, 0}, {4, 4, 1}, wgpu::TextureAspect::All, invalidOptions);
     }
 
     {
+        // not set conversionMatrix
         wgpu::CopyTextureForBrowserOptions invalidOptions = options;
-        if (UsesWire()) {
+        std::array<float, 7> srcTransferFunctionParameters = {};
-            invalidOptions.conversionMatrixElementsCount = 0;
+        std::array<float, 7> dstTransferFunctionParameters = {};
-        }
+        invalidOptions.srcTransferFunctionParameters = srcTransferFunctionParameters.data();
+        invalidOptions.dstTransferFunctionParameters = dstTransferFunctionParameters.data();
+        TestCopyTextureForBrowser(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0,
+                                  {0, 0, 0}, {4, 4, 1}, wgpu::TextureAspect::All, invalidOptions);
+
+        // set to nullptr
         invalidOptions.conversionMatrix = nullptr;
         TestCopyTextureForBrowser(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0,
                                   {0, 0, 0}, {4, 4, 1}, wgpu::TextureAspect::All, invalidOptions);