From 4bd14c0429d5b4fabe07740c2971bc49804a5bd5 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Thu, 26 Sep 2024 16:24:02 -0700 Subject: [PATCH 1/4] [Impeller] Apply some recent color filter fixes to BlendFilterContents::CreateForegroundAdvancedBlend Some recent changes affecting color filters (https://github.com/flutter/engine/pull/55411, https://github.com/flutter/engine/pull/55448) need to be done for both Porter-Duff and advanced blend modes --- impeller/display_list/aiks_dl_blend_unittests.cc | 13 +++++++++++-- .../contents/filters/blend_filter_contents.cc | 8 +++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/impeller/display_list/aiks_dl_blend_unittests.cc b/impeller/display_list/aiks_dl_blend_unittests.cc index 965959f33952c..3cad64b50f04f 100644 --- a/impeller/display_list/aiks_dl_blend_unittests.cc +++ b/impeller/display_list/aiks_dl_blend_unittests.cc @@ -248,11 +248,19 @@ TEST_P(AiksTest, ColorFilterBlend) { // Verification for: https://github.com/flutter/flutter/issues/155691 TEST_P(AiksTest, ColorFilterAdvancedBlend) { + std::array filter_blend_mode_names = {"SrcIn", "Multiply"}; + std::array filter_blend_modes = {DlBlendMode::kSrcIn, + DlBlendMode::kMultiply}; bool has_color_filter = true; + int selected_filter_blend_mode = 0; + auto callback = [&]() -> sk_sp { if (AiksTest::ImGuiBegin("Controls", nullptr, ImGuiWindowFlags_AlwaysAutoResize)) { ImGui::Checkbox("has color filter", &has_color_filter); + ImGui::Combo("Color filter blend mode", &selected_filter_blend_mode, + filter_blend_mode_names.data(), + filter_blend_mode_names.size()); ImGui::End(); } @@ -289,8 +297,9 @@ TEST_P(AiksTest, ColorFilterAdvancedBlend) { srcPaint.setBlendMode(blend_modes[i]); if (has_color_filter) { std::shared_ptr color_filter = - DlBlendColorFilter::Make(DlColor::RGBA(0.9, 0.5, 0.0, 1.0), - DlBlendMode::kSrcIn); + DlBlendColorFilter::Make( + DlColor::RGBA(0.9, 0.5, 0.0, 1.0), + filter_blend_modes[selected_filter_blend_mode]); srcPaint.setColorFilter(color_filter); } builder.DrawImage(src_image, {0, 0}, DlImageSampling::kMipmapLinear, diff --git a/impeller/entity/contents/filters/blend_filter_contents.cc b/impeller/entity/contents/filters/blend_filter_contents.cc index a1120d3f9039f..49c7d6b14cd24 100644 --- a/impeller/entity/contents/filters/blend_filter_contents.cc +++ b/impeller/entity/contents/filters/blend_filter_contents.cc @@ -303,7 +303,7 @@ std::optional BlendFilterContents::CreateForegroundAdvancedBlend( BlendModeToString(blend_mode))); #endif // IMPELLER_DEBUG pass.SetVertexBuffer(std::move(vtx_buffer)); - auto options = OptionsFromPass(pass); + auto options = OptionsFromPassAndEntity(pass, entity); options.primitive_type = PrimitiveType::kTriangleStrip; switch (blend_mode) { @@ -370,8 +370,9 @@ std::optional BlendFilterContents::CreateForegroundAdvancedBlend( FS::BindTextureSamplerDst(pass, dst_snapshot->texture, dst_sampler); frame_info.dst_y_coord_scale = dst_snapshot->texture->GetYCoordScale(); - frame_info.mvp = Entity::GetShaderTransform(entity.GetShaderClipDepth(), - pass, dst_snapshot->transform); + frame_info.mvp = Entity::GetShaderTransform( + entity.GetShaderClipDepth(), pass, + entity.GetTransform() * dst_snapshot->transform); blend_info.dst_input_alpha = absorb_opacity == ColorFilterContents::AbsorbOpacity::kYes @@ -402,6 +403,7 @@ std::optional BlendFilterContents::CreateForegroundAdvancedBlend( Entity sub_entity; sub_entity.SetContents(std::move(contents)); + sub_entity.SetBlendMode(entity.GetBlendMode()); return sub_entity; } From abc16c145c02d34c6515b6a519d344a0e0843b45 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Fri, 27 Sep 2024 09:35:50 -0700 Subject: [PATCH 2/4] framebuffer fetch disable test --- .../display_list/aiks_dl_blend_unittests.cc | 95 +++++++++++++++++-- .../display_list/aiks_dl_blur_unittests.cc | 4 - impeller/renderer/testing/mocks.h | 4 + 3 files changed, 89 insertions(+), 14 deletions(-) diff --git a/impeller/display_list/aiks_dl_blend_unittests.cc b/impeller/display_list/aiks_dl_blend_unittests.cc index 3cad64b50f04f..2118abd9634e6 100644 --- a/impeller/display_list/aiks_dl_blend_unittests.cc +++ b/impeller/display_list/aiks_dl_blend_unittests.cc @@ -22,6 +22,7 @@ #include "impeller/display_list/dl_dispatcher.h" #include "impeller/playground/playground.h" #include "impeller/playground/playground_test.h" +#include "impeller/renderer/testing/mocks.h" #include "include/core/SkMatrix.h" //////////////////////////////////////////////////////////////////////////////// @@ -248,19 +249,94 @@ TEST_P(AiksTest, ColorFilterBlend) { // Verification for: https://github.com/flutter/flutter/issues/155691 TEST_P(AiksTest, ColorFilterAdvancedBlend) { - std::array filter_blend_mode_names = {"SrcIn", "Multiply"}; - std::array filter_blend_modes = {DlBlendMode::kSrcIn, - DlBlendMode::kMultiply}; bool has_color_filter = true; - int selected_filter_blend_mode = 0; + auto callback = [&]() -> sk_sp { + if (AiksTest::ImGuiBegin("Controls", nullptr, + ImGuiWindowFlags_AlwaysAutoResize)) { + ImGui::Checkbox("has color filter", &has_color_filter); + ImGui::End(); + } + + DisplayListBuilder builder; + builder.Scale(GetContentScale().x, GetContentScale().y); + + auto src_image = + DlImageImpeller::Make(CreateTextureForFixture("blend_mode_src.png")); + auto dst_image = + DlImageImpeller::Make(CreateTextureForFixture("blend_mode_dst.png")); + + std::vector blend_modes = { + DlBlendMode::kScreen, DlBlendMode::kOverlay, + DlBlendMode::kDarken, DlBlendMode::kLighten, + DlBlendMode::kColorDodge, DlBlendMode::kColorBurn, + DlBlendMode::kHardLight, DlBlendMode::kSoftLight, + DlBlendMode::kDifference, DlBlendMode::kExclusion, + DlBlendMode::kMultiply, DlBlendMode::kHue, + DlBlendMode::kSaturation, DlBlendMode::kColor, + DlBlendMode::kLuminosity, + }; + + for (uint32_t i = 0; i < blend_modes.size(); ++i) { + builder.Save(); + builder.Translate((i % 5) * 200, (i / 5) * 200); + builder.Scale(0.4, 0.4); + { + DlPaint dstPaint; + builder.DrawImage(dst_image, {0, 0}, DlImageSampling::kMipmapLinear, + &dstPaint); + } + { + DlPaint srcPaint; + srcPaint.setBlendMode(blend_modes[i]); + if (has_color_filter) { + std::shared_ptr color_filter = + DlBlendColorFilter::Make(DlColor::RGBA(0.9, 0.5, 0.0, 1.0), + DlBlendMode::kSrcIn); + srcPaint.setColorFilter(color_filter); + } + builder.DrawImage(src_image, {0, 0}, DlImageSampling::kMipmapLinear, + &srcPaint); + } + builder.Restore(); + } + return builder.Build(); + }; + ASSERT_TRUE(OpenPlaygroundHere(callback)); +} +// Variant of the https://github.com/flutter/flutter/issues/155691 test that +// uses an advanced blend in the color filter and disables framebuffer fetch +// to force usage of BlendFilterContents::CreateForegroundAdvancedBlend. +TEST_P(AiksTest, ColorFilterAdvancedBlendNoFbFetch) { + std::shared_ptr old_capabilities = + GetContext()->GetCapabilities(); + auto mock_capabilities = std::make_shared(); + EXPECT_CALL(*mock_capabilities, SupportsFramebufferFetch()) + .Times(::testing::AtLeast(1)) + .WillRepeatedly(::testing::Return(false)); + FLT_FORWARD(mock_capabilities, old_capabilities, GetDefaultColorFormat); + FLT_FORWARD(mock_capabilities, old_capabilities, GetDefaultStencilFormat); + FLT_FORWARD(mock_capabilities, old_capabilities, + GetDefaultDepthStencilFormat); + FLT_FORWARD(mock_capabilities, old_capabilities, SupportsOffscreenMSAA); + FLT_FORWARD(mock_capabilities, old_capabilities, + SupportsImplicitResolvingMSAA); + FLT_FORWARD(mock_capabilities, old_capabilities, SupportsReadFromResolve); + FLT_FORWARD(mock_capabilities, old_capabilities, SupportsSSBO); + FLT_FORWARD(mock_capabilities, old_capabilities, SupportsCompute); + FLT_FORWARD(mock_capabilities, old_capabilities, + SupportsTextureToTextureBlits); + FLT_FORWARD(mock_capabilities, old_capabilities, GetDefaultGlyphAtlasFormat); + FLT_FORWARD(mock_capabilities, old_capabilities, SupportsTriangleFan); + FLT_FORWARD(mock_capabilities, old_capabilities, + SupportsDecalSamplerAddressMode); + ASSERT_TRUE(SetCapabilities(mock_capabilities).ok()); + + bool has_color_filter = true; auto callback = [&]() -> sk_sp { if (AiksTest::ImGuiBegin("Controls", nullptr, ImGuiWindowFlags_AlwaysAutoResize)) { ImGui::Checkbox("has color filter", &has_color_filter); - ImGui::Combo("Color filter blend mode", &selected_filter_blend_mode, - filter_blend_mode_names.data(), - filter_blend_mode_names.size()); ImGui::End(); } @@ -297,9 +373,8 @@ TEST_P(AiksTest, ColorFilterAdvancedBlend) { srcPaint.setBlendMode(blend_modes[i]); if (has_color_filter) { std::shared_ptr color_filter = - DlBlendColorFilter::Make( - DlColor::RGBA(0.9, 0.5, 0.0, 1.0), - filter_blend_modes[selected_filter_blend_mode]); + DlBlendColorFilter::Make(DlColor::RGBA(0.9, 0.5, 0.0, 1.0), + DlBlendMode::kMultiply); srcPaint.setColorFilter(color_filter); } builder.DrawImage(src_image, {0, 0}, DlImageSampling::kMipmapLinear, diff --git a/impeller/display_list/aiks_dl_blur_unittests.cc b/impeller/display_list/aiks_dl_blur_unittests.cc index 16c02df38baf5..7bd6a6af0537f 100644 --- a/impeller/display_list/aiks_dl_blur_unittests.cc +++ b/impeller/display_list/aiks_dl_blur_unittests.cc @@ -1041,10 +1041,6 @@ TEST_P(AiksTest, BlurredRectangleWithShader) { ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); } -#define FLT_FORWARD(mock, real, method) \ - EXPECT_CALL(*mock, method()) \ - .WillRepeatedly(::testing::Return(real->method())); - TEST_P(AiksTest, GaussianBlurWithoutDecalSupport) { if (GetParam() != PlaygroundBackend::kMetal) { GTEST_SKIP() diff --git a/impeller/renderer/testing/mocks.h b/impeller/renderer/testing/mocks.h index 6b80affd3a171..5b681b820ae82 100644 --- a/impeller/renderer/testing/mocks.h +++ b/impeller/renderer/testing/mocks.h @@ -16,6 +16,10 @@ #include "impeller/renderer/render_target.h" #include "impeller/renderer/sampler_library.h" +#define FLT_FORWARD(mock, real, method) \ + EXPECT_CALL(*mock, method()) \ + .WillRepeatedly(::testing::Return(real->method())); + namespace impeller { namespace testing { From 2655c4a289cd59063c9599f3de7ee75f11956023 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Fri, 27 Sep 2024 10:26:07 -0700 Subject: [PATCH 3/4] run test only on Metal playgrounds --- impeller/display_list/aiks_dl_blend_unittests.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/impeller/display_list/aiks_dl_blend_unittests.cc b/impeller/display_list/aiks_dl_blend_unittests.cc index 2118abd9634e6..fcdd859274cdb 100644 --- a/impeller/display_list/aiks_dl_blend_unittests.cc +++ b/impeller/display_list/aiks_dl_blend_unittests.cc @@ -308,6 +308,14 @@ TEST_P(AiksTest, ColorFilterAdvancedBlend) { // uses an advanced blend in the color filter and disables framebuffer fetch // to force usage of BlendFilterContents::CreateForegroundAdvancedBlend. TEST_P(AiksTest, ColorFilterAdvancedBlendNoFbFetch) { + if (GetParam() != PlaygroundBackend::kMetal) { + GTEST_SKIP() + << "This backend doesn't yet support setting device capabilities."; + } + if (!WillRenderSomething()) { + GTEST_SKIP() << "This test requires playgrounds."; + } + std::shared_ptr old_capabilities = GetContext()->GetCapabilities(); auto mock_capabilities = std::make_shared(); From c0de5817be58cfea3f017b4a9d27f87c0c85107c Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Fri, 27 Sep 2024 11:34:38 -0700 Subject: [PATCH 4/4] golden file --- testing/impeller_golden_tests_output.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/impeller_golden_tests_output.txt b/testing/impeller_golden_tests_output.txt index c1d2d87bb5e80..e8f2276cdc9a4 100644 --- a/testing/impeller_golden_tests_output.txt +++ b/testing/impeller_golden_tests_output.txt @@ -523,6 +523,7 @@ impeller_Play_AiksTest_CollapsedDrawPaintInSubpassBackdropFilter_Vulkan.png impeller_Play_AiksTest_CollapsedDrawPaintInSubpass_Metal.png impeller_Play_AiksTest_CollapsedDrawPaintInSubpass_OpenGLES.png impeller_Play_AiksTest_CollapsedDrawPaintInSubpass_Vulkan.png +impeller_Play_AiksTest_ColorFilterAdvancedBlendNoFbFetch_Metal.png impeller_Play_AiksTest_ColorFilterAdvancedBlend_Metal.png impeller_Play_AiksTest_ColorFilterAdvancedBlend_OpenGLES.png impeller_Play_AiksTest_ColorFilterAdvancedBlend_Vulkan.png