Skip to content
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

Add method to get "base" system UI color and system theme change callback. #87384

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Jan 19, 2024

Implements DisplayServer.get_base_color to get default control background color (on macOS and Windows).
Implements DisplayServer.set_system_theme_change_callback to set theme change callback (all platforms, except web, tested on Windows 11, macOS 14.2.1, PopOS, Android 14 and iOS 17.2).

func _theme_upd():
	$ColorRectBack.color = DisplayServer.get_base_color()
	$ColorRectFront.color = DisplayServer.get_accent_color()

func _ready():
	DisplayServer.set_system_theme_change_callback(Callable(self, "_theme_upd"))
	_theme_upd()

Note: theme_change callback is triggered by any OS UI style related change, it's not necessarily mean that dark/light mode was switched or accent/base color changed.

@YuriSizov YuriSizov self-requested a review January 19, 2024 20:13
@bruvzg bruvzg force-pushed the sys_base_color branch 2 times, most recently from 848adcf to 6d86579 Compare January 20, 2024 09:27
@bruvzg bruvzg changed the title [macOS / Windows] Add method to get "base" system UI color and system them change callback. Add method to get "base" system UI color and system theme change callback. Jan 20, 2024
@bruvzg
Copy link
Member Author

bruvzg commented Jan 20, 2024

Added theme change callback support for the rest of platforms (except web).

@bruvzg bruvzg force-pushed the sys_base_color branch 2 times, most recently from 90704c7 to 86f3c9d Compare January 21, 2024 10:02
@YuriSizov YuriSizov requested a review from m4gr3d January 22, 2024 11:04
@Riteo
Copy link
Contributor

Riteo commented Jan 22, 2024

@bruvzg

for Linux there's no standard "accent" color, maybe we can get something, but it will be at least DE specific.

There's an accent-color XDG Portal setting: https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.Settings.html

@bruvzg
Copy link
Member Author

bruvzg commented Jan 22, 2024

There's an accent-color XDG Portal setting: https://flatpak.github.io/xdg-desktop-portal/docs/doc-org.freedesktop.portal.Settings.html

This seems to be a new addition, so I'm not sure if any real DE support it, but since it's D-Bus we probably should try reading it anyway.

Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor feedback to address and the Android logic should be good to go!

Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Android section looks good!

@YuriSizov YuriSizov modified the milestones: 4.x, 4.3 Jan 24, 2024
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

I wonder if the "theme_change" methods/signals should be "system_theme_change" to make it clear that it's about the OS theme settings and not the Godot editor/project theme?

doc/classes/DisplayServer.xml Outdated Show resolved Hide resolved
platform/linuxbsd/x11/display_server_x11.cpp Outdated Show resolved Hide resolved
@bruvzg
Copy link
Member Author

bruvzg commented Feb 13, 2024

Renamed callback to system_theme_change and added in to Wayland DS.

@bruvzg bruvzg force-pushed the sys_base_color branch 2 times, most recently from 16dd61d to 31ae2ac Compare February 13, 2024 15:28
@akien-mga
Copy link
Member

Needs a rebase.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally on Fedora 39 KDE X11, this returns a fully transparent color for both base and accent color (so nothing is visible):

extends Node2D

func _theme_upd():
	print("Updating theme")
	$ColorRectBack.color = DisplayServer.get_base_color()
	print($ColorRectBack.color)
	$ColorRectFront.color = DisplayServer.get_accent_color()
	print($ColorRectFront.color)

func _ready():
	DisplayServer.set_system_theme_change_callback(Callable(self, "_theme_upd"))
	_theme_upd()

I'm using the Breeze Dark theme. The theme switch callback works (it's called if I switch system themes), but colors are still fully transparent.

@bruvzg
Copy link
Member Author

bruvzg commented Feb 13, 2024

Tested locally on Fedora 39 KDE X11

Colors are supported only on macOS and Windows, there's no standard Linux API for this.

@akien-mga akien-mga merged commit b6dee88 into godotengine:master Feb 13, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@bruvzg bruvzg deleted the sys_base_color branch February 14, 2024 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants