[Impeller] Make paths externally immutable, update all tests to use PathBuilder to create Path.

[jonahwilliams]

This is a design problem in the Flutter API, that in general you can't assume a Path hasn't changed from the last time you used it. We're so close, so just move the mutation methods to PathBuilder :)

Fixes flutter/flutter#133880

[chinmaygarde]

👍🏽

[jonahwilliams]

So we might have a bug in path builder, or I'm misunderstanding the point of the test. We had a test case "PathCreatePolyLineDoesNotDuplicatePoints"

engine/impeller/geometry/geometry_unittests.cc

Lines 2009 to 2026 in f151289

	TEST(GeometryTest, PathCreatePolyLineDoesNotDuplicatePoints) {
	Path path;
	path.AddContourComponent({10, 10});
	path.AddLinearComponent({10, 10}, {20, 20});
	path.AddLinearComponent({20, 20}, {30, 30});
	path.AddContourComponent({40, 40});
	path.AddLinearComponent({40, 40}, {50, 50});
	auto polyline = path.CreatePolyline(1.0f);
	ASSERT_EQ(polyline.contours.size(), 2u);
	ASSERT_EQ(polyline.points.size(), 5u);
	ASSERT_EQ(polyline.points[0].x, 10);
	ASSERT_EQ(polyline.points[1].x, 20);
	ASSERT_EQ(polyline.points[2].x, 30);
	ASSERT_EQ(polyline.points[3].x, 40);
	ASSERT_EQ(polyline.points[4].x, 50);
	}

I updated it to use the PathBuilder API in a way that I think is correct. However, now it seems like the polyline does in fact have duplicated points.

[jonahwilliams]

The points I get from the tests are now:

[ERROR:flutter/impeller/geometry/geometry_unittests.cc(2035)] (10, 10)
[ERROR:flutter/impeller/geometry/geometry_unittests.cc(2035)] (20, 20)
[ERROR:flutter/impeller/geometry/geometry_unittests.cc(2035)] (20, 20)
[ERROR:flutter/impeller/geometry/geometry_unittests.cc(2035)] (30, 30)
[ERROR:flutter/impeller/geometry/geometry_unittests.cc(2035)] (40, 40)
[ERROR:flutter/impeller/geometry/geometry_unittests.cc(2035)] (50, 50)

WHich I think is because PathBuilder is inserting Moves. However, we're only ever building paths with PathBuilder today, so I'm not sure if the expectation is overly specific to the original bug.

FYI @bdero

[bdero]

Add* inserts a new contour -- this actually needs to be LineTo to be equivalent.

builder.MoveTo({10, 10});
builder.LineTo({20, 20});
builder.LineTo({30, 30});

And no, you're not crazy. Yes, the linear component has redundant data.

[jonahwilliams]

Aghh, good catch. Thank you

[flutter-dashboard]

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.

Changes reported for pull request #45393 at sha 923f7b7