[Impeller] partially remove remap sampler support

impeller/compiler/compiler.cc

@@ -36,44 +36,6 @@ static CompilerBackend CreateMSLCompiler(const spirv_cross::ParsedIR& ir,
       spirv_cross::CompilerMSL::Options::make_msl_version(1, 2);
   sl_compiler->set_msl_options(sl_options);
 
-  // Set metal resource mappings to be consistent with location based mapping
-  // used on other backends when creating fragment shaders. This doesn't seem
-  // to work with the generated bindings for compute shaders, nor for certain
-  // shaders in the flutter/engine tree.
-  if (source_options.remap_samplers) {
-    std::vector<uint32_t> sampler_offsets;
-    ir.for_each_typed_id<spirv_cross::SPIRVariable>(
-        [&](uint32_t, const spirv_cross::SPIRVariable& var) {
-          if (var.storage != spv::StorageClassUniformConstant) {
-            return;
-          }
-          const auto spir_type = sl_compiler->get_type(var.basetype);
-          auto location = sl_compiler->get_decoration(
-              var.self, spv::Decoration::DecorationLocation);
-          if (spir_type.basetype ==
-              spirv_cross::SPIRType::BaseType::SampledImage) {
-            sampler_offsets.push_back(location);
-          }
-        });
-    if (sampler_offsets.size() > 0) {
-      auto start_offset =
-          *std::min_element(sampler_offsets.begin(), sampler_offsets.end());
-      for (auto offset : sampler_offsets) {
-        sl_compiler->add_msl_resource_binding({
-            .stage = spv::ExecutionModel::ExecutionModelFragment,
-            .basetype = spirv_cross::SPIRType::BaseType::SampledImage,
-            .binding = offset,
-            .count = 1u,
-            // A sampled image is both an image and a sampler, so both
-            // offsets need to be set or depending on the partiular shader
-            // the bindings may be incorrect.
-            .msl_texture = offset - start_offset,
-            .msl_sampler = offset - start_offset,
-        });
-      }
-    }
-  }
-
   return CompilerBackend(sl_compiler);
 }
 

impeller/entity/contents/runtime_effect_contents.cc

@@ -5,6 +5,7 @@
 #include "impeller/entity/contents/runtime_effect_contents.h"
 
 #include <future>
+#include <iostream>
 #include <memory>
 
 #include "flutter/fml/logging.h"
@@ -151,30 +152,26 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer,
   /// Fragment stage uniforms.
   ///
 
+  size_t minimum_sampler_index = 100000000;
   size_t buffer_index = 0;
   size_t buffer_offset = 0;
-  size_t sampler_index = 0;
   for (auto uniform : runtime_stage_->GetUniforms()) {
     // TODO(113715): Populate this metadata once GLES is able to handle
     //               non-struct uniform names.
     ShaderMetadata metadata;
 
     switch (uniform.type) {
       case kSampledImage: {
-        FML_DCHECK(sampler_index < texture_inputs_.size());
-        auto& input = texture_inputs_[sampler_index];
-
-        auto sampler =
-            context->GetSamplerLibrary()->GetSampler(input.sampler_descriptor);
-
-        SampledImageSlot image_slot;
-        image_slot.name = uniform.name.c_str();
-        image_slot.texture_index = sampler_index;
-        image_slot.sampler_index = sampler_index;
-        cmd.BindResource(ShaderStage::kFragment, image_slot, metadata,
-                         input.texture, sampler);
-
-        sampler_index++;
+        // Sampler uniforms are ordered in the IPLR according to their
+        // declaration and the uniform location reflects the correct offset to
+        // be mapped to - except that it may include all proceeding float
+        // uniforms. For example, a float sampler that comes after 4 float
+        // uniforms may have a location of 4. To convert to the actual offset we
+        // need to find the largest location assigned to a float uniform and
+        // then subtract this from all uniform locations. This is more or less
+        // the same operation we previously performed in the shader compiler.
+        minimum_sampler_index =
+            std::min(minimum_sampler_index, uniform.location);
         break;
       }
       case kFloat: {
@@ -210,6 +207,36 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer,
     }
   }
 
+  size_t sampler_index = 0;
+  FML_DCHECK(minimum_sampler_index >= 0);
+  for (auto uniform : runtime_stage_->GetUniforms()) {
+    // TODO(113715): Populate this metadata once GLES is able to handle
+    //               non-struct uniform names.
+    ShaderMetadata metadata;
+
+    switch (uniform.type) {
+      case kSampledImage: {
+        FML_DCHECK(sampler_index < texture_inputs_.size());
+        auto& input = texture_inputs_[sampler_index];
+
+        auto sampler =
+            context->GetSamplerLibrary()->GetSampler(input.sampler_descriptor);
+
+        SampledImageSlot image_slot;
+        image_slot.name = uniform.name.c_str();
+        image_slot.texture_index = uniform.location - minimum_sampler_index;
+        image_slot.sampler_index = uniform.location - minimum_sampler_index;
+        cmd.BindResource(ShaderStage::kFragment, image_slot, metadata,
+                         input.texture, sampler);
+
+        sampler_index++;
+        break;
+      }
+      default:
+        continue;
+    }
+  }
+
   pass.AddCommand(std::move(cmd));
 
   if (geometry_result.prevent_overdraw) {

testing/dart/fragment_shader_test.dart

@@ -332,34 +332,6 @@ void main() async {
     shader.dispose();
   });
 
-  // This test can't rely on actual pixels rendered since it needs to run on a
-  // metal shader on iOS. instead parse the source code.
-  test('impellerc orders samplers in metal shader according to declaration and not usage', () async {
-    if (!Platform.isMacOS) {
-      return;
-    }
-    final Directory directory = shaderDirectory('iplr-remap');
-    final String data = readAsStringLossy(File(path.join(directory.path, 'shader_with_samplers.frag.iplr')));
-
-    const String expected = 'texture2d<float> textureA [[texture(0)]],'
-      ' texture2d<float> textureB [[texture(1)]]';
-
-    expect(data, contains(expected));
-  });
-
-  test('impellerc orders samplers in metal shader according to declaration and not usage in glow', () async {
-    if (!Platform.isMacOS) {
-      return;
-    }
-    final Directory directory = shaderDirectory('iplr-remap');
-    final String data = readAsStringLossy(File(path.join(directory.path, 'glow_shader.frag.iplr')));
-
-    const String expected = 'texture2d<float> tInput [[texture(0)]], texture2d<float> tNoise [[texture(1)]], '
-      'sampler tInputSmplr [[sampler(0)]], sampler tNoiseSmplr [[sampler(1)]]';
-
-    expect(data, contains(expected));
-  });
-
   // Test all supported GLSL ops. See lib/spirv/lib/src/constants.dart
   final Map<String, FragmentProgram> iplrSupportedGLSLOpShaders = await _loadShaderAssets(
     path.join('supported_glsl_op_shaders', 'iplr'),