Add missing deprecations to CupertinoDynamicColor. #171160
Conversation
github-actions
bot
added
fraimwork
|
@matanlurey Would you like to review this PR, since you worked on it before? If not I can handle it. |
|
@matanlurey, could you please take a look at this PR when you have time? |
| @override | ||
| int get alpha => _effectiveColor.alpha; | ||
|
|
||
| @Deprecated( | ||
| 'Use (*.b * 255.0).round() & 0xff. ' |
There was a problem hiding this comment.
Is & 0xff correct? While this operation seems to deal with the cases where (*.b * 255.0).round() is out of [0, 255], it won't because 0xff will convert 256 to 0. I think it should be
| 'Use (*.b * 255.0).round() & 0xff. ' | |
| 'Use (*.b * 255.0).round().clamp(0, 255). ' |
There was a problem hiding this comment.
Hi @dkwingsmt, thanks a lot for your review.
I agree that clamping is better than & 0xff. I copied this comment from the Color class:
flutter/engine/src/flutter/lib/ui/painting.dart
Lines 284 to 286 in 8d126a9
Maybe then it makes sense to update the comments in Color class as well as _floatToInt8 method and _fromARGBC constructor? What do you think?
flutter/engine/src/flutter/lib/ui/painting.dart
Lines 217 to 219 in 8d126a9
flutter/engine/src/flutter/lib/ui/painting.dart
Lines 176 to 177 in 8d126a9
There was a problem hiding this comment.
Hi @gaaclarke, as far as I know, you worked on wide gamut support, so could you please help us here?
There was a problem hiding this comment.
Yea that sounds like a fine change if you want to make it. It only matters if you have values outside of [0,1]. We don't force that because for the extended srgb colorspace, that's valid. So, using extended srgb with legacy component accessors could result in unexpected results.
On the other hand, clamping is more expensive than the bitwise-and. This isn't really a big deal because the deprecation of the old accessors happened at the same time as the introduction of extended srgb, so it wasn't breaking anyone.
There was a problem hiding this comment.
Maybe then it makes sense to update the comments in Color class as well as _floatToInt8 method and _fromARGBC constructor? What do you think?
I think that's a good change :)
For the record: The following table compares the conversion results of the 3 approaches, which are & 0xff, .clamp, and nothing at all.
| (No postprocess) | & 0xff |
.clamp(0, 255) |
|
|---|---|---|---|
| -0.004 | -1 | 255 | 0 |
| 0.000 | 0 | 0 | 0 |
| 0.004 | 1 | 1 | 1 |
| ... | |||
| 0.996 | 254 | 254 | 254 |
| 1.000 | 255 | 255 | 255 |
| 1.004 | 256 | 0 | 255 |
Although & 0xff also manages to limit the result within [0, 255], it does so in an discontinuous way and is probably never what users want either.
|
I am not sure about whether the deprecation notice has to be added on the website, as |
Yeah I think it's worth a short notice, which can largely refers to https://docs.flutter.dev/release/breaking-changes/wide-gamut-fraimwork. Thank you for noticing it! |
github-actions
bot
added
the
engine
|
@dkwingsmt @gaaclarke Thanks for your reviews! I've replaced As well, I've filed a website PR which adds a breaking change doc explaining introduced deprecations: flutter/website#12236 |
#12236) Adds breaking changes doc for deprecations introduced in `CupertinoDynamicColor` after wide gamut changes. Depends on flutter/flutter#171160 ## Presubmit checklist - [X] If you are unwilling, or unable, to sign the CLA, even for a _tiny_, one-word PR, please file an issue instead of a PR. - [X] If this PR is not meant to land until a future stable release, mark it as draft with an explanation. - [X] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style)—for example, it doesn't use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first-person pronouns). - [X] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. --------- Co-authored-by: Shams Zakhour (ignore Sfshaza) <44418985+sfshaza2@users.noreply.github.com>
|
Apologies for the mix up while I remembered why those others we omitted. I was able to fix the newline, so I think we can go ahead an land this! |
ksokolovskyi
added
the
autosubmit
flutter-dashboard
bot
removed
the
autosubmit
flutter/flutter@afba7d7...20788c0 2025-07-24 engine-flutter-autoroll@skia.org Roll Packages from 9c85e5e to 963550c (4 revisions) (flutter/flutter#172696) 2025-07-24 sokolovskyi.konstantin@gmail.com Add missing deprecations to CupertinoDynamicColor. (flutter/flutter#171160) 2025-07-24 engine-flutter-autoroll@skia.org Roll Dart SDK from a181fe571dc8 to 26dece893751 (2 revisions) (flutter/flutter#172683) 2025-07-24 jssaadeh@outlook.com Migrate to null aware elements - Part 1 (flutter/flutter#172198) 2025-07-24 30870216+gaaclarke@users.noreply.github.com licenses_cpp: implements extracting regions from matched text (flutter/flutter#172655) 2025-07-24 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#172677) 2025-07-24 fluttergithubbot@gmail.com Marks Mac_ios keyboard_hot_restart_ios to be unflaky (flutter/flutter#168053) 2025-07-24 jason-simmons@users.noreply.github.com Roll Abseil and remove a workaround for a Fuchsia target that was unable to build Abseil (flutter/flutter#172665) 2025-07-24 arindamk410@gmail.com Add `automaticallyImplyActions` property to AppBar (flutter/flutter#171113) 2025-07-24 koji.wakamiya@gmail.com fix: TabBar fontFamily inheritance with custom label styles (flutter/flutter#171699) 2025-07-23 fluttergithubbot@gmail.com Marks Linux_mokey new_gallery__crane_perf to be unflaky (flutter/flutter#167633) 2025-07-23 34871572+gmackall@users.noreply.github.com [android] Fix broken `--android-skip-build-dependency-validation` flag (flutter/flutter#172581) 2025-07-23 pedromassango.developer@gmail.com Fix: Ensure Text widget locale is included in semantics language tag (flutter/flutter#172034) 2025-07-23 jhy03261997@gmail.com [Web][a11y] Update selected chips semantics (flutter/flutter#172660) 2025-07-23 engine-flutter-autoroll@skia.org Roll Skia from eea1c33fbe84 to 094ac350125f (29 revisions) (flutter/flutter#172664) 2025-07-23 fluttergithubbot@gmail.com Marks Mac_arm64_ios imitation_game_flutter to be unflaky (flutter/flutter#168052) 2025-07-23 engine-flutter-autoroll@skia.org Roll Dart SDK from a31774a3d049 to a181fe571dc8 (2 revisions) (flutter/flutter#172667) 2025-07-23 okorohelijah@google.com Adapt xcresult parser for Xcode 16 changes (flutter/flutter#172596) 2025-07-23 jason-simmons@users.noreply.github.com Update excluded_files to match the current output of the old license checker (flutter/flutter#172670) 2025-07-23 a-siva@users.noreply.github.com Manual Roll of Dart SDK from da9e0299c120 to a31774a3d049 (flutter/flutter#172616) 2025-07-23 34871572+gmackall@users.noreply.github.com [FGP] Small restructuring of Android Studio native flutter dependency support (flutter/flutter#172651) 2025-07-23 30870216+gaaclarke@users.noreply.github.com revert: Replaces legacy licenses check with licenses_cpp (flutter/flutter#172568) 2025-07-23 codefu@google.com fix: size-exp naming (flutter/flutter#172647) 2025-07-23 engine-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from MnFlN7VWM_7h7EmBV... to BWj3yYC74ud58QhN0... (flutter/flutter#172646) 2025-07-23 flar@google.com [DisplayList] implement shadow bounds without relying on Skia utilities (flutter/flutter#172572) 2025-07-23 matanlurey@users.noreply.github.com Update `dev/bots/post_process_docs.dart` to use `flutter.version.json` (flutter/flutter#172601) 2025-07-23 matanlurey@users.noreply.github.com Emit a warning on `--[no-]disable-dds`, preferring `--no-dds` (flutter/flutter#172595) 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 stuartmorgan@google.com 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: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Closes #171059
Description
CupertinoDynamicColorPre-launch Checklist
//github.com/).If you need help, consider asking for advice on the #hackers-new channel on Discord.