diff --git a/impeller/compiler/compiler.cc b/impeller/compiler/compiler.cc index fdb9ebd8f9130..edb9fe69354c8 100644 --- a/impeller/compiler/compiler.cc +++ b/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 sampler_offsets; - ir.for_each_typed_id( - [&](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); } diff --git a/impeller/entity/contents/runtime_effect_contents.cc b/impeller/entity/contents/runtime_effect_contents.cc index 68c95acefe8d5..f36e0c9b21efa 100644 --- a/impeller/entity/contents/runtime_effect_contents.cc +++ b/impeller/entity/contents/runtime_effect_contents.cc @@ -151,9 +151,9 @@ 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. @@ -161,20 +161,16 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, 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 +206,35 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, } } + 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 = 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) { diff --git a/testing/dart/fragment_shader_test.dart b/testing/dart/fragment_shader_test.dart index 7ce3edbe389b6..e97eb5128f6e5 100644 --- a/testing/dart/fragment_shader_test.dart +++ b/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 textureA [[texture(0)]],' - ' texture2d 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 tInput [[texture(0)]], texture2d 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 iplrSupportedGLSLOpShaders = await _loadShaderAssets( path.join('supported_glsl_op_shaders', 'iplr'),