Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[web] Fix transform not invalidating path bounds causing debugValidate failure #22172

Merged
ferhatb merged 4 commits into from
Oct 29, 2020
Merged

[web] Fix transform not invalidating path bounds causing debugValidate failure #22172

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0087911
Fix transform not invalidating path bounds causing debugValidate failure
ferhatb Oct 28, 2020
2294e2d
address reviewer comment
ferhatb Oct 28, 2020
a8d8789
Merge remote-tracking branch 'upstream/master' into unstable_path
ferhatb Oct 28, 2020
933034a
Merge remote-tracking branch 'upstream/master' into unstable_path
ferhatb Oct 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions lib/web_ui/lib/src/engine/html/path/path_ref.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ class PathRef {
static const int kInitialPointsCapacity = 8;
static const int kInitialVerbsCapacity = 8;

/// Bounds of points that define path.
ui.Rect? fBounds;
/// Computed tight bounds of path (may exclude curve control points).
ui.Rect? cachedBounds;
int _fPointsCapacity = 0;
int _fPointsLength = 0;
Expand Down Expand Up @@ -730,6 +732,7 @@ class PathRef {
fIsRRect = false;
fIsRect = false;
cachedBounds = null;
fBoundsIsDirty = true;
}

void setIsOval(bool isOval, bool isCCW, int start) {
Expand Down
14 changes: 14 additions & 0 deletions lib/web_ui/test/path_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,5 +536,19 @@ void testMain() {
end = iter.skipToNextContour();
expect(start, end);
});

/// Regression test for https://github.com/flutter/flutter/issues/68702.
test('Path should return correct bounds after transform', () {
final Path path1 = Path()
..moveTo(100, 100)
..lineTo(200, 100)
..lineTo(150, 200)
..close();
final SurfacePath path2 = Path.from(path1) as SurfacePath;
Rect bounds = path2.pathRef.getBounds();
SurfacePath transformedPath = path2.transform(
Matrix4.identity().scaled(0.5, 0.5).toFloat64());
expect(bounds != transformedPath.pathRef.getBounds(), isTrue);

@ditman ditman Oct 28, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(bounds != transformedPath.pathRef.getBounds(), isTrue);
expect(transformedPath.pathRef.getBounds(), isNot(bounds));

Instead of a "expected true but was false" test failure, you can get a slightly more explicit error message if you use isNot(equals()) or similar, at least you get the toString of the objects that were being compared :P

@ferhatb ferhatb Oct 28, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thank you for suggestion!

});
});
}