From 1e495ab7d870ba9df0991a7b6349d3a4ef6d1a4a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 24 Sep 2024 17:03:00 -0700 Subject: [PATCH 1/6] [Impeller] actually fix external texture. g --- impeller/aiks/canvas.cc | 18 ---- impeller/entity/contents/content_context.h | 2 +- impeller/entity/contents/texture_contents.cc | 37 +++++++- .../entity/contents/tiled_texture_contents.cc | 89 +++++-------------- .../shaders/tiled_texture_fill_external.frag | 2 +- 5 files changed, 60 insertions(+), 88 deletions(-) diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index b919b9f8fcc0e..805a4d7b3066d 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -744,24 +744,6 @@ void Canvas::DrawImageRect(const std::shared_ptr& image, return; } - if (image->GetTextureDescriptor().type == TextureType::kTextureExternalOES) { - auto texture_contents = std::make_shared(); - texture_contents->SetTexture(image); - texture_contents->SetGeometry(Geometry::MakeRect(dest)); - texture_contents->SetSamplerDescriptor(std::move(sampler)); - texture_contents->SetInheritedOpacity(paint.color.alpha); - - std::shared_ptr contents = texture_contents; - - Entity entity; - entity.SetBlendMode(paint.blend_mode); - entity.SetContents(paint.WithFilters(contents)); - entity.SetTransform(GetCurrentTransform()); - - AddRenderEntityToCurrentPass(std::move(entity)); - return; - } - auto texture_contents = TextureContents::MakeRect(dest); texture_contents->SetTexture(image); texture_contents->SetSourceRect(source); diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index 6814b1415397b..c2fe1fcd71896 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -237,7 +237,7 @@ using VerticesUberShader = RenderPipelineHandle; #endif // IMPELLER_ENABLE_OPENGLES diff --git a/impeller/entity/contents/texture_contents.cc b/impeller/entity/contents/texture_contents.cc index 1e2838b52381b..b3174780b4371 100644 --- a/impeller/entity/contents/texture_contents.cc +++ b/impeller/entity/contents/texture_contents.cc @@ -14,6 +14,7 @@ #include "impeller/entity/texture_fill.frag.h" #include "impeller/entity/texture_fill.vert.h" #include "impeller/entity/texture_fill_strict_src.frag.h" +#include "impeller/entity/tiled_texture_fill_external.frag.h" #include "impeller/geometry/constants.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/vertex_buffer_builder.h" @@ -112,15 +113,15 @@ bool TextureContents::Render(const ContentContext& renderer, using VS = TextureFillVertexShader; using FS = TextureFillFragmentShader; using FSStrict = TextureFillStrictSrcFragmentShader; + using FSExternal = TiledTextureFillExternalFragmentShader; if (destination_rect_.IsEmpty() || source_rect_.IsEmpty() || texture_ == nullptr || texture_->GetSize().IsEmpty()) { return true; // Nothing to render. } - [[maybe_unused]] bool is_external_texture = + bool is_external_texture = texture_->GetTextureDescriptor().type == TextureType::kTextureExternalOES; - FML_DCHECK(!is_external_texture); auto texture_coords = Rect::MakeSize(texture_->GetSize()).Project(source_rect_); @@ -159,9 +160,21 @@ bool TextureContents::Render(const ContentContext& renderer, pipeline_options.depth_write_enabled = stencil_enabled_ && pipeline_options.blend_mode == BlendMode::kSource; +#ifdef IMPELLER_ENABLE_OPENGLES + if (is_external_texture) { + pass.SetPipeline( + renderer.GetTiledTextureExternalPipeline(pipeline_options)); + } else { + pass.SetPipeline( + strict_source_rect_enabled_ + ? renderer.GetTextureStrictSrcPipeline(pipeline_options) + : renderer.GetTexturePipeline(pipeline_options)); + } +#else pass.SetPipeline(strict_source_rect_enabled_ ? renderer.GetTextureStrictSrcPipeline(pipeline_options) : renderer.GetTexturePipeline(pipeline_options)); +#endif // IMPELLER_ENABLE_OPENGLES pass.SetVertexBuffer(vertex_buffer); VS::BindFrameInfo(pass, host_buffer.EmplaceUniform(frame_info)); @@ -181,6 +194,26 @@ bool TextureContents::Render(const ContentContext& renderer, pass, texture_, renderer.GetContext()->GetSamplerLibrary()->GetSampler( sampler_descriptor_)); +#ifdef IMPELLER_ENABLE_OPENGLES + } else if (is_external_texture) { + FSExternal::FragInfo frag_info; + frag_info.x_tile_mode = + static_cast(sampler_descriptor_.width_address_mode); + frag_info.y_tile_mode = + static_cast(sampler_descriptor_.height_address_mode); + frag_info.alpha = GetOpacity(); + FSExternal::BindFragInfo(pass, host_buffer.EmplaceUniform(frag_info)); + + SamplerDescriptor sampler_desc; + // OES_EGL_image_external states that only CLAMP_TO_EDGE is valid, so + // we emulate all other tile modes here by remapping the texture + // coordinates. + sampler_desc.width_address_mode = SamplerAddressMode::kClampToEdge; + sampler_desc.height_address_mode = SamplerAddressMode::kClampToEdge; + FSExternal::BindSAMPLEREXTERNALOESTextureSampler( + pass, texture_, + renderer.GetContext()->GetSamplerLibrary()->GetSampler(sampler_desc)); +#endif // IMPELLER_ENABLE_OPENGLES } else { FS::FragInfo frag_info; frag_info.alpha = GetOpacity(); diff --git a/impeller/entity/contents/tiled_texture_contents.cc b/impeller/entity/contents/tiled_texture_contents.cc index 557d1257494d8..a750f21914b41 100644 --- a/impeller/entity/contents/tiled_texture_contents.cc +++ b/impeller/entity/contents/tiled_texture_contents.cc @@ -116,16 +116,12 @@ bool TiledTextureContents::Render(const ContentContext& renderer, using VS = TextureUvFillVertexShader; using FS = TiledTextureFillFragmentShader; - using FSExternal = TiledTextureFillExternalFragmentShader; const auto texture_size = texture_->GetSize(); if (texture_size.IsEmpty()) { return true; } - bool is_external_texture = - texture_->GetTextureDescriptor().type == TextureType::kTextureExternalOES; - VS::FrameInfo frame_info; frame_info.texture_sampler_y_coord_scale = texture_->GetYCoordScale(); frame_info.uv_transform = @@ -133,79 +129,40 @@ bool TiledTextureContents::Render(const ContentContext& renderer, GetInverseEffectTransform(); PipelineBuilderMethod pipeline_method; - -#ifdef IMPELLER_ENABLE_OPENGLES - if (is_external_texture) { - pipeline_method = &ContentContext::GetTiledTextureExternalPipeline; - } else { - pipeline_method = &ContentContext::GetTiledTexturePipeline; - } -#else - pipeline_method = &ContentContext::GetTiledTexturePipeline; -#endif // IMPELLER_ENABLE_OPENGLES - PipelineBuilderCallback pipeline_callback = [&renderer, &pipeline_method](ContentContextOptions options) { return (renderer.*pipeline_method)(options); }; return ColorSourceContents::DrawGeometry( renderer, entity, pass, pipeline_callback, frame_info, - [this, &renderer, &is_external_texture, &entity](RenderPass& pass) { + [this, &renderer, &entity](RenderPass& pass) { auto& host_buffer = renderer.GetTransientsBuffer(); - +#ifdef IMPELLER_DEBUG pass.SetCommandLabel("TextureFill"); - - if (is_external_texture) { - FSExternal::FragInfo frag_info; - frag_info.x_tile_mode = static_cast(x_tile_mode_); - frag_info.y_tile_mode = static_cast(y_tile_mode_); - frag_info.alpha = - GetOpacityFactor() * - GetGeometry()->ComputeAlphaCoverage(entity.GetTransform()); - FSExternal::BindFragInfo(pass, host_buffer.EmplaceUniform(frag_info)); +#endif // IMPELLER_DEBUG + + FS::FragInfo frag_info; + frag_info.x_tile_mode = static_cast(x_tile_mode_); + frag_info.y_tile_mode = static_cast(y_tile_mode_); + frag_info.alpha = + GetOpacityFactor() * + GetGeometry()->ComputeAlphaCoverage(entity.GetTransform()); + FS::BindFragInfo(pass, host_buffer.EmplaceUniform(frag_info)); + + if (color_filter_) { + auto filtered_texture = CreateFilterTexture(renderer); + if (!filtered_texture) { + return false; + } + FS::BindTextureSampler( + pass, filtered_texture, + renderer.GetContext()->GetSamplerLibrary()->GetSampler( + CreateSamplerDescriptor(renderer.GetDeviceCapabilities()))); } else { - FS::FragInfo frag_info; - frag_info.x_tile_mode = static_cast(x_tile_mode_); - frag_info.y_tile_mode = static_cast(y_tile_mode_); - frag_info.alpha = - GetOpacityFactor() * - GetGeometry()->ComputeAlphaCoverage(entity.GetTransform()); - FS::BindFragInfo(pass, host_buffer.EmplaceUniform(frag_info)); - } - - if (is_external_texture) { - SamplerDescriptor sampler_desc; - // OES_EGL_image_external states that only CLAMP_TO_EDGE is valid, so - // we emulate all other tile modes here by remapping the texture - // coordinates. - sampler_desc.width_address_mode = SamplerAddressMode::kClampToEdge; - sampler_desc.height_address_mode = SamplerAddressMode::kClampToEdge; - - // Also, external textures cannot be bound to color filters, so ignore - // this case for now. - FML_DCHECK(!color_filter_) << "Color filters are not currently " - "supported for external textures."; - - FSExternal::BindSAMPLEREXTERNALOESTextureSampler( + FS::BindTextureSampler( pass, texture_, renderer.GetContext()->GetSamplerLibrary()->GetSampler( - sampler_desc)); - } else { - if (color_filter_) { - auto filtered_texture = CreateFilterTexture(renderer); - if (!filtered_texture) { - return false; - } - FS::BindTextureSampler( - pass, filtered_texture, - renderer.GetContext()->GetSamplerLibrary()->GetSampler( - CreateSamplerDescriptor(renderer.GetDeviceCapabilities()))); - } else { - FS::BindTextureSampler( - pass, texture_, - renderer.GetContext()->GetSamplerLibrary()->GetSampler( - CreateSamplerDescriptor(renderer.GetDeviceCapabilities()))); - } + CreateSamplerDescriptor(renderer.GetDeviceCapabilities()))); } return true; diff --git a/impeller/entity/shaders/tiled_texture_fill_external.frag b/impeller/entity/shaders/tiled_texture_fill_external.frag index 7d744553ab066..610c4bb44936c 100644 --- a/impeller/entity/shaders/tiled_texture_fill_external.frag +++ b/impeller/entity/shaders/tiled_texture_fill_external.frag @@ -15,7 +15,7 @@ uniform FragInfo { } frag_info; -in vec2 v_texture_coords; +in highp vec2 v_texture_coords; out vec4 frag_color; From d62d75bd66ffae12aadfb39bcd49eede129fdc45 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 24 Sep 2024 17:17:59 -0700 Subject: [PATCH 2/6] ++ --- impeller/entity/contents/texture_contents.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/impeller/entity/contents/texture_contents.cc b/impeller/entity/contents/texture_contents.cc index b3174780b4371..abadb760fbd9c 100644 --- a/impeller/entity/contents/texture_contents.cc +++ b/impeller/entity/contents/texture_contents.cc @@ -120,8 +120,10 @@ bool TextureContents::Render(const ContentContext& renderer, return true; // Nothing to render. } +#ifdef IMPELLER_ENABLE_OPENGLES bool is_external_texture = texture_->GetTextureDescriptor().type == TextureType::kTextureExternalOES; +#endif // IMPELLER_ENABLE_OPENGLES auto texture_coords = Rect::MakeSize(texture_->GetSize()).Project(source_rect_); From 8d33e9fa2b484a42fb339d9037bfe5d8cbab2f7c Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 24 Sep 2024 17:33:24 -0700 Subject: [PATCH 3/6] Update texture_contents.cc --- impeller/entity/contents/texture_contents.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/entity/contents/texture_contents.cc b/impeller/entity/contents/texture_contents.cc index abadb760fbd9c..83454365a6ad1 100644 --- a/impeller/entity/contents/texture_contents.cc +++ b/impeller/entity/contents/texture_contents.cc @@ -113,7 +113,6 @@ bool TextureContents::Render(const ContentContext& renderer, using VS = TextureFillVertexShader; using FS = TextureFillFragmentShader; using FSStrict = TextureFillStrictSrcFragmentShader; - using FSExternal = TiledTextureFillExternalFragmentShader; if (destination_rect_.IsEmpty() || source_rect_.IsEmpty() || texture_ == nullptr || texture_->GetSize().IsEmpty()) { @@ -121,6 +120,7 @@ bool TextureContents::Render(const ContentContext& renderer, } #ifdef IMPELLER_ENABLE_OPENGLES + using FSExternal = TiledTextureFillExternalFragmentShader; bool is_external_texture = texture_->GetTextureDescriptor().type == TextureType::kTextureExternalOES; #endif // IMPELLER_ENABLE_OPENGLES From a7e0af161b1f0b76bdf4e2e011490754e528b419 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 25 Sep 2024 09:16:21 -0700 Subject: [PATCH 4/6] ++ --- impeller/entity/contents/tiled_texture_contents.cc | 5 ++--- .../entity/contents/tiled_texture_contents_unittests.cc | 6 ++++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/impeller/entity/contents/tiled_texture_contents.cc b/impeller/entity/contents/tiled_texture_contents.cc index a750f21914b41..549abf32ff99d 100644 --- a/impeller/entity/contents/tiled_texture_contents.cc +++ b/impeller/entity/contents/tiled_texture_contents.cc @@ -128,10 +128,9 @@ bool TiledTextureContents::Render(const ContentContext& renderer, Rect::MakeSize(texture_size).GetNormalizingTransform() * GetInverseEffectTransform(); - PipelineBuilderMethod pipeline_method; PipelineBuilderCallback pipeline_callback = - [&renderer, &pipeline_method](ContentContextOptions options) { - return (renderer.*pipeline_method)(options); + [&renderer](ContentContextOptions options) { + return renderer.GetTiledTexturePipeline(options); }; return ColorSourceContents::DrawGeometry( renderer, entity, pass, pipeline_callback, frame_info, diff --git a/impeller/entity/contents/tiled_texture_contents_unittests.cc b/impeller/entity/contents/tiled_texture_contents_unittests.cc index 36e21d644c506..4acc56ef27efb 100644 --- a/impeller/entity/contents/tiled_texture_contents_unittests.cc +++ b/impeller/entity/contents/tiled_texture_contents_unittests.cc @@ -44,8 +44,14 @@ TEST_P(EntityTest, TiledTextureContentsRendersWithCorrectPipeline) { const std::vector& commands = recording_pass->GetCommands(); ASSERT_EQ(commands.size(), 1u); +#ifdef IMPELLER_DEBUG EXPECT_TRUE(commands[0].pipeline->GetDescriptor().GetLabel().find( "TextureFill Pipeline") != std::string::npos); +#endif // IMPELLER_DEBUG + auto options = OptionsFromPassAndEntity(*recording_pass, {}); + options.primitive_type = PrimitiveType::kTriangleStrip; + EXPECT_EQ(commands[0].pipeline, + GetContentContext()->GetTiledTexturePipeline(options)); if (GetParam() == PlaygroundBackend::kMetal) { recording_pass->EncodeCommands(); From 614045a5a18284c1a342a9a725c39f3d345c44cc Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 25 Sep 2024 10:17:27 -0700 Subject: [PATCH 5/6] ++ --- .../tiled_texture_contents_unittests.cc | 7 ++- impeller/tools/malioc.json | 44 +++++++++---------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/impeller/entity/contents/tiled_texture_contents_unittests.cc b/impeller/entity/contents/tiled_texture_contents_unittests.cc index 4acc56ef27efb..42f06c9484006 100644 --- a/impeller/entity/contents/tiled_texture_contents_unittests.cc +++ b/impeller/entity/contents/tiled_texture_contents_unittests.cc @@ -90,8 +90,11 @@ TEST_P(EntityTest, TiledTextureContentsRendersWithCorrectPipelineExternalOES) { const std::vector& commands = render_pass->GetCommands(); ASSERT_EQ(commands.size(), 1u); - EXPECT_TRUE(commands[0].pipeline->GetDescriptor().GetLabel().find( - "TiledTextureFillExternal Pipeline") != std::string::npos); + + auto options = OptionsFromPassAndEntity(*render_pass, {}); + options.primitive_type = PrimitiveType::kTriangleStrip; + EXPECT_EQ(commands[0].pipeline, + GetContentContext()->GetTiledTextureExternalPipeline(options)); } #endif diff --git a/impeller/tools/malioc.json b/impeller/tools/malioc.json index 65d3b54f4eb3c..24b5da732460d 100644 --- a/impeller/tools/malioc.json +++ b/impeller/tools/malioc.json @@ -5547,7 +5547,7 @@ "uses_late_zs_update": false, "variants": { "Main": { - "fp16_arithmetic": 70, + "fp16_arithmetic": 35, "has_stack_spilling": false, "performance": { "longest_path_bound_pipelines": [ @@ -5560,7 +5560,7 @@ 0.421875, 0.0, 0.0, - 0.125, + 0.25, 0.25 ], "pipelines": [ @@ -5581,7 +5581,7 @@ 0.09375, 0.0, 0.0, - 0.125, + 0.25, 0.0 ], "total_bound_pipelines": [ @@ -5589,12 +5589,12 @@ "arith_cvt" ], "total_cycles": [ - 0.515625, + 0.546875, 0.359375, - 0.515625, + 0.546875, 0.0, 0.0, - 0.125, + 0.25, 0.25 ] }, @@ -5618,7 +5618,7 @@ "arithmetic" ], "longest_path_cycles": [ - 7.920000076293945, + 8.25, 1.0, 1.0 ], @@ -5639,7 +5639,7 @@ "arithmetic" ], "total_cycles": [ - 9.0, + 9.666666984558105, 1.0, 1.0 ] @@ -7971,7 +7971,7 @@ "uses_late_zs_update": false, "variants": { "Main": { - "fp16_arithmetic": 100, + "fp16_arithmetic": 72, "has_stack_spilling": false, "performance": { "longest_path_bound_pipelines": [ @@ -7979,12 +7979,12 @@ "arith_cvt" ], "longest_path_cycles": [ - 0.328125, + 0.375, 0.1875, - 0.328125, - 0.0, + 0.375, + 0.0625, 0.0, - 0.125, + 0.25, 0.25 ], "pipelines": [ @@ -8000,12 +8000,12 @@ "varying" ], "shortest_path_cycles": [ - 0.109375, + 0.140625, 0.03125, - 0.109375, - 0.0, + 0.140625, + 0.0625, 0.0, - 0.125, + 0.25, 0.0 ], "total_bound_pipelines": [ @@ -8013,19 +8013,19 @@ "arith_cvt" ], "total_cycles": [ - 0.390625, + 0.4375, 0.21875, - 0.390625, - 0.0, + 0.4375, + 0.0625, 0.0, - 0.125, + 0.25, 0.25 ] }, "stack_spill_bytes": 0, "thread_occupancy": 100, "uniform_registers_used": 6, - "work_registers_used": 6 + "work_registers_used": 7 } } } From 44a84162ec7fa38f763d9d72a2ad23a16d343f04 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 25 Sep 2024 12:17:35 -0700 Subject: [PATCH 6/6] ++ --- .../contents/tiled_texture_contents_unittests.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/impeller/entity/contents/tiled_texture_contents_unittests.cc b/impeller/entity/contents/tiled_texture_contents_unittests.cc index 42f06c9484006..dd732e36212a0 100644 --- a/impeller/entity/contents/tiled_texture_contents_unittests.cc +++ b/impeller/entity/contents/tiled_texture_contents_unittests.cc @@ -7,6 +7,7 @@ #include "impeller/core/formats.h" #include "impeller/core/texture_descriptor.h" #include "impeller/entity/contents/test/recording_render_pass.h" +#include "impeller/entity/contents/texture_contents.h" #include "impeller/entity/contents/tiled_texture_contents.h" #include "impeller/entity/entity_playground.h" #include "impeller/playground/playground_test.h" @@ -73,10 +74,10 @@ TEST_P(EntityTest, TiledTextureContentsRendersWithCorrectPipelineExternalOES) { texture_desc.storage_mode = StorageMode::kDevicePrivate; auto texture = GetContext()->GetResourceAllocator()->CreateTexture(texture_desc); - - TiledTextureContents contents; - contents.SetTexture(texture); - contents.SetGeometry(Geometry::MakeCover()); + auto contents = TextureContents::MakeRect(Rect::MakeSize(texture->GetSize())); + contents->SetTexture(texture); + contents->SetSourceRect(Rect::MakeSize(texture->GetSize())); + contents->SetStrictSourceRect(false); auto content_context = GetContentContext(); auto buffer = content_context->GetContext()->CreateCommandBuffer(); @@ -86,7 +87,7 @@ TEST_P(EntityTest, TiledTextureContentsRendersWithCorrectPipelineExternalOES) { /*mip_count=*/1); auto render_pass = buffer->CreateRenderPass(render_target); - ASSERT_TRUE(contents.Render(*GetContentContext(), {}, *render_pass)); + ASSERT_TRUE(contents->Render(*GetContentContext(), {}, *render_pass)); const std::vector& commands = render_pass->GetCommands(); ASSERT_EQ(commands.size(), 1u);