From 6f3b341232a7d4af7bcd276808bb83e3bf63adbb Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Sun, 20 Aug 2023 14:10:13 -0700 Subject: [PATCH] Revert "Implementing TextScaler for nonlinear text scaling" (flutter/engine#44882) Reverts flutter/engine#42062 Failing due to: >>>>>>>> 08-18 15:59:41.350 3439 3484 E flutter : [ERROR:flutter/shell/platform/android/platform_view_android_jni_impl.cc(902)] Could not locate FlutterJNI#getScaledFontSize method >>>>>>>> 08-18 15:59:41.350 3439 3484 F flutter : [FATAL:flutter/shell/platform/android/library_loader.cc(26)] Check failed: result. --- engine/src/flutter/lib/ui/dart_ui.cc | 1 - .../flutter/lib/ui/platform_dispatcher.dart | 130 +------------- .../lib/ui/window/platform_configuration.cc | 9 - .../lib/ui/window/platform_configuration.h | 25 --- .../lib/web_ui/lib/platform_dispatcher.dart | 2 - .../lib/src/engine/platform_dispatcher.dart | 3 - .../src/flutter/runtime/runtime_controller.cc | 5 - .../src/flutter/runtime/runtime_controller.h | 4 - engine/src/flutter/runtime/runtime_delegate.h | 3 - engine/src/flutter/shell/common/engine.cc | 5 - engine/src/flutter/shell/common/engine.h | 26 --- .../flutter/shell/common/engine_unittests.cc | 4 - .../src/flutter/shell/common/platform_view.cc | 8 - .../src/flutter/shell/common/platform_view.h | 22 --- engine/src/flutter/shell/common/shell.cc | 7 - engine/src/flutter/shell/common/shell.h | 4 - .../android/android_shell_holder_unittests.cc | 2 - .../embedding/android/FlutterView.java | 1 - .../flutter/embedding/engine/FlutterJNI.java | 17 -- .../systemchannels/SettingsChannel.java | 166 +----------------- .../shell/platform/android/jni/jni_mock.h | 5 - .../android/jni/platform_view_android_jni.h | 3 - .../platform/android/platform_view_android.cc | 5 - .../platform/android/platform_view_android.h | 3 - .../android/platform_view_android_jni_impl.cc | 27 --- .../android/platform_view_android_jni_impl.h | 3 - ...lutterActivityAndFragmentDelegateTest.java | 1 - .../systemchannels/SettingsChannelTest.java | 70 -------- .../src/flutter/testing/dart/window_test.dart | 8 - 29 files changed, 8 insertions(+), 561 deletions(-) delete mode 100644 engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/systemchannels/SettingsChannelTest.java diff --git a/engine/src/flutter/lib/ui/dart_ui.cc b/engine/src/flutter/lib/ui/dart_ui.cc index d25754fa96..02a6b79978 100644 --- a/engine/src/flutter/lib/ui/dart_ui.cc +++ b/engine/src/flutter/lib/ui/dart_ui.cc @@ -110,7 +110,6 @@ typedef CanvasPath Path; V(PlatformConfigurationNativeApi::GetRootIsolateToken, 0) \ V(PlatformConfigurationNativeApi::RegisterBackgroundIsolate, 1) \ V(PlatformConfigurationNativeApi::SendPortPlatformMessage, 4) \ - V(PlatformConfigurationNativeApi::GetScaledFontSize, 2) \ V(DartRuntimeHooks::Logger_PrintDebugString, 1) \ V(DartRuntimeHooks::Logger_PrintString, 1) \ V(DartRuntimeHooks::ScheduleMicrotask, 1) \ diff --git a/engine/src/flutter/lib/ui/platform_dispatcher.dart b/engine/src/flutter/lib/ui/platform_dispatcher.dart index 6d86cc6270..c8ebb246ae 100644 --- a/engine/src/flutter/lib/ui/platform_dispatcher.dart +++ b/engine/src/flutter/lib/ui/platform_dispatcher.dart @@ -1050,19 +1050,17 @@ class PlatformDispatcher { if (brieflyShowPassword != null) { _brieflyShowPassword = brieflyShowPassword; } - final Brightness platformBrightness = switch (data['platformBrightness']) { - 'dark' => Brightness.dark, - 'light' => Brightness.light, - final Object? value => throw StateError('$value is not a valid platformBrightness.'), - }; + final Brightness platformBrightness = + data['platformBrightness']! as String == 'dark' ? Brightness.dark : Brightness.light; final String? systemFontFamily = data['systemFontFamily'] as String?; - final int? configurationId = data['configurationId'] as int?; final _PlatformConfiguration previousConfiguration = _configuration; final bool platformBrightnessChanged = previousConfiguration.platformBrightness != platformBrightness; final bool textScaleFactorChanged = previousConfiguration.textScaleFactor != textScaleFactor; - final bool alwaysUse24HourFormatChanged = previousConfiguration.alwaysUse24HourFormat != alwaysUse24HourFormat; - final bool systemFontFamilyChanged = previousConfiguration.systemFontFamily != systemFontFamily; - if (!platformBrightnessChanged && !textScaleFactorChanged && !alwaysUse24HourFormatChanged && !systemFontFamilyChanged && configurationId == null) { + final bool alwaysUse24HourFormatChanged = + previousConfiguration.alwaysUse24HourFormat != alwaysUse24HourFormat; + final bool systemFontFamilyChanged = + previousConfiguration.systemFontFamily != systemFontFamily; + if (!platformBrightnessChanged && !textScaleFactorChanged && !alwaysUse24HourFormatChanged && !systemFontFamilyChanged) { return; } _configuration = previousConfiguration.copyWith( @@ -1070,11 +1068,9 @@ class PlatformDispatcher { alwaysUse24HourFormat: alwaysUse24HourFormat, platformBrightness: platformBrightness, systemFontFamily: systemFontFamily, - configurationId: configurationId, ); _invoke(onPlatformConfigurationChanged, _onPlatformConfigurationChangedZone); if (textScaleFactorChanged) { - _cachedFontSizes = null; _invoke(onTextScaleFactorChanged, _onTextScaleFactorChangedZone); } if (platformBrightnessChanged) { @@ -1247,99 +1243,6 @@ class PlatformDispatcher { @Native(symbol: 'PlatformConfigurationNativeApi::DefaultRouteName') external static String _defaultRouteName(); - - /// Computes the scaled font size from the given `unscaledFontSize`, according - /// to the user's platform preferences. - /// - /// Many platforms allow users to scale text globally for better readability. - /// Given the font size the app developer specified in logical pixels, this - /// method converts it to the preferred font size (also in logical pixels) that - /// accounts for platform-wide text scaling. The return value is always - /// non-negative. - /// - /// The scaled value of the same font size input may change if the user changes - /// the text scaling preference (in system settings for example). The - /// [onTextScaleFactorChanged] callback can be used to monitor such changes. - /// - /// Instead of directly calling this method, applications should typically use - /// [MediaQuery.textScalerOf] to retrive the scaled font size in a widget tree, - /// so text in the app resizes properly when the text scaling preference - /// changes. - double scaleFontSize(double unscaledFontSize) { - assert(unscaledFontSize >= 0); - assert(unscaledFontSize.isFinite); - - if (textScaleFactor == 1.0) { - return unscaledFontSize; - } - - final int unscaledFloor = unscaledFontSize.floor(); - final int unscaledCeil = unscaledFontSize.ceil(); - if (unscaledFloor == unscaledCeil) { - // No need to interpolate if the input value is an integer. - return _scaleAndMemoize(unscaledFloor) ?? unscaledFontSize * textScaleFactor; - } - assert(unscaledCeil - unscaledFloor == 1, 'Unexpected interpolation range: $unscaledFloor - $unscaledCeil.'); - - return switch ((_scaleAndMemoize(unscaledFloor), _scaleAndMemoize(unscaledCeil))) { - (null, _) || (_, null) => unscaledFontSize * textScaleFactor, - (final double lower, final double upper) => lower + (upper - lower) * (unscaledFontSize - unscaledFloor), - }; - } - - // The cache is cleared when the text scale factor changes. - Map? _cachedFontSizes; - // This method returns null if an error is encountered. - double? _scaleAndMemoize(int unscaledFontSize) { - final int? configurationId = _configuration.configurationId; - if (configurationId == null) { - // The platform uses linear scaling, or the platform hasn't sent us a - // configuration yet. - return null; - } - final double? cachedValue = _cachedFontSizes?[unscaledFontSize]; - if (cachedValue != null) { - assert(cachedValue >= 0); - return cachedValue; - } - - final double unscaledFontSizeDouble = unscaledFontSize.toDouble(); - final double fontSize = PlatformDispatcher._getScaledFontSize(unscaledFontSizeDouble, configurationId); - if (fontSize >= 0) { - return (_cachedFontSizes ??= {})[unscaledFontSize] = fontSize; - } - switch (fontSize) { - case -1: - // Invalid configuration id. This error can be unrecoverable as the - // _getScaledFontSize function can be destructive. - assert(false, 'Flutter Error: incorrect configuration id: $configurationId.'); - case final double errorCode: - assert(false, 'Unknown error: GetScaledFontSize failed with $errorCode.'); - } - return null; - } - - // Calls the platform's text scaling implementation to scale the given - // `unscaledFontSize`. - // - // The `configurationId` parameter tells the embedder which platform - // configuration to use for computing the scaled font size. When the user - // changes the platform configuration, the configuration data will first be - // made available on the platform thread before being dispatched asynchronously - // to the Flutter UI thread. Since this call is synchronous, without this - // identifier, it could call into the embber who's using a newer configuration - // that Flutter has not received yet. The `configurationId` parameter must be - // the lastest configuration id received from the platform - // (`_configuration.configurationId`). Using an incorrect id could result in - // an unrecoverable error. - // - // Currently this is only implemented on newer versions of Android (SDK level - // 34, using the `TypedValue#applyDimension` API). Platforms that do not have - // the capability will never send a `configurationId` to [PlatformDispatcher], - // and should not call this method. This method returns -1 when the specified - // configurationId does not match any configuration. - @Native(symbol: 'PlatformConfigurationNativeApi::GetScaledFontSize') - external static double _getScaledFontSize(double unscaledFontSize, int configurationId); } /// Configuration of the platform. @@ -1355,7 +1258,6 @@ class _PlatformConfiguration { this.locales = const [], this.defaultRouteName, this.systemFontFamily, - this.configurationId, }); _PlatformConfiguration copyWith({ @@ -1367,7 +1269,6 @@ class _PlatformConfiguration { List? locales, String? defaultRouteName, String? systemFontFamily, - int? configurationId, }) { return _PlatformConfiguration( accessibilityFeatures: accessibilityFeatures ?? this.accessibilityFeatures, @@ -1378,7 +1279,6 @@ class _PlatformConfiguration { locales: locales ?? this.locales, defaultRouteName: defaultRouteName ?? this.defaultRouteName, systemFontFamily: systemFontFamily ?? this.systemFontFamily, - configurationId: configurationId ?? this.configurationId, ); } @@ -1410,22 +1310,6 @@ class _PlatformConfiguration { /// The system-reported default font family. final String? systemFontFamily; - - /// A unique identifier for this [_PlatformConfiguration]. - /// - /// This unique identifier is optionally assigned by the platform embedder. - /// Dart code that runs on the Flutter UI thread and synchronously invokes - /// platform APIs can use this identifier to tell the embedder to use the - /// configuration that matches the current [_PlatformConfiguration] in - /// dart:ui. See the [_getScaledFontSize] function for an example. - /// - /// This field's nullability also indicates whether the platform supports - /// nonlinear text scaling (as it's the only feature that requires synchronous - /// invocation of platform APIs). This field is always null if the platform - /// does not use nonlinear text scaling, or when dart:ui has not received any - /// configuration updates from the embedder yet. The _getScaledFontSize - /// function should not be called in either case. - final int? configurationId; } /// An immutable view configuration. diff --git a/engine/src/flutter/lib/ui/window/platform_configuration.cc b/engine/src/flutter/lib/ui/window/platform_configuration.cc index a5f38a7b03..c796bdfac5 100644 --- a/engine/src/flutter/lib/ui/window/platform_configuration.cc +++ b/engine/src/flutter/lib/ui/window/platform_configuration.cc @@ -631,13 +631,4 @@ void PlatformConfigurationNativeApi::RegisterBackgroundIsolate( dart_state->SetPlatformMessageHandler(weak_platform_message_handler); } -double PlatformConfigurationNativeApi::GetScaledFontSize( - double unscaled_font_size, - int configuration_id) { - UIDartState::ThrowIfUIOperationsProhibited(); - return UIDartState::Current() - ->platform_configuration() - ->client() - ->GetScaledFontSize(unscaled_font_size, configuration_id); -} } // namespace flutter diff --git a/engine/src/flutter/lib/ui/window/platform_configuration.h b/engine/src/flutter/lib/ui/window/platform_configuration.h index c8bb6dcef5..90000890dd 100644 --- a/engine/src/flutter/lib/ui/window/platform_configuration.h +++ b/engine/src/flutter/lib/ui/window/platform_configuration.h @@ -207,28 +207,6 @@ class PlatformConfigurationClient { /// virtual void RequestDartDeferredLibrary(intptr_t loading_unit_id) = 0; - //-------------------------------------------------------------------------- - /// @brief Synchronously invokes platform-specific APIs to apply the - /// system text scaling on the given unscaled font size. - /// - /// Platforms that support this feature (currently it's only - /// implemented for Android SDK level 34+) will send a valid - /// configuration_id to potential callers, before this method can - /// be called. - /// - /// @param[in] unscaled_font_size The unscaled font size specified by the - /// app developer. The value is in logical - /// pixels, and is guaranteed to be finite and - /// non-negative. - /// @param[in] configuration_id The unique id of the configuration to use - /// for computing the scaled font size. - /// - /// @return The scaled font size in logical pixels, or -1 if the given - /// configuration_id did not match a valid configuration. - /// - virtual double GetScaledFontSize(double unscaled_font_size, - int configuration_id) const = 0; - protected: virtual ~PlatformConfigurationClient(); }; @@ -598,9 +576,6 @@ class PlatformConfigurationNativeApi { static void RegisterBackgroundIsolate(int64_t root_isolate_token); - static double GetScaledFontSize(double unscaled_font_size, - int configuration_id); - private: static Dart_PerformanceMode current_performance_mode_; }; diff --git a/engine/src/flutter/lib/web_ui/lib/platform_dispatcher.dart b/engine/src/flutter/lib/web_ui/lib/platform_dispatcher.dart index 9343eb9640..54453a0fff 100644 --- a/engine/src/flutter/lib/web_ui/lib/platform_dispatcher.dart +++ b/engine/src/flutter/lib/web_ui/lib/platform_dispatcher.dart @@ -146,8 +146,6 @@ abstract class PlatformDispatcher { VoidCallback? get onFrameDataChanged => null; set onFrameDataChanged(VoidCallback? callback) {} - - double scaleFontSize(double unscaledFontSize); } enum FramePhase { diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher.dart index cdd1a77e42..33a7b5fb38 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -1287,9 +1287,6 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { @override ui.FrameData get frameData => const ui.FrameData.webOnly(); - - @override - double scaleFontSize(double unscaledFontSize) => unscaledFontSize * textScaleFactor; } bool _handleWebTestEnd2EndMessage(MethodCodec codec, ByteData? data) { diff --git a/engine/src/flutter/runtime/runtime_controller.cc b/engine/src/flutter/runtime/runtime_controller.cc index 9eb97fa47d..a11b9c2ff2 100644 --- a/engine/src/flutter/runtime/runtime_controller.cc +++ b/engine/src/flutter/runtime/runtime_controller.cc @@ -545,11 +545,6 @@ bool RuntimeController::SetDisplays(const std::vector& displays) { return false; } -double RuntimeController::GetScaledFontSize(double unscaled_font_size, - int configuration_id) const { - return client_.GetScaledFontSize(unscaled_font_size, configuration_id); -} - RuntimeController::Locale::Locale(std::string language_code_, std::string country_code_, std::string script_code_, diff --git a/engine/src/flutter/runtime/runtime_controller.h b/engine/src/flutter/runtime/runtime_controller.h index 2a36211354..1d1b0d7407 100644 --- a/engine/src/flutter/runtime/runtime_controller.h +++ b/engine/src/flutter/runtime/runtime_controller.h @@ -683,10 +683,6 @@ class RuntimeController : public PlatformConfigurationClient { std::unique_ptr> ComputePlatformResolvedLocale( const std::vector& supported_locale_data) override; - // |PlatformConfigurationClient| - double GetScaledFontSize(double unscaled_font_size, - int configuration_id) const override; - FML_DISALLOW_COPY_AND_ASSIGN(RuntimeController); }; diff --git a/engine/src/flutter/runtime/runtime_delegate.h b/engine/src/flutter/runtime/runtime_delegate.h index 324ccf0a44..330fc46e1d 100644 --- a/engine/src/flutter/runtime/runtime_delegate.h +++ b/engine/src/flutter/runtime/runtime_delegate.h @@ -54,9 +54,6 @@ class RuntimeDelegate { virtual std::weak_ptr GetPlatformMessageHandler() const = 0; - virtual double GetScaledFontSize(double unscaled_font_size, - int configuration_id) const = 0; - protected: virtual ~RuntimeDelegate(); }; diff --git a/engine/src/flutter/shell/common/engine.cc b/engine/src/flutter/shell/common/engine.cc index 2228f855e7..d87acb8b87 100644 --- a/engine/src/flutter/shell/common/engine.cc +++ b/engine/src/flutter/shell/common/engine.cc @@ -500,11 +500,6 @@ std::unique_ptr> Engine::ComputePlatformResolvedLocale( return delegate_.ComputePlatformResolvedLocale(supported_locale_data); } -double Engine::GetScaledFontSize(double unscaled_font_size, - int configuration_id) const { - return delegate_.GetScaledFontSize(unscaled_font_size, configuration_id); -} - void Engine::SetNeedsReportTimings(bool needs_reporting) { delegate_.SetNeedsReportTimings(needs_reporting); } diff --git a/engine/src/flutter/shell/common/engine.h b/engine/src/flutter/shell/common/engine.h index f73d85ffa7..e53efc9eb3 100644 --- a/engine/src/flutter/shell/common/engine.h +++ b/engine/src/flutter/shell/common/engine.h @@ -293,28 +293,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// Flutter to the host platform (and its responses). virtual const std::shared_ptr& GetPlatformMessageHandler() const = 0; - - //-------------------------------------------------------------------------- - /// @brief Synchronously invokes platform-specific APIs to apply the - /// system text scaling on the given unscaled font size. - /// - /// Platforms that support this feature (currently it's only - /// implemented for Android SDK level 34+) will send a valid - /// configuration_id to potential callers, before this method - /// can be called. - /// - /// @param[in] unscaled_font_size The unscaled font size specified by the - /// app developer. The value is in logical - /// pixels, and is guaranteed to be finite - /// and non-negative. - /// @param[in] configuration_id The unique id of the configuration to - /// use for computing the scaled font size. - /// - /// @return The scaled font size in logical pixels, or -1 when the given - /// configuration_id did not match a valid configuration. - /// - virtual double GetScaledFontSize(double unscaled_font_size, - int configuration_id) const = 0; }; //---------------------------------------------------------------------------- @@ -977,10 +955,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { std::weak_ptr GetPlatformMessageHandler() const override; - // |RuntimeDelegate| - double GetScaledFontSize(double unscaled_font_size, - int configuration_id) const override; - void SetNeedsReportTimings(bool value) override; bool HandleLifecyclePlatformMessage(PlatformMessage* message); diff --git a/engine/src/flutter/shell/common/engine_unittests.cc b/engine/src/flutter/shell/common/engine_unittests.cc index 7e6a5e38d0..a6bbc12adb 100644 --- a/engine/src/flutter/shell/common/engine_unittests.cc +++ b/engine/src/flutter/shell/common/engine_unittests.cc @@ -39,8 +39,6 @@ class MockDelegate : public Engine::Delegate { MOCK_METHOD0(GetCurrentTimePoint, fml::TimePoint()); MOCK_CONST_METHOD0(GetPlatformMessageHandler, const std::shared_ptr&()); - MOCK_CONST_METHOD2(GetScaledFontSize, - double(double font_size, int configuration_id)); }; class MockResponse : public PlatformMessageResponse { @@ -69,8 +67,6 @@ class MockRuntimeDelegate : public RuntimeDelegate { MOCK_METHOD1(RequestDartDeferredLibrary, void(intptr_t)); MOCK_CONST_METHOD0(GetPlatformMessageHandler, std::weak_ptr()); - MOCK_CONST_METHOD2(GetScaledFontSize, - double(double font_size, int configuration_id)); }; class MockRuntimeController : public RuntimeController { diff --git a/engine/src/flutter/shell/common/platform_view.cc b/engine/src/flutter/shell/common/platform_view.cc index 76d93cfc7c..77a33bfb76 100644 --- a/engine/src/flutter/shell/common/platform_view.cc +++ b/engine/src/flutter/shell/common/platform_view.cc @@ -199,12 +199,4 @@ const Settings& PlatformView::GetSettings() const { return delegate_.OnPlatformViewGetSettings(); } -double PlatformView::GetScaledFontSize(double unscaled_font_size, - int configuration_id) const { - // Unreachable by default, as most platforms do not support nonlinear scaling - // and the Flutter application never invokes this method. - FML_UNREACHABLE(); - return -1; -} - } // namespace flutter diff --git a/engine/src/flutter/shell/common/platform_view.h b/engine/src/flutter/shell/common/platform_view.h index c5b4f1ea97..e6c7dab924 100644 --- a/engine/src/flutter/shell/common/platform_view.h +++ b/engine/src/flutter/shell/common/platform_view.h @@ -833,28 +833,6 @@ class PlatformView { /// const Settings& GetSettings() const; - //-------------------------------------------------------------------------- - /// @brief Synchronously invokes platform-specific APIs to apply the - /// system text scaling on the given unscaled font size. - /// - /// Platforms that support this feature (currently it's only - /// implemented for Android SDK level 34+) will send a valid - /// configuration_id to potential callers, before this method can - /// be called. - /// - /// @param[in] unscaled_font_size The unscaled font size specified by the - /// app developer. The value is in logical - /// pixels, and is guaranteed to be finite and - /// non-negative. - /// @param[in] configuration_id The unique id of the configuration to use - /// for computing the scaled font size. - /// - /// @return The scaled font size in logical pixels, or -1 if the given - /// configuration_id did not match a valid configuration. - /// - virtual double GetScaledFontSize(double unscaled_font_size, - int configuration_id) const; - protected: // This is the only method called on the raster task runner. virtual std::unique_ptr CreateRenderingSurface(); diff --git a/engine/src/flutter/shell/common/shell.cc b/engine/src/flutter/shell/common/shell.cc index 3994674ba4..7964a53003 100644 --- a/engine/src/flutter/shell/common/shell.cc +++ b/engine/src/flutter/shell/common/shell.cc @@ -1478,13 +1478,6 @@ void Shell::RequestDartDeferredLibrary(intptr_t loading_unit_id) { }); } -// |Engine::Delegate| -double Shell::GetScaledFontSize(double unscaled_font_size, - int configuration_id) const { - return platform_view_->GetScaledFontSize(unscaled_font_size, - configuration_id); -} - void Shell::ReportTimings() { FML_DCHECK(is_set_up_); FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread()); diff --git a/engine/src/flutter/shell/common/shell.h b/engine/src/flutter/shell/common/shell.h index 053dd834cd..c4b585550b 100644 --- a/engine/src/flutter/shell/common/shell.h +++ b/engine/src/flutter/shell/common/shell.h @@ -680,10 +680,6 @@ class Shell final : public PlatformView::Delegate, // |Engine::Delegate| fml::TimePoint GetCurrentTimePoint() override; - // |Engine::Delegate| - double GetScaledFontSize(double unscaled_font_size, - int configuration_id) const override; - // |Rasterizer::Delegate| void OnFrameRasterized(const FrameTiming&) override; diff --git a/engine/src/flutter/shell/platform/android/android_shell_holder_unittests.cc b/engine/src/flutter/shell/platform/android/android_shell_holder_unittests.cc index 242b6c6f20..831abc2cd6 100644 --- a/engine/src/flutter/shell/platform/android/android_shell_holder_unittests.cc +++ b/engine/src/flutter/shell/platform/android/android_shell_holder_unittests.cc @@ -60,8 +60,6 @@ class MockPlatformViewAndroidJNI : public PlatformViewAndroidJNI { MOCK_METHOD0(GetDisplayHeight, double()); MOCK_METHOD0(GetDisplayDensity, double()); MOCK_METHOD1(RequestDartDeferredLibrary, bool(int loading_unit_id)); - MOCK_CONST_METHOD2(FlutterViewGetScaledFontSize, - double(double font_size, int configuration_id)); }; class MockPlatformMessageResponse : public PlatformMessageResponse { diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/android/FlutterView.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/android/FlutterView.java index e248f324a8..45fae9f964 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/android/FlutterView.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/android/FlutterView.java @@ -1493,7 +1493,6 @@ public class FlutterView extends FrameLayout .getSettingsChannel() .startMessage() .setTextScaleFactor(getResources().getConfiguration().fontScale) - .setDisplayMetrics(getResources().getDisplayMetrics()) .setNativeSpellCheckServiceDefined(isNativeSpellCheckServiceDefined) .setBrieflyShowPassword( Settings.System.getInt( diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index a1532413b8..d579f1fceb 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -12,9 +12,7 @@ import android.graphics.ImageDecoder; import android.graphics.SurfaceTexture; import android.os.Build; import android.os.Looper; -import android.util.DisplayMetrics; import android.util.Size; -import android.util.TypedValue; import android.view.Surface; import android.view.SurfaceHolder; import androidx.annotation.Keep; @@ -29,7 +27,6 @@ import io.flutter.embedding.engine.deferredcomponents.DeferredComponentManager; import io.flutter.embedding.engine.mutatorsstack.FlutterMutatorsStack; import io.flutter.embedding.engine.renderer.FlutterUiDisplayListener; import io.flutter.embedding.engine.renderer.SurfaceTextureWrapper; -import io.flutter.embedding.engine.systemchannels.SettingsChannel; import io.flutter.plugin.common.StandardMessageCodec; import io.flutter.plugin.localization.LocalizationPlugin; import io.flutter.plugin.platform.PlatformViewsController; @@ -1307,20 +1304,6 @@ public class FlutterJNI { } // ----- End Localization Support ---- - @Nullable - public float getScaledFontSize(float fontSize, int configurationId) { - final DisplayMetrics metrics = SettingsChannel.getPastDisplayMetrics(configurationId); - if (metrics == null) { - Log.e( - TAG, - "getScaledFontSize called with configurationId " - + String.valueOf(configurationId) - + ", which can't be found."); - return -1f; - } - return TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_SP, fontSize, metrics) - / metrics.density; - } // ----- Start Deferred Components Support ---- diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/SettingsChannel.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/SettingsChannel.java index bdd7f0cba0..46bf51190c 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/SettingsChannel.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/SettingsChannel.java @@ -1,19 +1,12 @@ package io.flutter.embedding.engine.systemchannels; -import android.annotation.SuppressLint; -import android.os.Build; -import android.util.DisplayMetrics; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; -import androidx.annotation.UiThread; -import androidx.annotation.VisibleForTesting; import io.flutter.Log; import io.flutter.embedding.engine.dart.DartExecutor; import io.flutter.plugin.common.BasicMessageChannel; import io.flutter.plugin.common.JSONMessageCodec; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.ConcurrentLinkedQueue; public class SettingsChannel { private static final String TAG = "SettingsChannel"; @@ -24,10 +17,6 @@ public class SettingsChannel { private static final String BRIEFLY_SHOW_PASSWORD = "brieflyShowPassword"; private static final String ALWAYS_USE_24_HOUR_FORMAT = "alwaysUse24HourFormat"; private static final String PLATFORM_BRIGHTNESS = "platformBrightness"; - private static final String CONFIGURATION_ID = "configurationId"; - - // When hasNonlinearTextScalingSupport() returns false, this will not be initialized. - private static final ConfigurationQueue CONFIGURATION_QUEUE = new ConfigurationQueue(); @NonNull public final BasicMessageChannel channel; @@ -35,19 +24,6 @@ public class SettingsChannel { this.channel = new BasicMessageChannel<>(dartExecutor, CHANNEL_NAME, JSONMessageCodec.INSTANCE); } - @SuppressLint("AnnotateVersionCheck") - public static boolean hasNonlinearTextScalingSupport() { - return Build.VERSION.SDK_INT >= 34; - } - - // This method will only be called on Flutter's UI thread. - public static DisplayMetrics getPastDisplayMetrics(int configId) { - assert hasNonlinearTextScalingSupport(); - final ConfigurationQueue.SentConfiguration configuration = - CONFIGURATION_QUEUE.getConfiguration(configId); - return configuration == null ? null : configuration.displayMetrics; - } - @NonNull public MessageBuilder startMessage() { return new MessageBuilder(channel); @@ -56,18 +32,11 @@ public class SettingsChannel { public static class MessageBuilder { @NonNull private final BasicMessageChannel channel; @NonNull private Map message = new HashMap<>(); - @Nullable private DisplayMetrics displayMetrics; MessageBuilder(@NonNull BasicMessageChannel channel) { this.channel = channel; } - @NonNull - public MessageBuilder setDisplayMetrics(@NonNull DisplayMetrics displayMetrics) { - this.displayMetrics = displayMetrics; - return this; - } - @NonNull public MessageBuilder setTextScaleFactor(float textScaleFactor) { message.put(TEXT_SCALE_FACTOR, textScaleFactor); @@ -111,17 +80,7 @@ public class SettingsChannel { + "\n" + "platformBrightness: " + message.get(PLATFORM_BRIGHTNESS)); - final DisplayMetrics metrics = this.displayMetrics; - if (!hasNonlinearTextScalingSupport() || metrics == null) { - channel.send(message); - return; - } - final ConfigurationQueue.SentConfiguration sentConfiguration = - new ConfigurationQueue.SentConfiguration(metrics); - final BasicMessageChannel.Reply deleteCallback = - CONFIGURATION_QUEUE.enqueueConfiguration(sentConfiguration); - message.put(CONFIGURATION_ID, sentConfiguration.generationNumber); - channel.send(message, deleteCallback); + channel.send(message); } } @@ -141,127 +100,4 @@ public class SettingsChannel { this.name = name; } } - - /** - * A FIFO queue that maintains generations of configurations that are potentially used by the - * Flutter application. - * - *

Platform configurations needed by the Flutter app (for example, text scale factor) are - * retrived on the platform thread, serialized and sent to the Flutter application running on the - * Flutter UI thread. However, configurations exposed as functions that take parameters are - * typically not serializable. To allow the Flutter app to access these configurations, one - * possible solution is to create dart bindings that allow the Flutter framework to invoke these - * functions via JNI synchronously. To ensure the serialized configuration and these functions - * represent the same set of configurations at any given time, a "generation" id is used in these - * synchronous calls, to keep them consistent with the serialized configuration that the Flutter - * app most recently received and is currently using. - * - *

A unique generation identifier is generated by the {@link SettingsChannel} and associated - * with a configuration when it sends a serialized configuration to the Flutter framework. This - * queue keeps different generations of configurations that could be used by the Flutter - * framework, and cleans up old configurations that the Flutter framework no longer uses. When the - * Flutter framework invokes a function to access the configuration with a generation identifier, - * this queue finds the configuration with that identifier and also cleans up configurations that - * are no longer needed. - * - *

This mechanism is only needed because {@code TypedValue#applyDimension} does not take the - * current text scale factor as an input. Once the AndroidX API that allows us to query the scaled - * font size with a pure function is available, we can scrap this class and make the - * implementation much simpler. - */ - @VisibleForTesting - public static class ConfigurationQueue { - private final ConcurrentLinkedQueue sentQueue = - new ConcurrentLinkedQueue<>(); - - // The current SentConfiguration the Flutter application is using, according - // to the most recent getConfiguration call. - // - // This instance variable will only be accessed by getConfiguration, on - // Flutter's UI thread. - private SentConfiguration currentConfiguration; - - /** - * Returns the {@link SentConfiguration} associated with the given {@code configGeneration}, and - * removes configurations older than the returned configurations from the queue as they are no - * longer needed. - */ - public SentConfiguration getConfiguration(int configGeneration) { - if (currentConfiguration == null) { - currentConfiguration = sentQueue.poll(); - } - - // Remove the older entries, up to the entry associated with - // configGeneration. Here we assume the generationNumber never overflows. - while (currentConfiguration != null - && currentConfiguration.generationNumber < configGeneration) { - currentConfiguration = sentQueue.poll(); - } - - if (currentConfiguration == null) { - Log.e( - TAG, - "Cannot find config with generation: " - + String.valueOf(configGeneration) - + ", after exhausting the queue."); - return null; - } else if (currentConfiguration.generationNumber != configGeneration) { - Log.e( - TAG, - "Cannot find config with generation: " - + String.valueOf(configGeneration) - + ", the oldest config is now: " - + String.valueOf(currentConfiguration.generationNumber)); - return null; - } - return currentConfiguration; - } - - private SentConfiguration previousEnqueuedConfiguration; - - /** - * Adds the most recently sent {@link SentConfiguration} to the queue. - * - * @return a {@link BasicMessageChannel.Reply} whose {@code reply} method must be called when - * the embedder receives the reply for the sent configuration, to properly clean up older - * configurations in the queue. - */ - @UiThread - @Nullable - public BasicMessageChannel.Reply enqueueConfiguration(SentConfiguration config) { - sentQueue.add(config); - final SentConfiguration configurationToRemove = previousEnqueuedConfiguration; - previousEnqueuedConfiguration = config; - return configurationToRemove == null - ? null - : new BasicMessageChannel.Reply() { - @UiThread - @Override - public void reply(Object reply) { - // Removes the SentConfiguration sent right before `config`. Since - // platform messages are also FIFO older messages will be removed - // before newer ones. - sentQueue.remove(configurationToRemove); - if (!sentQueue.isEmpty()) { - Log.e( - TAG, - "The queue becomes empty after removing config generation " - + String.valueOf(configurationToRemove.generationNumber)); - } - } - }; - } - - public static class SentConfiguration { - private static int nextConfigGeneration = Integer.MIN_VALUE; - - @NonNull public final int generationNumber; - @NonNull private final DisplayMetrics displayMetrics; - - public SentConfiguration(@NonNull DisplayMetrics displayMetrics) { - this.generationNumber = nextConfigGeneration++; - this.displayMetrics = displayMetrics; - } - } - } } diff --git a/engine/src/flutter/shell/platform/android/jni/jni_mock.h b/engine/src/flutter/shell/platform/android/jni/jni_mock.h index 34848a66f9..8b84793844 100644 --- a/engine/src/flutter/shell/platform/android/jni/jni_mock.h +++ b/engine/src/flutter/shell/platform/android/jni/jni_mock.h @@ -123,11 +123,6 @@ class JNIMock final : public PlatformViewAndroidJNI { RequestDartDeferredLibrary, (int loading_unit_id), (override)); - - MOCK_METHOD(double, - FlutterViewGetScaledFontSize, - (double font_size, int configuration_id), - (const, override)); }; } // namespace flutter diff --git a/engine/src/flutter/shell/platform/android/jni/platform_view_android_jni.h b/engine/src/flutter/shell/platform/android/jni/platform_view_android_jni.h index e87355a473..c876b093dc 100644 --- a/engine/src/flutter/shell/platform/android/jni/platform_view_android_jni.h +++ b/engine/src/flutter/shell/platform/android/jni/platform_view_android_jni.h @@ -223,9 +223,6 @@ class PlatformViewAndroidJNI { virtual double GetDisplayDensity() = 0; virtual bool RequestDartDeferredLibrary(int loading_unit_id) = 0; - - virtual double FlutterViewGetScaledFontSize(double unscaled_font_size, - int configuration_id) const = 0; }; } // namespace flutter diff --git a/engine/src/flutter/shell/platform/android/platform_view_android.cc b/engine/src/flutter/shell/platform/android/platform_view_android.cc index ac6111c343..5a74c309e4 100644 --- a/engine/src/flutter/shell/platform/android/platform_view_android.cc +++ b/engine/src/flutter/shell/platform/android/platform_view_android.cc @@ -472,9 +472,4 @@ void PlatformViewAndroid::FireFirstFrameCallback() { jni_facade_->FlutterViewOnFirstFrame(); } -double PlatformViewAndroid::GetScaledFontSize(double unscaled_font_size, - int configuration_id) const { - return jni_facade_->FlutterViewGetScaledFontSize(unscaled_font_size, - configuration_id); -} } // namespace flutter diff --git a/engine/src/flutter/shell/platform/android/platform_view_android.h b/engine/src/flutter/shell/platform/android/platform_view_android.h index 5510fb65ad..014e2c8c6f 100644 --- a/engine/src/flutter/shell/platform/android/platform_view_android.h +++ b/engine/src/flutter/shell/platform/android/platform_view_android.h @@ -175,9 +175,6 @@ class PlatformViewAndroid final : public PlatformView { void FireFirstFrameCallback(); - double GetScaledFontSize(double unscaled_font_size, - int configuration_id) const override; - FML_DISALLOW_COPY_AND_ASSIGN(PlatformViewAndroid); }; } // namespace flutter diff --git a/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc b/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc index 7f32306435..b755e4f4d2 100644 --- a/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc +++ b/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc @@ -93,8 +93,6 @@ static jmethodID g_update_semantics_method = nullptr; static jmethodID g_update_custom_accessibility_actions_method = nullptr; -static jmethodID g_get_scaled_font_size_method = nullptr; - static jmethodID g_on_first_frame_method = nullptr; static jmethodID g_on_engine_restart_method = nullptr; @@ -895,14 +893,6 @@ bool RegisterApi(JNIEnv* env) { return false; } - g_get_scaled_font_size_method = env->GetMethodID( - g_flutter_jni_class->obj(), "getScaledFontSize", "(FJ)F"); - - if (g_get_scaled_font_size_method == nullptr) { - FML_LOG(ERROR) << "Could not locate FlutterJNI#getScaledFontSize method"; - return false; - } - g_update_semantics_method = env->GetMethodID( g_flutter_jni_class->obj(), "updateSemantics", "(Ljava/nio/ByteBuffer;[Ljava/lang/String;[Ljava/nio/ByteBuffer;)V"); @@ -1325,23 +1315,6 @@ void PlatformViewAndroidJNIImpl::FlutterViewHandlePlatformMessageResponse( FML_CHECK(fml::jni::CheckException(env)); } -double PlatformViewAndroidJNIImpl::FlutterViewGetScaledFontSize( - double font_size, - int configuration_id) const { - JNIEnv* env = fml::jni::AttachCurrentThread(); - - auto java_object = java_object_.get(env); - if (java_object.is_null()) { - return -3; - } - - const jfloat scaledSize = - env->CallFloatMethod(java_object.obj(), g_get_scaled_font_size_method, - (jfloat)font_size, (jint)configuration_id); - FML_CHECK(fml::jni::CheckException(env)); - return (double)scaledSize; -} - void PlatformViewAndroidJNIImpl::FlutterViewUpdateSemantics( std::vector buffer, std::vector strings, diff --git a/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.h b/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.h index d29c55c1f6..33de0ae255 100644 --- a/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.h +++ b/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.h @@ -99,9 +99,6 @@ class PlatformViewAndroidJNIImpl final : public PlatformViewAndroidJNI { bool RequestDartDeferredLibrary(int loading_unit_id) override; - double FlutterViewGetScaledFontSize(double unscaled_font_size, - int configuration_id) const override; - private: // Reference to FlutterJNI object. const fml::jni::JavaObjectWeakGlobalRef java_object_; diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java index 9ce5f8c913..86b5acfac0 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java @@ -1287,7 +1287,6 @@ public class FlutterActivityAndFragmentDelegateTest { when(fakeMessageBuilder.setPlatformBrightness(any(SettingsChannel.PlatformBrightness.class))) .thenReturn(fakeMessageBuilder); when(fakeMessageBuilder.setTextScaleFactor(any(Float.class))).thenReturn(fakeMessageBuilder); - when(fakeMessageBuilder.setDisplayMetrics(any())).thenReturn(fakeMessageBuilder); when(fakeMessageBuilder.setNativeSpellCheckServiceDefined(any(Boolean.class))) .thenReturn(fakeMessageBuilder); when(fakeMessageBuilder.setBrieflyShowPassword(any(Boolean.class))) diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/systemchannels/SettingsChannelTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/systemchannels/SettingsChannelTest.java deleted file mode 100644 index b1f54e8ddc..0000000000 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/systemchannels/SettingsChannelTest.java +++ /dev/null @@ -1,70 +0,0 @@ -package io.flutter.embedding.engine.systemchannels; - -import static junit.framework.TestCase.assertEquals; -import static org.mockito.Mockito.eq; -import static org.mockito.Mockito.isNull; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; - -import android.annotation.TargetApi; -import android.util.DisplayMetrics; -import androidx.test.ext.junit.runners.AndroidJUnit4; -import io.flutter.embedding.engine.dart.DartExecutor; -import io.flutter.plugin.common.BasicMessageChannel; -import java.nio.ByteBuffer; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.ArgumentCaptor; -import org.robolectric.annotation.Config; - -@Config(manifest = Config.NONE) -@RunWith(AndroidJUnit4.class) -public class SettingsChannelTest { - @Test - @TargetApi(33) - @Config(sdk = 33) - public void setDisplayMetricsDoesNothingOnAPILevel33() { - final DartExecutor executor = mock(DartExecutor.class); - executor.onAttachedToJNI(); - final SettingsChannel settingsChannel = new SettingsChannel(executor); - - final ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(ByteBuffer.class); - - settingsChannel.startMessage().setDisplayMetrics(mock(DisplayMetrics.class)).send(); - - verify(executor).send(eq("flutter/settings"), messageCaptor.capture(), isNull()); - } - - @Test - public void configurationQueueWorks() { - final SettingsChannel.ConfigurationQueue queue = new SettingsChannel.ConfigurationQueue(); - final int baseId = Integer.MIN_VALUE; - - queue.enqueueConfiguration( - new SettingsChannel.ConfigurationQueue.SentConfiguration(mock(DisplayMetrics.class))); - queue.enqueueConfiguration( - new SettingsChannel.ConfigurationQueue.SentConfiguration(mock(DisplayMetrics.class))); - assertEquals(baseId + 0, queue.getConfiguration(baseId + 0).generationNumber); - assertEquals(baseId + 1, queue.getConfiguration(baseId + 1).generationNumber); - assertEquals(baseId + 1, queue.getConfiguration(baseId + 1).generationNumber); - - queue.enqueueConfiguration( - new SettingsChannel.ConfigurationQueue.SentConfiguration(mock(DisplayMetrics.class))); - queue.enqueueConfiguration( - new SettingsChannel.ConfigurationQueue.SentConfiguration(mock(DisplayMetrics.class))); - assertEquals(baseId + 3, queue.getConfiguration(baseId + 3).generationNumber); - // Can get the same configuration more than once. - assertEquals(baseId + 3, queue.getConfiguration(baseId + 3).generationNumber); - - final BasicMessageChannel.Reply replyFor4 = - queue.enqueueConfiguration( - new SettingsChannel.ConfigurationQueue.SentConfiguration(mock(DisplayMetrics.class))); - final BasicMessageChannel.Reply replyFor5 = - queue.enqueueConfiguration( - new SettingsChannel.ConfigurationQueue.SentConfiguration(mock(DisplayMetrics.class))); - replyFor4.reply(null); - replyFor5.reply(null); - assertEquals(baseId + 5, queue.getConfiguration(baseId + 5).generationNumber); - assertEquals(baseId + 5, queue.getConfiguration(baseId + 5).generationNumber); - } -} diff --git a/engine/src/flutter/testing/dart/window_test.dart b/engine/src/flutter/testing/dart/window_test.dart index fc13eeb10b..a0c98c441e 100644 --- a/engine/src/flutter/testing/dart/window_test.dart +++ b/engine/src/flutter/testing/dart/window_test.dart @@ -95,12 +95,4 @@ void main() { expect(flutterView.viewId, 0); expect(flutterView.toString(), 'FlutterView(id: 0)'); }); - - test('scaleFontSize is the identity function by default when textScaleFactor = 1', () { - expect(PlatformDispatcher.instance.scaleFontSize(0), 0.0); - expect(PlatformDispatcher.instance.scaleFontSize(1), 1.0); - expect(PlatformDispatcher.instance.scaleFontSize(2), 2.0); - expect(PlatformDispatcher.instance.scaleFontSize(3), 3.0); - expect(PlatformDispatcher.instance.scaleFontSize(3.4), 3.4); - }); }