From 13bec286bdd109cd30ba26a8d62b637a3f225b76 Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Wed, 3 Apr 2024 15:49:38 -0700 Subject: [PATCH] Magnifier cleanup (#143558) - Introduces the ability to control whether RawMagnifier uses a clip on its decoration. (It still has to use a clip for its magnified image.) - Uses `BlurStyle.outer` to remove the clip for CupertinoMagnifier. - Many changes to the documentation around magnifiers and shadows. - Implements `BoxShadow.copyWith` which somehow we had never needed before. - Makes `debugDisableShadows` handle `BlurStyle.outer` correctly. - Adds `MagnifierInfo.toString`. - Aligns various `operator ==`s with the style guide. - Makes MagnifierDecoration a separate concept from Decoration, since it's not actually compatible anywhere. - Removes some dead code and makes other minor code simplifications. - Uses double syntax rather than integer syntax for various double literals for clarity. I expect one minor golden image change (antialiasing change on Cupertino's magnifier test). --- .../flutter/lib/src/cupertino/magnifier.dart | 42 ++- .../flutter/lib/src/material/magnifier.dart | 135 ++++++---- .../flutter/lib/src/material/shadows.dart | 9 + .../lib/src/painting/box_decoration.dart | 14 + .../flutter/lib/src/painting/box_shadow.dart | 30 ++- packages/flutter/lib/src/painting/debug.dart | 14 +- .../lib/src/painting/shape_decoration.dart | 30 +++ .../flutter/lib/src/widgets/magnifier.dart | 241 ++++++++++-------- .../test/cupertino/magnifier_test.dart | 17 +- .../flutter/test/material/magnifier_test.dart | 5 +- .../test/painting/decoration_test.dart | 13 + .../flutter/test/widgets/magnifier_test.dart | 40 +-- 12 files changed, 395 insertions(+), 195 deletions(-) diff --git a/packages/flutter/lib/src/cupertino/magnifier.dart b/packages/flutter/lib/src/cupertino/magnifier.dart index 74aef3fe26..f02803b508 100644 --- a/packages/flutter/lib/src/cupertino/magnifier.dart +++ b/packages/flutter/lib/src/cupertino/magnifier.dart @@ -245,29 +245,60 @@ class CupertinoMagnifier extends StatelessWidget { color: Color.fromARGB(25, 0, 0, 0), blurRadius: 11, spreadRadius: 0.2, + blurStyle: BlurStyle.outer, ), ], + this.clipBehavior = Clip.none, this.borderSide = const BorderSide(color: Color.fromARGB(255, 232, 232, 232)), this.inOutAnimation, }); - /// The shadows displayed under the magnifier. + /// A list of shadows cast by the [Magnifier]. + /// + /// If the shadows use a [BlurStyle] that paints inside the shape, or if they + /// are offset, then a [clipBehavior] that enables clipping (such as + /// [Clip.hardEdge]) is recommended, otherwise the shadow will occlude the + /// magnifier (the shadow is drawn above the magnifier so as to not be + /// included in the magnified image). + /// + /// A shadow that uses [BlurStyle.outer] and is not offset does not need + /// clipping. + /// + /// By default, the [shadows] are not offset and use [BlurStyle.outer], and + /// correspondingly the default [clipBehavior] is [Clip.none]. final List shadows; + /// Whether and how to clip the [shadows] that render inside the loupe. + /// + /// Defaults to [Clip.none], which is useful if the shadow will not paint + /// where the magnified image appears, or if doing so is intentional (e.g. to + /// blur the edges of the magnified image). + /// + /// The default configuration of [CupertinoMagnifier] does not render inside + /// the loupe (the shadows are not offset and use [BlurStyle.outer]). + /// + /// Other values (e.g. [Clip.hardEdge]) are recommended when the [shadows] + /// have an offset. + /// + /// See the discussion at [shadows]. + final Clip clipBehavior; + /// The border, or "rim", of this magnifier. + /// + /// This border is drawn on a [RoundedRectangleBorder] with radius + /// [borderRadius], and increases the [size] of the magnifier by the + /// [BorderSide.width]. final BorderSide borderSide; /// The vertical offset that the magnifier is along the Y axis above /// the focal point. - @visibleForTesting static const double kMagnifierAboveFocalPoint = -26; /// The default size of the magnifier. /// /// This is public so that positioners can choose to depend on it, although /// it is overridable. - @visibleForTesting static const Size kDefaultSize = Size(80, 47.5); /// The duration that this magnifier animates in / out for. @@ -278,9 +309,13 @@ class CupertinoMagnifier extends StatelessWidget { static const Duration _kInOutAnimationDuration = Duration(milliseconds: 150); /// The size of this magnifier. + /// + /// The size does not include the [borderSide] or [shadows]. final Size size; /// The border radius of this magnifier. + /// + /// The magnifier's shape is a [RoundedRectangleBorder] with this radius. final BorderRadius borderRadius; /// This [RawMagnifier]'s controller. @@ -317,6 +352,7 @@ class CupertinoMagnifier extends StatelessWidget { ), shadows: shadows, ), + clipBehavior: clipBehavior, ), ); } diff --git a/packages/flutter/lib/src/material/magnifier.dart b/packages/flutter/lib/src/material/magnifier.dart index 6206d0e16b..3d9c2a1669 100644 --- a/packages/flutter/lib/src/material/magnifier.dart +++ b/packages/flutter/lib/src/material/magnifier.dart @@ -7,39 +7,38 @@ import 'dart:async'; import 'package:flutter/cupertino.dart'; import 'package:flutter/foundation.dart'; -/// {@template widgets.material.magnifier.magnifier} /// A [Magnifier] positioned by rules dictated by the native Android magnifier. -/// {@endtemplate} /// -/// {@template widgets.material.magnifier.positionRules} -/// Positions itself based on [magnifierInfo]. Specifically, follows the -/// following rules: -/// - Tracks the gesture's x coordinate, but clamped to the beginning and end of the -/// currently editing line. -/// - Focal point may never contain anything out of bounds. -/// - Never goes out of bounds vertically; offset until the entire magnifier is in the screen. The -/// focal point, regardless of this transformation, always points to the touch y coordinate. -/// - If just jumped between lines (prevY != currentY) then animate for duration -/// [jumpBetweenLinesAnimationDuration]. -/// {@endtemplate} +/// The positioning rules are based on [magnifierInfo], as follows: +/// +/// - The loupe tracks the gesture's _x_ coordinate, clamping to the beginning +/// and end of the currently editing line. +/// +/// - The focal point never contains anything out of the bounds of the text +/// field or other widget being magnified (the [MagnifierInfo.fieldBounds]). +/// +/// - The focal point always remains aligned with the _y_ coordinate of the touch. +/// +/// - The loupe always remains on the screen. +/// +/// - When the line targeted by the touch's _y_ coordinate changes, the position +/// is animated over [jumpBetweenLinesAnimationDuration]. +/// +/// This behavior was based on the Android 12 source code, where possible, and +/// on eyeballing a Pixel 6 running Android 12 otherwise. class TextMagnifier extends StatefulWidget { - /// {@macro widgets.material.magnifier.magnifier} + /// Creates a [TextMagnifier]. /// - /// {@template widgets.material.magnifier.androidDisclaimer} - /// These constants and default parameters were taken from the - /// Android 12 source code where directly transferable, and eyeballed on - /// a Pixel 6 running Android 12 otherwise. - /// {@endtemplate} - /// - /// {@macro widgets.material.magnifier.positionRules} + /// The [magnifierInfo] must be provided, and must be updated with new values + /// as the user's touch changes. const TextMagnifier({ super.key, required this.magnifierInfo, }); - /// A [TextMagnifierConfiguration] that returns a [CupertinoTextMagnifier] on iOS, - /// [TextMagnifier] on Android, and null on all other platforms, and shows the editing handles - /// only on iOS. + /// A [TextMagnifierConfiguration] that returns a [CupertinoTextMagnifier] on + /// iOS, [TextMagnifier] on Android, and null on all other platforms, and + /// shows the editing handles only on iOS. static TextMagnifierConfiguration adaptiveMagnifierConfiguration = TextMagnifierConfiguration( shouldDisplayHandlesInMagnifier: defaultTargetPlatform == TargetPlatform.iOS, magnifierBuilder: ( @@ -55,7 +54,7 @@ class TextMagnifier extends StatefulWidget { ); case TargetPlatform.android: return TextMagnifier( - magnifierInfo: magnifierInfo, + magnifierInfo: magnifierInfo, ); case TargetPlatform.fuchsia: case TargetPlatform.linux: @@ -68,15 +67,14 @@ class TextMagnifier extends StatefulWidget { /// The duration that the position is animated if [TextMagnifier] just switched /// between lines. - @visibleForTesting - static const Duration jumpBetweenLinesAnimationDuration = - Duration(milliseconds: 70); + static const Duration jumpBetweenLinesAnimationDuration = Duration(milliseconds: 70); - /// [TextMagnifier] positions itself based on [magnifierInfo]. + /// The current status of the user's touch. /// - /// {@macro widgets.material.magnifier.positionRules} - final ValueNotifier - magnifierInfo; + /// As the value of the [magnifierInfo] changes, the position of the loupe is + /// adjusted automatically, according to the rules described in the + /// [TextMagnifier] class description. + final ValueNotifier magnifierInfo; @override State createState() => _TextMagnifierState(); @@ -130,7 +128,6 @@ class _TextMagnifierState extends State { super.didUpdateWidget(oldWidget); } - /// {@macro widgets.material.magnifier.positionRules} void _determineMagnifierPositionAndFocalPoint() { final MagnifierInfo selectionInfo = widget.magnifierInfo.value; @@ -250,16 +247,19 @@ class _TextMagnifierState extends State { } } -/// A Material styled magnifying glass. +/// A Material-styled magnifying glass. /// /// {@macro flutter.widgets.magnifier.intro} /// -/// This widget focuses on mimicking the _style_ of the magnifier on material. For a -/// widget that is focused on mimicking the behavior of a material magnifier, see [TextMagnifier]. +/// This widget focuses on mimicking the _style_ of the magnifier on material. +/// For a widget that is focused on mimicking the _behavior_ of a material +/// magnifier, see [TextMagnifier], which uses [Magnifier]. +/// +/// The styles implemented in this widget were based on the Android 12 source +/// code, where possible, and on eyeballing a Pixel 6 running Android 12 +/// otherwise. class Magnifier extends StatelessWidget { /// Creates a [RawMagnifier] in the Material style. - /// - /// {@macro widgets.material.magnifier.androidDisclaimer} const Magnifier({ super.key, this.additionalFocalPointOffset = Offset.zero, @@ -267,11 +267,13 @@ class Magnifier extends StatelessWidget { this.filmColor = const Color.fromARGB(8, 158, 158, 158), this.shadows = const [ BoxShadow( - blurRadius: 1.5, - offset: Offset(0, 2), - spreadRadius: 0.75, - color: Color.fromARGB(25, 0, 0, 0)) + blurRadius: 1.5, + offset: Offset(0.0, 2.0), + spreadRadius: 0.75, + color: Color.fromARGB(25, 0, 0, 0), + ) ], + this.clipBehavior = Clip.hardEdge, this.size = Magnifier.kDefaultMagnifierSize, }); @@ -280,14 +282,13 @@ class Magnifier extends StatelessWidget { /// The size of the magnifier may be modified through the constructor; /// [kDefaultMagnifierSize] is extracted from the default parameter of /// [Magnifier]'s constructor so that positioners may depend on it. - @visibleForTesting static const Size kDefaultMagnifierSize = Size(77.37, 37.9); /// The vertical distance that the magnifier should be above the focal point. /// - /// [kStandardVerticalFocalPointShift] is an unmodifiable constant so that positioning of this - /// [Magnifier] can be done with a guaranteed size, as opposed to an estimate. - @visibleForTesting + /// The [kStandardVerticalFocalPointShift] value is a constant so that + /// positioning of this [Magnifier] can be done with a guaranteed size, as + /// opposed to an estimate. static const double kStandardVerticalFocalPointShift = 22; static const double _borderRadius = 40; @@ -296,11 +297,16 @@ class Magnifier extends StatelessWidget { /// Any additional offset the focal point requires to "point" /// to the correct place. /// - /// This is useful for instances where the magnifier is not pointing to something - /// directly below it. + /// This value is added to [kStandardVerticalFocalPointShift] to obtain the + /// actual offset. + /// + /// This is useful for instances where the magnifier is not pointing to + /// something directly below it. final Offset additionalFocalPointOffset; /// The border radius for this magnifier. + /// + /// The magnifier's shape is a [RoundedRectangleBorder] with this radius. final BorderRadius borderRadius; /// The color to tint the image in this [Magnifier]. @@ -310,12 +316,35 @@ class Magnifier extends StatelessWidget { /// the background. final Color filmColor; - /// The shadows for this [Magnifier]. + /// A list of shadows cast by the [Magnifier]. + /// + /// If the shadows use a [BlurStyle] that paints inside the shape, or if they + /// are offset, then a [clipBehavior] that enables clipping (such as the + /// default [Clip.hardEdge]) is recommended, otherwise the shadow will occlude + /// the magnifier (the shadow is drawn above the magnifier so as to not be + /// included in the magnified image). + /// + /// By default, the shadows are offset vertically by two logical pixels, so + /// clipping is recommended. + /// + /// A shadow that uses [BlurStyle.outer] and is not offset does not need + /// clipping; in that case, consider setting [clipBehavior] to [Clip.none]. final List shadows; + /// Whether and how to clip the [shadows] that render inside the loupe. + /// + /// Defaults to [Clip.hardEdge]. + /// + /// A value of [Clip.none] can be used if the shadow will not paint where the + /// magnified image appears, or if doing so is intentional (e.g. to blur the + /// edges of the magnified image). + /// + /// See the discussion at [shadows]. + final Clip clipBehavior; + /// The [Size] of this [Magnifier]. /// - /// This size does not include the border. + /// The [shadows] are drawn outside of the [size]. final Size size; @override @@ -325,11 +354,17 @@ class Magnifier extends StatelessWidget { shape: RoundedRectangleBorder(borderRadius: borderRadius), shadows: shadows, ), + clipBehavior: clipBehavior, magnificationScale: _magnification, focalPointOffset: additionalFocalPointOffset + Offset(0, kStandardVerticalFocalPointShift + kDefaultMagnifierSize.height / 2), size: size, child: ColoredBox( + // This couldn't be part of the decoration (even if the + // MagnifierDecoration supported specifying a color) because the + // decoration's shadows are offset and therefore we set a clipBehavior + // that clips the inner part of the decoration to avoid occluding the + // magnified image with the shadow. color: filmColor, ), ); diff --git a/packages/flutter/lib/src/material/shadows.dart b/packages/flutter/lib/src/material/shadows.dart index 35947a61b8..00dc20a1fe 100644 --- a/packages/flutter/lib/src/material/shadows.dart +++ b/packages/flutter/lib/src/material/shadows.dart @@ -18,6 +18,15 @@ import 'package:flutter/painting.dart'; /// This is useful when simulating a shadow with a [BoxDecoration] or other /// class that uses a list of [BoxShadow] objects. /// +/// Shadows defined by [kElevationToShadow] use [BlurStyle.normal]. To convert a +/// shadow from [kElevationToShadow] to use a different [BlurStyle] (e.g. to use +/// it in a [MagnifierDecoration]), consider an expression such as the +/// following: +/// +/// ```dart +/// kElevationToShadow[12]!.map((BoxShadow shadow) => shadow.copyWith(blurStyle: BlurStyle.outer)).toList(), +/// ``` +/// /// See also: /// /// * [Material], which takes an arbitrary double for its elevation and generates diff --git a/packages/flutter/lib/src/painting/box_decoration.dart b/packages/flutter/lib/src/painting/box_decoration.dart index 26328607cc..9cba29aad3 100644 --- a/packages/flutter/lib/src/painting/box_decoration.dart +++ b/packages/flutter/lib/src/painting/box_decoration.dart @@ -11,6 +11,7 @@ import 'border_radius.dart'; import 'box_border.dart'; import 'box_shadow.dart'; import 'colors.dart'; +import 'debug.dart'; import 'decoration.dart'; import 'decoration_image.dart'; import 'edge_insets.dart'; @@ -439,7 +440,20 @@ class _BoxDecorationPainter extends BoxPainter { for (final BoxShadow boxShadow in _decoration.boxShadow!) { final Paint paint = boxShadow.toPaint(); final Rect bounds = rect.shift(boxShadow.offset).inflate(boxShadow.spreadRadius); + assert(() { + if (debugDisableShadows && boxShadow.blurStyle == BlurStyle.outer) { + canvas.save(); + canvas.clipRect(bounds); + } + return true; + }()); _paintBox(canvas, bounds, paint, textDirection); + assert(() { + if (debugDisableShadows && boxShadow.blurStyle == BlurStyle.outer) { + canvas.restore(); + } + return true; + }()); } } diff --git a/packages/flutter/lib/src/painting/box_shadow.dart b/packages/flutter/lib/src/painting/box_shadow.dart index 9c49d1771a..6cd3a14559 100644 --- a/packages/flutter/lib/src/painting/box_shadow.dart +++ b/packages/flutter/lib/src/painting/box_shadow.dart @@ -44,6 +44,9 @@ class BoxShadow extends ui.Shadow { /// The [BlurStyle] to use for this shadow. /// /// Defaults to [BlurStyle.normal]. + /// + /// When [debugDisableShadows] is true, [toPaint] ignores the [blurStyle] and + /// acts as if [BlurStyle.normal] was used. final BlurStyle blurStyle; /// Create the [Paint] object that corresponds to this shadow description. @@ -52,6 +55,12 @@ class BoxShadow extends ui.Shadow { /// To honor those as well, the shape should be inflated by [spreadRadius] pixels /// in every direction and then translated by [offset] before being filled using /// this [Paint]. + /// + /// The [blurStyle] is ignored if [debugDisableShadows] is true. This causes + /// an especially significant change to the rendering when [BlurStyle.outer] + /// is used; the caller is responsible for adjusting for that case if + /// necessary. (This only matters when using [debugDisableShadows], e.g. in + /// tests that use [matchesGoldenFile].) @override Paint toPaint() { final Paint result = Paint() @@ -66,7 +75,8 @@ class BoxShadow extends ui.Shadow { return result; } - /// Returns a new box shadow with its offset, blurRadius, and spreadRadius scaled by the given factor. + /// Returns a new box shadow with its offset, blurRadius, and spreadRadius + /// scaled by the given factor. @override BoxShadow scale(double factor) { return BoxShadow( @@ -78,6 +88,24 @@ class BoxShadow extends ui.Shadow { ); } + /// Creates a copy of this object but with the given fields replaced with the + /// new values. + BoxShadow copyWith({ + Color? color, + Offset? offset, + double? blurRadius, + double? spreadRadius, + BlurStyle? blurStyle, + }) { + return BoxShadow( + color: color ?? this.color, + offset: offset ?? this.offset, + blurRadius: blurRadius ?? this.blurRadius, + spreadRadius: spreadRadius ?? this.spreadRadius, + blurStyle: blurStyle ?? this.blurStyle, + ); + } + /// Linearly interpolate between two box shadows. /// /// If either box shadow is null, this function linearly interpolates from diff --git a/packages/flutter/lib/src/painting/debug.dart b/packages/flutter/lib/src/painting/debug.dart index cbb6e48561..53c9e5baf1 100644 --- a/packages/flutter/lib/src/painting/debug.dart +++ b/packages/flutter/lib/src/painting/debug.dart @@ -13,11 +13,17 @@ import 'package:flutter/foundation.dart'; /// the rendering of shadows is not guaranteed to be pixel-for-pixel identical from /// version to version (or even from run to run). /// -/// In those tests, this is usually set to false at the beginning of a test and back -/// to true before the end of the test case. +/// This is set to true in [AutomatedTestWidgetsFlutterBinding]. Tests will fail +/// if they change this value and do not reset it before the end of the test. /// -/// If it remains true when the test ends, an exception is thrown to avoid state -/// leaking from one test case to another. +/// When this is set, [BoxShadow.toPaint] acts as if the [BoxShadow.blurStyle] +/// was [BlurStyle.normal] regardless of the actual specified blur style. This +/// is compensated for in [BoxDecoration] and [ShapeDecoration] but may need to +/// be explicitly considered in other situations. +/// +/// This property should not be changed during a frame (e.g. during a call to +/// [ShapeBorder.paintInterior] or [ShapeBorder.getOuterPath]); doing so may +/// cause undefined effects. bool debugDisableShadows = false; /// Signature for a method that returns an [HttpClient]. diff --git a/packages/flutter/lib/src/painting/shape_decoration.dart b/packages/flutter/lib/src/painting/shape_decoration.dart index fbe3c60ad5..a19f502d15 100644 --- a/packages/flutter/lib/src/painting/shape_decoration.dart +++ b/packages/flutter/lib/src/painting/shape_decoration.dart @@ -11,6 +11,7 @@ import 'box_decoration.dart'; import 'box_shadow.dart'; import 'circle_border.dart'; import 'colors.dart'; +import 'debug.dart'; import 'decoration.dart'; import 'decoration_image.dart'; import 'edge_insets.dart'; @@ -359,14 +360,43 @@ class _ShapeDecorationPainter extends BoxPainter { } void _paintShadows(Canvas canvas, Rect rect, TextDirection? textDirection) { + // The debugHandleDisabledShadowStart and debugHandleDisabledShadowEnd + // methods are used in debug mode only to support BlurStyle.outer when + // debugDisableShadows is set. Without these clips, the shadows would extend + // to the inside of the shape, which would likely obscure important + // portions of the rendering and would cause unit tests of widgets that use + // BlurStyle.outer to significantly diverge from the original intent. + // It is assumed that [debugDisableShadows] will not change when calling + // paintInterior or getOuterPath; if it does, the results are undefined. + bool debugHandleDisabledShadowStart(Canvas canvas, BoxShadow boxShadow, Path path) { + if (debugDisableShadows && boxShadow.blurStyle == BlurStyle.outer) { + canvas.save(); + final Path clipPath = Path(); + clipPath.fillType = PathFillType.evenOdd; + clipPath.addRect(Rect.largest); + clipPath.addPath(path, Offset.zero); + canvas.clipPath(clipPath); + } + return true; + } + bool debugHandleDisabledShadowEnd(Canvas canvas, BoxShadow boxShadow) { + if (debugDisableShadows && boxShadow.blurStyle == BlurStyle.outer) { + canvas.restore(); + } + return true; + } if (_shadowCount != null) { if (_decoration.shape.preferPaintInterior) { for (int index = 0; index < _shadowCount!; index += 1) { + assert(debugHandleDisabledShadowStart(canvas, _decoration.shadows![index], _decoration.shape.getOuterPath(_shadowBounds[index], textDirection: textDirection))); _decoration.shape.paintInterior(canvas, _shadowBounds[index], _shadowPaints[index], textDirection: textDirection); + assert(debugHandleDisabledShadowEnd(canvas, _decoration.shadows![index])); } } else { for (int index = 0; index < _shadowCount!; index += 1) { + assert(debugHandleDisabledShadowStart(canvas, _decoration.shadows![index], _shadowPaths[index])); canvas.drawPath(_shadowPaths[index], _shadowPaints[index]); + assert(debugHandleDisabledShadowEnd(canvas, _decoration.shadows![index])); } } } diff --git a/packages/flutter/lib/src/widgets/magnifier.dart b/packages/flutter/lib/src/widgets/magnifier.dart index e6bb7abcf5..463f017b1a 100644 --- a/packages/flutter/lib/src/widgets/magnifier.dart +++ b/packages/flutter/lib/src/widgets/magnifier.dart @@ -3,9 +3,9 @@ // found in the LICENSE file. import 'dart:async'; -import 'dart:math' as math; import 'dart:ui'; +import 'package:flutter/foundation.dart'; import 'package:flutter/rendering.dart'; import 'basic.dart'; @@ -68,8 +68,8 @@ class MagnifierInfo { @override bool operator ==(Object other) { - if (identical(this, other)) { - return true; + if (other.runtimeType != runtimeType) { + return false; } return other is MagnifierInfo && other.globalGesturePosition == globalGesturePosition @@ -85,6 +85,16 @@ class MagnifierInfo { fieldBounds, currentLineBoundaries, ); + + @override + String toString() { + return '${objectRuntimeType(this, 'MagnifierInfo')}(' + 'position: $globalGesturePosition, ' + 'line: $currentLineBoundaries, ' + 'caret: $caretRect, ' + 'field: $fieldBounds' + ')'; + } } /// A configuration object for a magnifier (e.g. in a text field). @@ -135,9 +145,6 @@ class TextMagnifierConfiguration { /// [Overlay]. /// /// To check the status of the magnifier, see [MagnifierController.shown]. -// TODO(antholeole): This whole paradigm can be removed once portals -// lands - then the magnifier can be controlled though a widget in the tree. -// https://github.com/flutter/flutter/pull/105335 class MagnifierController { /// If there is no in / out animation for the magnifier, [animationController] should be left /// null. @@ -350,37 +357,83 @@ class MagnifierController { } } -/// A decoration for a [RawMagnifier]. +/// The decorations to put around the loupe in a [RawMagnifier]. /// -/// [MagnifierDecoration] does not expose [ShapeDecoration.color], [ShapeDecoration.image], -/// or [ShapeDecoration.gradient], since they will be covered by the [RawMagnifier]'s lens. +/// See also: /// -/// Also takes an [opacity] (see https://github.com/flutter/engine/pull/34435). -class MagnifierDecoration extends ShapeDecoration { +/// * [Decoration], a more general solution for [DecoratedBox]. +@immutable +class MagnifierDecoration { /// Constructs a [MagnifierDecoration]. /// - /// By default, [MagnifierDecoration] is a rectangular magnifier with no shadows, and - /// fully opaque. + /// By default, [MagnifierDecoration] is a rectangular magnifier with no + /// shadows, and fully opaque. const MagnifierDecoration({ - this.opacity = 1, - super.shadows, - super.shape = const RoundedRectangleBorder(), + this.opacity = 1.0, + this.shadows, + this.shape = const RoundedRectangleBorder(), }); - /// The magnifier's opacity. + // TODO(ianh): deprecate [opacity] (moving it to [RawMagnifier]), and then + // once [opacity] can be removed, replace [MagnifierDecoration] with a + // `typedef` to [ShapeDecoration] and make anywhere that accepts a + // [MagnifierDecoration] accept a [ShapeDecoration] instead. This would allow + // magnifiers that don't offset the shadows to use the decoration to paint + // over the loupe rather than having to have a Stack of widgets to do so. + + /// The opacity of the magnifier and decorations around the magnifier. + /// + /// When this is 1.0, the magnified image shows in the [shape] of the + /// magnifier. When this is less than 1.0, the magnified image is transparent + /// and shows through the unmagnified background. + /// + /// Generally this is only useful for animating the magnifier in and out, as a + /// transparent magnifier looks quite confusing. final double opacity; + /// A list of shadows cast by the [shape]. + /// + /// If the shadows are offset, consider setting [RawMagnifier.clipBehavior] to + /// [Clip.hardEdge] (or similar) to ensure the shadow does not occlude the + /// magnifier (the shadow is drawn above the magnifier). + /// + /// If the shadows are _not_ offset, consider using [BlurStyle.outer] in the + /// shadows instead, to avoid having to introduce a clip. + /// + /// In the event that [shape] consists of a stack of borders, the shadow is + /// drawn using the bounds of the last one. + /// + /// See also: + /// + /// * [kElevationToShadow], which defines some shadows for Material design. + /// Those shadows use [BlurStyle.normal] and may need to be converted to + /// [BlurStyle.outer] for use with [MagnifierDecoration]. + final List? shadows; + + /// The shape of the magnifier and the outline (border) around it. + /// + /// Shapes can be stacked (using the `+` operator). In that case, the + /// magnifier and shadow are drawn according to the outside edge of the last + /// shape, with the borders painted on top. + final ShapeBorder shape; + @override bool operator ==(Object other) { - if (identical(this, other)) { - return true; + if (other.runtimeType != runtimeType) { + return false; } - - return super == other && other is MagnifierDecoration && other.opacity == opacity; + return other is MagnifierDecoration + && other.opacity == opacity + && listEquals(other.shadows, shadows) + && other.shape == shape; } @override - int get hashCode => Object.hash(super.hashCode, opacity); + int get hashCode => Object.hash( + opacity, + shape, + shadows == null ? null : Object.hashAll(shadows!), + ); } /// A common base class for magnifiers. @@ -406,17 +459,16 @@ class MagnifierDecoration extends ShapeDecoration { class RawMagnifier extends StatelessWidget { /// Constructs a [RawMagnifier]. /// - /// {@template flutter.widgets.magnifier.RawMagnifier.invisibility_warning} - /// By default, this magnifier uses the default [MagnifierDecoration], - /// the focal point is directly under the magnifier, and there is no magnification: - /// This means that a default magnifier will be entirely invisible to the naked eye, - /// since it is painting exactly what is under it, exactly where it was painted - /// originally. - /// {@endtemplate} + /// By default, this magnifier uses the default [MagnifierDecoration] (which + /// draws nothing), the focal point is directly under the magnifier, and there + /// is no magnification; this means that a default magnifier will be entirely + /// invisible to the naked eye, painting exactly what is under it, exactly + /// where it was painted originally. const RawMagnifier({ super.key, this.child, this.decoration = const MagnifierDecoration(), + this.clipBehavior = Clip.none, this.focalPointOffset = Offset.zero, this.magnificationScale = 1, required this.size, @@ -426,14 +478,29 @@ class RawMagnifier extends StatelessWidget { /// An optional widget to position inside the len of the [RawMagnifier]. /// /// This is positioned over the [RawMagnifier] - it may be useful for tinting the - /// [RawMagnifier], or drawing a crosshair like UI. + /// [RawMagnifier], or drawing a crosshair-like UI. final Widget? child; /// This magnifier's decoration. /// - /// {@macro flutter.widgets.magnifier.RawMagnifier.invisibility_warning} + /// This sets the shape of the loupe, plus any borders and shadows that it + /// casts. The default has no border and no shadow; combined with the default + /// [magnificationScale] of 1.0, this results in the magnifier having no + /// visible effect. + /// + /// If the [decoration] has a [MagnifierDecoration.shadows] that uses offset + /// shadows or uses a [BlurStyle] that would obscure the magnified image, + /// consider setting [clipBehavior] to [Clip.hardEdge] (or similar) to ensure + /// the magnified image is visible. final MagnifierDecoration decoration; + /// Whether and how to clip the parts of [decoration] that render inside the + /// loupe. + /// + /// Defaults to [Clip.none]. + /// + /// See the discussion at [decoration]. + final Clip clipBehavior; /// The offset of the magnifier from [RawMagnifier]'s center. /// @@ -448,11 +515,13 @@ class RawMagnifier extends StatelessWidget { final Offset focalPointOffset; /// How "zoomed in" the magnification subject is in the lens. + /// + /// The default is 1.0, which is no magnification. final double magnificationScale; /// The size of the magnifier. /// - /// This does not include added border; it only includes + /// This does not include the border from the [decoration]; it only includes /// the size of the magnifier. final Size size; @@ -462,12 +531,12 @@ class RawMagnifier extends StatelessWidget { clipBehavior: Clip.none, alignment: Alignment.center, children: [ + // The magnified image is clipped to the outer path of the shape. ClipPath.shape( shape: decoration.shape, child: Opacity( opacity: decoration.opacity, child: _Magnifier( - shape: decoration.shape, focalPointOffset: focalPointOffset, magnificationScale: magnificationScale, child: SizedBox.fromSize( @@ -477,86 +546,53 @@ class RawMagnifier extends StatelessWidget { ), ), ), - // Because `BackdropFilter` will filter any widgets before it, we should - // apply the style after (i.e. in a younger sibling) to avoid the magnifier + // Because `BackdropFilter` will filter any widgets before it, we apply + // these styles after (i.e. in a younger sibling) to avoid the magnifier // from seeing its own styling. - Opacity( - opacity: decoration.opacity, - child: _MagnifierStyle( - decoration, - size: size, + IgnorePointer( + child: Opacity( + opacity: decoration.opacity, + child: ClipPath( + clipBehavior: clipBehavior, + clipper: _NegativeClip(shape: decoration.shape), + child: DecoratedBox( + decoration: ShapeDecoration( + shape: decoration.shape, + shadows: decoration.shadows, + ), + child: SizedBox.fromSize( + size: size, + ), + ), + ), ), - ) + ), ], ); } } -class _MagnifierStyle extends StatelessWidget { - const _MagnifierStyle(this.decoration, {required this.size}); +// A clip that renders everything except the inside of a shape. +class _NegativeClip extends CustomClipper { + _NegativeClip({required this.shape}); - final MagnifierDecoration decoration; - final Size size; - - @override - Widget build(BuildContext context) { - double largestShadow = 0; - for (final BoxShadow shadow in decoration.shadows ?? []) { - largestShadow = math.max( - largestShadow, - (shadow.blurRadius + shadow.spreadRadius) + - math.max(shadow.offset.dy.abs(), shadow.offset.dx.abs())); - } - - return ClipPath( - clipBehavior: Clip.hardEdge, - clipper: _DonutClip( - shape: decoration.shape, - spreadRadius: largestShadow, - ), - child: DecoratedBox( - decoration: decoration, - child: SizedBox.fromSize( - size: size, - ), - ), - ); - } -} - -/// A `clipPath` that looks like a donut if you were to fill its area. -/// -/// This is necessary because the shadow must be added after the magnifier is drawn, -/// so that the shadow does not end up in the magnifier. Without this clip, the magnifier would be -/// entirely covered by the shadow. -/// -/// The negative space of the donut is clipped out (the donut hole, outside the donut). -/// The donut hole is cut out exactly like the shape of the magnifier. -class _DonutClip extends CustomClipper { - _DonutClip({required this.shape, required this.spreadRadius}); - - final double spreadRadius; final ShapeBorder shape; @override Path getClip(Size size) { - final Path path = Path(); - final Rect rect = Offset.zero & size; - - path.fillType = PathFillType.evenOdd; - path.addPath(shape.getOuterPath(rect.inflate(spreadRadius)), Offset.zero); - path.addPath(shape.getInnerPath(rect), Offset.zero); - return path; + return Path() + ..fillType = PathFillType.evenOdd + ..addRect(Rect.largest) + ..addPath(shape.getInnerPath(Offset.zero & size), Offset.zero); } @override - bool shouldReclip(_DonutClip oldClipper) => oldClipper.shape != shape; + bool shouldReclip(_NegativeClip oldClipper) => oldClipper.shape != shape; } class _Magnifier extends SingleChildRenderObjectWidget { const _Magnifier({ super.child, - required this.shape, this.magnificationScale = 1, this.focalPointOffset = Offset.zero, }); @@ -571,12 +607,9 @@ class _Magnifier extends SingleChildRenderObjectWidget { // If greater than 1.0, the content appears bigger in the magnifier. final double magnificationScale; - // Shape of the magnifier. - final ShapeBorder shape; - @override RenderObject createRenderObject(BuildContext context) { - return _RenderMagnification(focalPointOffset, magnificationScale, shape); + return _RenderMagnification(focalPointOffset, magnificationScale); } @override @@ -584,7 +617,6 @@ class _Magnifier extends SingleChildRenderObjectWidget { BuildContext context, _RenderMagnification renderObject) { renderObject ..focalPointOffset = focalPointOffset - ..shape = shape ..magnificationScale = magnificationScale; } } @@ -592,8 +624,7 @@ class _Magnifier extends SingleChildRenderObjectWidget { class _RenderMagnification extends RenderProxyBox { _RenderMagnification( this._focalPointOffset, - this._magnificationScale, - this._shape, { + this._magnificationScale, { RenderBox? child, }) : super(child); @@ -617,16 +648,6 @@ class _RenderMagnification extends RenderProxyBox { markNeedsPaint(); } - ShapeBorder get shape => _shape; - ShapeBorder _shape; - set shape(ShapeBorder value) { - if (_shape == value) { - return; - } - _shape = value; - markNeedsPaint(); - } - @override bool get alwaysNeedsCompositing => true; diff --git a/packages/flutter/test/cupertino/magnifier_test.dart b/packages/flutter/test/cupertino/magnifier_test.dart index ba90c936ce..01e7794326 100644 --- a/packages/flutter/test/cupertino/magnifier_test.dart +++ b/packages/flutter/test/cupertino/magnifier_test.dart @@ -23,15 +23,14 @@ void main() { ValueNotifier magnifierInfo, ) async { final Future magnifierShown = magnifierController.show( - context: context, - builder: (_) => CupertinoTextMagnifier( - controller: magnifierController, - magnifierInfo: magnifierInfo, - )); - - WidgetsBinding.instance.scheduleFrame(); - await tester.pumpAndSettle(); - + context: context, + builder: (BuildContext context) => CupertinoTextMagnifier( + controller: magnifierController, + magnifierInfo: magnifierInfo, + ), + ); + await tester.pump(); + await tester.pump(const Duration(seconds: 2)); await magnifierShown; } diff --git a/packages/flutter/test/material/magnifier_test.dart b/packages/flutter/test/material/magnifier_test.dart index b1e3555735..da2db52167 100644 --- a/packages/flutter/test/material/magnifier_test.dart +++ b/packages/flutter/test/material/magnifier_test.dart @@ -168,8 +168,9 @@ void main() { await showMagnifier(context, tester, magnifierInfo); - // Should show two red squares; original, and one in the magnifier, - // directly ontop of one another. + // Should show two red crossed-out squares: the original in the center, + // and one in the magnifier, in the upper half of the image, surrounded + // by a faint offset rounded rectangle shadow. await expectLater( find.byType(MaterialApp), matchesGoldenFile('magnifier.position.default.png'), diff --git a/packages/flutter/test/painting/decoration_test.dart b/packages/flutter/test/painting/decoration_test.dart index dbb4642a15..de7ac97282 100644 --- a/packages/flutter/test/painting/decoration_test.dart +++ b/packages/flutter/test/painting/decoration_test.dart @@ -812,4 +812,17 @@ void main() { info.dispose(); }, skip: kIsWeb); // https://github.com/flutter/flutter/issues/87442 + + test('BoxShadow.copyWith', () { + expect(const BoxShadow(), isNot(const BoxShadow(color: Color(0xFF112233)))); + expect(const BoxShadow().copyWith(color: const Color(0xFF112233)), const BoxShadow(color: Color(0xFF112233))); + expect(const BoxShadow(), isNot(const BoxShadow(offset: Offset(1.0, 2.0)))); + expect(const BoxShadow().copyWith(offset: const Offset(1.0, 2.0)), const BoxShadow(offset: Offset(1.0, 2.0))); + expect(const BoxShadow(), isNot(const BoxShadow(blurRadius: 123.0))); + expect(const BoxShadow().copyWith(blurRadius: 123.0), const BoxShadow(blurRadius: 123.0)); + expect(const BoxShadow(), isNot(const BoxShadow(spreadRadius: 123.0))); + expect(const BoxShadow().copyWith(spreadRadius: 123.0), const BoxShadow(spreadRadius: 123.0)); + expect(const BoxShadow(), isNot(const BoxShadow(blurStyle: BlurStyle.outer))); + expect(const BoxShadow().copyWith(blurStyle: BlurStyle.outer), const BoxShadow(blurStyle: BlurStyle.outer)); + }); } diff --git a/packages/flutter/test/widgets/magnifier_test.dart b/packages/flutter/test/widgets/magnifier_test.dart index 922e2e9d31..67ec028eae 100644 --- a/packages/flutter/test/widgets/magnifier_test.dart +++ b/packages/flutter/test/widgets/magnifier_test.dart @@ -53,7 +53,7 @@ void main() { await tester.pumpWidget(MaterialApp( key: appKey, home: Container( - color: Colors.orange, + color: Colors.blue, width: double.infinity, height: double.infinity, child: Stack( @@ -64,7 +64,7 @@ void main() { left: magnifierPosition.dx + magnifierFocalPoint.dx, top: magnifierPosition.dy + magnifierFocalPoint.dy, child: Container( - color: Colors.pink, + color: Colors.black, // Since it is the size of the magnifier but over its // magnificationScale, it should take up the whole magnifier. width: (magnifierSize.width * 1.5) / magnificationScale, @@ -78,26 +78,27 @@ void main() { size: magnifierSize, focalPointOffset: magnifierFocalPoint, magnificationScale: magnificationScale, - decoration: MagnifierDecoration(shadows: [ - BoxShadow( - spreadRadius: 10, - blurRadius: 10, - color: Colors.green, - offset: Offset(5, 5), - ), - ]), + clipBehavior: Clip.hardEdge, + decoration: MagnifierDecoration( + shadows: [ + BoxShadow( + spreadRadius: 10.0, + blurRadius: 10.0, + color: Colors.yellow, + offset: Offset(5.0, 5.0), + ), + ], + opacity: 0.5, + ), ), ), ], ), ))); - await tester.pumpAndSettle(); - - // Should look like an orange screen, with two pink boxes. - // One pink box is in the magnifier (so has a green shadow) and is double - // size (from magnification). Also, the magnifier should be slightly orange - // since it has opacity. + // Should look like a blue screen, with two black boxes. The larger black + // box is in the magnifier, is outlined in yellow, and is doubled in size + // (from magnification). The magnifier should be slightly transparent. await expectLater( find.byKey(appKey), matchesGoldenFile('widgets.magnifier.styled.png'), @@ -325,4 +326,11 @@ void main() { } }); }); + + testWidgets('MagnifierInfo.toString', (WidgetTester tester) async { + expect(MagnifierInfo.empty.toString(), + 'MagnifierInfo(position: Offset(0.0, 0.0), line: Rect.fromLTRB(0.0, 0.0, 0.0, 0.0), ' + 'caret: Rect.fromLTRB(0.0, 0.0, 0.0, 0.0), field: Rect.fromLTRB(0.0, 0.0, 0.0, 0.0))', + ); + }); }