diff --git a/impeller/compiler/reflector.cc b/impeller/compiler/reflector.cc index e60613e0c5b26..d3ce90e396a39 100644 --- a/impeller/compiler/reflector.cc +++ b/impeller/compiler/reflector.cc @@ -376,6 +376,8 @@ std::shared_ptr Reflector::GenerateRuntimeStageData() const auto& ubo = ubos[0]; + size_t binding = + compiler_->get_decoration(ubo.id, spv::Decoration::DecorationBinding); auto members = ReadStructMembers(ubo.type_id); std::vector struct_layout; size_t float_count = 0; @@ -410,9 +412,8 @@ std::shared_ptr Reflector::GenerateRuntimeStageData() } data->uniforms.emplace_back(UniformDescription{ .name = ubo.name, - .location = 64, // Magic constant that must match the descriptor set - // location for fragment programs. - .binding = 64, + .location = binding, + .binding = binding, .type = spirv_cross::SPIRType::Struct, .struct_layout = std::move(struct_layout), .struct_float_count = float_count, diff --git a/impeller/fixtures/BUILD.gn b/impeller/fixtures/BUILD.gn index 367f1b70f8bf4..2253fad68b414 100644 --- a/impeller/fixtures/BUILD.gn +++ b/impeller/fixtures/BUILD.gn @@ -70,6 +70,8 @@ impellerc("runtime_stages") { "ink_sparkle.frag", "runtime_stage_example.frag", "gradient.frag", + "uniforms_and_sampler_1.frag", + "uniforms_and_sampler_2.frag", ] sl_file_extension = "iplr" diff --git a/impeller/fixtures/uniforms_and_sampler_1.frag b/impeller/fixtures/uniforms_and_sampler_1.frag new file mode 100644 index 0000000000000..196ff621764ed --- /dev/null +++ b/impeller/fixtures/uniforms_and_sampler_1.frag @@ -0,0 +1,13 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// sampler is specifically ordered before color. +uniform sampler2D u_texture; +uniform vec4 u_color; + +out vec4 frag_color; + +void main() { + frag_color = u_color + texture(u_texture, vec2(0.5, 0.5)); +} diff --git a/impeller/fixtures/uniforms_and_sampler_2.frag b/impeller/fixtures/uniforms_and_sampler_2.frag new file mode 100644 index 0000000000000..77706675594cf --- /dev/null +++ b/impeller/fixtures/uniforms_and_sampler_2.frag @@ -0,0 +1,13 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +uniform vec4 u_color; +// sampler is specifically ordered after color. +uniform sampler2D u_texture; + +out vec4 frag_color; + +void main() { + frag_color = u_color + texture(u_texture, vec2(0.5, 0.5)); +} diff --git a/impeller/runtime_stage/runtime_stage.cc b/impeller/runtime_stage/runtime_stage.cc index 9e6b3eb42cc22..bf46e805a93eb 100644 --- a/impeller/runtime_stage/runtime_stage.cc +++ b/impeller/runtime_stage/runtime_stage.cc @@ -89,6 +89,12 @@ RuntimeStage::RuntimeStage(const fb::RuntimeStage* runtime_stage, entrypoint_ = runtime_stage->entrypoint()->str(); auto* uniforms = runtime_stage->uniforms(); + + // Note: image bindings are screwy and will always have the same offset. + // track the binding of the UBO to determine where the image bindings go. + // This is only guaranteed to give us the correct bindings if there is a + // single sampler2D. + std::optional ubo_id; if (uniforms) { for (auto i = uniforms->begin(), end = uniforms->end(); i != end; i++) { RuntimeUniformDescription desc; @@ -96,6 +102,10 @@ RuntimeStage::RuntimeStage(const fb::RuntimeStage* runtime_stage, desc.location = i->location(); desc.binding = i->binding(); desc.type = ToType(i->type()); + if (desc.type == kStruct) { + ubo_id = desc.location; + desc.binding = desc.location; + } desc.dimensions = RuntimeUniformDimensions{ static_cast(i->rows()), static_cast(i->columns())}; desc.bit_width = i->bit_width(); @@ -105,7 +115,6 @@ RuntimeStage::RuntimeStage(const fb::RuntimeStage* runtime_stage, desc.struct_layout.push_back(static_cast(byte_type)); } } - desc.binding = i->binding(); desc.struct_float_count = i->struct_float_count(); uniforms_.push_back(std::move(desc)); } @@ -117,6 +126,20 @@ RuntimeStage::RuntimeStage(const fb::RuntimeStage* runtime_stage, [payload = payload_](auto, auto) {} // ); + size_t binding = 64; + if (ubo_id.has_value() && ubo_id.value() == binding) { + binding++; + } + for (auto& uniform : uniforms_) { + if (uniform.type == kSampledImage) { + uniform.binding = binding; + binding++; + if (ubo_id.has_value() && ubo_id.value() == binding) { + binding++; + } + } + } + for (const auto& uniform : GetUniforms()) { if (uniform.type == kStruct) { descriptor_set_layouts_.push_back(DescriptorSetLayout{ @@ -132,7 +155,6 @@ RuntimeStage::RuntimeStage(const fb::RuntimeStage* runtime_stage, }); } } - is_valid_ = true; } diff --git a/impeller/runtime_stage/runtime_stage_unittests.cc b/impeller/runtime_stage/runtime_stage_unittests.cc index 2dc646d3a640b..484fe457d16f3 100644 --- a/impeller/runtime_stage/runtime_stage_unittests.cc +++ b/impeller/runtime_stage/runtime_stage_unittests.cc @@ -7,6 +7,7 @@ #include "flutter/fml/make_copyable.h" #include "flutter/testing/testing.h" #include "gmock/gmock.h" +#include "gtest/gtest.h" #include "impeller/base/allocation.h" #include "impeller/base/validation.h" #include "impeller/core/runtime_types.h" @@ -225,6 +226,56 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { } } +TEST_P(RuntimeStageTest, CanReadUniformsSamplerBeforeUBO) { + if (GetBackend() != PlaygroundBackend::kVulkan) { + GTEST_SKIP() << "Test only relevant for Vulkan"; + } + const std::shared_ptr fixture = + flutter::testing::OpenFixtureAsMapping( + "uniforms_and_sampler_1.frag.iplr"); + ASSERT_TRUE(fixture); + ASSERT_GT(fixture->GetSize(), 0u); + auto stages = RuntimeStage::DecodeRuntimeStages(fixture); + auto stage = stages[PlaygroundBackendToRuntimeStageBackend(GetBackend())]; + + EXPECT_EQ(stage->GetUniforms().size(), 2u); + auto uni = stage->GetUniform(RuntimeStage::kVulkanUBOName); + ASSERT_TRUE(uni); + // Struct must be offset at 65. + EXPECT_EQ(uni->type, RuntimeUniformType::kStruct); + EXPECT_EQ(uni->binding, 65u); + // Sampler should be offset at 64 but due to current bug + // has offset of 0, the correct offset is computed at runtime. + auto sampler_uniform = stage->GetUniform("u_texture"); + EXPECT_EQ(sampler_uniform->type, RuntimeUniformType::kSampledImage); + EXPECT_EQ(sampler_uniform->binding, 64u); +} + +TEST_P(RuntimeStageTest, CanReadUniformsSamplerAfterUBO) { + if (GetBackend() != PlaygroundBackend::kVulkan) { + GTEST_SKIP() << "Test only relevant for Vulkan"; + } + const std::shared_ptr fixture = + flutter::testing::OpenFixtureAsMapping( + "uniforms_and_sampler_2.frag.iplr"); + ASSERT_TRUE(fixture); + ASSERT_GT(fixture->GetSize(), 0u); + auto stages = RuntimeStage::DecodeRuntimeStages(fixture); + auto stage = stages[PlaygroundBackendToRuntimeStageBackend(GetBackend())]; + + EXPECT_EQ(stage->GetUniforms().size(), 2u); + auto uni = stage->GetUniform(RuntimeStage::kVulkanUBOName); + ASSERT_TRUE(uni); + // Struct must be offset at 45. + EXPECT_EQ(uni->type, RuntimeUniformType::kStruct); + EXPECT_EQ(uni->binding, 64u); + // Sampler should be offset at 64 but due to current bug + // has offset of 0, the correct offset is computed at runtime. + auto sampler_uniform = stage->GetUniform("u_texture"); + EXPECT_EQ(sampler_uniform->type, RuntimeUniformType::kSampledImage); + EXPECT_EQ(sampler_uniform->binding, 65u); +} + TEST_P(RuntimeStageTest, CanRegisterStage) { const std::shared_ptr fixture = flutter::testing::OpenFixtureAsMapping("ink_sparkle.frag.iplr");