From 04a60d6b27fdb7eb4d6e6d647887ab553bf4c781 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 8 Jan 2025 12:23:57 -0800 Subject: [PATCH] [Impeller] reland: fix porterduff shader and handle optimized out texture binding in GLES backend. (#161326) Reland and forward fix for current tree breakage. GLES may optimize out the shader binding in a way that behaves differently from Metal/Vulkan. Don't treat this as an error. This happens because certain combinations of porter duff constants result in a 0 * texture lookup. --- .../entity/contents/atlas_contents.cc | 15 +- .../entity/contents/content_context.cc | 57 ++++++- .../entity/contents/content_context.h | 139 +++++++++++++++++- .../contents/filters/blend_filter_contents.cc | 10 +- .../entity/contents/vertices_contents.cc | 15 +- .../shaders/blending/porter_duff_blend.frag | 20 +-- .../backend/gles/buffer_bindings_gles.cc | 3 +- engine/src/flutter/impeller/tools/malioc.json | 12 +- 8 files changed, 217 insertions(+), 54 deletions(-) diff --git a/engine/src/flutter/impeller/entity/contents/atlas_contents.cc b/engine/src/flutter/impeller/entity/contents/atlas_contents.cc index d23c63d933..d0bd6e9625 100644 --- a/engine/src/flutter/impeller/entity/contents/atlas_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/atlas_contents.cc @@ -97,8 +97,10 @@ bool AtlasContents::Render(const ContentContext& renderer, pass.SetCommandLabel("DrawAtlas Blend"); #endif // IMPELLER_DEBUG pass.SetVertexBuffer(geometry_->CreateBlendVertexBuffer(host_buffer)); - pass.SetPipeline( - renderer.GetPorterDuffBlendPipeline(OptionsFromPass(pass))); + auto inverted_blend_mode = + InvertPorterDuffBlend(blend_mode).value_or(BlendMode::kSource); + pass.SetPipeline(renderer.GetPorterDuffPipeline(inverted_blend_mode, + OptionsFromPass(pass))); FS::FragInfo frag_info; VS::FrameInfo frame_info; @@ -110,15 +112,6 @@ bool AtlasContents::Render(const ContentContext& renderer, frag_info.output_alpha = alpha_; frag_info.input_alpha = 1.0; - auto inverted_blend_mode = - InvertPorterDuffBlend(blend_mode).value_or(BlendMode::kSource); - auto blend_coefficients = - kPorterDuffCoefficients[static_cast(inverted_blend_mode)]; - frag_info.src_coeff = blend_coefficients[0]; - frag_info.src_coeff_dst_alpha = blend_coefficients[1]; - frag_info.dst_coeff = blend_coefficients[2]; - frag_info.dst_coeff_src_alpha = blend_coefficients[3]; - frag_info.dst_coeff_src_color = blend_coefficients[4]; // These values are ignored on platforms that natively support decal. frag_info.tmx = static_cast(Entity::TileMode::kDecal); frag_info.tmy = static_cast(Entity::TileMode::kDecal); diff --git a/engine/src/flutter/impeller/entity/contents/content_context.cc b/engine/src/flutter/impeller/entity/contents/content_context.cc index 149c544a89..7cf890768a 100644 --- a/engine/src/flutter/impeller/entity/contents/content_context.cc +++ b/engine/src/flutter/impeller/entity/contents/content_context.cc @@ -220,6 +220,28 @@ void ContentContextOptions::ApplyToPipelineDescriptor( desc.SetPolygonMode(wireframe ? PolygonMode::kLine : PolygonMode::kFill); } +std::array, 15> GetPorterDuffSpecConstants( + bool supports_decal) { + Scalar x = supports_decal ? 1 : 0; + return {{ + {x, 0, 0, 0, 0, 0}, // Clear + {x, 1, 0, 0, 0, 0}, // Source + {x, 0, 0, 1, 0, 0}, // Destination + {x, 1, 0, 1, -1, 0}, // SourceOver + {x, 1, -1, 1, 0, 0}, // DestinationOver + {x, 0, 1, 0, 0, 0}, // SourceIn + {x, 0, 0, 0, 1, 0}, // DestinationIn + {x, 1, -1, 0, 0, 0}, // SourceOut + {x, 0, 0, 1, -1, 0}, // DestinationOut + {x, 0, 1, 1, -1, 0}, // SourceATop + {x, 1, -1, 0, 1, 0}, // DestinationATop + {x, 1, -1, 1, -1, 0}, // Xor + {x, 1, 0, 1, 0, 0}, // Plus + {x, 0, 0, 0, 0, 1}, // Modulate + {x, 0, 0, 1, 0, -1}, // Screen + }}; +} + template static std::unique_ptr CreateDefaultPipeline( const Context& context) { @@ -352,9 +374,40 @@ ContentContext::ContentContext( border_mask_blur_pipelines_.CreateDefault(*context_, options_trianglestrip); color_matrix_color_filter_pipelines_.CreateDefault(*context_, options_trianglestrip); - porter_duff_blend_pipelines_.CreateDefault(*context_, options_trianglestrip, - {supports_decal}); vertices_uber_shader_.CreateDefault(*context_, options, {supports_decal}); + + const std::array, 15> porter_duff_constants = + GetPorterDuffSpecConstants(supports_decal); + clear_blend_pipelines_.CreateDefault(*context_, options_trianglestrip, + porter_duff_constants[0]); + source_blend_pipelines_.CreateDefault(*context_, options_trianglestrip, + porter_duff_constants[1]); + destination_blend_pipelines_.CreateDefault(*context_, options_trianglestrip, + porter_duff_constants[2]); + source_over_blend_pipelines_.CreateDefault(*context_, options_trianglestrip, + porter_duff_constants[3]); + destination_over_blend_pipelines_.CreateDefault( + *context_, options_trianglestrip, porter_duff_constants[4]); + source_in_blend_pipelines_.CreateDefault(*context_, options_trianglestrip, + porter_duff_constants[5]); + destination_in_blend_pipelines_.CreateDefault( + *context_, options_trianglestrip, porter_duff_constants[6]); + source_out_blend_pipelines_.CreateDefault(*context_, options_trianglestrip, + porter_duff_constants[7]); + destination_out_blend_pipelines_.CreateDefault( + *context_, options_trianglestrip, porter_duff_constants[8]); + source_a_top_blend_pipelines_.CreateDefault( + *context_, options_trianglestrip, porter_duff_constants[9]); + destination_a_top_blend_pipelines_.CreateDefault( + *context_, options_trianglestrip, porter_duff_constants[10]); + xor_blend_pipelines_.CreateDefault(*context_, options_trianglestrip, + porter_duff_constants[11]); + plus_blend_pipelines_.CreateDefault(*context_, options_trianglestrip, + porter_duff_constants[12]); + modulate_blend_pipelines_.CreateDefault(*context_, options_trianglestrip, + porter_duff_constants[13]); + screen_blend_pipelines_.CreateDefault(*context_, options_trianglestrip, + porter_duff_constants[14]); } if (context_->GetCapabilities()->SupportsFramebufferFetch()) { diff --git a/engine/src/flutter/impeller/entity/contents/content_context.h b/engine/src/flutter/impeller/entity/contents/content_context.h index 8a0ed4ec8a..7446b31b44 100644 --- a/engine/src/flutter/impeller/entity/contents/content_context.h +++ b/engine/src/flutter/impeller/entity/contents/content_context.h @@ -15,6 +15,7 @@ #include "impeller/base/validation.h" #include "impeller/core/formats.h" #include "impeller/core/host_buffer.h" +#include "impeller/geometry/color.h" #include "impeller/renderer/capabilities.h" #include "impeller/renderer/command_buffer.h" #include "impeller/renderer/pipeline.h" @@ -522,8 +523,121 @@ class ContentContext { return GetPipeline(yuv_to_rgb_filter_pipelines_, opts); } - PipelineRef GetPorterDuffBlendPipeline(ContentContextOptions opts) const { - return GetPipeline(porter_duff_blend_pipelines_, opts); + // Porter-Duff combined blends. + PipelineRef GetPorterDuffPipeline(BlendMode mode, + ContentContextOptions opts) const { + switch (mode) { + case BlendMode::kClear: + return GetClearBlendPipeline(opts); + case BlendMode::kSource: + return GetSourceBlendPipeline(opts); + case BlendMode::kDestination: + return GetDestinationBlendPipeline(opts); + case BlendMode::kSourceOver: + return GetSourceOverBlendPipeline(opts); + case BlendMode::kDestinationOver: + return GetDestinationOverBlendPipeline(opts); + case BlendMode::kSourceIn: + return GetSourceInBlendPipeline(opts); + case BlendMode::kDestinationIn: + return GetDestinationInBlendPipeline(opts); + case BlendMode::kSourceOut: + return GetSourceOutBlendPipeline(opts); + case BlendMode::kDestinationOut: + return GetDestinationOutBlendPipeline(opts); + case BlendMode::kSourceATop: + return GetSourceATopBlendPipeline(opts); + case BlendMode::kDestinationATop: + return GetDestinationATopBlendPipeline(opts); + case BlendMode::kXor: + return GetXorBlendPipeline(opts); + case BlendMode::kPlus: + return GetPlusBlendPipeline(opts); + case BlendMode::kModulate: + return GetModulateBlendPipeline(opts); + case BlendMode::kScreen: + return GetScreenBlendPipeline(opts); + case BlendMode::kOverlay: + case BlendMode::kDarken: + case BlendMode::kLighten: + case BlendMode::kColorDodge: + case BlendMode::kColorBurn: + case BlendMode::kHardLight: + case BlendMode::kSoftLight: + case BlendMode::kDifference: + case BlendMode::kExclusion: + case BlendMode::kMultiply: + case BlendMode::kHue: + case BlendMode::kSaturation: + case BlendMode::kColor: + case BlendMode::kLuminosity: + VALIDATION_LOG << "Invalid porter duff blend mode " + << BlendModeToString(mode); + return GetClearBlendPipeline(opts); + break; + } + } + + PipelineRef GetClearBlendPipeline(ContentContextOptions opts) const { + return GetPipeline(clear_blend_pipelines_, opts); + } + + PipelineRef GetSourceBlendPipeline(ContentContextOptions opts) const { + return GetPipeline(source_blend_pipelines_, opts); + } + + PipelineRef GetDestinationBlendPipeline(ContentContextOptions opts) const { + return GetPipeline(destination_blend_pipelines_, opts); + } + + PipelineRef GetSourceOverBlendPipeline(ContentContextOptions opts) const { + return GetPipeline(source_over_blend_pipelines_, opts); + } + + PipelineRef GetDestinationOverBlendPipeline( + ContentContextOptions opts) const { + return GetPipeline(destination_over_blend_pipelines_, opts); + } + + PipelineRef GetSourceInBlendPipeline(ContentContextOptions opts) const { + return GetPipeline(source_in_blend_pipelines_, opts); + } + + PipelineRef GetDestinationInBlendPipeline(ContentContextOptions opts) const { + return GetPipeline(destination_in_blend_pipelines_, opts); + } + + PipelineRef GetSourceOutBlendPipeline(ContentContextOptions opts) const { + return GetPipeline(source_out_blend_pipelines_, opts); + } + + PipelineRef GetDestinationOutBlendPipeline(ContentContextOptions opts) const { + return GetPipeline(destination_out_blend_pipelines_, opts); + } + + PipelineRef GetSourceATopBlendPipeline(ContentContextOptions opts) const { + return GetPipeline(source_a_top_blend_pipelines_, opts); + } + + PipelineRef GetDestinationATopBlendPipeline( + ContentContextOptions opts) const { + return GetPipeline(destination_a_top_blend_pipelines_, opts); + } + + PipelineRef GetXorBlendPipeline(ContentContextOptions opts) const { + return GetPipeline(xor_blend_pipelines_, opts); + } + + PipelineRef GetPlusBlendPipeline(ContentContextOptions opts) const { + return GetPipeline(plus_blend_pipelines_, opts); + } + + PipelineRef GetModulateBlendPipeline(ContentContextOptions opts) const { + return GetPipeline(modulate_blend_pipelines_, opts); + } + + PipelineRef GetScreenBlendPipeline(ContentContextOptions opts) const { + return GetPipeline(screen_blend_pipelines_, opts); } // Advanced blends. @@ -826,7 +940,7 @@ class ContentContext { void CreateDefault(const Context& context, const ContentContextOptions& options, - const std::initializer_list& constants = {}) { + const std::vector& constants = {}) { auto desc = PipelineHandleT::Builder::MakeDefaultPipelineDescriptor( context, constants); if (!desc.has_value()) { @@ -916,7 +1030,24 @@ class ContentContext { mutable Variants clip_pipelines_; mutable Variants glyph_atlas_pipelines_; mutable Variants yuv_to_rgb_filter_pipelines_; - mutable Variants porter_duff_blend_pipelines_; + + // Porter Duff Blends. + mutable Variants clear_blend_pipelines_; + mutable Variants source_blend_pipelines_; + mutable Variants destination_blend_pipelines_; + mutable Variants source_over_blend_pipelines_; + mutable Variants destination_over_blend_pipelines_; + mutable Variants source_in_blend_pipelines_; + mutable Variants destination_in_blend_pipelines_; + mutable Variants source_out_blend_pipelines_; + mutable Variants destination_out_blend_pipelines_; + mutable Variants source_a_top_blend_pipelines_; + mutable Variants destination_a_top_blend_pipelines_; + mutable Variants xor_blend_pipelines_; + mutable Variants plus_blend_pipelines_; + mutable Variants modulate_blend_pipelines_; + mutable Variants screen_blend_pipelines_; + // Advanced blends. mutable Variants blend_color_pipelines_; mutable Variants blend_colorburn_pipelines_; diff --git a/engine/src/flutter/impeller/entity/contents/filters/blend_filter_contents.cc b/engine/src/flutter/impeller/entity/contents/filters/blend_filter_contents.cc index 3549dd0272..a276b575ff 100644 --- a/engine/src/flutter/impeller/entity/contents/filters/blend_filter_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/filters/blend_filter_contents.cc @@ -470,7 +470,7 @@ std::optional BlendFilterContents::CreateForegroundPorterDuffBlend( pass.SetVertexBuffer(std::move(vtx_buffer)); auto options = OptionsFromPassAndEntity(pass, entity); options.primitive_type = PrimitiveType::kTriangleStrip; - pass.SetPipeline(renderer.GetPorterDuffBlendPipeline(options)); + pass.SetPipeline(renderer.GetPorterDuffPipeline(blend_mode, options)); FS::FragInfo frag_info; VS::FrameInfo frame_info; @@ -497,14 +497,6 @@ std::optional BlendFilterContents::CreateForegroundPorterDuffBlend( : 1.0; frag_info.output_alpha = 1.0; - auto blend_coefficients = - kPorterDuffCoefficients[static_cast(blend_mode)]; - frag_info.src_coeff = blend_coefficients[0]; - frag_info.src_coeff_dst_alpha = blend_coefficients[1]; - frag_info.dst_coeff = blend_coefficients[2]; - frag_info.dst_coeff_src_alpha = blend_coefficients[3]; - frag_info.dst_coeff_src_color = blend_coefficients[4]; - FS::BindFragInfo(pass, host_buffer.EmplaceUniform(frag_info)); VS::BindFrameInfo(pass, host_buffer.EmplaceUniform(frame_info)); diff --git a/engine/src/flutter/impeller/entity/contents/vertices_contents.cc b/engine/src/flutter/impeller/entity/contents/vertices_contents.cc index 49ced2d8b7..4bbe9d9fd9 100644 --- a/engine/src/flutter/impeller/entity/contents/vertices_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/vertices_contents.cc @@ -151,7 +151,10 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer, auto options = OptionsFromPassAndEntity(pass, entity); options.primitive_type = geometry_result.type; - pass.SetPipeline(renderer.GetPorterDuffBlendPipeline(options)); + auto inverted_blend_mode = + InvertPorterDuffBlend(blend_mode).value_or(BlendMode::kSource); + pass.SetPipeline( + renderer.GetPorterDuffPipeline(inverted_blend_mode, options)); FS::BindTextureSamplerDst(pass, texture, dst_sampler); @@ -164,16 +167,6 @@ bool VerticesSimpleBlendContents::Render(const ContentContext& renderer, frag_info.output_alpha = alpha_; frag_info.input_alpha = 1.0; - auto inverted_blend_mode = - InvertPorterDuffBlend(blend_mode).value_or(BlendMode::kSource); - auto blend_coefficients = - kPorterDuffCoefficients[static_cast(inverted_blend_mode)]; - frag_info.src_coeff = blend_coefficients[0]; - frag_info.src_coeff_dst_alpha = blend_coefficients[1]; - frag_info.dst_coeff = blend_coefficients[2]; - frag_info.dst_coeff_src_alpha = blend_coefficients[3]; - frag_info.dst_coeff_src_color = blend_coefficients[4]; - // These values are ignored if the platform supports native decal mode. frag_info.tmx = static_cast(tile_mode_x_); frag_info.tmy = static_cast(tile_mode_y_); diff --git a/engine/src/flutter/impeller/entity/shaders/blending/porter_duff_blend.frag b/engine/src/flutter/impeller/entity/shaders/blending/porter_duff_blend.frag index 348253387e..d7eb34caf5 100644 --- a/engine/src/flutter/impeller/entity/shaders/blending/porter_duff_blend.frag +++ b/engine/src/flutter/impeller/entity/shaders/blending/porter_duff_blend.frag @@ -9,16 +9,17 @@ precision mediump float; #include #include +// see GetPorterDuffSpecConstants in content_context.cc for actual constants layout(constant_id = 0) const float supports_decal = 1.0; +layout(constant_id = 1) const float src_coeff = 1.0; +layout(constant_id = 2) const float src_coeff_dst_alpha = 1.0; +layout(constant_id = 3) const float dst_coeff = 1.0; +layout(constant_id = 4) const float dst_coeff_src_alpha = 1.0; +layout(constant_id = 5) const float dst_coeff_src_color = 1.0; uniform f16sampler2D texture_sampler_dst; uniform FragInfo { - float16_t src_coeff; - float16_t src_coeff_dst_alpha; - float16_t dst_coeff; - float16_t dst_coeff_src_alpha; - float16_t dst_coeff_src_color; float16_t input_alpha; float16_t output_alpha; float tmx; @@ -29,7 +30,7 @@ frag_info; in vec2 v_texture_coords; in f16vec4 v_color; -out f16vec4 frag_color; +out vec4 frag_color; f16vec4 Sample(f16sampler2D texture_sampler, vec2 texture_coords, @@ -46,9 +47,8 @@ void main() { frag_info.tmy) * frag_info.input_alpha; f16vec4 src = v_color; - frag_color = - src * (frag_info.src_coeff + dst.a * frag_info.src_coeff_dst_alpha) + - dst * (frag_info.dst_coeff + src.a * frag_info.dst_coeff_src_alpha + - src * frag_info.dst_coeff_src_color); + frag_color = f16vec4(src * (src_coeff + dst.a * src_coeff_dst_alpha) + + dst * (dst_coeff + src.a * dst_coeff_src_alpha + + src * dst_coeff_src_color)); frag_color *= frag_info.output_alpha; } diff --git a/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.cc index 570615c224..a73e9823ba 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.cc @@ -462,7 +462,8 @@ std::optional BufferBindingsGLES::BindTextures( auto location = ComputeTextureLocation(data.texture.GetMetadata()); if (location == -1) { - return std::nullopt; + // The texture binding was optimized out of the shader. Continue. + continue; } //-------------------------------------------------------------------------- diff --git a/engine/src/flutter/impeller/tools/malioc.json b/engine/src/flutter/impeller/tools/malioc.json index 3e75278656..f4dbfdb0b8 100644 --- a/engine/src/flutter/impeller/tools/malioc.json +++ b/engine/src/flutter/impeller/tools/malioc.json @@ -3812,8 +3812,8 @@ }, "stack_spill_bytes": 0, "thread_occupancy": 100, - "uniform_registers_used": 8, - "work_registers_used": 19 + "uniform_registers_used": 4, + "work_registers_used": 20 } } }, @@ -3830,7 +3830,7 @@ "arithmetic" ], "longest_path_cycles": [ - 1.649999976158142, + 1.3200000524520874, 1.0, 1.0 ], @@ -3843,7 +3843,7 @@ "arithmetic" ], "shortest_path_cycles": [ - 1.649999976158142, + 1.3200000524520874, 1.0, 1.0 ], @@ -3851,7 +3851,7 @@ "arithmetic" ], "total_cycles": [ - 2.0, + 1.6666666269302368, 1.0, 1.0 ] @@ -7251,7 +7251,7 @@ }, "stack_spill_bytes": 0, "thread_occupancy": 100, - "uniform_registers_used": 14, + "uniform_registers_used": 6, "work_registers_used": 11 } }