diff --git a/engine/src/flutter/impeller/compiler/runtime_stage_data.cc b/engine/src/flutter/impeller/compiler/runtime_stage_data.cc index c7b28a6184f..b745ec94673 100644 --- a/engine/src/flutter/impeller/compiler/runtime_stage_data.cc +++ b/engine/src/flutter/impeller/compiler/runtime_stage_data.cc @@ -181,6 +181,7 @@ static std::optional ToJsonType( FML_UNREACHABLE(); } +static const char* kFormatVersionKey = "format_version"; static const char* kStageKey = "stage"; static const char* kTargetPlatformKey = "target_platform"; static const char* kEntrypointKey = "entrypoint"; @@ -212,6 +213,7 @@ static std::string RuntimeStageBackendToString(RuntimeStageBackend backend) { std::shared_ptr RuntimeStageData::CreateJsonMapping() const { // Runtime Stage Data JSON format // { + // "format_version": 1, // "sksl": { // "stage": 0, // "entrypoint": "", @@ -232,6 +234,8 @@ std::shared_ptr RuntimeStageData::CreateJsonMapping() const { // }, nlohmann::json root; + root[kFormatVersionKey] = + static_cast(fb::RuntimeStagesFormatVersion::kVersion); for (const auto& kvp : data_) { nlohmann::json platform_object; @@ -370,6 +374,8 @@ RuntimeStageData::CreateMultiStageFlatbuffer() const { // The high level object API is used here for writing to the buffer. This is // just a convenience. auto runtime_stages = std::make_unique(); + runtime_stages->format_version = + static_cast(fb::RuntimeStagesFormatVersion::kVersion); for (const auto& kvp : data_) { auto runtime_stage = CreateStageFlatbuffer(kvp.first); diff --git a/engine/src/flutter/impeller/compiler/shader_bundle.cc b/engine/src/flutter/impeller/compiler/shader_bundle.cc index 155029148cc..d178e998178 100644 --- a/engine/src/flutter/impeller/compiler/shader_bundle.cc +++ b/engine/src/flutter/impeller/compiler/shader_bundle.cc @@ -194,6 +194,8 @@ std::optional GenerateShaderBundleFlatbuffer( /// fb::shaderbundle::ShaderBundleT shader_bundle; + shader_bundle.format_version = static_cast( + fb::shaderbundle::ShaderBundleFormatVersion::kVersion); for (const auto& [shader_name, shader_config] : bundle_config.value()) { std::unique_ptr shader = diff --git a/engine/src/flutter/impeller/golden_tests/golden_playground_test_mac.cc b/engine/src/flutter/impeller/golden_tests/golden_playground_test_mac.cc index ae7dae00c15..d8e43e74bd2 100644 --- a/engine/src/flutter/impeller/golden_tests/golden_playground_test_mac.cc +++ b/engine/src/flutter/impeller/golden_tests/golden_playground_test_mac.cc @@ -281,7 +281,11 @@ RuntimeStage::Map GoldenPlaygroundTest::OpenAssetAsRuntimeStage( if (!fixture || fixture->GetSize() == 0) { return {}; } - return RuntimeStage::DecodeRuntimeStages(fixture); + auto stages = RuntimeStage::DecodeRuntimeStages(fixture); + if (!stages.ok()) { + return {}; + } + return stages.value(); } std::shared_ptr GoldenPlaygroundTest::GetContext() const { diff --git a/engine/src/flutter/impeller/playground/playground_test.cc b/engine/src/flutter/impeller/playground/playground_test.cc index e6f9f491d61..8b5be9c8608 100644 --- a/engine/src/flutter/impeller/playground/playground_test.cc +++ b/engine/src/flutter/impeller/playground/playground_test.cc @@ -93,7 +93,11 @@ RuntimeStage::Map PlaygroundTest::OpenAssetAsRuntimeStage( if (!fixture || fixture->GetSize() == 0) { return {}; } - return RuntimeStage::DecodeRuntimeStages(fixture); + auto stages = RuntimeStage::DecodeRuntimeStages(fixture); + if (!stages.ok()) { + return {}; + } + return stages.value(); } // |Playground| diff --git a/engine/src/flutter/impeller/renderer/backend/gles/shader_library_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/shader_library_gles.cc index 0d52f6c176b..876341e5d6e 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/shader_library_gles.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/shader_library_gles.cc @@ -67,12 +67,12 @@ ShaderLibraryGLES::ShaderLibraryGLES( return true; }; for (auto library : shader_libraries) { - auto blob_library = ShaderArchive{std::move(library)}; - if (!blob_library.IsValid()) { + auto blob_library = ShaderArchive::Create(std::move(library)); + if (!blob_library.ok()) { VALIDATION_LOG << "Could not construct blob library for shaders."; return; } - blob_library.IterateAllShaders(iterator); + blob_library->IterateAllShaders(iterator); } functions_ = functions; diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/shader_library_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/shader_library_vk.cc index 3748328b3ea..2782c65ad1b 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/shader_library_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/shader_library_vk.cc @@ -67,12 +67,12 @@ ShaderLibraryVK::ShaderLibraryVK( return true; }; for (const auto& library_data : shader_libraries_data) { - auto blob_library = ShaderArchive{library_data}; - if (!blob_library.IsValid()) { + auto blob_library = ShaderArchive::Create(library_data); + if (!blob_library.ok()) { VALIDATION_LOG << "Could not construct shader blob library."; return; } - blob_library.IterateAllShaders(iterator); + blob_library->IterateAllShaders(iterator); } if (!success) { diff --git a/engine/src/flutter/impeller/runtime_stage/BUILD.gn b/engine/src/flutter/impeller/runtime_stage/BUILD.gn index 8cc4f3b0a22..ff059a55abd 100644 --- a/engine/src/flutter/impeller/runtime_stage/BUILD.gn +++ b/engine/src/flutter/impeller/runtime_stage/BUILD.gn @@ -38,6 +38,7 @@ impeller_component("runtime_stage") { "../base", "../core", "//flutter/fml", + "//third_party/abseil-cpp/absl/status:statusor", ] } diff --git a/engine/src/flutter/impeller/runtime_stage/runtime_stage.cc b/engine/src/flutter/impeller/runtime_stage/runtime_stage.cc index fb98148dfea..598f24631f7 100644 --- a/engine/src/flutter/impeller/runtime_stage/runtime_stage.cc +++ b/engine/src/flutter/impeller/runtime_stage/runtime_stage.cc @@ -6,6 +6,7 @@ #include #include +#include #include "fml/mapping.h" #include "impeller/base/validation.h" @@ -59,17 +60,32 @@ std::unique_ptr RuntimeStage::RuntimeStageIfPresent( new RuntimeStage(runtime_stage, payload)); } -RuntimeStage::Map RuntimeStage::DecodeRuntimeStages( +absl::StatusOr RuntimeStage::DecodeRuntimeStages( const std::shared_ptr& payload) { if (payload == nullptr || !payload->GetMapping()) { - return {}; + return absl::InvalidArgumentError("Payload is null or empty."); } if (!fb::RuntimeStagesBufferHasIdentifier(payload->GetMapping())) { - return {}; + return absl::InvalidArgumentError( + "Payload does not have valid identifier."); } auto raw_stages = fb::GetRuntimeStages(payload->GetMapping()); - return { + if (!raw_stages) { + return absl::InvalidArgumentError("Failed to get runtime stages."); + } + + const uint32_t version = raw_stages->format_version(); + const auto expected = + static_cast(fb::RuntimeStagesFormatVersion::kVersion); + if (version != expected) { + std::stringstream stream; + stream << "Unsupported runtime stages format version. Expected " << expected + << ", got " << version << "."; + return absl::InvalidArgumentError(stream.str()); + } + + return Map{ {RuntimeStageBackend::kSkSL, RuntimeStageIfPresent(raw_stages->sksl(), payload)}, {RuntimeStageBackend::kMetal, diff --git a/engine/src/flutter/impeller/runtime_stage/runtime_stage.h b/engine/src/flutter/impeller/runtime_stage/runtime_stage.h index 86af3111f84..e8b4d7e090b 100644 --- a/engine/src/flutter/impeller/runtime_stage/runtime_stage.h +++ b/engine/src/flutter/impeller/runtime_stage/runtime_stage.h @@ -14,6 +14,7 @@ #include "flutter/impeller/core/runtime_types.h" #include "impeller/core/shader_types.h" #include "runtime_stage_types_flatbuffers.h" +#include "third_party/abseil-cpp/absl/status/statusor.h" namespace impeller { @@ -22,7 +23,8 @@ class RuntimeStage { static const char* kVulkanUBOName; using Map = std::map>; - static Map DecodeRuntimeStages(const std::shared_ptr& payload); + static absl::StatusOr DecodeRuntimeStages( + const std::shared_ptr& payload); RuntimeStage(const fb::RuntimeStage* runtime_stage, const std::shared_ptr& payload); diff --git a/engine/src/flutter/impeller/runtime_stage/runtime_stage_types.fbs b/engine/src/flutter/impeller/runtime_stage/runtime_stage_types.fbs index dd9f7f5a66d..941184b7d6c 100644 --- a/engine/src/flutter/impeller/runtime_stage/runtime_stage_types.fbs +++ b/engine/src/flutter/impeller/runtime_stage/runtime_stage_types.fbs @@ -4,6 +4,13 @@ namespace impeller.fb; +// This is the runtime stage format version number - it's value should be +// incremented any time there is a change to this file which changes the +// resulting flat buffer format. +enum RuntimeStagesFormatVersion:uint32 { + kVersion = 1 +} + enum Stage:byte { kVertex, kFragment, @@ -12,7 +19,7 @@ enum Stage:byte { // The subset of impeller::ShaderType that may be used for uniform bindings. // kStruct is only supported for Impeller Vulkan. -// kFloat encompases multiple float-based types e.g. vec2. +// kFloat encompasses multiple float-based types e.g. vec2. enum UniformDataType:uint32 { kFloat, kSampledImage, @@ -85,4 +92,5 @@ table RuntimeStages { opengles: RuntimeStage; opengles3: RuntimeStage; vulkan: RuntimeStage; + format_version: uint32; // This is intended to hold RuntimeStagesFormatVersion.kVersion. } diff --git a/engine/src/flutter/impeller/runtime_stage/runtime_stage_unittests.cc b/engine/src/flutter/impeller/runtime_stage/runtime_stage_unittests.cc index 8d6e466e04e..c26d8034cbc 100644 --- a/engine/src/flutter/impeller/runtime_stage/runtime_stage_unittests.cc +++ b/engine/src/flutter/impeller/runtime_stage/runtime_stage_unittests.cc @@ -18,7 +18,9 @@ #include "impeller/renderer/pipeline_library.h" #include "impeller/renderer/shader_library.h" #include "impeller/runtime_stage/runtime_stage.h" +#include "impeller/runtime_stage/runtime_stage_flatbuffers.h" #include "impeller/runtime_stage/runtime_stage_playground.h" +#include "runtime_stage_types_flatbuffers.h" namespace impeller { namespace testing { @@ -32,11 +34,26 @@ TEST_P(RuntimeStageTest, CanReadValidBlob) { ASSERT_TRUE(fixture); ASSERT_GT(fixture->GetSize(), 0u); auto stages = RuntimeStage::DecodeRuntimeStages(fixture); - auto stage = stages[PlaygroundBackendToRuntimeStageBackend(GetBackend())]; + ASSERT_TRUE(stages.ok()); + auto stage = + stages.value()[PlaygroundBackendToRuntimeStageBackend(GetBackend())]; ASSERT_TRUE(stage->IsValid()); ASSERT_EQ(stage->GetShaderStage(), RuntimeShaderStage::kFragment); } +TEST_P(RuntimeStageTest, RejectInvalidFormatVersion) { + flatbuffers::FlatBufferBuilder builder; + fb::RuntimeStagesBuilder stages_builder(builder); + stages_builder.add_format_version(0); + auto stages = stages_builder.Finish(); + builder.Finish(stages, fb::RuntimeStagesIdentifier()); + auto mapping = std::make_shared( + builder.GetBufferPointer(), builder.GetSize()); + auto runtime_stages = RuntimeStage::DecodeRuntimeStages(mapping); + EXPECT_FALSE(runtime_stages.ok()); + EXPECT_EQ(runtime_stages.status().code(), absl::StatusCode::kInvalidArgument); +} + TEST_P(RuntimeStageTest, CanRejectInvalidBlob) { ScopedValidationDisable disable_validation; const std::shared_ptr fixture = @@ -50,7 +67,7 @@ TEST_P(RuntimeStageTest, CanRejectInvalidBlob) { junk_allocation->GetLength().GetByteSize()); auto stages = RuntimeStage::DecodeRuntimeStages( CreateMappingFromAllocation(junk_allocation)); - ASSERT_FALSE(stages[PlaygroundBackendToRuntimeStageBackend(GetBackend())]); + ASSERT_FALSE(stages.ok()); } TEST_P(RuntimeStageTest, CanReadUniforms) { @@ -59,7 +76,9 @@ TEST_P(RuntimeStageTest, CanReadUniforms) { ASSERT_TRUE(fixture); ASSERT_GT(fixture->GetSize(), 0u); auto stages = RuntimeStage::DecodeRuntimeStages(fixture); - auto stage = stages[PlaygroundBackendToRuntimeStageBackend(GetBackend())]; + ASSERT_TRUE(stages.ok()); + auto stage = + stages.value()[PlaygroundBackendToRuntimeStageBackend(GetBackend())]; ASSERT_TRUE(stage->IsValid()); switch (GetBackend()) { @@ -237,7 +256,9 @@ TEST_P(RuntimeStageTest, CanReadUniformsSamplerBeforeUBO) { ASSERT_TRUE(fixture); ASSERT_GT(fixture->GetSize(), 0u); auto stages = RuntimeStage::DecodeRuntimeStages(fixture); - auto stage = stages[PlaygroundBackendToRuntimeStageBackend(GetBackend())]; + ASSERT_TRUE(stages.ok()); + auto stage = + stages.value()[PlaygroundBackendToRuntimeStageBackend(GetBackend())]; EXPECT_EQ(stage->GetUniforms().size(), 2u); auto uni = stage->GetUniform(RuntimeStage::kVulkanUBOName); @@ -262,7 +283,9 @@ TEST_P(RuntimeStageTest, CanReadUniformsSamplerAfterUBO) { ASSERT_TRUE(fixture); ASSERT_GT(fixture->GetSize(), 0u); auto stages = RuntimeStage::DecodeRuntimeStages(fixture); - auto stage = stages[PlaygroundBackendToRuntimeStageBackend(GetBackend())]; + ASSERT_TRUE(stages.ok()); + auto stage = + stages.value()[PlaygroundBackendToRuntimeStageBackend(GetBackend())]; EXPECT_EQ(stage->GetUniforms().size(), 2u); auto uni = stage->GetUniform(RuntimeStage::kVulkanUBOName); @@ -283,7 +306,9 @@ TEST_P(RuntimeStageTest, CanRegisterStage) { ASSERT_TRUE(fixture); ASSERT_GT(fixture->GetSize(), 0u); auto stages = RuntimeStage::DecodeRuntimeStages(fixture); - auto stage = stages[PlaygroundBackendToRuntimeStageBackend(GetBackend())]; + ASSERT_TRUE(stages.ok()); + auto stage = + stages.value()[PlaygroundBackendToRuntimeStageBackend(GetBackend())]; ASSERT_TRUE(stage->IsValid()); std::promise registration; auto future = registration.get_future(); diff --git a/engine/src/flutter/impeller/shader_archive/BUILD.gn b/engine/src/flutter/impeller/shader_archive/BUILD.gn index 7e749b93722..dc23d67e6d0 100644 --- a/engine/src/flutter/impeller/shader_archive/BUILD.gn +++ b/engine/src/flutter/impeller/shader_archive/BUILD.gn @@ -29,6 +29,7 @@ impeller_component("shader_archive") { ":shader_archive_flatbuffers", "../base", "//flutter/fml", + "//third_party/abseil-cpp/absl/status:statusor", ] } diff --git a/engine/src/flutter/impeller/shader_archive/shader_archive.cc b/engine/src/flutter/impeller/shader_archive/shader_archive.cc index 4cf2d3e1b56..7c8ee3bcd57 100644 --- a/engine/src/flutter/impeller/shader_archive/shader_archive.cc +++ b/engine/src/flutter/impeller/shader_archive/shader_archive.cc @@ -5,6 +5,7 @@ #include "impeller/shader_archive/shader_archive.h" #include +#include #include #include @@ -25,48 +26,56 @@ constexpr ArchiveShaderType ToShaderType(fb::Stage stage) { FML_UNREACHABLE(); } -ShaderArchive::ShaderArchive(std::shared_ptr payload) - : payload_(std::move(payload)) { - if (!payload_ || payload_->GetMapping() == nullptr) { - VALIDATION_LOG << "Shader mapping was absent."; - return; +absl::StatusOr ShaderArchive::Create( + std::shared_ptr payload) { + if (!payload || payload->GetMapping() == nullptr) { + return absl::InvalidArgumentError("Shader mapping was absent."); } - if (!fb::ShaderArchiveBufferHasIdentifier(payload_->GetMapping())) { - VALIDATION_LOG << "Invalid shader magic."; - return; + if (!fb::ShaderArchiveBufferHasIdentifier(payload->GetMapping())) { + return absl::InvalidArgumentError("Invalid shader magic."); } - auto shader_archive = fb::GetShaderArchive(payload_->GetMapping()); + auto shader_archive = fb::GetShaderArchive(payload->GetMapping()); if (!shader_archive) { - return; + return absl::InvalidArgumentError("Could not read shader archive."); } + const auto version = shader_archive->format_version(); + const auto expected = + static_cast(fb::ShaderArchiveFormatVersion::kVersion); + if (version != expected) { + std::stringstream stream; + stream << "Unsupported shader archive format version. Expected: " + << expected << ", Got: " << version; + return absl::InvalidArgumentError(stream.str()); + } + + Shaders shaders; if (auto items = shader_archive->items()) { for (auto i = items->begin(), end = items->end(); i != end; i++) { ShaderKey key; key.name = i->name()->str(); key.type = ToShaderType(i->stage()); - shaders_[key] = std::make_shared( - i->mapping()->Data(), i->mapping()->size(), - [payload = payload_](auto, auto) { + shaders[key] = std::make_shared( + i->mapping()->Data(), i->mapping()->size(), [payload](auto, auto) { // The pointers are into the base payload. Instead of copying the // data, just hold onto the payload. }); } } - is_valid_ = true; + return ShaderArchive(std::move(payload), std::move(shaders)); } +ShaderArchive::ShaderArchive(std::shared_ptr payload, + Shaders shaders) + : payload_(std::move(payload)), shaders_(std::move(shaders)) {} + ShaderArchive::ShaderArchive(ShaderArchive&&) = default; ShaderArchive::~ShaderArchive() = default; -bool ShaderArchive::IsValid() const { - return is_valid_; -} - size_t ShaderArchive::GetShaderCount() const { return shaders_.size(); } @@ -86,7 +95,7 @@ size_t ShaderArchive::IterateAllShaders( const std::string& name, const std::shared_ptr& mapping)>& callback) const { - if (!IsValid() || !callback) { + if (!callback) { return 0u; } size_t count = 0u; diff --git a/engine/src/flutter/impeller/shader_archive/shader_archive.fbs b/engine/src/flutter/impeller/shader_archive/shader_archive.fbs index 0077f54a353..d8ac2d1d34f 100644 --- a/engine/src/flutter/impeller/shader_archive/shader_archive.fbs +++ b/engine/src/flutter/impeller/shader_archive/shader_archive.fbs @@ -4,6 +4,13 @@ namespace impeller.fb; +// This is the shader archive format version number - it's value should be +// incremented any time there is a change to this file which changes the +// resulting flat buffer format. +enum ShaderArchiveFormatVersion:uint32 { + kVersion = 1 +} + enum Stage:byte { kVertex, kFragment, @@ -18,6 +25,7 @@ table ShaderBlob { table ShaderArchive { items: [ShaderBlob]; + format_version: uint32; // This is intended to hold ShaderArchiveFormatVersion.kVersion. } root_type ShaderArchive; diff --git a/engine/src/flutter/impeller/shader_archive/shader_archive.h b/engine/src/flutter/impeller/shader_archive/shader_archive.h index dc9c243785c..1fcae5cc04f 100644 --- a/engine/src/flutter/impeller/shader_archive/shader_archive.h +++ b/engine/src/flutter/impeller/shader_archive/shader_archive.h @@ -12,19 +12,19 @@ #include "flutter/fml/hash_combine.h" #include "flutter/fml/mapping.h" #include "impeller/shader_archive/shader_archive_types.h" +#include "third_party/abseil-cpp/absl/status/statusor.h" namespace impeller { class ShaderArchive { public: - explicit ShaderArchive(std::shared_ptr payload); + static absl::StatusOr Create( + std::shared_ptr payload); ShaderArchive(ShaderArchive&&); ~ShaderArchive(); - bool IsValid() const; - size_t GetShaderCount() const; std::shared_ptr GetMapping(ArchiveShaderType type, @@ -37,6 +37,8 @@ class ShaderArchive { const; private: + explicit ShaderArchive(std::shared_ptr payload); + struct ShaderKey { ArchiveShaderType type = ArchiveShaderType::kFragment; std::string name; @@ -63,7 +65,8 @@ class ShaderArchive { std::shared_ptr payload_; Shaders shaders_; - bool is_valid_ = false; + + ShaderArchive(std::shared_ptr payload, Shaders shaders); ShaderArchive(const ShaderArchive&) = delete; diff --git a/engine/src/flutter/impeller/shader_archive/shader_archive_unittests.cc b/engine/src/flutter/impeller/shader_archive/shader_archive_unittests.cc index ba4a3072c91..eeaada19744 100644 --- a/engine/src/flutter/impeller/shader_archive/shader_archive_unittests.cc +++ b/engine/src/flutter/impeller/shader_archive/shader_archive_unittests.cc @@ -2,11 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include #include #include "flutter/fml/mapping.h" #include "flutter/testing/testing.h" #include "impeller/shader_archive/shader_archive.h" +#include "impeller/shader_archive/shader_archive_flatbuffers.h" #include "impeller/shader_archive/shader_archive_writer.h" namespace impeller { @@ -41,17 +43,39 @@ TEST(ShaderArchiveTest, CanReadAndWriteBlobs) { auto mapping = writer.CreateMapping(); ASSERT_NE(mapping, nullptr); - ShaderArchive library(mapping); - ASSERT_TRUE(library.IsValid()); - ASSERT_EQ(library.GetShaderCount(), 5u); + auto library = ShaderArchive::Create(mapping); + ASSERT_TRUE(library.ok()); + ASSERT_EQ(library->GetShaderCount(), 5u); // Wrong type. - ASSERT_EQ(library.GetMapping(ArchiveShaderType::kFragment, "Hello"), nullptr); + ASSERT_EQ(library->GetMapping(ArchiveShaderType::kFragment, "Hello"), + nullptr); - auto hello_vtx = library.GetMapping(ArchiveShaderType::kVertex, "Hello"); + auto hello_vtx = library->GetMapping(ArchiveShaderType::kVertex, "Hello"); ASSERT_NE(hello_vtx, nullptr); ASSERT_EQ(CreateStringFromMapping(*hello_vtx), "World"); } +TEST(ShaderArchiveTest, ReturnsErrorOnInvalidVersion) { + fb::ShaderArchiveT shader_archive; + shader_archive.format_version = -1; + + auto builder = std::make_shared(); + builder->Finish(fb::ShaderArchive::Pack(*builder.get(), &shader_archive), + fb::ShaderArchiveIdentifier()); + auto mapping = std::make_shared( + builder->GetBufferPointer(), builder->GetSize(), + [builder](auto, auto) {}); + + auto library = ShaderArchive::Create(mapping); + ASSERT_FALSE(library.ok()); + ASSERT_EQ(library.status().code(), absl::StatusCode::kInvalidArgument); + std::stringstream stream; + stream << "Unsupported shader archive format version. Expected: " + << static_cast(fb::ShaderArchiveFormatVersion::kVersion) + << ", Got: 4294967295"; + ASSERT_EQ(library.status().message(), stream.str()); +} + } // namespace testing } // namespace impeller diff --git a/engine/src/flutter/impeller/shader_archive/shader_archive_writer.cc b/engine/src/flutter/impeller/shader_archive/shader_archive_writer.cc index 070c5c23d76..95482877d35 100644 --- a/engine/src/flutter/impeller/shader_archive/shader_archive_writer.cc +++ b/engine/src/flutter/impeller/shader_archive/shader_archive_writer.cc @@ -106,6 +106,8 @@ constexpr fb::Stage ToStage(ArchiveShaderType type) { std::shared_ptr ShaderArchiveWriter::CreateMapping() const { fb::ShaderArchiveT shader_archive; + shader_archive.format_version = + static_cast(fb::ShaderArchiveFormatVersion::kVersion); for (const auto& shader_description : shader_descriptions_) { auto mapping = shader_description.mapping; if (!mapping) { diff --git a/engine/src/flutter/impeller/shader_bundle/shader_bundle.fbs b/engine/src/flutter/impeller/shader_bundle/shader_bundle.fbs index 490312223d5..e63876dc76a 100644 --- a/engine/src/flutter/impeller/shader_bundle/shader_bundle.fbs +++ b/engine/src/flutter/impeller/shader_bundle/shader_bundle.fbs @@ -4,6 +4,13 @@ namespace impeller.fb.shaderbundle; +// This is the shader bundle format version number - it's value should be +// incremented any time there is a change to this file which changes the +// resulting flat buffer format. +enum ShaderBundleFormatVersion:uint32 { + kVersion = 1 +} + enum ShaderStage:byte { kVertex, kFragment, @@ -102,6 +109,7 @@ table Shader { table ShaderBundle { shaders: [Shader]; + format_version: uint32; // This is intended to hold ShaderBundleFormatVersion.kVersion. } root_type ShaderBundle; diff --git a/engine/src/flutter/impeller/toolkit/interop/fragment_program.cc b/engine/src/flutter/impeller/toolkit/interop/fragment_program.cc index 593a84c9f99..bdc8c1c7f5f 100644 --- a/engine/src/flutter/impeller/toolkit/interop/fragment_program.cc +++ b/engine/src/flutter/impeller/toolkit/interop/fragment_program.cc @@ -15,8 +15,12 @@ FragmentProgram::FragmentProgram(const std::shared_ptr& data) { } auto stages = RuntimeStage::DecodeRuntimeStages(data); + if (!stages.ok()) { + VALIDATION_LOG << "Failed to decode runtime stages: " << stages.status(); + return; + } - for (const auto& stage : stages) { + for (const auto& stage : stages.value()) { if (auto data = stage.second) { stages_[stage.first] = std::move(data); } diff --git a/engine/src/flutter/lib/gpu/shader_library.cc b/engine/src/flutter/lib/gpu/shader_library.cc index b3fa5ec9969..2387bdaada9 100644 --- a/engine/src/flutter/lib/gpu/shader_library.cc +++ b/engine/src/flutter/lib/gpu/shader_library.cc @@ -185,6 +185,15 @@ fml::RefPtr ShaderLibrary::MakeFromFlatbuffer( return nullptr; } + const auto version = bundle->format_version(); + const auto expected = static_cast( + impeller::fb::shaderbundle::ShaderBundleFormatVersion::kVersion); + if (version != expected) { + VALIDATION_LOG << "Unsupported shader bundle format version: " << version + << ", expected: " << expected; + return nullptr; + } + ShaderLibrary::ShaderMap shader_map; for (const auto* bundled_shader : *bundle->shaders()) { diff --git a/engine/src/flutter/lib/ui/painting/fragment_program.cc b/engine/src/flutter/lib/ui/painting/fragment_program.cc index 379c33a49b8..ae705901f8f 100644 --- a/engine/src/flutter/lib/ui/painting/fragment_program.cc +++ b/engine/src/flutter/lib/ui/painting/fragment_program.cc @@ -112,7 +112,13 @@ std::string FragmentProgram::initFromAsset(const std::string& asset_name) { auto runtime_stages = impeller::RuntimeStage::DecodeRuntimeStages(std::move(data)); - if (runtime_stages.empty()) { + if (!runtime_stages.ok()) { + return std::string("Asset '") + asset_name + + std::string("' manifest could not be decoded: ") + + runtime_stages.status().ToString(); + } + + if (runtime_stages->empty()) { return std::string("Asset '") + asset_name + std::string("' does not contain any shader data."); } @@ -120,7 +126,7 @@ std::string FragmentProgram::initFromAsset(const std::string& asset_name) { impeller::RuntimeStageBackend backend = ui_dart_state->GetRuntimeStageBackend(); std::shared_ptr runtime_stage = - runtime_stages[backend]; + (*runtime_stages)[backend]; if (!runtime_stage) { std::ostringstream stream; stream << "Asset '" << asset_name @@ -128,7 +134,7 @@ std::string FragmentProgram::initFromAsset(const std::string& asset_name) { "backend (" << RuntimeStageBackendToString(backend) << ")." << std::endl << "Found stages: "; - for (const auto& kvp : runtime_stages) { + for (const auto& kvp : *runtime_stages) { if (kvp.second) { stream << RuntimeStageBackendToString(kvp.first) << " "; } diff --git a/engine/src/flutter/testing/dart/fragment_shader_test.dart b/engine/src/flutter/testing/dart/fragment_shader_test.dart index 11b3a1043c4..fabaeaee01c 100644 --- a/engine/src/flutter/testing/dart/fragment_shader_test.dart +++ b/engine/src/flutter/testing/dart/fragment_shader_test.dart @@ -25,7 +25,7 @@ void main() async { expect(rawData is Map, true); final data = rawData! as Map; - expect(data.keys.toList(), ['sksl']); + expect(data.keys.toList(), ['format_version', 'sksl']); expect(data['sksl'] is Map, true); final skslData = data['sksl']! as Map;