-
Notifications
You must be signed in to change notification settings - Fork 29.7k
[Impeller] add support for rational bezier conics to Path #163282
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
jonahwilliams
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| points[3] = sub_p2; | ||
| points[4] = p2; | ||
|
|
||
| // Update w. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commented out code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was from the original Sk code that was allowing for further sub-division. We may eventually want to keep sub-dividing so that code might be useful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would leave a note so it doesn't look accidental.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reworded the note to make it more obvious it is a placeholder for future functionality.
| // We only need one value, but the underlying storage allocation is always | ||
| // a multiple of Point objects. To avoid confusion over which field the | ||
| // weight is stored in, and what the value of the other field may be, we | ||
| // store it in both x,y components of the |weight| field. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
The latest round of fixes will very likely not fix the current golden failures, they are feedback changes I wanted to get in while I continue to investigate why the new method that returns the same numbers as the old method causes those golden issues. |
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
It looks like the conics being generated in these examples are degenerate (all 3 points identical) and so the attempt to subdivide them leaves 3 slightly different points that are different as per round-off error and those freak out the stroking math. This isn't a problem created by this change, it's just that we aren't identical in the math (we use non-vectorized equations) and so we end up with "different freak-outs". Looking at a solution for the degenerate conic issue and considering whether to fix that while I'm here. |
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
The degenerate conic protection added here relies on the inefficiency of the code in the star border tests to test it. I'm not sure if we should add an explicit "empty conic test" to prevent regressions in case Flutter changes the star border code to recognize and omit degenerate conic corners for efficiency...? |
jonahwilliams
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Reason for revert: This PR is closing the tree. |
…163282)" (#163573) <!-- start_original_pr_link --> Reverts: #163282 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: victorsanni <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: This PR is closing the tree. <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: flar <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {jonahwilliams} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: Add basic support for storing rational bezier conics in impeller::Path. The support is very thin and just degrades the conics into a pair of quadratic curves just as Impeller has always done for the conic segments that it receives via SkPath objects, but it puts in place the framework for eventually handling the conics more directly and allows the unit tests to be rewritten on top of Impeller paths rather than SkPaths, paving the way for reduced internal API dependencies. <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
|
I think this pull request might have made the tree red. https://flutter-dashboard.appspot.com/#/build?repo=flutter&taskFilter=Mac+framework_tests_impeller&branch=master |
|
|
Unable to create the revert pull request due to ProcessException: Standard error |
…utter#163282)" This reverts commit db0bdfd.
…3282)" (#163645) Reverts #163624 Relands #163282 Add basic support for storing rational bezier conics in impeller::Path. The support is very thin and just degrades the conics into a pair of quadratic curves just as Impeller has always done for the conic segments that it receives via SkPath objects, but it puts in place the framework for eventually handling the conics more directly and allows the unit tests to be rewritten on top of Impeller paths rather than SkPaths, paving the way for reduced internal API dependencies.
Roll Flutter from e8f34a9 to 39b4951 (95 revisions) flutter/flutter@e8f34a9...39b4951 2025-02-18 [email protected] Marks Windows_arm64 plugin_test_windows to be flaky (flutter/flutter#163123) 2025-02-18 [email protected] [android] use macro definition to shrink repetitive JNI code size. (flutter/flutter#163395) 2025-02-18 [email protected] Roll pub packages (flutter/flutter#163474) 2025-02-18 [email protected] [web] Cleanup everything HTML from the engine (outside html/ folder) (flutter/flutter#162840) 2025-02-18 [email protected] [Impeller] add support for rational bezier conics to Path (flutter/flutter#163282) 2025-02-18 [email protected] Change `cardTheme`, `dialogTheme`, and `tabBarTheme` type to `xxxThemeData` (flutter/flutter#157292) 2025-02-18 [email protected] Update integration test and benchmark Android .gitignore files to match the current app template (flutter/flutter#163276) 2025-02-18 [email protected] [Impeller] Don't create a redundant typography context. (flutter/flutter#163513) 2025-02-18 [email protected] Roll Dart SDK from fcef25f18e4d to 023ac80cef14 (1 revision) (flutter/flutter#163110) 2025-02-18 [email protected] update module_host_with_custom_build_v2_embedding to target android 35 and to use the latest gradle and agp versions (flutter/flutter#163542) 2025-02-18 [email protected] [Impeller] when binding to READ_FRAMEBUFFER, treat multisampled textures as single sampled. (flutter/flutter#163345) 2025-02-18 [email protected] Add missing properties to _ArcPaintPredicate. (flutter/flutter#162572) 2025-02-18 [email protected] Roll Packages from 8542af3 to cb4fb13 (4 revisions) (flutter/flutter#163544) 2025-02-18 [email protected] Remove bringup for android_display_cutout (flutter/flutter#163312) 2025-02-18 [email protected] Create VersionUtils class and unit tests and extract logic out of flutter.groovy (flutter/flutter#163166) 2025-02-18 [email protected] Invalidate `pod install` output if `.flutter-plugins-dependencies` content changes. (flutter/flutter#163275) 2025-02-18 [email protected] Replace hard coded numbers with mouse button defines (flutter/flutter#163503) 2025-02-18 [email protected] Roll Skia from 9147a9654043 to 6da10829d017 (1 revision) (flutter/flutter#163531) 2025-02-18 [email protected] Roll Skia from 92aaa4e20ea7 to 9147a9654043 (2 revisions) (flutter/flutter#163512) 2025-02-17 [email protected] Roll Skia from 71a160edc9d9 to 92aaa4e20ea7 (1 revision) (flutter/flutter#163488) 2025-02-17 [email protected] Roll Packages from 625023a to 8542af3 (21 revisions) (flutter/flutter#163484) 2025-02-17 [email protected] Roll Skia from 7e3129d5db11 to 71a160edc9d9 (1 revision) (flutter/flutter#163459) 2025-02-17 [email protected] Roll Skia from 40ce5ef20d22 to 7e3129d5db11 (1 revision) (flutter/flutter#163451) 2025-02-16 [email protected] Roll Skia from bb166c85957b to 40ce5ef20d22 (1 revision) (flutter/flutter#163403) 2025-02-15 [email protected] Roll Skia from ff94581f1f8a to bb166c85957b (1 revision) (flutter/flutter#163381) 2025-02-15 [email protected] Move DWDS initialization into the onLoadEndCallback for the DDC library bundle format (flutter/flutter#163338) 2025-02-14 [email protected] [canvaskit] Handle MakeGrContext returning null (flutter/flutter#163332) 2025-02-14 [email protected] Roll Skia from 13a3b6f34ee8 to ff94581f1f8a (1 revision) (flutter/flutter#163347) 2025-02-14 [email protected] Fix failing chrome_dev_mode tests (flutter/flutter#163346) 2025-02-14 [email protected] Adds all semantics roles (flutter/flutter#163075) 2025-02-14 [email protected] Roll Skia from 20924303cc25 to 13a3b6f34ee8 (1 revision) (flutter/flutter#163336) 2025-02-14 [email protected] [ Widget Preview ] Add experimental support for web-based widget preview environment (flutter/flutter#163154) 2025-02-14 [email protected] [Impeller] don't use glFramebufferBlit for onscreen restore. (flutter/flutter#163327) 2025-02-14 [email protected] Align web terminal messages with the VM (flutter/flutter#163268) 2025-02-14 [email protected] Manually roll customer_testing to enable rfw tests (flutter/flutter#163030) 2025-02-14 [email protected] Refactor SliverMainAxisGroup for reverse mode. (flutter/flutter#161849) 2025-02-14 [email protected] Tweaked TextContents math to avoid floating point errors (flutter/flutter#162480) 2025-02-14 [email protected] [release] Update cherry-pick CHANGELOG requirements (flutter/flutter#163318) 2025-02-14 [email protected] Roll Skia from 85722a1db585 to 20924303cc25 (2 revisions) (flutter/flutter#163293) 2025-02-14 [email protected] Roll Skia from 5a38d23ee247 to 85722a1db585 (1 revision) (flutter/flutter#163286) 2025-02-14 [email protected] Add `.flutter-plugins-dependencies` to `FlutterBuildSystem`; update logic, add tests. (flutter/flutter#163278) 2025-02-14 [email protected] Add table related semantics role (flutter/flutter#162339) 2025-02-14 [email protected] [Impeller] Call glDebugMessageControlKHR only if the KHR_debug extension is available (flutter/flutter#163273) 2025-02-14 [email protected] Roll Skia from 748415976ad1 to 5a38d23ee247 (3 revisions) (flutter/flutter#163271) 2025-02-14 [email protected] [canvaskit] Use `transferToImageBitmap` instead of `createImageBitmap` (flutter/flutter#163175) 2025-02-14 [email protected] [skwasm] Use `transferToImageBitmap` instead of `createImageBitmap` (flutter/flutter#163251) ...
Roll Flutter from e8f34a9 to 39b4951 (95 revisions) flutter/flutter@e8f34a9...39b4951 2025-02-18 [email protected] Marks Windows_arm64 plugin_test_windows to be flaky (flutter/flutter#163123) 2025-02-18 [email protected] [android] use macro definition to shrink repetitive JNI code size. (flutter/flutter#163395) 2025-02-18 [email protected] Roll pub packages (flutter/flutter#163474) 2025-02-18 [email protected] [web] Cleanup everything HTML from the engine (outside html/ folder) (flutter/flutter#162840) 2025-02-18 [email protected] [Impeller] add support for rational bezier conics to Path (flutter/flutter#163282) 2025-02-18 [email protected] Change `cardTheme`, `dialogTheme`, and `tabBarTheme` type to `xxxThemeData` (flutter/flutter#157292) 2025-02-18 [email protected] Update integration test and benchmark Android .gitignore files to match the current app template (flutter/flutter#163276) 2025-02-18 [email protected] [Impeller] Don't create a redundant typography context. (flutter/flutter#163513) 2025-02-18 [email protected] Roll Dart SDK from fcef25f18e4d to 023ac80cef14 (1 revision) (flutter/flutter#163110) 2025-02-18 [email protected] update module_host_with_custom_build_v2_embedding to target android 35 and to use the latest gradle and agp versions (flutter/flutter#163542) 2025-02-18 [email protected] [Impeller] when binding to READ_FRAMEBUFFER, treat multisampled textures as single sampled. (flutter/flutter#163345) 2025-02-18 [email protected] Add missing properties to _ArcPaintPredicate. (flutter/flutter#162572) 2025-02-18 [email protected] Roll Packages from 8542af3 to cb4fb13 (4 revisions) (flutter/flutter#163544) 2025-02-18 [email protected] Remove bringup for android_display_cutout (flutter/flutter#163312) 2025-02-18 [email protected] Create VersionUtils class and unit tests and extract logic out of flutter.groovy (flutter/flutter#163166) 2025-02-18 [email protected] Invalidate `pod install` output if `.flutter-plugins-dependencies` content changes. (flutter/flutter#163275) 2025-02-18 [email protected] Replace hard coded numbers with mouse button defines (flutter/flutter#163503) 2025-02-18 [email protected] Roll Skia from 9147a9654043 to 6da10829d017 (1 revision) (flutter/flutter#163531) 2025-02-18 [email protected] Roll Skia from 92aaa4e20ea7 to 9147a9654043 (2 revisions) (flutter/flutter#163512) 2025-02-17 [email protected] Roll Skia from 71a160edc9d9 to 92aaa4e20ea7 (1 revision) (flutter/flutter#163488) 2025-02-17 [email protected] Roll Packages from 625023a to 8542af3 (21 revisions) (flutter/flutter#163484) 2025-02-17 [email protected] Roll Skia from 7e3129d5db11 to 71a160edc9d9 (1 revision) (flutter/flutter#163459) 2025-02-17 [email protected] Roll Skia from 40ce5ef20d22 to 7e3129d5db11 (1 revision) (flutter/flutter#163451) 2025-02-16 [email protected] Roll Skia from bb166c85957b to 40ce5ef20d22 (1 revision) (flutter/flutter#163403) 2025-02-15 [email protected] Roll Skia from ff94581f1f8a to bb166c85957b (1 revision) (flutter/flutter#163381) 2025-02-15 [email protected] Move DWDS initialization into the onLoadEndCallback for the DDC library bundle format (flutter/flutter#163338) 2025-02-14 [email protected] [canvaskit] Handle MakeGrContext returning null (flutter/flutter#163332) 2025-02-14 [email protected] Roll Skia from 13a3b6f34ee8 to ff94581f1f8a (1 revision) (flutter/flutter#163347) 2025-02-14 [email protected] Fix failing chrome_dev_mode tests (flutter/flutter#163346) 2025-02-14 [email protected] Adds all semantics roles (flutter/flutter#163075) 2025-02-14 [email protected] Roll Skia from 20924303cc25 to 13a3b6f34ee8 (1 revision) (flutter/flutter#163336) 2025-02-14 [email protected] [ Widget Preview ] Add experimental support for web-based widget preview environment (flutter/flutter#163154) 2025-02-14 [email protected] [Impeller] don't use glFramebufferBlit for onscreen restore. (flutter/flutter#163327) 2025-02-14 [email protected] Align web terminal messages with the VM (flutter/flutter#163268) 2025-02-14 [email protected] Manually roll customer_testing to enable rfw tests (flutter/flutter#163030) 2025-02-14 [email protected] Refactor SliverMainAxisGroup for reverse mode. (flutter/flutter#161849) 2025-02-14 [email protected] Tweaked TextContents math to avoid floating point errors (flutter/flutter#162480) 2025-02-14 [email protected] [release] Update cherry-pick CHANGELOG requirements (flutter/flutter#163318) 2025-02-14 [email protected] Roll Skia from 85722a1db585 to 20924303cc25 (2 revisions) (flutter/flutter#163293) 2025-02-14 [email protected] Roll Skia from 5a38d23ee247 to 85722a1db585 (1 revision) (flutter/flutter#163286) 2025-02-14 [email protected] Add `.flutter-plugins-dependencies` to `FlutterBuildSystem`; update logic, add tests. (flutter/flutter#163278) 2025-02-14 [email protected] Add table related semantics role (flutter/flutter#162339) 2025-02-14 [email protected] [Impeller] Call glDebugMessageControlKHR only if the KHR_debug extension is available (flutter/flutter#163273) 2025-02-14 [email protected] Roll Skia from 748415976ad1 to 5a38d23ee247 (3 revisions) (flutter/flutter#163271) 2025-02-14 [email protected] [canvaskit] Use `transferToImageBitmap` instead of `createImageBitmap` (flutter/flutter#163175) 2025-02-14 [email protected] [skwasm] Use `transferToImageBitmap` instead of `createImageBitmap` (flutter/flutter#163251) ...
Roll Flutter from e8f34a9 to 39b4951 (95 revisions) flutter/flutter@e8f34a9...39b4951 2025-02-18 [email protected] Marks Windows_arm64 plugin_test_windows to be flaky (flutter/flutter#163123) 2025-02-18 [email protected] [android] use macro definition to shrink repetitive JNI code size. (flutter/flutter#163395) 2025-02-18 [email protected] Roll pub packages (flutter/flutter#163474) 2025-02-18 [email protected] [web] Cleanup everything HTML from the engine (outside html/ folder) (flutter/flutter#162840) 2025-02-18 [email protected] [Impeller] add support for rational bezier conics to Path (flutter/flutter#163282) 2025-02-18 [email protected] Change `cardTheme`, `dialogTheme`, and `tabBarTheme` type to `xxxThemeData` (flutter/flutter#157292) 2025-02-18 [email protected] Update integration test and benchmark Android .gitignore files to match the current app template (flutter/flutter#163276) 2025-02-18 [email protected] [Impeller] Don't create a redundant typography context. (flutter/flutter#163513) 2025-02-18 [email protected] Roll Dart SDK from fcef25f18e4d to 023ac80cef14 (1 revision) (flutter/flutter#163110) 2025-02-18 [email protected] update module_host_with_custom_build_v2_embedding to target android 35 and to use the latest gradle and agp versions (flutter/flutter#163542) 2025-02-18 [email protected] [Impeller] when binding to READ_FRAMEBUFFER, treat multisampled textures as single sampled. (flutter/flutter#163345) 2025-02-18 [email protected] Add missing properties to _ArcPaintPredicate. (flutter/flutter#162572) 2025-02-18 [email protected] Roll Packages from 8542af3 to cb4fb13 (4 revisions) (flutter/flutter#163544) 2025-02-18 [email protected] Remove bringup for android_display_cutout (flutter/flutter#163312) 2025-02-18 [email protected] Create VersionUtils class and unit tests and extract logic out of flutter.groovy (flutter/flutter#163166) 2025-02-18 [email protected] Invalidate `pod install` output if `.flutter-plugins-dependencies` content changes. (flutter/flutter#163275) 2025-02-18 [email protected] Replace hard coded numbers with mouse button defines (flutter/flutter#163503) 2025-02-18 [email protected] Roll Skia from 9147a9654043 to 6da10829d017 (1 revision) (flutter/flutter#163531) 2025-02-18 [email protected] Roll Skia from 92aaa4e20ea7 to 9147a9654043 (2 revisions) (flutter/flutter#163512) 2025-02-17 [email protected] Roll Skia from 71a160edc9d9 to 92aaa4e20ea7 (1 revision) (flutter/flutter#163488) 2025-02-17 [email protected] Roll Packages from 625023a to 8542af3 (21 revisions) (flutter/flutter#163484) 2025-02-17 [email protected] Roll Skia from 7e3129d5db11 to 71a160edc9d9 (1 revision) (flutter/flutter#163459) 2025-02-17 [email protected] Roll Skia from 40ce5ef20d22 to 7e3129d5db11 (1 revision) (flutter/flutter#163451) 2025-02-16 [email protected] Roll Skia from bb166c85957b to 40ce5ef20d22 (1 revision) (flutter/flutter#163403) 2025-02-15 [email protected] Roll Skia from ff94581f1f8a to bb166c85957b (1 revision) (flutter/flutter#163381) 2025-02-15 [email protected] Move DWDS initialization into the onLoadEndCallback for the DDC library bundle format (flutter/flutter#163338) 2025-02-14 [email protected] [canvaskit] Handle MakeGrContext returning null (flutter/flutter#163332) 2025-02-14 [email protected] Roll Skia from 13a3b6f34ee8 to ff94581f1f8a (1 revision) (flutter/flutter#163347) 2025-02-14 [email protected] Fix failing chrome_dev_mode tests (flutter/flutter#163346) 2025-02-14 [email protected] Adds all semantics roles (flutter/flutter#163075) 2025-02-14 [email protected] Roll Skia from 20924303cc25 to 13a3b6f34ee8 (1 revision) (flutter/flutter#163336) 2025-02-14 [email protected] [ Widget Preview ] Add experimental support for web-based widget preview environment (flutter/flutter#163154) 2025-02-14 [email protected] [Impeller] don't use glFramebufferBlit for onscreen restore. (flutter/flutter#163327) 2025-02-14 [email protected] Align web terminal messages with the VM (flutter/flutter#163268) 2025-02-14 [email protected] Manually roll customer_testing to enable rfw tests (flutter/flutter#163030) 2025-02-14 [email protected] Refactor SliverMainAxisGroup for reverse mode. (flutter/flutter#161849) 2025-02-14 [email protected] Tweaked TextContents math to avoid floating point errors (flutter/flutter#162480) 2025-02-14 [email protected] [release] Update cherry-pick CHANGELOG requirements (flutter/flutter#163318) 2025-02-14 [email protected] Roll Skia from 85722a1db585 to 20924303cc25 (2 revisions) (flutter/flutter#163293) 2025-02-14 [email protected] Roll Skia from 5a38d23ee247 to 85722a1db585 (1 revision) (flutter/flutter#163286) 2025-02-14 [email protected] Add `.flutter-plugins-dependencies` to `FlutterBuildSystem`; update logic, add tests. (flutter/flutter#163278) 2025-02-14 [email protected] Add table related semantics role (flutter/flutter#162339) 2025-02-14 [email protected] [Impeller] Call glDebugMessageControlKHR only if the KHR_debug extension is available (flutter/flutter#163273) 2025-02-14 [email protected] Roll Skia from 748415976ad1 to 5a38d23ee247 (3 revisions) (flutter/flutter#163271) 2025-02-14 [email protected] [canvaskit] Use `transferToImageBitmap` instead of `createImageBitmap` (flutter/flutter#163175) 2025-02-14 [email protected] [skwasm] Use `transferToImageBitmap` instead of `createImageBitmap` (flutter/flutter#163251) ...
Add basic support for storing rational bezier conics in impeller::Path. The support is very thin and just degrades the conics into a pair of quadratic curves just as Impeller has always done for the conic segments that it receives via SkPath objects, but it puts in place the framework for eventually handling the conics more directly and allows the unit tests to be rewritten on top of Impeller paths rather than SkPaths, paving the way for reduced internal API dependencies.