Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete Settings::msaa_samples. #52780

Merged
merged 3 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion common/graphics/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
17 changes: 0 additions & 17 deletions common/graphics/msaa_sample_count.h

This file was deleted.

7 changes: 0 additions & 7 deletions common/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 1 addition & 2 deletions shell/common/shell_test_platform_view_metal.mm
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,7 @@ GPUMTLTextureInfo offscreen_texture_info() const {
return std::make_unique<GPUSurfaceMetalImpeller>(this,
[metal_context_->impeller_context() context]);
}
return std::make_unique<GPUSurfaceMetalSkia>(this, [metal_context_->context() mainContext],
MsaaSampleCount::kNone);
return std::make_unique<GPUSurfaceMetalSkia>(this, [metal_context_->context() mainContext]);
}

// |PlatformView|
Expand Down
22 changes: 0 additions & 22 deletions shell/common/switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
7 changes: 0 additions & 7 deletions shell/common/switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down
18 changes: 0 additions & 18 deletions shell/common/switches_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions shell/gpu/gpu_surface_metal_skia.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -19,7 +18,6 @@ class SK_API_AVAILABLE_CA_METAL_LAYER GPUSurfaceMetalSkia : public Surface {
public:
GPUSurfaceMetalSkia(GPUSurfaceMetalDelegate* delegate,
sk_sp<GrDirectContext> context,
MsaaSampleCount msaa_samples,
bool render_to_surface = true);

// |Surface|
Expand All @@ -33,7 +31,6 @@ class SK_API_AVAILABLE_CA_METAL_LAYER GPUSurfaceMetalSkia : public Surface {
const MTLRenderTargetType render_target_type_;
sk_sp<GrDirectContext> 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
Expand Down
14 changes: 5 additions & 9 deletions shell/gpu/gpu_surface_metal_skia.mm
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
sk_sp<SkSurface> CreateSurfaceFromMetalTexture(GrDirectContext* context,
id<MTLTexture> texture,
GrSurfaceOrigin origin,
MsaaSampleCount sample_cnt,
SkColorType color_type,
sk_sp<SkColorSpace> color_space,
const SkSurfaceProps* props,
Expand All @@ -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<int>(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<GrDirectContext> 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 =
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -215,8 +211,8 @@
}

sk_sp<SkSurface> surface = CreateSurfaceFromMetalTexture(
context_.get(), mtl_texture, kTopLeft_GrSurfaceOrigin, msaa_samples_, kBGRA_8888_SkColorType,
nullptr, nullptr, static_cast<SkSurfaces::TextureReleaseProc>(texture.destruction_callback),
context_.get(), mtl_texture, kTopLeft_GrSurfaceOrigin, kBGRA_8888_SkColorType, nullptr,
nullptr, static_cast<SkSurfaces::TextureReleaseProc>(texture.destruction_callback),
texture.destruction_context);

if (!surface) {
Expand Down
12 changes: 3 additions & 9 deletions shell/platform/android/android_context_gl_skia.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ static EGLResult<EGLContext> CreateContext(EGLDisplay display,
return {context != EGL_NO_CONTEXT, context};
}

static EGLResult<EGLConfig> ChooseEGLConfiguration(EGLDisplay display,
uint8_t msaa_samples) {
EGLint sample_buffers = msaa_samples > 1 ? 1 : 0;
static EGLResult<EGLConfig> ChooseEGLConfiguration(EGLDisplay display) {
EGLint attributes[] = {
// clang-format off
EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
Expand All @@ -37,8 +35,6 @@ static EGLResult<EGLConfig> ChooseEGLConfiguration(EGLDisplay display,
EGL_ALPHA_SIZE, 8,
EGL_DEPTH_SIZE, 0,
EGL_STENCIL_SIZE, 0,
EGL_SAMPLES, static_cast<EGLint>(msaa_samples),
EGL_SAMPLE_BUFFERS, sample_buffers,
EGL_NONE, // termination sentinel
// clang-format on
};
Expand Down Expand Up @@ -66,8 +62,7 @@ static bool TeardownContext(EGLDisplay display, EGLContext context) {

AndroidContextGLSkia::AndroidContextGLSkia(
fml::RefPtr<AndroidEnvironmentGL> environment,
const TaskRunners& task_runners,
uint8_t msaa_samples)
const TaskRunners& task_runners)
: AndroidContext(AndroidRenderingAPI::kSkiaOpenGLES),
environment_(std::move(environment)),
task_runners_(task_runners) {
Expand All @@ -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();
Expand Down
3 changes: 1 addition & 2 deletions shell/platform/android/android_context_gl_skia.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ class AndroidEGLSurface;
class AndroidContextGLSkia : public AndroidContext {
public:
AndroidContextGLSkia(fml::RefPtr<AndroidEnvironmentGL> environment,
const TaskRunners& taskRunners,
uint8_t msaa_samples);
const TaskRunners& taskRunners);

~AndroidContextGLSkia();

Expand Down
33 changes: 5 additions & 28 deletions shell/platform/android/android_context_gl_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ TEST(AndroidContextGl, Create) {
ThreadHost::Type::kIo));
TaskRunners task_runners = MakeTaskRunners(thread_label, thread_host);
auto context =
std::make_unique<AndroidContextGLSkia>(environment, task_runners, 0);
std::make_unique<AndroidContextGLSkia>(environment, task_runners);
context->SetMainSkiaContext(main_context);
EXPECT_NE(context.get(), nullptr);
context.reset();
Expand Down Expand Up @@ -141,7 +141,7 @@ TEST(AndroidContextGl, CreateSingleThread) {
TaskRunners(thread_label, platform_runner, platform_runner,
platform_runner, platform_runner);
auto context =
std::make_unique<AndroidContextGLSkia>(environment, task_runners, 0);
std::make_unique<AndroidContextGLSkia>(environment, task_runners);
context->SetMainSkiaContext(main_context);
EXPECT_NE(context.get(), nullptr);
context.reset();
Expand All @@ -160,7 +160,7 @@ TEST(AndroidSurfaceGL, CreateSnapshopSurfaceWhenOnscreenSurfaceIsNotNull) {
ThreadHost::Type::kIo));
TaskRunners task_runners = MakeTaskRunners(thread_label, thread_host);
auto android_context =
std::make_shared<AndroidContextGLSkia>(environment, task_runners, 0);
std::make_shared<AndroidContextGLSkia>(environment, task_runners);
auto android_surface =
std::make_unique<AndroidSurfaceGLSkia>(android_context);
auto window = fml::MakeRefCounted<AndroidNativeWindow>(
Expand All @@ -187,37 +187,14 @@ TEST(AndroidSurfaceGL, CreateSnapshopSurfaceWhenOnscreenSurfaceIsNull) {
ThreadHost thread_host(host_config);
TaskRunners task_runners = MakeTaskRunners(thread_label, thread_host);
auto android_context =
std::make_shared<AndroidContextGLSkia>(environment, task_runners, 0);
std::make_shared<AndroidContextGLSkia>(environment, task_runners);
auto android_surface =
std::make_unique<AndroidSurfaceGLSkia>(android_context);
EXPECT_EQ(android_surface->GetOnscreenSurface(), nullptr);
android_surface->CreateSnapshotSurface();
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<GrDirectContext> main_context =
GrDirectContext::MakeMock(&main_context_options);
auto environment = fml::MakeRefCounted<AndroidEnvironmentGL>();
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<AndroidContextGLSkia>(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<GrDirectContext> main_context =
Expand All @@ -231,7 +208,7 @@ TEST(AndroidContextGl, EnsureMakeCurrentChecksCurrentContextStatus) {
ThreadHost::Type::kIo));
TaskRunners task_runners = MakeTaskRunners(thread_label, thread_host);
auto context =
std::make_unique<AndroidContextGLSkia>(environment, task_runners, 0);
std::make_unique<AndroidContextGLSkia>(environment, task_runners);

auto pbuffer_surface = context->CreatePbufferSurface();
auto status = pbuffer_surface->MakeCurrent();
Expand Down
3 changes: 1 addition & 2 deletions shell/platform/android/android_shell_holder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions shell/platform/android/platform_view_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ std::unique_ptr<AndroidSurface> AndroidSurfaceFactoryImpl::CreateSurface() {
static std::shared_ptr<flutter::AndroidContext> 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,
Expand All @@ -81,8 +80,7 @@ static std::shared_ptr<flutter::AndroidContext> CreateAndroidContext(
case AndroidRenderingAPI::kSkiaOpenGLES:
return std::make_unique<AndroidContextGLSkia>(
fml::MakeRefCounted<AndroidEnvironmentGL>(), //
task_runners, //
msaa_samples //
task_runners //
);
}
FML_UNREACHABLE();
Expand All @@ -92,16 +90,14 @@ PlatformViewAndroid::PlatformViewAndroid(
PlatformView::Delegate& delegate,
const flutter::TaskRunners& task_runners,
const std::shared_ptr<PlatformViewAndroidJNI>& jni_facade,
bool use_software_rendering,
uint8_t msaa_samples)
bool use_software_rendering)
: PlatformViewAndroid(
delegate,
task_runners,
jni_facade,
CreateAndroidContext(
use_software_rendering,
task_runners,
msaa_samples,
delegate.OnPlatformViewGetSettings().android_rendering_api,
delegate.OnPlatformViewGetSettings().enable_vulkan_validation,
delegate.OnPlatformViewGetSettings().enable_opengl_gpu_tracing,
Expand Down
3 changes: 1 addition & 2 deletions shell/platform/android/platform_view_android.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ class PlatformViewAndroid final : public PlatformView {
PlatformViewAndroid(PlatformView::Delegate& delegate,
const flutter::TaskRunners& task_runners,
const std::shared_ptr<PlatformViewAndroidJNI>& jni_facade,
bool use_software_rendering,
uint8_t msaa_samples);
bool use_software_rendering);

//----------------------------------------------------------------------------
/// @brief Creates a new PlatformViewAndroid but using an existing
Expand Down
Loading