Added special case for fat width arcs #161255

gaaclarke · 2025-01-07T19:24:56Z

This draws arcs as oval sectors when the stroke width is large enough.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

gaaclarke · 2025-01-07T19:29:28Z

I discovered some other cases where the arc code is making weird choices. I'll file an issue for those separately since they aren't exactly what the OP is asking for and I'm not sure if the framework will run into them ( see #158567 (comment) )

jonahwilliams · 2025-01-07T19:35:29Z

engine/src/flutter/impeller/display_list/dl_dispatcher.cc

-  builder.AddArc(oval_bounds, Degrees(start_degrees), Degrees(sweep_degrees),
-                 use_center);
-  GetCanvas().DrawPath(builder.TakePath(), paint_);
+  if (paint_.stroke_width >


Maybe add a breadcrumb back to the original issue here, or a comment explaining why we're doing this conversion.

jonahwilliams

LGTM

auto-submit · 2025-01-07T20:56:04Z

auto label is removed for flutter/flutter/161255, due to - The status or check suite Merge Queue Guard has failed. Please fix the issues identified (or deflake) before re-applying this label.

flutter-dashboard · 2025-01-09T01:21:17Z

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 package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #161255 at sha b26bb7b

flutter-dashboard · 2025-01-09T19:24:31Z

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #161255 at sha 8c3c01a

jonahwilliams · 2025-01-13T21:05:23Z

@gaaclarke please make sure this lands

gaaclarke · 2025-01-13T21:08:11Z

@gaaclarke please make sure this lands

It's on autosubmit, it's been blocked for 4 days because of the tree-status. I'm still seeing people landing stuff (but the tree is red), is my CI check stuck?

jonahwilliams · 2025-01-13T21:09:02Z

Google testing appears to be stuck

gaaclarke · 2025-01-13T21:15:07Z

Google testing appears to be stuck

I've hit the "Test all" in the FROB UI. Let's see if that pokes it. The way the UI is setup it looks like Google testing is waiting on tree-status, but that can't be true as evidenced by the fact it's been frozen 4 days and things have landed in that time.

jonahwilliams · 2025-01-13T21:16:16Z

I would mark it as passing with an infra issue, I don't expect google testing to provide value for this PR.

gaaclarke · 2025-01-13T21:18:44Z

I would mark it as passing with an infra issue, I don't expect google testing to provide value for this PR.

done

gaaclarke · 2025-01-14T16:50:07Z

Checking up on this, the tree is red now, the autosubmit didn't seem to work since some PR's have landed. I guess I'll have to manually submit this to the merge queue when the tree is open.

Roll Flutter from 72db8f6 to 40c2b86 (33 revisions) flutter/flutter@72db8f6...40c2b86 2025-01-14 [email protected] update changelog for 3.27.2 release (flutter/flutter#161569) 2025-01-14 [email protected] Fix `showLicensePage` does not inherit ambient `Theme` (flutter/flutter#161599) 2025-01-14 [email protected] Added special case for fat width arcs (flutter/flutter#161255) 2025-01-14 [email protected] Replace `fetch `with `gclient sync`. (flutter/flutter#161565) 2025-01-14 [email protected] Roll Packages from 3c3bc68 to d1fd623 (4 revisions) (flutter/flutter#161597) 2025-01-14 [email protected] Remove unused method (flutter/flutter#161572) 2025-01-14 [email protected] Fix crash when closing a window with `Alt+F4` in multi-win Flutter on Windows (flutter/flutter#161375) 2025-01-14 [email protected] Update InputDecoration.border documentation (flutter/flutter#161415) 2025-01-14 [email protected] [Web] Allow specifying the strategy on when to use <img> element to display images (flutter/flutter#159917) 2025-01-14 [email protected] Roll Dart to Version 3.7.0-323.0.dev (flutter/flutter#161567) 2025-01-14 [email protected] Use wildcards (flutter/flutter#161548) 2025-01-14 [email protected] Autocomplete Options Width (flutter/flutter#143249) 2025-01-13 [email protected] Move the analyzer_benchmark to Mac arm64 devicelab bots (flutter/flutter#161405) 2025-01-13 [email protected] Remove references to `cirrus`, mostly in doc comments. (flutter/flutter#161529) 2025-01-13 [email protected] Fix paths when running clang-tidy on git diffs (flutter/flutter#161496) 2025-01-13 [email protected] [web:a11y] treat empty tappables as buttons (flutter/flutter#161360) 2025-01-13 [email protected] Add route settings to CupertinoSheetRoute (flutter/flutter#161528) 2025-01-13 [email protected] Copy `linux_host_engine` as `linux_host_engine_test`, removing `archives: [...]`. (flutter/flutter#161532) 2025-01-13 [email protected] Remove last two references to Cirrus CI. (flutter/flutter#161530) 2025-01-13 [email protected] Mark `Mac_mokey microbenchmarks` as flakey (flutter/flutter#161550) 2025-01-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (#160241)" (flutter/flutter#161555) 2025-01-13 [email protected] Add validator execution times to `flutter doctor --verbose` (flutter/flutter#158124) 2025-01-13 [email protected] Explain more specifically how to use `flutter drive`/what it does (flutter/flutter#161450) 2025-01-13 [email protected] Fixed repeated strings for incompatible Gradle or AGP version in `create` command (flutter/flutter#161223) 2025-01-13 [email protected] Remove `WEB_SHARD_COUNT`, which no longer exists post-Cirrus. (flutter/flutter#161527) 2025-01-13 [email protected] [Impeller] Update guidance on prebuilt artifacts. (flutter/flutter#161251) 2025-01-13 [email protected] Migrate DisplayList unit tests to DL/Impeller geometry classes (flutter/flutter#161453) 2025-01-13 [email protected] Context menu button callback docs clarification (flutter/flutter#161451) 2025-01-13 [email protected] Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (flutter/flutter#160241) 2025-01-13 [email protected] Udpate documentation on the third_party directories (flutter/flutter#161407) 2025-01-13 [email protected] Propagate environment variables when `flutter drive` is invoked. (flutter/flutter#161452) 2025-01-13 [email protected] Convert base application name handling to kotlin source (start of FGP kt conversion) (flutter/flutter#155963) 2025-01-13 [email protected] [Impeller] remove API 30 restriction for SurfaceControl testing. (flutter/flutter#161438) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: ...

Roll Flutter from 72db8f6 to 40c2b86 (33 revisions) flutter/flutter@72db8f6...40c2b86 2025-01-14 [email protected] update changelog for 3.27.2 release (flutter/flutter#161569) 2025-01-14 [email protected] Fix `showLicensePage` does not inherit ambient `Theme` (flutter/flutter#161599) 2025-01-14 [email protected] Added special case for fat width arcs (flutter/flutter#161255) 2025-01-14 [email protected] Replace `fetch `with `gclient sync`. (flutter/flutter#161565) 2025-01-14 [email protected] Roll Packages from 3c3bc68 to d1fd623 (4 revisions) (flutter/flutter#161597) 2025-01-14 [email protected] Remove unused method (flutter/flutter#161572) 2025-01-14 [email protected] Fix crash when closing a window with `Alt+F4` in multi-win Flutter on Windows (flutter/flutter#161375) 2025-01-14 [email protected] Update InputDecoration.border documentation (flutter/flutter#161415) 2025-01-14 [email protected] [Web] Allow specifying the strategy on when to use <img> element to display images (flutter/flutter#159917) 2025-01-14 [email protected] Roll Dart to Version 3.7.0-323.0.dev (flutter/flutter#161567) 2025-01-14 [email protected] Use wildcards (flutter/flutter#161548) 2025-01-14 [email protected] Autocomplete Options Width (flutter/flutter#143249) 2025-01-13 [email protected] Move the analyzer_benchmark to Mac arm64 devicelab bots (flutter/flutter#161405) 2025-01-13 [email protected] Remove references to `cirrus`, mostly in doc comments. (flutter/flutter#161529) 2025-01-13 [email protected] Fix paths when running clang-tidy on git diffs (flutter/flutter#161496) 2025-01-13 [email protected] [web:a11y] treat empty tappables as buttons (flutter/flutter#161360) 2025-01-13 [email protected] Add route settings to CupertinoSheetRoute (flutter/flutter#161528) 2025-01-13 [email protected] Copy `linux_host_engine` as `linux_host_engine_test`, removing `archives: [...]`. (flutter/flutter#161532) 2025-01-13 [email protected] Remove last two references to Cirrus CI. (flutter/flutter#161530) 2025-01-13 [email protected] Mark `Mac_mokey microbenchmarks` as flakey (flutter/flutter#161550) 2025-01-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (#160241)" (flutter/flutter#161555) 2025-01-13 [email protected] Add validator execution times to `flutter doctor --verbose` (flutter/flutter#158124) 2025-01-13 [email protected] Explain more specifically how to use `flutter drive`/what it does (flutter/flutter#161450) 2025-01-13 [email protected] Fixed repeated strings for incompatible Gradle or AGP version in `create` command (flutter/flutter#161223) 2025-01-13 [email protected] Remove `WEB_SHARD_COUNT`, which no longer exists post-Cirrus. (flutter/flutter#161527) 2025-01-13 [email protected] [Impeller] Update guidance on prebuilt artifacts. (flutter/flutter#161251) 2025-01-13 [email protected] Migrate DisplayList unit tests to DL/Impeller geometry classes (flutter/flutter#161453) 2025-01-13 [email protected] Context menu button callback docs clarification (flutter/flutter#161451) 2025-01-13 [email protected] Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (flutter/flutter#160241) 2025-01-13 [email protected] Udpate documentation on the third_party directories (flutter/flutter#161407) 2025-01-13 [email protected] Propagate environment variables when `flutter drive` is invoked. (flutter/flutter#161452) 2025-01-13 [email protected] Convert base application name handling to kotlin source (start of FGP kt conversion) (flutter/flutter#155963) 2025-01-13 [email protected] [Impeller] remove API 30 restriction for SurfaceControl testing. (flutter/flutter#161438) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: ...

github-actions bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Jan 7, 2025

gaaclarke marked this pull request as ready for review January 7, 2025 19:28

gaaclarke requested review from jason-simmons and jonahwilliams January 7, 2025 19:28

jonahwilliams reviewed Jan 7, 2025

View reviewed changes

jonahwilliams approved these changes Jan 7, 2025

View reviewed changes

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 7, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 7, 2025

gaaclarke added 3 commits January 7, 2025 13:03

Added special case for fat width arcs

9c65efb

expanded gui options

52a80f7

added comment

4069f78

gaaclarke force-pushed the fat-stroke-arc branch from cc39018 to 4069f78 Compare January 7, 2025 21:03

gaaclarke mentioned this pull request Jan 7, 2025

[impeller] drawing fat arcs has artifacts sometimes #161266

Open

lint

aadb026

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 8, 2025

updated golden text

b26bb7b

flutter-dashboard bot added the will affect goldens Changes to golden files label Jan 9, 2025

added grey background

8c3c01a

gaaclarke added this pull request to the merge queue Jan 14, 2025

Merged via the queue into flutter:master with commit e683e10 Jan 14, 2025
179 checks passed

gaaclarke deleted the fat-stroke-arc branch January 14, 2025 18:51

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 14, 2025

engine-flutter-autoroll mentioned this pull request Jan 14, 2025

Roll Flutter from 72db8f69e339 to 40c2b86fdf65 (33 revisions) flutter/packages#8428

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2025

e683e10 Added special case for fat width arcs (flutter/flutter#161255)

9a87982

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025

e683e10 Added special case for fat width arcs (flutter/flutter#161255)

88e016d

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

e683e10 Added special case for fat width arcs (flutter/flutter#161255)

5c2af23

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

e683e10 Added special case for fat width arcs (flutter/flutter#161255)

0124f61

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

e683e10 Added special case for fat width arcs (flutter/flutter#161255)

9fab743

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025

e683e10 Added special case for fat width arcs (flutter/flutter#161255)

7d856b7

Added special case for fat width arcs #161255

Added special case for fat width arcs #161255

Uh oh!

Conversation

gaaclarke commented Jan 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

gaaclarke commented Jan 7, 2025

Uh oh!

jonahwilliams Jan 7, 2025

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jan 7, 2025

Choose a reason for hiding this comment

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Jan 7, 2025

Uh oh!

flutter-dashboard bot commented Jan 9, 2025

Uh oh!

flutter-dashboard bot commented Jan 9, 2025

Uh oh!

jonahwilliams commented Jan 13, 2025

Uh oh!

gaaclarke commented Jan 13, 2025

Uh oh!

jonahwilliams commented Jan 13, 2025

Uh oh!

gaaclarke commented Jan 13, 2025

Uh oh!

jonahwilliams commented Jan 13, 2025

Uh oh!

gaaclarke commented Jan 13, 2025

Uh oh!

gaaclarke commented Jan 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gaaclarke commented Jan 7, 2025 •

edited

Loading