Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Commit

Permalink
Fix applyBoxFit's handling of fitWidth and fitHeight. (#117185)
Browse files Browse the repository at this point in the history
* Fix applyBoxFit's handling of fitWidth and fitHeight.

Previously, in `fitWidth` mode, if the input size had a wider aspect
ratio than the output size, `applyBoxFit` would make the source rect
taller than the input size in order to match the aspect ratio of the
destination rect.  Similarly, in `fitHeight` mode, if the input size
had a taller aspect ratio than the output size, `applyBoxFit` would
make the source rect wider than the input size in to match the aspect
ratio of the destination rect.  This is in contrast to all the other
modes, which never output a source rect that's larger than the input
size.

Most of the time this worked as intended (since attempting to blit
pixels that are outside the source image has no effect), however it
meant that if a user attempted to create a `BoxDecoration` that used
both `fitWidth` and `repeatY` (e.g. in an attempt to tile a background
image), the image would not actually appear to repeat, since the logic
in `paintImage` for determining the proper tiling stride is based on
the destination image size, meaning that the entire destination rect
would be covered in a single tile.

This change modifies `applyBoxFit` so that in `fitWidth` mode, if the
input size has a wider aspect ratio than the output size, it uses
formulas that are equivalent to `contain`, whereas if the input size
has a taller aspect ratio than the output size, it uses formulas that
are equivalent to `cover`.  And vice versa for `fitHeight` mode.  This
produces source and destination rects that match the behaviour
specified in https://api.flutter.dev/flutter/painting/BoxFit.html.

* Apply suggestions from code review

Co-authored-by: Michael Goderbauer <goderbauer@google.com>
  • Loading branch information
stereotype441 and goderbauer committed Jan 18, 2023
1 parent 6277520 commit 997d436
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 4 deletions.
22 changes: 18 additions & 4 deletions packages/flutter/lib/src/painting/box_fit.dart
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,26 @@ FittedSizes applyBoxFit(BoxFit fit, Size inputSize, Size outputSize) {
destinationSize = outputSize;
break;
case BoxFit.fitWidth:
sourceSize = Size(inputSize.width, inputSize.width * outputSize.height / outputSize.width);
destinationSize = Size(outputSize.width, sourceSize.height * outputSize.width / sourceSize.width);
if (outputSize.width / outputSize.height > inputSize.width / inputSize.height) {
// Like "cover"
sourceSize = Size(inputSize.width, inputSize.width * outputSize.height / outputSize.width);
destinationSize = outputSize;
} else {
// Like "contain"
sourceSize = inputSize;
destinationSize = Size(outputSize.width, sourceSize.height * outputSize.width / sourceSize.width);
}
break;
case BoxFit.fitHeight:
sourceSize = Size(inputSize.height * outputSize.width / outputSize.height, inputSize.height);
destinationSize = Size(sourceSize.width * outputSize.height / sourceSize.height, outputSize.height);
if (outputSize.width / outputSize.height > inputSize.width / inputSize.height) {
// Like "contain"
sourceSize = inputSize;
destinationSize = Size(sourceSize.width * outputSize.height / sourceSize.height, outputSize.height);
} else {
// Like "cover"
sourceSize = Size(inputSize.height * outputSize.width / outputSize.height, inputSize.height);
destinationSize = outputSize;
}
break;
case BoxFit.none:
sourceSize = Size(math.min(inputSize.width, outputSize.width), math.min(inputSize.height, outputSize.height));
Expand Down
8 changes: 8 additions & 0 deletions packages/flutter/test/painting/box_fit_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,18 @@ void main() {
expect(result.source, equals(const Size(2000.0, 200.0)));
expect(result.destination, equals(const Size(1000.0, 100.0)));

result = applyBoxFit(BoxFit.fitWidth, const Size(2000.0, 400.0), const Size(1000.0, 300.0));
expect(result.source, equals(const Size(2000.0, 400.0)));
expect(result.destination, equals(const Size(1000.0, 200.0)));

result = applyBoxFit(BoxFit.fitHeight, const Size(400.0, 2000.0), const Size(100.0, 1000.0));
expect(result.source, equals(const Size(200.0, 2000.0)));
expect(result.destination, equals(const Size(100.0, 1000.0)));

result = applyBoxFit(BoxFit.fitHeight, const Size(400.0, 2000.0), const Size(300.0, 1000.0));
expect(result.source, equals(const Size(400.0, 2000.0)));
expect(result.destination, equals(const Size(200.0, 1000.0)));

_testZeroAndNegativeSizes(BoxFit.fill);
_testZeroAndNegativeSizes(BoxFit.contain);
_testZeroAndNegativeSizes(BoxFit.cover);
Expand Down
80 changes: 80 additions & 0 deletions packages/flutter/test/painting/decoration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,86 @@ void main() {
}
});

test('paintImage with repeatX and fitHeight', () async {
final TestCanvas canvas = TestCanvas();

// Paint a square image into an output rect that is twice as wide as it is
// tall. Two copies of the image should be painted, one next to the other.
const Rect outputRect = Rect.fromLTWH(30.0, 30.0, 400.0, 200.0);
final ui.Image image = await createTestImage(width: 100, height: 100);

paintImage(
canvas: canvas,
rect: outputRect,
image: image,
alignment: Alignment.topLeft,
fit: BoxFit.fitHeight,
repeat: ImageRepeat.repeatX,
);

const Size imageSize = Size(100.0, 100.0);

final List<Invocation> calls = canvas.invocations.where((Invocation call) => call.memberName == #drawImageRect).toList();
final Set<Rect> tileRects = <Rect>{};

expect(calls, hasLength(2));
for (final Invocation call in calls) {
expect(call.isMethod, isTrue);
expect(call.positionalArguments, hasLength(4));

expect(call.positionalArguments[0], isA<ui.Image>());

// sourceRect should contain all pixels of the source image
expect(call.positionalArguments[1], Offset.zero & imageSize);

tileRects.add(call.positionalArguments[2] as Rect);

expect(call.positionalArguments[3], isA<Paint>());
}

expect(tileRects, <Rect>{const Rect.fromLTWH(30.0, 30.0, 200.0, 200.0), const Rect.fromLTWH(230.0, 30.0, 200.0, 200.0)});
});

test('paintImage with repeatY and fitWidth', () async {
final TestCanvas canvas = TestCanvas();

// Paint a square image into an output rect that is twice as tall as it is
// wide. Two copies of the image should be painted, one above the other.
const Rect outputRect = Rect.fromLTWH(30.0, 30.0, 200.0, 400.0);
final ui.Image image = await createTestImage(width: 100, height: 100);

paintImage(
canvas: canvas,
rect: outputRect,
image: image,
alignment: Alignment.topLeft,
fit: BoxFit.fitWidth,
repeat: ImageRepeat.repeatY,
);

const Size imageSize = Size(100.0, 100.0);

final List<Invocation> calls = canvas.invocations.where((Invocation call) => call.memberName == #drawImageRect).toList();
final Set<Rect> tileRects = <Rect>{};

expect(calls, hasLength(2));
for (final Invocation call in calls) {
expect(call.isMethod, isTrue);
expect(call.positionalArguments, hasLength(4));

expect(call.positionalArguments[0], isA<ui.Image>());

// sourceRect should contain all pixels of the source image
expect(call.positionalArguments[1], Offset.zero & imageSize);

tileRects.add(call.positionalArguments[2] as Rect);

expect(call.positionalArguments[3], isA<Paint>());
}

expect(tileRects, <Rect>{const Rect.fromLTWH(30.0, 30.0, 200.0, 200.0), const Rect.fromLTWH(30.0, 230.0, 200.0, 200.0)});
});

test('DecorationImage scale test', () async {
final ui.Image image = await createTestImage(width: 100, height: 100);
final DecorationImage backgroundImage = DecorationImage(
Expand Down

0 comments on commit 997d436

Please sign in to comment.