[Impeller] Faster path for circle geometry, rename compute path geometry. #47845
Conversation
|
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. |
chinmaygarde
changed the title
| points[offset++] = origin; | ||
| points[offset++] = pt2; | ||
| pt2 = center + angle_table[j + 3]; | ||
| points[offset++] = pt2; |
There was a problem hiding this comment.
You could reduce the verts by 1/3 by making this a triangle strip
There was a problem hiding this comment.
Ahh, I had a brain fart. The reason this wasn't working is that this code is also used for draw points, so I'd also need to insert padding to break the strip. I'd rather leave this as is for now.
| auto elapsed_angle = radian_start; | ||
| std::vector<Point> angle_table(vertices_per_geom); | ||
| for (auto i = 0u; i < vertices_per_geom; i++) { | ||
| angle_table[i] = Point(cos(elapsed_angle), sin(elapsed_angle)) * radius; |
There was a problem hiding this comment.
Instead of generating a table for the whole circle, you could just generate points for the 0 to PI/2 range and reuse the table for all 4 quadrants.
|
From offline discussion: this code can be simplified by using a Triangle strip. |
|
I think this looks right but I want to double check goldens. |
|
Testing BTW |
|
Golden file changes are available for triage from new commit, Click here to view. |
|
Golden file changes are available for triage from new commit, Click here to view. |
| return {}; | ||
| } | ||
|
|
||
| Scalar min_size = 1.0f / sqrt(std::abs(determinant)); |
There was a problem hiding this comment.
min_size is appropriate for strokes (i.e. when this is created from DrawPoints), but not when we are filling an actual circle (i.e. when created from DrawCircle).
|
One general comment is that I think it might be more useful to split this into Circle and Point geometry classes, but have their common code shared either via global/static methods that tesselate a circle or through a parent "CircleTesselatingGeometry" mixin class to inherit from. While both DrawPoints and DrawCircle may end up tesselating a circle, they generate them from different kinds of data and have different options. For example, DrawPoints may not involve circles at all and even when it does generate circles it will need to filter the radius through the minimum scale before it invokes the tesselation. A couple of lines of code in the split Geometry classes can do this work and then delegate to a common tesselation mechanism. |
…7846) Impeller implements the DrawLine primitive as DrawPath on a path containing a single line. Benchmarks show that this can cost 30% overhead on apps that use a lot of DrawLine primitives. This PR creates a more direct Entity that can tesselate the geometry of a line directly. The reduced overhead should help with flutter/flutter#138004 The current code will back off to Path rendering for round caps. When the circle geometry work is finished (#47845) we can come back and implement round caps using the code refactored in that PR.
|
Closing in favor of flutter/flutter#138254 |
|
Oops i mean: flutter/flutter#138254 |
Work towards flutter/flutter#138004