Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #169293 at sha 3c6f85a

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #169293 at sha 8b26ebc

😭 Hi @justinmc, @dkwingsmt, @Piinks, could you please review this PR? Thanks! 😭

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

After an internal discussion between @Piinks, a couple other folks, and I, we've come to agree with the current direction to add the shader to the framework as long as it aligns with how we implement the ink sparkle. We'll take a more detailed review at this PR soon, but I just want to let you know ASAP so you don't have to look into the engine code. Sorry for the confusion!

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #169293 at sha 93ad7c2

I’ve tried to engage constructively and move this forward, but I need your response to make progress.

Just a quick follow-up — I've proposed a resolution to the concern at https://github.com/flutter/flutter/pull/169293/files#r2165791595 with a StretchEffect abstraction. If there's no objection, I'll proceed with that implementation and mark the thread as resolved.

Please let me know if you have any blockers. Otherwise, I’ll consider the discussion resolved and move forward.

I still haven't seen https://github.com/flutter/flutter/pull/169293/files#r2165791595 addressed.

@dkwingsmt I suggested something in response to your review — did you get a chance to read my comment?

Ah, my apology. I probably have missed the reply. Can you find me the link to your reply (since there are too many items above)?

I still haven't seen https://github.com/flutter/flutter/pull/169293/files#r2165791595 addressed.

@dkwingsmt I suggested something in response to your review — did you get a chance to read my comment?

Ah, my apology. I probably have missed the reply. Can you find me the link to your reply (since there are too many items above)?

That comment is outdated.

Comment

@dkwingsmt @Piinks considering the concerns around making StretchOverscrollEffect public due to its renderer dependency, what do you think about introducing a new widget like StretchEffect?

This widget would act as a safe abstraction, internally switching between StretchOverscrollEffect (when supported) and a fallback like Transform.

That way we can still expose a reusable, scroll-independent stretch effect as @Piinks suggested, while encapsulating renderer-specific logic and preventing misuse — and this structure also allows us to make StretchOverscrollEffect private, as you suggested.

I think this could be a good way to address both of your concerns. I'd love to hear your thoughts on it.

Example

class StretchEffect extends StatelessWidget {
  const StretchEffect({
    super.key,
    this.stretchStrength = 0.0,
    required this.axis,
    required this.child,
  });

  final double stretchStrength;
  final Axis axis;
  final Widget child;

  @override
  Widget build(BuildContext context) {
    if (ImageFilter.isShaderFilterSupported) {
      return _StretchOverscrollEffect(...);
    } else {
      return Transform(transform: ...)
    }
  }
}

// private
class _StretchOverscrollEffect {}

I see, and you have moved forward with it. It looks good to me at a first glance, but let me take a closer look!

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #169293 at sha f7f1129

Sorry, my reply was in a pending state. You should be able to see it now!

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #169293 at sha c97605a

Triaged images 👍

Thank you again for contributing this!

reason for revert: 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).

@MTtankkeo Can you open a new PR to try to reland this when you get a chance? The best way is probably to press the "Revrt" button on #173865. We do still want to solve this!

We're trying to figure out what caused the error posted above. Probably it's a configuration thing and not related to your shader code.

@justinmc new PR: #173885