Gradient transform #42484

dnfield · 2019-10-11T00:01:15Z

Description

Exposes the transform parameter for gradients into the framework via a GradientTransform class, which is akin to Alignment or EdgeInsets in that it resolves an actual Matrix4 by means of a Rect (and potentially a TextDirection). This enables more easily creating rotated gradient effects without having to rotate the entire canvas.

Fixes #23648

Related Issues

#23648

Tests

I added the following tests:

Golden tests for transformed gradients.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

Yes, this is a breaking change (Please read Handling breaking changes). Replace this with a link to the e-mail where you asked for input on this proposed change.
No, this is not a breaking change.

dnfield · 2019-10-11T00:11:46Z

packages/flutter/lib/src/painting/gradient.dart

+  ///   // Calculate the point to rotate about.
+  ///   final double sinRadians = math.sin(radians);
+  ///   final double oneMinusCosRadians = 1 - math.cos(radians);
+  ///   final Offset center = rect.center;
+  ///   final double originX = sinRadians * center.dy + oneMinusCosRadians * center.dx;
+  ///   final double originY = -sinRadians * center.dx + oneMinusCosRadians * center.dy;
+  ///
+  ///   final Matrix4 transform = Matrix4.identity()
+  ///     ..translate(originX, originY)
+  ///     ..rotateZ(radians);


I suspect having amethod like this in the framework might be helpful to people, but I'm not sure where it would go. MatrixUtils?

@goderbauer wdyt?

Hixie · 2019-10-11T00:12:09Z

Would it make more sense for the matrix to be a property rather than something you pass to createShader? The shader is created pretty low-down, most people don't use that API.

dnfield · 2019-10-11T00:20:48Z

The thing is, you're generally going to need to know the Rect that's being used for createShader to make sense of what to do with your matrix. And the people requesting this in the linked bug were already using createShader to apply this to a Paint object's shader property in things like a CustomPaint anyway.

It would be nice but I'm not sure how we'd really abstract the rect away properly.

dnfield · 2019-10-11T00:26:39Z

We could maybe give it a callback where you're given a rect and you give back a transform, like:

typdef MatrixBuilder = Matrix4 Function(Rect rect);

And then createShader would call that and use the transform created by that to transform the shader. That would make this more useable in something like a BoxDecoration. WDYT?

Hixie · 2019-10-11T01:10:51Z

We could also take a new kind of class that represents a matrix transform. That class would have a method on it that takes a rect to do the actual transform computation. This is analogous to what we do wiht Alignment or EdgeInsets (both of which need the actual Rect to do their actual work, but don't have it available when you specify them).

dnfield · 2019-10-11T03:46:01Z

I've added a GradientTransform and a GradientRotation class.

We could trivially implement things like GradientTranslate and GradientSkew and the like - rotation is probably the hardest of the bunch though and it's the only one being asked for right now.

Hixie · 2019-10-11T06:58:38Z

packages/flutter/lib/src/painting/gradient.dart

+///
+/// For example, a [SweepGradient] normally starts its gradation at 3 o'clock
+/// and draws clockwise. To have the sweep appear to start at 6 o'clock, supply
+/// a [GradientRotation] of `0.785398` radians (i.e. `45` degrees).


trivial nit: I think our style is to not use backticks for literals.

Hixie · 2019-10-11T06:58:51Z

packages/flutter/lib/src/painting/gradient.dart

+/// a [GradientRotation] of `0.785398` radians (i.e. `45` degrees).
+@immutable
+abstract class GradientTransform {
+  /// A const constructor that allows subclasses to be const.


there's a boilerplate paragraph we use for this kind of thing.

Hixie · 2019-10-11T07:00:07Z

packages/flutter/lib/src/painting/gradient.dart

+///   colors: colors,
+///   transform: GradientRotation(0.785398),
+/// );
+/// ```


we should use the sample code logic for this

Hixie · 2019-10-11T07:00:18Z

packages/flutter/lib/src/painting/gradient.dart

+/// ```
+@immutable
+class GradientRotation extends GradientTransform {
+  /// Constructs a `GradientRotation` for the specified angle.


use [] for identifiers

Hixie · 2019-10-11T07:00:56Z

packages/flutter/lib/src/painting/gradient.dart

+  /// The angle is in radians in the clockwise direction.
+  const GradientRotation(this.radians);
+
+  /// The angle of rotation.


This needs more detail, e.g. direction of rotation (clockwise, counter-clockwise), the units (though it's pretty obvious from the name).

Hixie · 2019-10-11T07:02:02Z

packages/flutter/lib/src/painting/gradient.dart

+  ///
+  /// The [transform] argument can be applied to transform _only_ the gradient,
+  /// without rotating the canvas itself or other geometry on the canvas. For
+  /// example, a `GradientRotation(0.785398)` will result in a [SweepGradient]


here and elsewhere, rather than 0.785398 we should say pi/4

Hixie · 2019-10-11T07:04:37Z

This looks great!

dnfield · 2019-10-11T15:43:06Z

In case it's not obvious - I've rev'd goldens a few times in this because I've made very minor changes to the tests that are resulting in miniscule pixel differences.

…gradient_transform

dnfield added 9 commits May 16, 2019 10:10

initial

8329cf6

Merge remote-tracking branch 'upstream/master' into gradient_transform

3844496

Merge remote-tracking branch 'upstream/master' into gradient_transform

a32c949

..

d522884

fix exmaple

b345595

tests

cf2e0a3

tests

db13dc8

..

f2db848

tests

db8df6b

dnfield requested review from goderbauer and Piinks October 11, 2019 00:01

fluttergithubbot added the framework flutter/packages/flutter repository. See also f: labels. label Oct 11, 2019

googlebot added the cla: yes label Oct 11, 2019

dnfield added a commit to flutter/goldens that referenced this pull request Oct 11, 2019

Goldens for flutter/flutter#42484

c0dcafd

dnfield commented Oct 11, 2019

View reviewed changes

dnfield added a commit to flutter/goldens that referenced this pull request Oct 11, 2019

Update Goldens for flutter/flutter#42484

9a2854e

GradientTransform

13d7f99

dnfield force-pushed the gradient_transform branch from d1cdc44 to 13d7f99 Compare October 11, 2019 03:33

dnfield added 2 commits October 10, 2019 20:43

sample

6060c6b

unused import

7662328

dnfield requested a review from Hixie October 11, 2019 03:45

dnfield added 2 commits October 10, 2019 20:48

reapply golden update...

3c964f7

fix sample

122e5aa

Hixie reviewed Oct 11, 2019

View reviewed changes

ened mentioned this pull request Oct 11, 2019

Performance issue when drawing a rotated gradient into a clipped button #36935

Closed

dnfield added 2 commits October 11, 2019 08:04

review .. may require on emore goldnes bump

326acfa

sample

36b0cf8

dnfield added a commit to flutter/goldens that referenced this pull request Oct 11, 2019

Update Goldens for flutter/flutter#42484 again

fa13c1b

rev goldens again

b13c116

dnfield added the will affect goldens Changes to golden files label Oct 11, 2019

dnfield added 3 commits October 11, 2019 10:00

Merge remote-tracking branch 'upstream/master' into gradient_transform

4b66a31

Fix sample agian.

cbde192

Merge branch 'gradient_transform' of github.com:dnfield/flutter into …

cd6e736

…gradient_transform

dnfield merged commit de499c2 into flutter:master Oct 11, 2019

dnfield deleted the gradient_transform branch October 11, 2019 20:40

yjbanov mentioned this pull request Oct 14, 2019

Reland: fix and enable painting web tests #42689

Merged

9 tasks

Piinks mentioned this pull request Nov 22, 2019

Set custom size for your golden files #44185

Closed

Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019

Gradient transform (flutter#42484)

e065a0a

github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gradient transform #42484

Gradient transform #42484

dnfield commented Oct 11, 2019 •

edited

dnfield Oct 11, 2019

Hixie commented Oct 11, 2019

dnfield commented Oct 11, 2019

dnfield commented Oct 11, 2019

Hixie commented Oct 11, 2019

dnfield commented Oct 11, 2019

Hixie Oct 11, 2019

Hixie Oct 11, 2019

Hixie Oct 11, 2019

Hixie Oct 11, 2019

Hixie Oct 11, 2019

Hixie Oct 11, 2019

Hixie commented Oct 11, 2019

dnfield commented Oct 11, 2019

Gradient transform #42484

Gradient transform #42484

Conversation

dnfield commented Oct 11, 2019 • edited

Description

Related Issues

Tests

Checklist

Breaking Change

dnfield Oct 11, 2019

Choose a reason for hiding this comment

Hixie commented Oct 11, 2019

dnfield commented Oct 11, 2019

dnfield commented Oct 11, 2019

Hixie commented Oct 11, 2019

dnfield commented Oct 11, 2019

Hixie Oct 11, 2019

Choose a reason for hiding this comment

Hixie Oct 11, 2019

Choose a reason for hiding this comment

Hixie Oct 11, 2019

Choose a reason for hiding this comment

Hixie Oct 11, 2019

Choose a reason for hiding this comment

Hixie Oct 11, 2019

Choose a reason for hiding this comment

Hixie Oct 11, 2019

Choose a reason for hiding this comment

Hixie commented Oct 11, 2019

dnfield commented Oct 11, 2019

dnfield commented Oct 11, 2019 •

edited