Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] partially remove remap sampler support #39147

Merged
merged 2 commits into from
Jan 26, 2023
Merged

[Impeller] partially remove remap sampler support #39147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0f649de
partially remove remap sampler support
jonahwilliams Jan 26, 2023
980859e
Update runtime_effect_contents.cc
jonahwilliams Jan 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 0 additions & 38 deletions impeller/compiler/compiler.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
55 changes: 40 additions & 15 deletions impeller/entity/contents/runtime_effect_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,30 +151,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: {
Expand Down Expand Up @@ -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) {
Expand Down
28 changes: 0 additions & 28 deletions testing/dart/fragment_shader_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the test on line 275 sufficient? Does that test need to have a sampler or two thrown in?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't apply either way, its not running on impeller or metal?

// 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'),
Expand Down