writers: Duplicate cube height for textureDimensions()

All shader backend languages describe cubes as width / height, but give no way to query depth. WGSL however returns a vec3<i32> for cube textures when calling textureDimensions().
As cube textures must be square (width == height == depth), just replicate the height for the depth.

See https://github.com/gpuweb/gpuweb/issues/1345

Bug: tint:140
Bug: tint:437
Change-Id: I76ef18ee4bd8b53d5f9d9d3f1c10c3f7cb23e137
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/37446
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>

src/writer/hlsl/generator_impl.cc

@@ -745,7 +745,8 @@ bool GeneratorImpl::EmitTextureCall(std::ostream& pre,
   auto const kNotUsed = ast::intrinsic::TextureSignature::Parameters::kNotUsed;
 
   auto* texture = params[pidx.texture];
-  auto* texture_type = texture->result_type()->UnwrapPtrIfNeeded();
+  auto* texture_type =
+      texture->result_type()->UnwrapPtrIfNeeded()->As<ast::type::Texture>();
 
   if (ident->intrinsic() == ast::Intrinsic::kTextureDimensions) {
     // Declare a variable to hold the texture dimensions
@@ -763,17 +764,29 @@ bool GeneratorImpl::EmitTextureCall(std::ostream& pre,
     if (pidx.level != kNotUsed) {
       pre << pidx.level << ", ";
     }
-    if (auto* vec = expr->result_type()->As<ast::type::Vector>()) {
+    switch (texture_type->dim()) {
-      for (uint32_t i = 0; i < vec->size(); i++) {
+      case ast::type::TextureDimension::kNone:
-        if (i > 0) {
+        error_ = "texture dimension is kNone";
-          pre << ", ";
+        return false;
+      case ast::type::TextureDimension::k1d:
+      case ast::type::TextureDimension::k1dArray:
+        pre << dims << ");";
+        break;
+      case ast::type::TextureDimension::k2d:
+      case ast::type::TextureDimension::k2dArray:
+        pre << dims << "[0], " << dims << "[1]);";
+        break;
+      case ast::type::TextureDimension::k3d:
+        pre << dims << "[0], " << dims << "[1], " << dims << "[2]);";
+        break;
+      case ast::type::TextureDimension::kCube:
+      case ast::type::TextureDimension::kCubeArray:
+        // width == height == depth for cubes
+        // See https://github.com/gpuweb/gpuweb/issues/1345
+        pre << dims << "[0], " << dims << "[1]);\n";
+        pre << dims << "[2] = " << dims << "[1];";  // dims[2] = dims[1]
+        break;
     }
-        pre << dims << "[" << i << "]";
-      }
-    } else {
-      pre << dims;
-    }
-    pre << ");";
 
     // The result of the textureDimensions() call is now the temporary variable.
     out << dims;

src/writer/hlsl/generator_impl_intrinsic_texture_test.cc

@@ -67,10 +67,6 @@ ExpectedResult expected_texture_overload(
           "_tint_tmp",
       };
     case ValidTextureOverload::kDimensions3d:
-    case ValidTextureOverload::kDimensionsCube:
-    case ValidTextureOverload::kDimensionsCubeArray:
-    case ValidTextureOverload::kDimensionsDepthCube:
-    case ValidTextureOverload::kDimensionsDepthCubeArray:
     case ValidTextureOverload::kDimensionsStorageRO3d:
     case ValidTextureOverload::kDimensionsStorageWO3d:
       return {
@@ -79,6 +75,16 @@ ExpectedResult expected_texture_overload(
           "_tint_tmp[2]);",
           "_tint_tmp",
       };
+    case ValidTextureOverload::kDimensionsCube:
+    case ValidTextureOverload::kDimensionsCubeArray:
+    case ValidTextureOverload::kDimensionsDepthCube:
+    case ValidTextureOverload::kDimensionsDepthCubeArray:
+      return {
+          "int3 _tint_tmp;\n"
+          "texture_tint_0.GetDimensions(_tint_tmp[0], _tint_tmp[1]);\n"
+          "_tint_tmp[2] = _tint_tmp[1];",
+          "_tint_tmp",
+      };
     case ValidTextureOverload::kDimensions2dLevel:
     case ValidTextureOverload::kDimensions2dArrayLevel:
     case ValidTextureOverload::kDimensionsDepth2dLevel:
@@ -89,14 +95,20 @@ ExpectedResult expected_texture_overload(
           "_tint_tmp",
       };
     case ValidTextureOverload::kDimensions3dLevel:
+      return {
+          "int3 _tint_tmp;\n"
+          "texture_tint_0.GetDimensions(1, _tint_tmp[0], _tint_tmp[1], "
+          "_tint_tmp[2]);",
+          "_tint_tmp",
+      };
     case ValidTextureOverload::kDimensionsCubeLevel:
     case ValidTextureOverload::kDimensionsCubeArrayLevel:
     case ValidTextureOverload::kDimensionsDepthCubeLevel:
     case ValidTextureOverload::kDimensionsDepthCubeArrayLevel:
       return {
           "int3 _tint_tmp;\n"
-          "texture_tint_0.GetDimensions(1, _tint_tmp[0], _tint_tmp[1], "
+          "texture_tint_0.GetDimensions(1, _tint_tmp[0], _tint_tmp[1]);\n"
-          "_tint_tmp[2]);",
+          "_tint_tmp[2] = _tint_tmp[1];",
           "_tint_tmp",
       };
     case ValidTextureOverload::kSample1dF32:

src/writer/msl/generator_impl.cc

@@ -665,7 +665,37 @@ bool GeneratorImpl::EmitTextureCall(ast::CallExpression* expr) {
   auto& pidx = signature->params.idx;
   auto const kNotUsed = ast::intrinsic::TextureSignature::Parameters::kNotUsed;
 
+  assert(pidx.texture != kNotUsed);
+  auto* texture_type = params[pidx.texture]
+                           ->result_type()
+                           ->UnwrapAll()
+                           ->As<ast::type::Texture>();
+
   if (ident->intrinsic() == ast::Intrinsic::kTextureDimensions) {
+    std::vector<const char*> dims;
+    switch (texture_type->dim()) {
+      case ast::type::TextureDimension::kNone:
+        error_ = "texture dimension is kNone";
+        return false;
+      case ast::type::TextureDimension::k1d:
+      case ast::type::TextureDimension::k1dArray:
+        dims = {"width"};
+        break;
+      case ast::type::TextureDimension::k2d:
+      case ast::type::TextureDimension::k2dArray:
+        dims = {"width", "height"};
+        break;
+      case ast::type::TextureDimension::k3d:
+        dims = {"width", "height", "depth"};
+        break;
+      case ast::type::TextureDimension::kCube:
+      case ast::type::TextureDimension::kCubeArray:
+        // width == height == depth for cubes
+        // See https://github.com/gpuweb/gpuweb/issues/1345
+        dims = {"width", "height", "height"};
+        break;
+    }
+
     auto get_dim = [&](const char* name) {
       if (!EmitExpression(params[pidx.texture])) {
         return false;
@@ -678,32 +708,18 @@ bool GeneratorImpl::EmitTextureCall(ast::CallExpression* expr) {
       return true;
     };
 
-    size_t dims = 1;
+    if (dims.size() == 1) {
-    if (auto* vec = expr->result_type()->As<ast::type::Vector>()) {
+      get_dim(dims[0]);
-      dims = vec->size();
+    } else {
+      EmitType(expr->result_type(), "");
+      out_ << "(";
+      for (size_t i = 0; i < dims.size(); i++) {
+        if (i > 0) {
+          out_ << ", ";
+        }
+        get_dim(dims[i]);
       }
-    switch (dims) {
-      case 1:
-        get_dim("width");
-        break;
-      case 2:
-        EmitType(expr->result_type(), "");
-        out_ << "(";
-        get_dim("width");
-        out_ << ", ";
-        get_dim("height");
       out_ << ")";
-        break;
-      case 3:
-        EmitType(expr->result_type(), "");
-        out_ << "(";
-        get_dim("width");
-        out_ << ", ";
-        get_dim("height");
-        out_ << ", ";
-        get_dim("depth");
-        out_ << ")";
-        break;
     }
     return true;
   }

src/writer/msl/generator_impl_intrinsic_texture_test.cc

@@ -51,24 +51,26 @@ std::string expected_texture_overload(
     case ValidTextureOverload::kDimensionsStorageWO2dArray:
       return R"(int2(texture_tint_0.get_width(), texture_tint_0.get_height()))";
     case ValidTextureOverload::kDimensions3d:
+    case ValidTextureOverload::kDimensionsStorageRO3d:
+    case ValidTextureOverload::kDimensionsStorageWO3d:
+      return R"(int3(texture_tint_0.get_width(), texture_tint_0.get_height(), texture_tint_0.get_depth()))";
     case ValidTextureOverload::kDimensionsCube:
     case ValidTextureOverload::kDimensionsCubeArray:
     case ValidTextureOverload::kDimensionsDepthCube:
     case ValidTextureOverload::kDimensionsDepthCubeArray:
-    case ValidTextureOverload::kDimensionsStorageRO3d:
+      return R"(int3(texture_tint_0.get_width(), texture_tint_0.get_height(), texture_tint_0.get_height()))";
-    case ValidTextureOverload::kDimensionsStorageWO3d:
-      return R"(int3(texture_tint_0.get_width(), texture_tint_0.get_height(), texture_tint_0.get_depth()))";
     case ValidTextureOverload::kDimensions2dLevel:
     case ValidTextureOverload::kDimensions2dArrayLevel:
     case ValidTextureOverload::kDimensionsDepth2dLevel:
     case ValidTextureOverload::kDimensionsDepth2dArrayLevel:
       return R"(int2(texture_tint_0.get_width(1), texture_tint_0.get_height(1)))";
     case ValidTextureOverload::kDimensions3dLevel:
+      return R"(int3(texture_tint_0.get_width(1), texture_tint_0.get_height(1), texture_tint_0.get_depth(1)))";
     case ValidTextureOverload::kDimensionsCubeLevel:
     case ValidTextureOverload::kDimensionsCubeArrayLevel:
     case ValidTextureOverload::kDimensionsDepthCubeLevel:
     case ValidTextureOverload::kDimensionsDepthCubeArrayLevel:
-      return R"(int3(texture_tint_0.get_width(1), texture_tint_0.get_height(1), texture_tint_0.get_depth(1)))";
+      return R"(int3(texture_tint_0.get_width(1), texture_tint_0.get_height(1), texture_tint_0.get_height(1)))";
     case ValidTextureOverload::kSample1dF32:
       return R"(texture_tint_0.sample(sampler_tint_0, 1.0f))";
     case ValidTextureOverload::kSample1dArrayF32:

src/writer/spirv/builder.cc

@@ -272,6 +272,14 @@ uint32_t intrinsic_to_glsl_method(ast::type::Type* type,
   return 0;
 }
 
+/// @return the vector element type if ty is a vector, otherwise return ty.
+ast::type::Type* ElementTypeOf(ast::type::Type* ty) {
+  if (auto* v = ty->As<ast::type::Vector>()) {
+    return v->type();
+  }
+  return ty;
+}
+
 }  // namespace
 
 Builder::AccessorInfo::AccessorInfo() : source_id(0), source_type(nullptr) {}
@@ -1987,9 +1995,6 @@ bool Builder::GenerateTextureIntrinsic(ast::IdentifierExpression* ident,
                                        ast::CallExpression* call,
                                        Operand result_type,
                                        Operand result_id) {
-  auto* texture_type =
-      call->params()[0]->result_type()->UnwrapAll()->As<ast::type::Texture>();
-
   auto* sig = static_cast<const ast::intrinsic::TextureSignature*>(
       ident->intrinsic_signature());
   assert(sig != nullptr);
@@ -1997,6 +2002,10 @@ bool Builder::GenerateTextureIntrinsic(ast::IdentifierExpression* ident,
   auto const kNotUsed = ast::intrinsic::TextureSignature::Parameters::kNotUsed;
 
   assert(pidx.texture != kNotUsed);
+  auto* texture_type = call->params()[pidx.texture]
+                           ->result_type()
+                           ->UnwrapAll()
+                           ->As<ast::type::Texture>();
 
   auto op = spv::Op::OpNop;
 
@@ -2070,50 +2079,76 @@ bool Builder::GenerateTextureIntrinsic(ast::IdentifierExpression* ident,
 
   switch (ident->intrinsic()) {
     case ast::Intrinsic::kTextureDimensions: {
-      if (ast::type::IsTextureArray(texture_type->dim())) {
+      // Number of returned elements from OpImageQuerySize[Lod] may not match
-        // OpImageQuerySize[Lod] will append another element to the returned
+      // those of textureDimensions().
-        // vector describing the number of array elements. textureDimensions()
+      // This might be due to an extra vector scalar describing the number of
-        // does not include this in the returned vector, so it needs to be
+      // array elements or textureDimensions() returning a vec3 for cubes
-        // stripped from the resulting vector.
+      // when only width / height is returned by OpImageQuerySize[Lod]
-        auto unstripped_result = result_op();
+      // (see https://github.com/gpuweb/gpuweb/issues/1345).
+      // Handle these mismatches by swizzling the returned vector.
+      std::vector<uint32_t> swizzle;
+      uint32_t spirv_dims = 0;
+      switch (texture_type->dim()) {
+        case ast::type::TextureDimension::kNone:
+          error_ = "texture dimension is kNone";
+          return false;
+        case ast::type::TextureDimension::k1d:
+        case ast::type::TextureDimension::k2d:
+        case ast::type::TextureDimension::k3d:
+          break;  // No swizzle needed
+        case ast::type::TextureDimension::k1dArray:
+          swizzle = {0};   // Strip array index
+          spirv_dims = 2;  // [width, array count]
+          break;
+        case ast::type::TextureDimension::kCube:
+          swizzle = {0, 1, 1};  // Duplicate height for depth
+          spirv_dims = 2;       // [width, height]
+          break;
+        case ast::type::TextureDimension::k2dArray:
+          swizzle = {0, 1};  // Strip array index
+          spirv_dims = 3;    // [width, height, array_count]
+          break;
+        case ast::type::TextureDimension::kCubeArray:
+          swizzle = {0, 1, 1};  // Strip array index, duplicate height for depth
+          spirv_dims = 3;       // [width, height, array_count]
+          break;
+      }
 
-        ast::type::Type* unstripped_result_type;
+      if (swizzle.empty()) {
-        if (auto* v = call->result_type()->As<ast::type::Vector>()) {
+        append_result_type_and_id_to_spirv_params();
-          unstripped_result_type =
+      } else {
-              mod_->create<ast::type::Vector>(v->type(), v->size() + 1);
+        // Assign post_emission to swizzle the result of the call to
+        // OpImageQuerySize[Lod].
+        auto* element_type = ElementTypeOf(call->result_type());
+        auto spirv_result = result_op();
+        auto* spirv_result_type =
+            mod_->create<ast::type::Vector>(element_type, spirv_dims);
+        if (swizzle.size() > 1) {
           post_emission = [=] {
-            // Swizzle the unstripped vector to form a vec2 or vec3
             OperandList operands{
                 result_type,
                 result_id,
-                unstripped_result,
+                spirv_result,
-                unstripped_result,
+                spirv_result,
             };
-            for (uint32_t i = 0; i < v->size(); i++) {
+            for (auto idx : swizzle) {
-              operands.emplace_back(Operand::Int(i));
+              operands.emplace_back(Operand::Int(idx));
             }
             return push_function_inst(spv::Op::OpVectorShuffle, operands);
           };
         } else {
-          unstripped_result_type =
-              mod_->create<ast::type::Vector>(call->result_type(), 2);
           post_emission = [=] {
-            // Extract the first element of the unstripped vec2 to form a scalar
+            return push_function_inst(spv::Op::OpCompositeExtract,
-            return push_function_inst(
+                                      {result_type, result_id, spirv_result,
-                spv::Op::OpCompositeExtract,
+                                       Operand::Int(swizzle[0])});
-                {result_type, result_id, unstripped_result, Operand::Int(0)});
           };
         }
-
+        auto spirv_result_type_id = GenerateTypeIfNeeded(spirv_result_type);
-        auto unstripped_result_type_id =
+        if (spirv_result_type_id == 0) {
-            GenerateTypeIfNeeded(unstripped_result_type);
-        if (unstripped_result_type_id == 0) {
           return false;
         }
-        spirv_params.emplace_back(Operand::Int(unstripped_result_type_id));
+        spirv_params.emplace_back(Operand::Int(spirv_result_type_id));
-        spirv_params.emplace_back(unstripped_result);
+        spirv_params.emplace_back(spirv_result);
-      } else {
-        append_result_type_and_id_to_spirv_params();
       }
 
       spirv_params.emplace_back(gen_param(pidx.texture));

src/writer/spirv/builder_intrinsic_texture_test.cc

@@ -226,11 +226,13 @@ OpCapability ImageQuery
 %5 = OpVariable %6 UniformConstant
 %10 = OpTypeInt 32 1
 %9 = OpTypeVector %10 3
-%12 = OpConstant %10 0
+%12 = OpTypeVector %10 2
+%14 = OpConstant %10 0
 )",
           R"(
-%11 = OpLoad %3 %1
+%13 = OpLoad %3 %1
-%8 = OpImageQuerySizeLod %9 %11 %12
+%11 = OpImageQuerySizeLod %12 %13 %14
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
 )",
           R"(
 OpCapability ImageQuery
@@ -247,11 +249,13 @@ OpCapability ImageQuery
 %5 = OpVariable %6 UniformConstant
 %10 = OpTypeInt 32 1
 %9 = OpTypeVector %10 3
-%12 = OpConstant %10 1
+%12 = OpTypeVector %10 2
+%14 = OpConstant %10 1
 )",
           R"(
-%11 = OpLoad %3 %1
+%13 = OpLoad %3 %1
-%8 = OpImageQuerySizeLod %9 %11 %12
+%11 = OpImageQuerySizeLod %12 %13 %14
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
 )",
           R"(
 OpCapability ImageQuery
@@ -268,13 +272,12 @@ OpCapability ImageQuery
 %5 = OpVariable %6 UniformConstant
 %10 = OpTypeInt 32 1
 %9 = OpTypeVector %10 3
-%12 = OpTypeVector %10 4
+%13 = OpConstant %10 0
-%14 = OpConstant %10 0
 )",
           R"(
-%13 = OpLoad %3 %1
+%12 = OpLoad %3 %1
-%11 = OpImageQuerySizeLod %12 %13 %14
+%11 = OpImageQuerySizeLod %9 %12 %13
-%8 = OpVectorShuffle %9 %11 %11 0 1 2
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
 )",
           R"(
 OpCapability SampledCubeArray
@@ -292,13 +295,12 @@ OpCapability ImageQuery
 %5 = OpVariable %6 UniformConstant
 %10 = OpTypeInt 32 1
 %9 = OpTypeVector %10 3
-%12 = OpTypeVector %10 4
+%13 = OpConstant %10 1
-%14 = OpConstant %10 1
 )",
           R"(
-%13 = OpLoad %3 %1
+%12 = OpLoad %3 %1
-%11 = OpImageQuerySizeLod %12 %13 %14
+%11 = OpImageQuerySizeLod %9 %12 %13
-%8 = OpVectorShuffle %9 %11 %11 0 1 2
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
 )",
           R"(
 OpCapability SampledCubeArray
@@ -446,11 +448,13 @@ OpCapability ImageQuery
 %5 = OpVariable %6 UniformConstant
 %10 = OpTypeInt 32 1
 %9 = OpTypeVector %10 3
-%12 = OpConstant %10 0
+%12 = OpTypeVector %10 2
+%14 = OpConstant %10 0
 )",
           R"(
-%11 = OpLoad %3 %1
+%13 = OpLoad %3 %1
-%8 = OpImageQuerySizeLod %9 %11 %12
+%11 = OpImageQuerySizeLod %12 %13 %14
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
 )",
           R"(
 OpCapability ImageQuery
@@ -467,11 +471,13 @@ OpCapability ImageQuery
 %5 = OpVariable %6 UniformConstant
 %10 = OpTypeInt 32 1
 %9 = OpTypeVector %10 3
-%12 = OpConstant %10 1
+%12 = OpTypeVector %10 2
+%14 = OpConstant %10 1
 )",
           R"(
-%11 = OpLoad %3 %1
+%13 = OpLoad %3 %1
-%8 = OpImageQuerySizeLod %9 %11 %12
+%11 = OpImageQuerySizeLod %12 %13 %14
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
 )",
           R"(
 OpCapability ImageQuery
@@ -488,13 +494,12 @@ OpCapability ImageQuery
 %5 = OpVariable %6 UniformConstant
 %10 = OpTypeInt 32 1
 %9 = OpTypeVector %10 3
-%12 = OpTypeVector %10 4
+%13 = OpConstant %10 0
-%14 = OpConstant %10 0
 )",
           R"(
-%13 = OpLoad %3 %1
+%12 = OpLoad %3 %1
-%11 = OpImageQuerySizeLod %12 %13 %14
+%11 = OpImageQuerySizeLod %9 %12 %13
-%8 = OpVectorShuffle %9 %11 %11 0 1 2
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
 )",
           R"(
 OpCapability SampledCubeArray
@@ -512,13 +517,12 @@ OpCapability ImageQuery
 %5 = OpVariable %6 UniformConstant
 %10 = OpTypeInt 32 1
 %9 = OpTypeVector %10 3
-%12 = OpTypeVector %10 4
+%13 = OpConstant %10 1
-%14 = OpConstant %10 1
 )",
           R"(
-%13 = OpLoad %3 %1
+%12 = OpLoad %3 %1
-%11 = OpImageQuerySizeLod %12 %13 %14
+%11 = OpImageQuerySizeLod %9 %12 %13
-%8 = OpVectorShuffle %9 %11 %11 0 1 2
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
 )",
           R"(
 OpCapability SampledCubeArray