Skip to content

Commit

Permalink
fix: Clamp opacity set by the ColorEffect to 1.0 (#3069)
Browse files Browse the repository at this point in the history
Clamp opacity set by the ColorEffect to 1.0; this was causing an issue
when the tween returned values greater than 1.

The test shows that when dt is greater than 0.5 and less than 0.75 for a
period of 1 the tween returns approx 1.2 which causes Color to throw an
exception.

This has now been fixed.

Since flutter/flutter#89433 has also now been
fixed, I have changed the min opacity to 0.
  • Loading branch information
billy1380 committed Mar 6, 2024
1 parent 12841d6 commit 9282cc3
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 9 deletions.
5 changes: 1 addition & 4 deletions packages/flame/lib/src/effects/color_effect.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,7 @@ class ColorEffect extends ComponentEffect<HasPaint> {
@override
void apply(double progress) {
final currentColor = color.withOpacity(
// Currently there is a bug when opacity is 0 in the color filter.
// "Expected a value of type 'SkDeletable', but got one of type 'Null'"
// https://github.com/flutter/flutter/issues/89433
max(_tween.transform(progress), 1 / 255),
min(max(_tween.transform(progress), 0), 1),
);
target.tint(currentColor, paintId: paintId);
}
Expand Down
58 changes: 53 additions & 5 deletions packages/flame/test/effects/color_effect_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ void main() {
game.update(0);
expect(
component.paint.colorFilter.toString(),
// Once https://github.com/flutter/flutter/issues/89433 has been fixed
// the two equals lines should be swapped and the ColorEffect should go
// to opacity 0.
//equals('ColorFilter.mode(Color(0x00f44336), BlendMode.srcATop)'),
equals('ColorFilter.mode(Color(0x01f44336), BlendMode.srcATop)'),
equals('ColorFilter.mode(Color(0x00f44336), BlendMode.srcATop)'),
);

game.update(0.5);
Expand Down Expand Up @@ -62,6 +58,36 @@ void main() {
},
);

testWithFlameGame(
'resets the color filter to the original state',
(game) async {
final component = _PaintComponent();
await game.ensureAdd(component);

final originalColorFilter = component.paint.colorFilter;
const color = Colors.red;

final effect = ColorEffect(
color,
EffectController(duration: 1),
);
await component.add(effect);
game.update(0.5);

expect(
originalColorFilter,
isNot(equals(component.paint.colorFilter)),
);

effect.reset();

expect(
originalColorFilter,
equals(component.paint.colorFilter),
);
},
);

testWithFlameGame(
'can be re-added in the component tree',
(game) async {
Expand All @@ -84,6 +110,28 @@ void main() {
},
);

testWithFlameGame(
'will clamp controllers that over or under set the progress value',
(game) async {
final component = _PaintComponent();
await game.ensureAdd(component);

final effect = ColorEffect(
opacityFrom: 1,
opacityTo: 0,
Colors.black,
ZigzagEffectController(
period: 1,
),
);
await component.ensureAdd(effect);
expect(
() => game.update(0.56),
returnsNormally,
);
},
);

test('Validates opacity values', () {
expect(
() => ColorEffect(
Expand Down

0 comments on commit 9282cc3

Please sign in to comment.