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>
diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc
index 7c36f18..d95769f 100644
--- a/src/writer/hlsl/generator_impl.cc
+++ b/src/writer/hlsl/generator_impl.cc
@@ -745,7 +745,8 @@
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 @@
if (pidx.level != kNotUsed) {
pre << pidx.level << ", ";
}
- if (auto* vec = expr->result_type()->As<ast::type::Vector>()) {
- for (uint32_t i = 0; i < vec->size(); i++) {
- if (i > 0) {
- pre << ", ";
- }
- pre << dims << "[" << i << "]";
- }
- } else {
- pre << 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:
+ 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 << ");";
// The result of the textureDimensions() call is now the temporary variable.
out << dims;
diff --git a/src/writer/hlsl/generator_impl_intrinsic_texture_test.cc b/src/writer/hlsl/generator_impl_intrinsic_texture_test.cc
index f9d6448..bd4029b 100644
--- a/src/writer/hlsl/generator_impl_intrinsic_texture_test.cc
+++ b/src/writer/hlsl/generator_impl_intrinsic_texture_test.cc
@@ -67,10 +67,6 @@
"_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 @@
"_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 @@
"_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], "
- "_tint_tmp[2]);",
+ "texture_tint_0.GetDimensions(1, _tint_tmp[0], _tint_tmp[1]);\n"
+ "_tint_tmp[2] = _tint_tmp[1];",
"_tint_tmp",
};
case ValidTextureOverload::kSample1dF32:
diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc
index 1769301..5343a11 100644
--- a/src/writer/msl/generator_impl.cc
+++ b/src/writer/msl/generator_impl.cc
@@ -665,7 +665,37 @@
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 @@
return true;
};
- size_t dims = 1;
- if (auto* vec = expr->result_type()->As<ast::type::Vector>()) {
- dims = vec->size();
- }
- 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;
+ if (dims.size() == 1) {
+ get_dim(dims[0]);
+ } else {
+ EmitType(expr->result_type(), "");
+ out_ << "(";
+ for (size_t i = 0; i < dims.size(); i++) {
+ if (i > 0) {
+ out_ << ", ";
+ }
+ get_dim(dims[i]);
+ }
+ out_ << ")";
}
return true;
}
diff --git a/src/writer/msl/generator_impl_intrinsic_texture_test.cc b/src/writer/msl/generator_impl_intrinsic_texture_test.cc
index c7921d9..fcc6bbe 100644
--- a/src/writer/msl/generator_impl_intrinsic_texture_test.cc
+++ b/src/writer/msl/generator_impl_intrinsic_texture_test.cc
@@ -51,24 +51,26 @@
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:
- case ValidTextureOverload::kDimensionsStorageWO3d:
- return R"(int3(texture_tint_0.get_width(), texture_tint_0.get_height(), texture_tint_0.get_depth()))";
+ return R"(int3(texture_tint_0.get_width(), texture_tint_0.get_height(), texture_tint_0.get_height()))";
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:
diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc
index bd6fda7..68ca025 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -272,6 +272,14 @@
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 @@
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 @@
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 @@
switch (ident->intrinsic()) {
case ast::Intrinsic::kTextureDimensions: {
- if (ast::type::IsTextureArray(texture_type->dim())) {
- // OpImageQuerySize[Lod] will append another element to the returned
- // vector describing the number of array elements. textureDimensions()
- // does not include this in the returned vector, so it needs to be
- // stripped from the resulting vector.
- auto unstripped_result = result_op();
+ // Number of returned elements from OpImageQuerySize[Lod] may not match
+ // those of textureDimensions().
+ // This might be due to an extra vector scalar describing the number of
+ // array elements or textureDimensions() returning a vec3 for cubes
+ // when only width / height is returned by OpImageQuerySize[Lod]
+ // (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 (auto* v = call->result_type()->As<ast::type::Vector>()) {
- unstripped_result_type =
- mod_->create<ast::type::Vector>(v->type(), v->size() + 1);
+ if (swizzle.empty()) {
+ append_result_type_and_id_to_spirv_params();
+ } else {
+ // 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,
- unstripped_result,
+ spirv_result,
+ spirv_result,
};
- for (uint32_t i = 0; i < v->size(); i++) {
- operands.emplace_back(Operand::Int(i));
+ for (auto idx : swizzle) {
+ 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,
- {result_type, result_id, unstripped_result, Operand::Int(0)});
+ return push_function_inst(spv::Op::OpCompositeExtract,
+ {result_type, result_id, spirv_result,
+ Operand::Int(swizzle[0])});
};
}
-
- auto unstripped_result_type_id =
- GenerateTypeIfNeeded(unstripped_result_type);
- if (unstripped_result_type_id == 0) {
+ auto spirv_result_type_id = GenerateTypeIfNeeded(spirv_result_type);
+ if (spirv_result_type_id == 0) {
return false;
}
- spirv_params.emplace_back(Operand::Int(unstripped_result_type_id));
- spirv_params.emplace_back(unstripped_result);
- } else {
- append_result_type_and_id_to_spirv_params();
+ spirv_params.emplace_back(Operand::Int(spirv_result_type_id));
+ spirv_params.emplace_back(spirv_result);
}
spirv_params.emplace_back(gen_param(pidx.texture));
diff --git a/src/writer/spirv/builder_intrinsic_texture_test.cc b/src/writer/spirv/builder_intrinsic_texture_test.cc
index 760af94..a86ccb2 100644
--- a/src/writer/spirv/builder_intrinsic_texture_test.cc
+++ b/src/writer/spirv/builder_intrinsic_texture_test.cc
@@ -226,11 +226,13 @@
%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
-%8 = OpImageQuerySizeLod %9 %11 %12
+%13 = OpLoad %3 %1
+%11 = OpImageQuerySizeLod %12 %13 %14
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
)",
R"(
OpCapability ImageQuery
@@ -247,11 +249,13 @@
%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
-%8 = OpImageQuerySizeLod %9 %11 %12
+%13 = OpLoad %3 %1
+%11 = OpImageQuerySizeLod %12 %13 %14
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
)",
R"(
OpCapability ImageQuery
@@ -268,13 +272,12 @@
%5 = OpVariable %6 UniformConstant
%10 = OpTypeInt 32 1
%9 = OpTypeVector %10 3
-%12 = OpTypeVector %10 4
-%14 = OpConstant %10 0
+%13 = OpConstant %10 0
)",
R"(
-%13 = OpLoad %3 %1
-%11 = OpImageQuerySizeLod %12 %13 %14
-%8 = OpVectorShuffle %9 %11 %11 0 1 2
+%12 = OpLoad %3 %1
+%11 = OpImageQuerySizeLod %9 %12 %13
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
)",
R"(
OpCapability SampledCubeArray
@@ -292,13 +295,12 @@
%5 = OpVariable %6 UniformConstant
%10 = OpTypeInt 32 1
%9 = OpTypeVector %10 3
-%12 = OpTypeVector %10 4
-%14 = OpConstant %10 1
+%13 = OpConstant %10 1
)",
R"(
-%13 = OpLoad %3 %1
-%11 = OpImageQuerySizeLod %12 %13 %14
-%8 = OpVectorShuffle %9 %11 %11 0 1 2
+%12 = OpLoad %3 %1
+%11 = OpImageQuerySizeLod %9 %12 %13
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
)",
R"(
OpCapability SampledCubeArray
@@ -446,11 +448,13 @@
%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
-%8 = OpImageQuerySizeLod %9 %11 %12
+%13 = OpLoad %3 %1
+%11 = OpImageQuerySizeLod %12 %13 %14
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
)",
R"(
OpCapability ImageQuery
@@ -467,11 +471,13 @@
%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
-%8 = OpImageQuerySizeLod %9 %11 %12
+%13 = OpLoad %3 %1
+%11 = OpImageQuerySizeLod %12 %13 %14
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
)",
R"(
OpCapability ImageQuery
@@ -488,13 +494,12 @@
%5 = OpVariable %6 UniformConstant
%10 = OpTypeInt 32 1
%9 = OpTypeVector %10 3
-%12 = OpTypeVector %10 4
-%14 = OpConstant %10 0
+%13 = OpConstant %10 0
)",
R"(
-%13 = OpLoad %3 %1
-%11 = OpImageQuerySizeLod %12 %13 %14
-%8 = OpVectorShuffle %9 %11 %11 0 1 2
+%12 = OpLoad %3 %1
+%11 = OpImageQuerySizeLod %9 %12 %13
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
)",
R"(
OpCapability SampledCubeArray
@@ -512,13 +517,12 @@
%5 = OpVariable %6 UniformConstant
%10 = OpTypeInt 32 1
%9 = OpTypeVector %10 3
-%12 = OpTypeVector %10 4
-%14 = OpConstant %10 1
+%13 = OpConstant %10 1
)",
R"(
-%13 = OpLoad %3 %1
-%11 = OpImageQuerySizeLod %12 %13 %14
-%8 = OpVectorShuffle %9 %11 %11 0 1 2
+%12 = OpLoad %3 %1
+%11 = OpImageQuerySizeLod %9 %12 %13
+%8 = OpVectorShuffle %9 %11 %11 0 1 1
)",
R"(
OpCapability SampledCubeArray
