From 770c68e48b0182a0b4cb38819c824914a2490756 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Mon, 13 May 2024 14:42:12 -0700 Subject: [PATCH 1/3] Delete Settings::msaa_samples. Fixes https://github.com/flutter/flutter/issues/148257 --- common/graphics/BUILD.gn | 1 - common/graphics/msaa_sample_count.h | 17 ---------- common/settings.h | 7 ---- .../common/shell_test_platform_view_metal.mm | 3 +- shell/common/switches.cc | 22 ------------- shell/common/switches.h | 7 ---- shell/common/switches_unittests.cc | 18 ---------- shell/gpu/gpu_surface_metal_skia.h | 3 -- shell/gpu/gpu_surface_metal_skia.mm | 14 +++----- .../android/android_context_gl_skia.cc | 12 ++----- .../android/android_context_gl_skia.h | 3 +- .../android/android_context_gl_unittests.cc | 33 +++---------------- .../platform/android/android_shell_holder.cc | 3 +- .../embedding/engine/FlutterShellArgs.java | 6 ---- .../platform/android/platform_view_android.cc | 8 ++--- .../platform/android/platform_view_android.h | 3 +- .../engine/FlutterShellArgsTest.java | 23 ------------- .../Source/FlutterEnginePlatformViewTest.mm | 32 ------------------ shell/platform/darwin/ios/ios_context.h | 10 +----- shell/platform/darwin/ios/ios_context.mm | 6 ++-- .../darwin/ios/ios_context_metal_impeller.mm | 3 +- .../darwin/ios/ios_context_metal_skia.h | 2 +- .../darwin/ios/ios_context_metal_skia.mm | 2 +- .../darwin/ios/ios_context_software.mm | 2 +- .../darwin/ios/ios_surface_metal_skia.mm | 5 ++- .../platform/darwin/ios/platform_view_ios.mm | 18 +++++----- shell/platform/embedder/embedder.cc | 10 +++--- .../embedder/embedder_surface_metal.mm | 3 +- 28 files changed, 41 insertions(+), 235 deletions(-) delete mode 100644 common/graphics/msaa_sample_count.h diff --git a/common/graphics/BUILD.gn b/common/graphics/BUILD.gn index a9db34c011c91..d4209ec849854 100644 --- a/common/graphics/BUILD.gn +++ b/common/graphics/BUILD.gn @@ -10,7 +10,6 @@ source_set("graphics") { sources = [ "gl_context_switch.cc", "gl_context_switch.h", - "msaa_sample_count.h", "persistent_cache.cc", "persistent_cache.h", "texture.cc", diff --git a/common/graphics/msaa_sample_count.h b/common/graphics/msaa_sample_count.h deleted file mode 100644 index c1bf89c8eb18d..0000000000000 --- a/common/graphics/msaa_sample_count.h +++ /dev/null @@ -1,17 +0,0 @@ -// 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. - -#ifndef FLUTTER_COMMON_GRAPHICS_MSAA_SAMPLE_COUNT_H_ -#define FLUTTER_COMMON_GRAPHICS_MSAA_SAMPLE_COUNT_H_ - -// Supported MSAA sample count values. -enum class MsaaSampleCount { - kNone = 1, - kTwo = 2, - kFour = 4, - kEight = 8, - kSixteen = 16, -}; - -#endif // FLUTTER_COMMON_GRAPHICS_MSAA_SAMPLE_COUNT_H_ diff --git a/common/settings.h b/common/settings.h index a7b78a9c70a6e..b2872abdcd33b 100644 --- a/common/settings.h +++ b/common/settings.h @@ -346,13 +346,6 @@ struct Settings { // Max bytes threshold of resource cache, or 0 for unlimited. size_t resource_cache_max_bytes_threshold = 0; - /// The minimum number of samples to require in multipsampled anti-aliasing. - /// - /// Setting this value to 0 or 1 disables MSAA. - /// If it is not 0 or 1, it must be one of 2, 4, 8, or 16. However, if the - /// GPU does not support the requested sampling value, MSAA will be disabled. - uint8_t msaa_samples = 0; - /// Enable embedder api on the embedder. /// /// This is currently only used by iOS. diff --git a/shell/common/shell_test_platform_view_metal.mm b/shell/common/shell_test_platform_view_metal.mm index 93cf262e4a810..41322490f8252 100644 --- a/shell/common/shell_test_platform_view_metal.mm +++ b/shell/common/shell_test_platform_view_metal.mm @@ -116,8 +116,7 @@ GPUMTLTextureInfo offscreen_texture_info() const { return std::make_unique(this, [metal_context_->impeller_context() context]); } - return std::make_unique(this, [metal_context_->context() mainContext], - MsaaSampleCount::kNone); + return std::make_unique(this, [metal_context_->context() mainContext]); } // |PlatformView| diff --git a/shell/common/switches.cc b/shell/common/switches.cc index 06d49f2c48d47..6aa2b21ea8ce3 100644 --- a/shell/common/switches.cc +++ b/shell/common/switches.cc @@ -519,28 +519,6 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) { std::stoi(resource_cache_max_bytes_threshold); } - if (command_line.HasOption(FlagForSwitch(Switch::MsaaSamples))) { - std::string msaa_samples; - command_line.GetOptionValue(FlagForSwitch(Switch::MsaaSamples), - &msaa_samples); - if (msaa_samples == "0") { - settings.msaa_samples = 0; - } else if (msaa_samples == "1") { - settings.msaa_samples = 1; - } else if (msaa_samples == "2") { - settings.msaa_samples = 2; - } else if (msaa_samples == "4") { - settings.msaa_samples = 4; - } else if (msaa_samples == "8") { - settings.msaa_samples = 8; - } else if (msaa_samples == "16") { - settings.msaa_samples = 16; - } else { - FML_DLOG(ERROR) << "Invalid value for --msaa-samples: '" << msaa_samples - << "' (expected 0, 1, 2, 4, 8, or 16)."; - } - } - settings.enable_platform_isolates = command_line.HasOption(FlagForSwitch(Switch::EnablePlatformIsolates)); diff --git a/shell/common/switches.h b/shell/common/switches.h index e2a3279b46d72..9d27d16fbc6d7 100644 --- a/shell/common/switches.h +++ b/shell/common/switches.h @@ -288,13 +288,6 @@ DEF_SWITCH(LeakVM, "When the last shell shuts down, the shared VM is leaked by default " "(the leak_vm in VM settings is true). To clean up the leak VM, set " "this value to false.") -DEF_SWITCH( - MsaaSamples, - "msaa-samples", - "The minimum number of samples to require for multisampled anti-aliasing. " - "Setting this value to 0 or 1 disables MSAA. If it is not 0 or 1, it must " - "be one of 2, 4, 8, or 16. However, if the GPU does not support the " - "requested sampling value, MSAA will be disabled.") DEF_SWITCH(EnableEmbedderAPI, "enable-embedder-api", "Enable the embedder api. Defaults to false. iOS only.") diff --git a/shell/common/switches_unittests.cc b/shell/common/switches_unittests.cc index b4494a2968170..217ad9153c637 100644 --- a/shell/common/switches_unittests.cc +++ b/shell/common/switches_unittests.cc @@ -69,24 +69,6 @@ TEST(SwitchesTest, RouteParsedFlag) { EXPECT_TRUE(settings.route.empty()); } -TEST(SwitchesTest, MsaaSamples) { - for (int samples : {0, 1, 2, 4, 8, 16}) { - fml::CommandLine command_line = fml::CommandLineFromInitializerList( - {"command", ("--msaa-samples=" + std::to_string(samples)).c_str()}); - Settings settings = SettingsFromCommandLine(command_line); - EXPECT_EQ(settings.msaa_samples, samples); - } - fml::CommandLine command_line = - fml::CommandLineFromInitializerList({"command", "--msaa-samples=3"}); - Settings settings = SettingsFromCommandLine(command_line); - EXPECT_EQ(settings.msaa_samples, 0); - - command_line = - fml::CommandLineFromInitializerList({"command", "--msaa-samples=foobar"}); - settings = SettingsFromCommandLine(command_line); - EXPECT_EQ(settings.msaa_samples, 0); -} - TEST(SwitchesTest, EnableEmbedderAPI) { { // enable diff --git a/shell/gpu/gpu_surface_metal_skia.h b/shell/gpu/gpu_surface_metal_skia.h index f6f4601ce0759..ab7ec9512fe70 100644 --- a/shell/gpu/gpu_surface_metal_skia.h +++ b/shell/gpu/gpu_surface_metal_skia.h @@ -7,7 +7,6 @@ #if !SLIMPELLER -#include "flutter/common/graphics/msaa_sample_count.h" #include "flutter/flow/surface.h" #include "flutter/fml/macros.h" #include "flutter/shell/gpu/gpu_surface_metal_delegate.h" @@ -19,7 +18,6 @@ class SK_API_AVAILABLE_CA_METAL_LAYER GPUSurfaceMetalSkia : public Surface { public: GPUSurfaceMetalSkia(GPUSurfaceMetalDelegate* delegate, sk_sp context, - MsaaSampleCount msaa_samples, bool render_to_surface = true); // |Surface| @@ -33,7 +31,6 @@ class SK_API_AVAILABLE_CA_METAL_LAYER GPUSurfaceMetalSkia : public Surface { const MTLRenderTargetType render_target_type_; sk_sp context_; GrDirectContext* precompiled_sksl_context_ = nullptr; - MsaaSampleCount msaa_samples_ = MsaaSampleCount::kNone; // TODO(38466): Refactor GPU surface APIs take into account the fact that an // external view embedder may want to render to the root surface. This is a // hack to make avoid allocating resources for the root surface when an diff --git a/shell/gpu/gpu_surface_metal_skia.mm b/shell/gpu/gpu_surface_metal_skia.mm index 8c6dc133e52f7..c945c8894a70c 100644 --- a/shell/gpu/gpu_surface_metal_skia.mm +++ b/shell/gpu/gpu_surface_metal_skia.mm @@ -41,7 +41,6 @@ sk_sp CreateSurfaceFromMetalTexture(GrDirectContext* context, id texture, GrSurfaceOrigin origin, - MsaaSampleCount sample_cnt, SkColorType color_type, sk_sp color_space, const SkSurfaceProps* props, @@ -51,20 +50,18 @@ info.fTexture.reset([texture retain]); GrBackendTexture backend_texture = GrBackendTextures::MakeMtl(texture.width, texture.height, skgpu::Mipmapped::kNo, info); - return SkSurfaces::WrapBackendTexture( - context, backend_texture, origin, static_cast(sample_cnt), color_type, - std::move(color_space), props, release_proc, release_context); + return SkSurfaces::WrapBackendTexture(context, backend_texture, origin, 1, color_type, + std::move(color_space), props, release_proc, + release_context); } } // namespace GPUSurfaceMetalSkia::GPUSurfaceMetalSkia(GPUSurfaceMetalDelegate* delegate, sk_sp context, - MsaaSampleCount msaa_samples, bool render_to_surface) : delegate_(delegate), render_target_type_(delegate->GetRenderTargetType()), context_(std::move(context)), - msaa_samples_(msaa_samples), render_to_surface_(render_to_surface) { // If this preference is explicitly set, we allow for disabling partial repaint. NSNumber* disablePartialRepaint = @@ -143,7 +140,6 @@ auto surface = CreateSurfaceFromMetalTexture(context_.get(), drawable.get().texture, kTopLeft_GrSurfaceOrigin, // origin - msaa_samples_, // sample count kBGRA_8888_SkColorType, // color type nullptr, // colorspace nullptr, // surface properties @@ -215,8 +211,8 @@ } sk_sp surface = CreateSurfaceFromMetalTexture( - context_.get(), mtl_texture, kTopLeft_GrSurfaceOrigin, msaa_samples_, kBGRA_8888_SkColorType, - nullptr, nullptr, static_cast(texture.destruction_callback), + context_.get(), mtl_texture, kTopLeft_GrSurfaceOrigin, kBGRA_8888_SkColorType, nullptr, + nullptr, static_cast(texture.destruction_callback), texture.destruction_context); if (!surface) { diff --git a/shell/platform/android/android_context_gl_skia.cc b/shell/platform/android/android_context_gl_skia.cc index 8ad49cf82c5e8..652cefc864f7f 100644 --- a/shell/platform/android/android_context_gl_skia.cc +++ b/shell/platform/android/android_context_gl_skia.cc @@ -24,9 +24,7 @@ static EGLResult CreateContext(EGLDisplay display, return {context != EGL_NO_CONTEXT, context}; } -static EGLResult ChooseEGLConfiguration(EGLDisplay display, - uint8_t msaa_samples) { - EGLint sample_buffers = msaa_samples > 1 ? 1 : 0; +static EGLResult ChooseEGLConfiguration(EGLDisplay display) { EGLint attributes[] = { // clang-format off EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT, @@ -37,8 +35,6 @@ static EGLResult ChooseEGLConfiguration(EGLDisplay display, EGL_ALPHA_SIZE, 8, EGL_DEPTH_SIZE, 0, EGL_STENCIL_SIZE, 0, - EGL_SAMPLES, static_cast(msaa_samples), - EGL_SAMPLE_BUFFERS, sample_buffers, EGL_NONE, // termination sentinel // clang-format on }; @@ -66,8 +62,7 @@ static bool TeardownContext(EGLDisplay display, EGLContext context) { AndroidContextGLSkia::AndroidContextGLSkia( fml::RefPtr environment, - const TaskRunners& task_runners, - uint8_t msaa_samples) + const TaskRunners& task_runners) : AndroidContext(AndroidRenderingAPI::kSkiaOpenGLES), environment_(std::move(environment)), task_runners_(task_runners) { @@ -79,8 +74,7 @@ AndroidContextGLSkia::AndroidContextGLSkia( bool success = false; // Choose a valid configuration. - std::tie(success, config_) = - ChooseEGLConfiguration(environment_->Display(), msaa_samples); + std::tie(success, config_) = ChooseEGLConfiguration(environment_->Display()); if (!success) { FML_LOG(ERROR) << "Could not choose an EGL configuration."; LogLastEGLError(); diff --git a/shell/platform/android/android_context_gl_skia.h b/shell/platform/android/android_context_gl_skia.h index af19ef789d8de..d5dc6d17dbe0d 100644 --- a/shell/platform/android/android_context_gl_skia.h +++ b/shell/platform/android/android_context_gl_skia.h @@ -28,8 +28,7 @@ class AndroidEGLSurface; class AndroidContextGLSkia : public AndroidContext { public: AndroidContextGLSkia(fml::RefPtr environment, - const TaskRunners& taskRunners, - uint8_t msaa_samples); + const TaskRunners& taskRunners); ~AndroidContextGLSkia(); diff --git a/shell/platform/android/android_context_gl_unittests.cc b/shell/platform/android/android_context_gl_unittests.cc index 0bc2848f292b4..5173f59d673db 100644 --- a/shell/platform/android/android_context_gl_unittests.cc +++ b/shell/platform/android/android_context_gl_unittests.cc @@ -109,7 +109,7 @@ TEST(AndroidContextGl, Create) { ThreadHost::Type::kIo)); TaskRunners task_runners = MakeTaskRunners(thread_label, thread_host); auto context = - std::make_unique(environment, task_runners, 0); + std::make_unique(environment, task_runners); context->SetMainSkiaContext(main_context); EXPECT_NE(context.get(), nullptr); context.reset(); @@ -141,7 +141,7 @@ TEST(AndroidContextGl, CreateSingleThread) { TaskRunners(thread_label, platform_runner, platform_runner, platform_runner, platform_runner); auto context = - std::make_unique(environment, task_runners, 0); + std::make_unique(environment, task_runners); context->SetMainSkiaContext(main_context); EXPECT_NE(context.get(), nullptr); context.reset(); @@ -160,7 +160,7 @@ TEST(AndroidSurfaceGL, CreateSnapshopSurfaceWhenOnscreenSurfaceIsNotNull) { ThreadHost::Type::kIo)); TaskRunners task_runners = MakeTaskRunners(thread_label, thread_host); auto android_context = - std::make_shared(environment, task_runners, 0); + std::make_shared(environment, task_runners); auto android_surface = std::make_unique(android_context); auto window = fml::MakeRefCounted( @@ -187,7 +187,7 @@ TEST(AndroidSurfaceGL, CreateSnapshopSurfaceWhenOnscreenSurfaceIsNull) { ThreadHost thread_host(host_config); TaskRunners task_runners = MakeTaskRunners(thread_label, thread_host); auto android_context = - std::make_shared(environment, task_runners, 0); + std::make_shared(environment, task_runners); auto android_surface = std::make_unique(android_context); EXPECT_EQ(android_surface->GetOnscreenSurface(), nullptr); @@ -195,29 +195,6 @@ TEST(AndroidSurfaceGL, CreateSnapshopSurfaceWhenOnscreenSurfaceIsNull) { EXPECT_NE(android_surface->GetOnscreenSurface(), nullptr); } -// TODO(https://github.com/flutter/flutter/issues/104463): Flaky test. -TEST(AndroidContextGl, DISABLED_MSAAx4) { - GrMockOptions main_context_options; - sk_sp main_context = - GrDirectContext::MakeMock(&main_context_options); - auto environment = fml::MakeRefCounted(); - std::string thread_label = - ::testing::UnitTest::GetInstance()->current_test_info()->name(); - - ThreadHost thread_host(ThreadHost::ThreadHostConfig( - thread_label, ThreadHost::Type::kUi | ThreadHost::Type::kRaster | - ThreadHost::Type::kIo)); - TaskRunners task_runners = MakeTaskRunners(thread_label, thread_host); - auto context = - std::make_unique(environment, task_runners, 4); - context->SetMainSkiaContext(main_context); - - EGLint sample_count; - eglGetConfigAttrib(environment->Display(), context->Config(), EGL_SAMPLES, - &sample_count); - EXPECT_EQ(sample_count, 4); -} - TEST(AndroidContextGl, EnsureMakeCurrentChecksCurrentContextStatus) { GrMockOptions main_context_options; sk_sp main_context = @@ -231,7 +208,7 @@ TEST(AndroidContextGl, EnsureMakeCurrentChecksCurrentContextStatus) { ThreadHost::Type::kIo)); TaskRunners task_runners = MakeTaskRunners(thread_label, thread_host); auto context = - std::make_unique(environment, task_runners, 0); + std::make_unique(environment, task_runners); auto pbuffer_surface = context->CreatePbufferSurface(); auto status = pbuffer_surface->MakeCurrent(); diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index e732958906328..3f425b73a62d5 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -118,8 +118,7 @@ AndroidShellHolder::AndroidShellHolder( shell.GetTaskRunners(), // task runners jni_facade, // JNI interop shell.GetSettings() - .enable_software_rendering, // use software rendering - shell.GetSettings().msaa_samples // msaa sample count + .enable_software_rendering // use software rendering ); weak_platform_view = platform_view_android->GetWeakPtr(); return platform_view_android; diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterShellArgs.java b/shell/platform/android/io/flutter/embedding/engine/FlutterShellArgs.java index b3775217a86fe..36a40d3923295 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterShellArgs.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterShellArgs.java @@ -65,8 +65,6 @@ public class FlutterShellArgs { public static final String ARG_KEY_OBSERVATORY_PORT = "observatory-port"; public static final String ARG_KEY_DART_FLAGS = "dart-flags"; public static final String ARG_DART_FLAGS = "--dart-flags"; - public static final String ARG_KEY_MSAA_SAMPLES = "msaa-samples"; - public static final String ARG_MSAA_SAMPLES = "--msaa-samples"; @NonNull public static FlutterShellArgs fromIntent(@NonNull Intent intent) { @@ -143,10 +141,6 @@ public static FlutterShellArgs fromIntent(@NonNull Intent intent) { if (intent.getBooleanExtra(ARG_KEY_VERBOSE_LOGGING, false)) { args.add(ARG_VERBOSE_LOGGING); } - final int msaaSamples = intent.getIntExtra("msaa-samples", 0); - if (msaaSamples > 1) { - args.add("--msaa-samples=" + Integer.toString(msaaSamples)); - } // NOTE: all flags provided with this argument are subject to filtering // based on a list of allowed flags in shell/common/switches.cc. If any diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 15abb985bb73a..64b39cbda1dae 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -63,7 +63,6 @@ std::unique_ptr AndroidSurfaceFactoryImpl::CreateSurface() { static std::shared_ptr CreateAndroidContext( bool use_software_rendering, const flutter::TaskRunners& task_runners, - uint8_t msaa_samples, AndroidRenderingAPI android_rendering_api, bool enable_vulkan_validation, bool enable_opengl_gpu_tracing, @@ -81,8 +80,7 @@ static std::shared_ptr CreateAndroidContext( case AndroidRenderingAPI::kSkiaOpenGLES: return std::make_unique( fml::MakeRefCounted(), // - task_runners, // - msaa_samples // + task_runners // ); } FML_UNREACHABLE(); @@ -92,8 +90,7 @@ PlatformViewAndroid::PlatformViewAndroid( PlatformView::Delegate& delegate, const flutter::TaskRunners& task_runners, const std::shared_ptr& jni_facade, - bool use_software_rendering, - uint8_t msaa_samples) + bool use_software_rendering) : PlatformViewAndroid( delegate, task_runners, @@ -101,7 +98,6 @@ PlatformViewAndroid::PlatformViewAndroid( CreateAndroidContext( use_software_rendering, task_runners, - msaa_samples, delegate.OnPlatformViewGetSettings().android_rendering_api, delegate.OnPlatformViewGetSettings().enable_vulkan_validation, delegate.OnPlatformViewGetSettings().enable_opengl_gpu_tracing, diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index d5f34ffc777f7..5310826c52e24 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -46,8 +46,7 @@ class PlatformViewAndroid final : public PlatformView { PlatformViewAndroid(PlatformView::Delegate& delegate, const flutter::TaskRunners& task_runners, const std::shared_ptr& jni_facade, - bool use_software_rendering, - uint8_t msaa_samples); + bool use_software_rendering); //---------------------------------------------------------------------------- /// @brief Creates a new PlatformViewAndroid but using an existing diff --git a/shell/platform/android/test/io/flutter/embedding/engine/FlutterShellArgsTest.java b/shell/platform/android/test/io/flutter/embedding/engine/FlutterShellArgsTest.java index 334b88f73aa1e..954b857a89fe5 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/FlutterShellArgsTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/FlutterShellArgsTest.java @@ -31,27 +31,4 @@ public void itProcessesShellFlags() { assertTrue(argValues.contains("--dart-flags=--observe --no-hot --no-pub")); assertTrue(argValues.contains("--trace-skia-allowlist=skia.a,skia.b")); } - - @Test - public void itHandles4xMsaaFlag() { - Intent intent = new Intent(); - intent.putExtra("msaa-samples", 4); - - FlutterShellArgs args = FlutterShellArgs.fromIntent(intent); - HashSet argValues = new HashSet(Arrays.asList(args.toArray())); - - assertEquals(1, argValues.size()); - assertTrue(argValues.contains("--msaa-samples=4")); - } - - @Test - public void itHandles1xMsaaFlag() { - Intent intent = new Intent(); - intent.putExtra("msaa-samples", 1); - - FlutterShellArgs args = FlutterShellArgs.fromIntent(intent); - HashSet argValues = new HashSet(Arrays.asList(args.toArray())); - - assertEquals(0, argValues.size()); - } } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm index 2e2515e3eca3e..5ff2ee11e12f6 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm @@ -96,38 +96,6 @@ - (void)tearDown { return weak_factory->GetWeakPtr(); } -- (void)testMsaaSampleCount { - if (fake_delegate.settings_.enable_impeller) { - // Default should be 4 for Impeller. - XCTAssertEqual(platform_view->GetIosContext()->GetMsaaSampleCount(), MsaaSampleCount::kFour); - } else { - // Default should be 1 for Skia. - XCTAssertEqual(platform_view->GetIosContext()->GetMsaaSampleCount(), MsaaSampleCount::kNone); - } - - // Verify the platform view creates a new context with updated msaa_samples. - // Need to use Metal, since this is ignored for Software/GL. - fake_delegate.settings_.msaa_samples = 4; - - auto thread_task_runner = fml::MessageLoop::GetCurrent().GetTaskRunner(); - auto sync_switch = std::make_shared(); - flutter::TaskRunners runners(/*label=*/self.name.UTF8String, - /*platform=*/thread_task_runner, - /*raster=*/thread_task_runner, - /*ui=*/thread_task_runner, - /*io=*/thread_task_runner); - auto msaa_4x_platform_view = std::make_unique( - /*delegate=*/fake_delegate, - /*rendering_api=*/flutter::IOSRenderingAPI::kMetal, - /*platform_views_controller=*/nil, - /*task_runners=*/runners, - /*worker_task_runner=*/nil, - /*is_gpu_disabled_sync_switch=*/sync_switch); - - XCTAssertEqual(msaa_4x_platform_view->GetIosContext()->GetMsaaSampleCount(), - MsaaSampleCount::kFour); -} - - (void)testCallsNotifyLowMemory { FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"tester"]; XCTAssertNotNil(engine); diff --git a/shell/platform/darwin/ios/ios_context.h b/shell/platform/darwin/ios/ios_context.h index 04743b4c3a63b..d142e142d2ab8 100644 --- a/shell/platform/darwin/ios/ios_context.h +++ b/shell/platform/darwin/ios/ios_context.h @@ -8,7 +8,6 @@ #include #include "flutter/common/graphics/gl_context_switch.h" -#include "flutter/common/graphics/msaa_sample_count.h" #include "flutter/common/graphics/texture.h" #include "flutter/fml/concurrent_message_loop.h" #include "flutter/fml/macros.h" @@ -50,16 +49,12 @@ class IOSContext { /// engine/platform. /// @param[in] backend A client rendering backend supported by the /// engine/platform. - /// @param[in] msaa_samples - /// The number of MSAA samples to use. Only supplied to - /// Skia, must be either 0, 1, 2, 4, or 8. /// /// @return A valid context on success. `nullptr` on failure. /// static std::unique_ptr Create( IOSRenderingAPI api, IOSRenderingBackend backend, - MsaaSampleCount msaa_samples, const std::shared_ptr& is_gpu_disabled_sync_switch); //---------------------------------------------------------------------------- @@ -145,13 +140,10 @@ class IOSContext { virtual std::shared_ptr GetImpellerContext() const; - MsaaSampleCount GetMsaaSampleCount() const { return msaa_samples_; } - protected: - explicit IOSContext(MsaaSampleCount msaa_samples); + explicit IOSContext(); private: - MsaaSampleCount msaa_samples_ = MsaaSampleCount::kNone; FML_DISALLOW_COPY_AND_ASSIGN(IOSContext); }; diff --git a/shell/platform/darwin/ios/ios_context.mm b/shell/platform/darwin/ios/ios_context.mm index f425b7e2e193a..7ddd252b64852 100644 --- a/shell/platform/darwin/ios/ios_context.mm +++ b/shell/platform/darwin/ios/ios_context.mm @@ -14,14 +14,13 @@ namespace flutter { -IOSContext::IOSContext(MsaaSampleCount msaa_samples) : msaa_samples_(msaa_samples) {} +IOSContext::IOSContext() = default; IOSContext::~IOSContext() = default; std::unique_ptr IOSContext::Create( IOSRenderingAPI api, IOSRenderingBackend backend, - MsaaSampleCount msaa_samples, const std::shared_ptr& is_gpu_disabled_sync_switch) { switch (api) { case IOSRenderingAPI::kSoftware: @@ -35,8 +34,7 @@ switch (backend) { case IOSRenderingBackend::kSkia: #if !SLIMPELLER - return std::make_unique(msaa_samples); - + return std::make_unique(); #else // !SLIMPELLER FML_LOG(FATAL) << "Impeller opt-out unavailable."; return nullptr; diff --git a/shell/platform/darwin/ios/ios_context_metal_impeller.mm b/shell/platform/darwin/ios/ios_context_metal_impeller.mm index 146980c341746..92a59a8326c96 100644 --- a/shell/platform/darwin/ios/ios_context_metal_impeller.mm +++ b/shell/platform/darwin/ios/ios_context_metal_impeller.mm @@ -12,8 +12,7 @@ IOSContextMetalImpeller::IOSContextMetalImpeller( const std::shared_ptr& is_gpu_disabled_sync_switch) - : IOSContext(MsaaSampleCount::kFour), - darwin_context_metal_impeller_(fml::scoped_nsobject{ + : darwin_context_metal_impeller_(fml::scoped_nsobject{ [[FlutterDarwinContextMetalImpeller alloc] init:is_gpu_disabled_sync_switch]}) {} IOSContextMetalImpeller::~IOSContextMetalImpeller() = default; diff --git a/shell/platform/darwin/ios/ios_context_metal_skia.h b/shell/platform/darwin/ios/ios_context_metal_skia.h index 32e67e4744a38..c69aee44a6fe6 100644 --- a/shell/platform/darwin/ios/ios_context_metal_skia.h +++ b/shell/platform/darwin/ios/ios_context_metal_skia.h @@ -20,7 +20,7 @@ namespace flutter { class IOSContextMetalSkia final : public IOSContext { public: - explicit IOSContextMetalSkia(MsaaSampleCount msaa_samples); + explicit IOSContextMetalSkia(); ~IOSContextMetalSkia(); diff --git a/shell/platform/darwin/ios/ios_context_metal_skia.mm b/shell/platform/darwin/ios/ios_context_metal_skia.mm index 9bdfa1a405a76..9dd4db5ea211f 100644 --- a/shell/platform/darwin/ios/ios_context_metal_skia.mm +++ b/shell/platform/darwin/ios/ios_context_metal_skia.mm @@ -16,7 +16,7 @@ namespace flutter { -IOSContextMetalSkia::IOSContextMetalSkia(MsaaSampleCount msaa_samples) : IOSContext(msaa_samples) { +IOSContextMetalSkia::IOSContextMetalSkia() { darwin_context_metal_ = fml::scoped_nsobject{ [[FlutterDarwinContextMetalSkia alloc] initWithDefaultMTLDevice]}; } diff --git a/shell/platform/darwin/ios/ios_context_software.mm b/shell/platform/darwin/ios/ios_context_software.mm index 2c24ce6e158fc..dcf63c223d942 100644 --- a/shell/platform/darwin/ios/ios_context_software.mm +++ b/shell/platform/darwin/ios/ios_context_software.mm @@ -9,7 +9,7 @@ namespace flutter { -IOSContextSoftware::IOSContextSoftware() : IOSContext(MsaaSampleCount::kNone) {} +IOSContextSoftware::IOSContextSoftware() = default; // |IOSContext| IOSContextSoftware::~IOSContextSoftware() = default; diff --git a/shell/platform/darwin/ios/ios_surface_metal_skia.mm b/shell/platform/darwin/ios/ios_surface_metal_skia.mm index b16daf45e6dbe..32465e2153201 100644 --- a/shell/platform/darwin/ios/ios_surface_metal_skia.mm +++ b/shell/platform/darwin/ios/ios_surface_metal_skia.mm @@ -51,9 +51,8 @@ - (void)flutterPrepareForPresent:(nonnull id)commandBuffer; // |IOSSurface| std::unique_ptr IOSSurfaceMetalSkia::CreateGPUSurface(GrDirectContext* context) { FML_DCHECK(context); - return std::make_unique(this, // delegate - sk_ref_sp(context), // context - GetContext()->GetMsaaSampleCount() // sample count + return std::make_unique(this, // delegate + sk_ref_sp(context) // context ); } diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index 262df21d3c07e..4cda02ca7f3e6 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -58,16 +58,14 @@ new PlatformMessageHandlerIos(task_runners.GetPlatformTaskRunner())) {} const flutter::TaskRunners& task_runners, const std::shared_ptr& worker_task_runner, const std::shared_ptr& is_gpu_disabled_sync_switch) - : PlatformViewIOS( - delegate, - IOSContext::Create( - rendering_api, - delegate.OnPlatformViewGetSettings().enable_impeller ? IOSRenderingBackend::kImpeller - : IOSRenderingBackend::kSkia, - static_cast(delegate.OnPlatformViewGetSettings().msaa_samples), - is_gpu_disabled_sync_switch), - platform_views_controller, - task_runners) {} + : PlatformViewIOS(delegate, + IOSContext::Create(rendering_api, + delegate.OnPlatformViewGetSettings().enable_impeller + ? IOSRenderingBackend::kImpeller + : IOSRenderingBackend::kSkia, + is_gpu_disabled_sync_switch), + platform_views_controller, + task_runners) {} PlatformViewIOS::~PlatformViewIOS() = default; diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index d81373edaacdc..2d156a64e5b42 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -939,12 +939,10 @@ static sk_sp MakeSkSurfaceFromBackingStore( context, // context backend_texture, // back-end texture kTopLeft_GrSurfaceOrigin, // surface origin - // TODO(dnfield): Update this when embedders support MSAA, see - // https://github.com/flutter/flutter/issues/100392 - 1, // sample count - kBGRA_8888_SkColorType, // color type - nullptr, // color space - &surface_properties, // surface properties + 1, // sample count + kBGRA_8888_SkColorType, // color type + nullptr, // color space + &surface_properties, // surface properties static_cast( metal->texture.destruction_callback), // release proc metal->texture.user_data // release context diff --git a/shell/platform/embedder/embedder_surface_metal.mm b/shell/platform/embedder/embedder_surface_metal.mm index 58c08e47d4fdd..771f9f2c26372 100644 --- a/shell/platform/embedder/embedder_surface_metal.mm +++ b/shell/platform/embedder/embedder_surface_metal.mm @@ -50,8 +50,7 @@ } const bool render_to_surface = !external_view_embedder_; - auto surface = std::make_unique(this, main_context_, MsaaSampleCount::kNone, - render_to_surface); + auto surface = std::make_unique(this, main_context_, render_to_surface); if (!surface->IsValid()) { return nullptr; From 11b055c7590f0cb90c349f5b3d88483e557165cb Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Mon, 13 May 2024 15:28:16 -0700 Subject: [PATCH 2/3] PR --- shell/platform/embedder/embedder_surface_metal.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/embedder/embedder_surface_metal.mm b/shell/platform/embedder/embedder_surface_metal.mm index 771f9f2c26372..e11de9153cc0a 100644 --- a/shell/platform/embedder/embedder_surface_metal.mm +++ b/shell/platform/embedder/embedder_surface_metal.mm @@ -8,7 +8,6 @@ #include "flutter/shell/platform/embedder/embedder_surface_metal.h" -#include "flutter/common/graphics/msaa_sample_count.h" #include "flutter/fml/logging.h" #include "flutter/shell/gpu/gpu_surface_metal_delegate.h" #include "flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalSkia.h" From 9f888963cd6cf8c0f8a234325312610e18228ac6 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Mon, 13 May 2024 15:50:42 -0700 Subject: [PATCH 3/3] PR --- ci/licenses_golden/licenses_flutter | 2 -- 1 file changed, 2 deletions(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index b5e1ce6b66ea5..1573a261b3a93 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -39872,7 +39872,6 @@ ORIGIN: ../../../flutter/benchmarking/library.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/common/constants.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/common/graphics/gl_context_switch.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/common/graphics/gl_context_switch.h + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/common/graphics/msaa_sample_count.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/common/graphics/persistent_cache.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/common/graphics/persistent_cache.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/common/graphics/texture.cc + ../../../flutter/LICENSE @@ -42748,7 +42747,6 @@ FILE: ../../../flutter/common/exported_test_symbols.sym FILE: ../../../flutter/common/exported_test_symbols_mac.sym FILE: ../../../flutter/common/graphics/gl_context_switch.cc FILE: ../../../flutter/common/graphics/gl_context_switch.h -FILE: ../../../flutter/common/graphics/msaa_sample_count.h FILE: ../../../flutter/common/graphics/persistent_cache.cc FILE: ../../../flutter/common/graphics/persistent_cache.h FILE: ../../../flutter/common/graphics/texture.cc