GLSL: fix textureLoad() and textureStore(), depth textures, and more.

The CombineSamplers transform was incorrectly flagging StorageTexture
(which in GLSL ends up as image2D) as needing to be combined with a
sampler, or at least renamed. This is incorrect: StorageTexture never
has an associated sampler, so don't try to pair it up and just output
it as image* in GLSL.

In GLSL, textureLoad (aka texelFetch) of depth textures is not allowed.
The fix is to bind the depth texture as the corresponding f32 texture
instead (e.g., texture_depth_2d -> texture_2d<f32>,
texture_depth_cube -> texture_cube<f32>, etc). This requires changing
both the uniform globals and function parameter types. We're now going
to receive a vec4 instead of a float from texelFetch, so add a ".x"
member accessor to retrieve the first component. (Note that we don't
do this inside a CallStatement since this gives the CloneContext
indigestion, and CallStatement is going to ignore the result of the
call anyway.)

We were failing to find the dummy samplers that Dawn creates for the
calls that actually do require a dummy sampler, since the old Inspector
implementation of GetSamplerTextureUses() does not find them. The fix
is to implement a new Inspector call to return the texture/sampler
pairs the Resolver found during resolution. This will include the
dummy sampler as a null variable pointer.

In order to identify the placeholder sampler, we pass in a BindingPair
to represent it. When we discover a null sampler in the variable pair,
we return the passed-in placeholder binding point to the caller (Dawn).
(Dawn will use a group of kMaxBindGroups, to ensure that it never
collides with an existing sampler.)

Bug: tint:1298
Change-Id: I82e142c2b4318608c27a9fa9521c27f15a6214cd
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/78820
Reviewed-by: Ben Clayton <[email protected]>
Kokoro: Kokoro <[email protected]>
Commit-Queue: Stephen White <[email protected]>

src/inspector/inspector.cc

@@ -535,6 +535,29 @@ std::vector<sem::SamplerTexturePair> Inspector::GetSamplerTextureUses(
   return it->second;
 }
 
+std::vector<sem::SamplerTexturePair> Inspector::GetSamplerTextureUses(
+    const std::string& entry_point,
+    const sem::BindingPoint& placeholder) {
+  auto* func = FindEntryPointByName(entry_point);
+  if (!func) {
+    return {};
+  }
+  auto* func_sem = program_->Sem().Get(func);
+
+  std::vector<sem::SamplerTexturePair> new_pairs;
+  for (auto pair : func_sem->TextureSamplerPairs()) {
+    auto* texture = pair.first->As<sem::GlobalVariable>();
+    auto* sampler =
+        pair.second ? pair.second->As<sem::GlobalVariable>() : nullptr;
+    SamplerTexturePair new_pair;
+    new_pair.sampler_binding_point =
+        sampler ? sampler->BindingPoint() : placeholder;
+    new_pair.texture_binding_point = texture->BindingPoint();
+    new_pairs.push_back(new_pair);
+  }
+  return new_pairs;
+}
+
 uint32_t Inspector::GetWorkgroupStorageSize(const std::string& entry_point) {
   auto* func = FindEntryPointByName(entry_point);
   if (!func) {

src/inspector/inspector.h

@@ -135,6 +135,15 @@ class Inspector {
   std::vector<sem::SamplerTexturePair> GetSamplerTextureUses(
       const std::string& entry_point);
 
+  /// @param entry_point name of the entry point to get information about.
+  /// @param placeholder the sampler binding point to use for texture-only
+  /// access (e.g., textureLoad)
+  /// @returns vector of all of the sampler/texture sampling pairs that are used
+  /// by that entry point.
+  std::vector<sem::SamplerTexturePair> GetSamplerTextureUses(
+      const std::string& entry_point,
+      const sem::BindingPoint& placeholder);
+
   /// @param entry_point name of the entry point to get information about.
   /// @returns the total size in bytes of all Workgroup storage-class storage
   /// referenced transitively by the entry point.

src/resolver/resolver.cc

@@ -1400,12 +1400,14 @@ sem::Call* Resolver::BuiltinCall(const ast::CallExpression* expr,
     }
 
     auto* texture = args[texture_index]->As<sem::VariableUser>()->Variable();
-    int sampler_index = signature.IndexOf(sem::ParameterUsage::kSampler);
-    const sem::Variable* sampler =
-        sampler_index != -1
-            ? args[sampler_index]->As<sem::VariableUser>()->Variable()
-            : nullptr;
-    current_function_->AddTextureSamplerPair(texture, sampler);
+    if (!texture->Type()->UnwrapRef()->Is<sem::StorageTexture>()) {
+      int sampler_index = signature.IndexOf(sem::ParameterUsage::kSampler);
+      const sem::Variable* sampler =
+          sampler_index != -1
+              ? args[sampler_index]->As<sem::VariableUser>()->Variable()
+              : nullptr;
+      current_function_->AddTextureSamplerPair(texture, sampler);
+    }
   }
 
   if (!ValidateBuiltinCall(call)) {

src/transform/combine_samplers.cc

@@ -39,8 +39,9 @@ bool IsGlobal(const tint::sem::VariablePair& pair) {
 namespace tint {
 namespace transform {
 
-CombineSamplers::BindingInfo::BindingInfo(const BindingMap& map)
-    : binding_map(map) {}
+CombineSamplers::BindingInfo::BindingInfo(const BindingMap& map,
+                                          const sem::BindingPoint& placeholder)
+    : binding_map(map), placeholder_binding_point(placeholder) {}
 CombineSamplers::BindingInfo::BindingInfo(const BindingInfo& other) = default;
 CombineSamplers::BindingInfo::~BindingInfo() = default;
 
@@ -106,12 +107,12 @@ struct CombineSamplers::State {
         texture_var->As<sem::GlobalVariable>()->BindingPoint();
     bp_pair.sampler_binding_point =
         sampler_var ? sampler_var->As<sem::GlobalVariable>()->BindingPoint()
-                    : BindingPoint();
+                    : binding_info->placeholder_binding_point;
     auto it = binding_info->binding_map.find(bp_pair);
     if (it != binding_info->binding_map.end()) {
       name = it->second;
     }
-    const ast::Type* type = CreateASTTypeFor(ctx, texture_var->Type());
+    const ast::Type* type = CreateCombinedASTTypeFor(texture_var, sampler_var);
     Symbol symbol = ctx.dst->Symbols().New(name);
     return ctx.dst->Global(symbol, type, Attributes());
   }
@@ -128,6 +129,24 @@ struct CombineSamplers::State {
     return ctx.dst->Global(symbol, type, Attributes());
   }
 
+  /// Creates ast::Type for a given texture and sampler variable pair.
+  /// Depth textures with no samplers are turned into the corresponding
+  /// f32 texture (e.g., texture_depth_2d -> texture_2d<f32>).
+  /// @param texture the texture variable of interest
+  /// @param sampler the texture variable of interest
+  /// @returns the newly-created type
+  const ast::Type* CreateCombinedASTTypeFor(const sem::Variable* texture,
+                                            const sem::Variable* sampler) {
+    const sem::Type* texture_type = texture->Type()->UnwrapRef();
+    const sem::DepthTexture* depth = texture_type->As<sem::DepthTexture>();
+    if (depth && !sampler) {
+      return ctx.dst->create<ast::SampledTexture>(depth->dim(),
+                                                  ctx.dst->create<ast::F32>());
+    } else {
+      return CreateASTTypeFor(ctx, texture_type);
+    }
+  }
+
   /// Performs the transformation
   void Run() {
     auto& sem = ctx.src->Sem();
@@ -136,7 +155,8 @@ struct CombineSamplers::State {
     // by combined samplers.
     for (auto* var : ctx.src->AST().GlobalVariables()) {
       auto* type = sem.Get(var->type);
-      if (type && type->IsAnyOf<sem::Texture, sem::Sampler>()) {
+      if (type && type->IsAnyOf<sem::Texture, sem::Sampler>() &&
+          !type->Is<sem::StorageTexture>()) {
         ctx.Remove(ctx.src->AST().GlobalDeclarations(), var);
       } else if (auto binding_point = var->BindingPoint()) {
         if (binding_point.group->value == 0 &&
@@ -175,7 +195,8 @@ struct CombineSamplers::State {
           } else {
             // Either texture or sampler (or both) is a function parameter;
             // add a new function parameter to represent the combined sampler.
-            const ast::Type* type = CreateASTTypeFor(ctx, texture_var->Type());
+            const ast::Type* type =
+                CreateCombinedASTTypeFor(texture_var, sampler_var);
             const ast::Variable* var =
                 ctx.dst->Param(ctx.dst->Symbols().New(name), type);
             params.push_back(var);
@@ -219,6 +240,11 @@ struct CombineSamplers::State {
             return nullptr;
           }
           const sem::Expression* texture = call->Arguments()[texture_index];
+          // We don't want to combine storage textures with anything, since
+          // they never have associated samplers in GLSL.
+          if (texture->Type()->UnwrapRef()->Is<sem::StorageTexture>()) {
+            return nullptr;
+          }
           const sem::Expression* sampler =
               sampler_index != -1 ? call->Arguments()[sampler_index] : nullptr;
           auto* texture_var = texture->As<sem::VariableUser>()->Variable();
@@ -246,7 +272,14 @@ struct CombineSamplers::State {
               args.push_back(ctx.Clone(arg));
             }
           }
-          return ctx.dst->Call(ctx.Clone(expr->target.name), args);
+          const ast::Expression* value =
+              ctx.dst->Call(ctx.Clone(expr->target.name), args);
+          if (builtin->Type() == sem::BuiltinType::kTextureLoad &&
+              texture_var->Type()->UnwrapRef()->Is<sem::DepthTexture>() &&
+              !call->Stmt()->Declaration()->Is<ast::CallStatement>()) {
+            value = ctx.dst->MemberAccessor(value, "x");
+          }
+          return value;
         }
         // Replace all function calls.
         if (auto* callee = call->Target()->As<sem::Function>()) {

src/transform/combine_samplers.h

@@ -66,7 +66,8 @@ class CombineSamplers : public Castable<CombineSamplers, Transform> {
   struct BindingInfo : public Castable<Data, transform::Data> {
     /// Constructor
     /// @param map the map of all (texture, sampler) -> (combined) pairs
-    explicit BindingInfo(const BindingMap& map);
+    /// @param placeholder the binding point to use for placeholder samplers.
+    BindingInfo(const BindingMap& map, const sem::BindingPoint& placeholder);
 
     /// Copy constructor
     /// @param other the other BindingInfo to copy
@@ -77,6 +78,9 @@ class CombineSamplers : public Castable<CombineSamplers, Transform> {
 
     /// A map of bindings from (texture, sampler) -> combined sampler.
     BindingMap binding_map;
+
+    /// The binding point to use for placeholder samplers.
+    sem::BindingPoint placeholder_binding_point;
   };
 
   /// Constructor