[Linux] revise dark theme detection #25535

jpnurmi · 2021-04-10T18:34:37Z

Check whether the window text color is light or dark. This detects dark mode also for other themes than the dark variants of Yaru & Adwaita.

Fixes: flutter/flutter#80199

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] and the [C++, Objective-C, Java style guides].
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the [CLA].
All existing and new tests are passing.
The reviewer has submitted any presubmit flakes in this PR using the [engine presubmit flakes form] before re-triggering the failure.

Check whether the window text color is light or dark, or whether the theme name contains 'dark' (fallback). This detects dark mode also for other themes than the dark variants of Yaru & Adwaita.

flutter-dashboard · 2021-04-10T18:34:39Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

- don't require GtkApp - gtk_window_list_toplevels() is fine - windowless fallback is not needed because fl_engine_start() is called from fl_view_realize()

chinmaygarde · 2021-04-22T20:15:15Z

@cbracken @robert-ancell This looks good to me but I am not familiar with GTK. Can someone take a pass over this review please?

jpnurmi · 2021-04-23T07:33:18Z

If accessing the top-level window to get ahold of the style context is a problem (e.g. in headless mode?), we could revert to checking if the theme name ends with "-dark", which is a pretty common convention for GTK themes.

cbracken

Thanks for taking a look at this. Added a couple comments.

shell/platform/linux/fl_settings_plugin.cc

cbracken · 2021-05-01T19:43:24Z

Thanks! Looks good!

/cc @robert-ancell should we keep the "-dark" heuristic, or do you think this is adequate as-is? In particular, @jpnurmi have you verified this works for the adwaita-dark and yaru-dark themes?

jpnurmi · 2021-05-02T20:17:37Z

should we keep the "-dark" heuristic, or do you think this is adequate as-is?

I had a "-dark" check as a fallback at first just in case a window wouldn't exist, but then I realized that fl_engine_start(), where the settings plugin is created, is called from fl_view_realize() after the window has been created.

In particular, @jpnurmi have you verified this works for the adwaita-dark and yaru-dark themes?

Yes, I have tested the light and dark variants of Adwaita, Yaru, and Materia.

cbracken · 2021-05-03T16:14:46Z

Perfect -- thanks! When you merge, can you udpate the description to remove the bit about -dark detection?

Check whether the window text color is light or dark ~~, or whether the theme name contains 'dark' (fallback)~~. This detects dark mode also for other themes than the dark variants of Yaru & Adwaita.

jpnurmi · 2021-05-03T16:24:18Z

Oops, done!🙂

robert-ancell

It's such a shame we have to jump though these hoops to determine if a theme is dark :(

Changes look fine to me, thanks @jpnurmi!

* ccaae8d Roll Skia from 537293bf155f to adadb95a9f1e (1 revision) (flutter/engine#25905) * 2e3a7f8 Roll buildroot to pick change to cppwinrt invocation (flutter/engine#25993) * 1c22286 [Linux] revise dark theme detection (flutter/engine#25535) * 7424400 Moved PlatformMessage's to unique_ptrs (flutter/engine#25860) * 406c4da Ensure that AutoIsolateShutdown drops its reference to the DartIsolate on the intended task runner (flutter/engine#25899) * 72c2fda [web] Fix incorrect physical size due to visualviewport api on iOS (flutter/engine#25895) * a712ffe Roll Dart SDK from b8f4018535fa to 86c749398b3a (16 revisions) (flutter/engine#25999) * edbbb12 Roll Fuchsia Mac SDK from uQgs5ZmFq... to aCsEHpnS0... (flutter/engine#26001) * afbbeac Implement smooth resizing for Linux (flutter/engine#25884) * 3ffb8ef Roll Dart SDK from 86c749398b3a to b4210cc43086 (2 revisions) (flutter/engine#26006) * 039dcd9 pull googletest from github instead of fuchsia.googlesource (flutter/engine#25907) * 9c793f1 Roll Skia from adadb95a9f1e to 1dc2d0fe0fa0 (98 revisions) (flutter/engine#26009) * 82efb9a Roll Skia from 1dc2d0fe0fa0 to 115645ee9b1b (2 revisions) (flutter/engine#26012) * 91a4c72 Delete unused method from engine_layer.h (flutter/engine#25924) * 619f82f fuchsia: Fix multi-views fallout (flutter/engine#25984) * 0b4bf7e Fixes BUILD.gn if is_fuchsia (legacy embedder) and is_debug (flutter/engine#25858) * 2e9de09 Streamline frame timings recording (flutter/engine#25892) * 1077da8 [vsync_waiter] add AwaitVSyncForSecondaryCallback() (flutter/engine#25787) * 0053bef [build_fuchsia_artifacts] Move license copying into BuildBucket(). (flutter/engine#25815) * e8b80e7 Roll Skia from 115645ee9b1b to c411429239e9 (7 revisions) (flutter/engine#26018) * d3353b2 Move more parts to libraries. (flutter/engine#25863) * d1a1182 Roll Dart SDK from b4210cc43086 to 04e55dad908d (2 revisions) (flutter/engine#26020) * f631f5b Exclude third_party/dart/third_party/devtools from the license script (flutter/engine#25918) * 304539b Roll Fuchsia Mac SDK from aCsEHpnS0... to OyXxehV6e... (flutter/engine#26022) * c3e28ae Roll Fuchsia Linux SDK from 4numS0K6T... to -FIIsjZj2... (flutter/engine#26023) * 6cacb53 Roll Skia from c411429239e9 to 72de83df3a03 (1 revision) (flutter/engine#26024) * bfccba5 Add dependency on Windows SDK CIPD package (flutter/engine#26003) * 06cbf1e Roll Dart SDK from 04e55dad908d to 094c9024373c (1 revision) (flutter/engine#26027) * 73aaeea Roll Skia from 72de83df3a03 to dabb2891c4a1 (4 revisions) (flutter/engine#26025) * e229649 Extract Windows string_conversion target (flutter/engine#26029) * 3d73e06 Extract FML command_line target (flutter/engine#26028) * 4aace54 Make SceneBuilder.push* not return nullable objects (flutter/engine#25991) * 0d62a56 Roll Dart SDK from 094c9024373c to 2ea89ef8f6de (1 revision) (flutter/engine#26030) * 5a26c1f Use string_view inputs for conversion functions (flutter/engine#26031) * 4f28de4 Support windows registry access (flutter/engine#26032) * 234fae1 Roll Fuchsia Linux SDK from -FIIsjZj2... to KZCe5FqMb... (flutter/engine#26034) * 7a47d3b Roll Fuchsia Mac SDK from OyXxehV6e... to DSk0IzBHv... (flutter/engine#26035) * 01f9bd8 Roll Skia from dabb2891c4a1 to 686dd910dd6c (1 revision) (flutter/engine#26036) * 1825bef Revert Dart SDK to b8f4018535fa792891e2add3a475f35e3ec156ab (flutter/engine#26037)

Some users have reported that toggling between light and dark system theme results in Flutter detecting the opposite theme. This is because the window text color, which is used to calculate the brightness, may not have been yet refreshed when the theme change signal is emitted. This commit replaces the unreliable (and untestable) window text color -based theme detection with the simpler alternative proposed in flutter#25535 to check for a "-dark" suffix in the theme name, which is used by convention in dark GTK theme names (Yaru vs. Yaru-dark, Adwaita vs. Adwaita-dark, Pop vs. Pop-dark, ...). Ref: flutter/flutter#101438

[Linux] revise dark theme detection

023b9e6

Check whether the window text color is light or dark, or whether the theme name contains 'dark' (fallback). This detects dark mode also for other themes than the dark variants of Yaru & Adwaita.

flutter-dashboard bot added the platform-linux label Apr 10, 2021

google-cla bot added the cla: yes label Apr 10, 2021

Cleanup

b9c06bc

- don't require GtkApp - gtk_window_list_toplevels() is fine - windowless fallback is not needed because fl_engine_start() is called from fl_view_realize()

jpnurmi force-pushed the linux-dark-theme branch from 5d0526f to b9c06bc Compare April 11, 2021 18:48

jonahwilliams requested a review from robert-ancell April 14, 2021 10:59

jpnurmi mentioned this pull request Apr 15, 2021

Dark system themes are not correctly detected on Linux flutter/flutter#80199

Closed

cbracken suggested changes Apr 29, 2021

View reviewed changes

shell/platform/linux/fl_settings_plugin.cc Show resolved Hide resolved

shell/platform/linux/fl_settings_plugin.cc Show resolved Hide resolved

jpnurmi added 2 commits April 29, 2021 19:17

Merge remote-tracking branch 'upstream/master' into linux-dark-theme

05c6855

Resolve review comments

0ab4578

cbracken approved these changes May 1, 2021

View reviewed changes

robert-ancell approved these changes May 4, 2021

View reviewed changes

chinmaygarde added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label May 6, 2021

fluttergithubbot merged commit 1c22286 into flutter:master May 7, 2021

engine-flutter-autoroll mentioned this pull request May 7, 2021

Roll Engine from f57e986aa80c to f631f5bcb2d7 (23 revisions) flutter/flutter#82070

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 7, 2021

1c22286 [Linux] revise dark theme detection (flutter/engine#25535)

9d48b7d

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to bfccba5ac00c (27 revisions) flutter/flutter#82079

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

1c22286 [Linux] revise dark theme detection (flutter/engine#25535)

4f68133

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to 06cbf1e0d773 (28 revisions) flutter/flutter#82083

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

1c22286 [Linux] revise dark theme detection (flutter/engine#25535)

c8c7e75

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to 73aaeea34640 (29 revisions) flutter/flutter#82090

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

1c22286 [Linux] revise dark theme detection (flutter/engine#25535)

99ea4a9

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to e229649a7a97 (30 revisions) flutter/flutter#82096

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

1c22286 [Linux] revise dark theme detection (flutter/engine#25535)

f4a1821

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to 4aace5476b33 (32 revisions) flutter/flutter#82097

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

1c22286 [Linux] revise dark theme detection (flutter/engine#25535)

944d393

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to 0d62a560fe6a (33 revisions) flutter/flutter#82120

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

1c22286 [Linux] revise dark theme detection (flutter/engine#25535)

a60304d

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to 4f28de494253 (35 revisions) flutter/flutter#82121

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

1c22286 [Linux] revise dark theme detection (flutter/engine#25535)

242212b

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to 234fae1b8d1d (36 revisions) flutter/flutter#82122

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

1c22286 [Linux] revise dark theme detection (flutter/engine#25535)

22e363a

engine-flutter-autoroll mentioned this pull request May 8, 2021

Roll Engine from f57e986aa80c to 7a47d3b32567 (37 revisions) flutter/flutter#82127

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 8, 2021

1c22286 [Linux] revise dark theme detection (flutter/engine#25535)

3a6b665

engine-flutter-autoroll mentioned this pull request May 9, 2021

Roll Engine from f57e986aa80c to 01f9bd8649cd (38 revisions) flutter/flutter#82130

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 9, 2021

1c22286 [Linux] revise dark theme detection (flutter/engine#25535)

49eba21

engine-flutter-autoroll mentioned this pull request May 9, 2021

Roll Engine from f57e986aa80c to 1825befbc72d (39 revisions) flutter/flutter#82132

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 9, 2021

1c22286 [Linux] revise dark theme detection (flutter/engine#25535)

ae81158

TahaTesser mentioned this pull request May 17, 2021

Gtk theme does not comply with selected one flutter/flutter#82620

Closed

naudzghebre pushed a commit to naudzghebre/engine that referenced this pull request May 21, 2021

[Linux] revise dark theme detection (flutter#25535)

c96d040

DanTup mentioned this pull request Jul 13, 2021

Running a flutter project from within vscode no longer follows the system theme Dart-Code/Dart-Code#3454

Closed

jpnurmi deleted the linux-dark-theme branch April 12, 2022 06:33

jpnurmi mentioned this pull request Apr 19, 2022

[Linux] fix and test light vs. dark theme detection #32618

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Linux] revise dark theme detection #25535

[Linux] revise dark theme detection #25535

Uh oh!

jpnurmi commented Apr 10, 2021 •

edited

Loading

Uh oh!

flutter-dashboard bot commented Apr 10, 2021

Uh oh!

chinmaygarde commented Apr 22, 2021

Uh oh!

jpnurmi commented Apr 23, 2021

Uh oh!

cbracken left a comment

Uh oh!

Uh oh!

Uh oh!

cbracken commented May 1, 2021

Uh oh!

jpnurmi commented May 2, 2021

Uh oh!

cbracken commented May 3, 2021 •

edited

Loading

Uh oh!

jpnurmi commented May 3, 2021

Uh oh!

robert-ancell left a comment

Uh oh!

Uh oh!

[Linux] revise dark theme detection #25535

[Linux] revise dark theme detection #25535

Uh oh!

Conversation

jpnurmi commented Apr 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Apr 10, 2021

Uh oh!

chinmaygarde commented Apr 22, 2021

Uh oh!

jpnurmi commented Apr 23, 2021

Uh oh!

cbracken left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cbracken commented May 1, 2021

Uh oh!

jpnurmi commented May 2, 2021

Uh oh!

cbracken commented May 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jpnurmi commented May 3, 2021

Uh oh!

robert-ancell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jpnurmi commented Apr 10, 2021 •

edited

Loading

cbracken commented May 3, 2021 •

edited

Loading