Skip to content

Commit 8bd06c0

Browse files
auto-submit[bot]auto-submit[bot]
andauthored
Reverts "Implements the Android native stretch effect as a fragment shader (Impeller-only). (flutter#169293)" (flutter#173865)
<!-- start_original_pr_link --> Reverts: flutter#169293 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: matanlurey <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: Does not work on Metal (iOS/macOS): ``` The following _Exception was thrown running a test: Exception: Asset 'shaders/stretch_effect.frag' does not contain appropriate runtime stage data for current backend (Metal). ``` <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: MTtankkeo <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {dkwingsmt, justinmc, chunhtai} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: > Sorry, Closing PR flutter#169196 and reopening this in a new PR for clarity and a cleaner commit history. Fixes flutter#82906 In the existing Flutter implementation, the Android stretch overscroll effect is achieved using Transform. However, this approach does not evenly stretch the screen as it does in the native Android environment. Therefore, in the Impeller environment, add or modify files to implement the effect using a fragment shader identical to the one used in native Android. > [!NOTE] > - The addition of a separate test file for StretchOverscrollEffect was not included because it would likely duplicate coverage already provided by the changes made in overscroll_stretch_indicator_test.dart within this PR. >> However, since determining whether stretching occurred by referencing global coordinates is currently considered impossible with the new Fragment Shader approach, the code was modified to partially exclude the relevant test logic in the Impeller. >> >> For reference, in the page_view_test.dart test, there was an issue with referencing the child Transform widget, but because the StretchOverscrollEffect widget is defined instead of the Transform widget in the Impeller environment, the code logic was adjusted accordingly. > > - Golden image tests were updated as the visual effect changes. ## Reference Source - https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/libs/hwui/effects/StretchEffect.cpp ## `Old` Skia (Using Transform) https://github.com/user-attachments/assets/22d8ff96-d875-4722-bf6f-f0ad15b9077d ## `New` Impeller (Using fragment shader) https://github.com/user-attachments/assets/73b6ef18-06b2-42ea-9793-c391ec2ce282 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
1 parent e0c1c49 commit 8bd06c0

File tree

10 files changed

+709
-1249
lines changed

10 files changed

+709
-1249
lines changed

packages/flutter/lib/src/material/shaders/stretch_effect.frag

Lines changed: 0 additions & 159 deletions
This file was deleted.

packages/flutter/lib/src/widgets/overscroll_indicator.dart

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import 'framework.dart';
2323
import 'media_query.dart';
2424
import 'notification_listener.dart';
2525
import 'scroll_notification.dart';
26-
import 'stretch_effect.dart';
2726
import 'ticker_provider.dart';
2827
import 'transitions.dart';
2928

@@ -775,6 +774,20 @@ class _StretchingOverscrollIndicatorState extends State<StretchingOverscrollIndi
775774
return false;
776775
}
777776

777+
AlignmentGeometry _getAlignmentForAxisDirection(_StretchDirection stretchDirection) {
778+
// Accounts for reversed scrollables by checking the AxisDirection
779+
final AxisDirection direction = switch (stretchDirection) {
780+
_StretchDirection.trailing => widget.axisDirection,
781+
_StretchDirection.leading => flipAxisDirection(widget.axisDirection),
782+
};
783+
return switch (direction) {
784+
AxisDirection.up => AlignmentDirectional.topCenter,
785+
AxisDirection.down => AlignmentDirectional.bottomCenter,
786+
AxisDirection.left => Alignment.centerLeft,
787+
AxisDirection.right => Alignment.centerRight,
788+
};
789+
}
790+
778791
@override
779792
void dispose() {
780793
_stretchController.dispose();
@@ -789,34 +802,30 @@ class _StretchingOverscrollIndicatorState extends State<StretchingOverscrollIndi
789802
animation: _stretchController,
790803
builder: (BuildContext context, Widget? child) {
791804
final double stretch = _stretchController.value;
805+
double x = 1.0;
806+
double y = 1.0;
792807
final double mainAxisSize;
793808

794809
switch (widget.axis) {
795810
case Axis.horizontal:
811+
x += stretch;
796812
mainAxisSize = MediaQuery.widthOf(context);
797813
case Axis.vertical:
814+
y += stretch;
798815
mainAxisSize = MediaQuery.heightOf(context);
799816
}
800817

818+
final AlignmentGeometry alignment = _getAlignmentForAxisDirection(
819+
_stretchController.stretchDirection,
820+
);
821+
801822
final double viewportDimension =
802823
_lastOverscrollNotification?.metrics.viewportDimension ?? mainAxisSize;
803-
804-
double overscroll = stretch;
805-
806-
if (_stretchController.stretchDirection == _StretchDirection.trailing) {
807-
overscroll = -overscroll;
808-
}
809-
810-
// Adjust overscroll for reverse scroll directions.
811-
if (widget.axisDirection == AxisDirection.up ||
812-
widget.axisDirection == AxisDirection.left) {
813-
overscroll = -overscroll;
814-
}
815-
816-
final Widget transform = StretchEffect(
817-
stretchStrength: overscroll,
818-
axis: widget.axis,
819-
child: widget.child!,
824+
final Widget transform = Transform(
825+
alignment: alignment,
826+
transform: Matrix4.diagonal3Values(x, y, 1.0),
827+
filterQuality: stretch == 0 ? null : FilterQuality.medium,
828+
child: widget.child,
820829
);
821830

822831
// Only clip if the viewport dimension is smaller than that of the

0 commit comments

Comments
 (0)