[Windows] HDR Output: Emit window events when HDR state changes

[DarkKilauea]

Depends on: #94496

This adds a small enhancement to the HDR output feature for windows that triggers a WindowEvent whenever the HDR output state changes.

I noticed when working on my own projects and in sample applications that @allenwp and I put together, that polling was required to respond to external changes to the HDR state. This enhancement tries to address that by allowing UI, game effects, and other nodes to respond to the HDR state actually changing.

Please see the last commit for the changes added by this PR.

[DarkKilauea]

Whoops, forgot to update the documentation. That's fixed now.

[ArchercatNEO]

Since emitting an event is also possible on Wayland (in fact, the Wayland API only provides event-based queries) should this also be implemented for Wayland?

I could rebase on top of this PR but that feels very indirect, I could also wait until both the original and this PR are merged then rebase onto the then-master but that depends on how closely each merge is done.

[DarkKilauea]

@ArchercatNEO You could also create another PR that implements just the WindowEvent emission and have that depend on both your HDR PR and this one.

It is my hope that once we get the first HDR PR merged, the others will be able to follow shortly after.

[DarkKilauea]

@bruvzg Are you the correct person to review this?

[bruvzg]

Code looks good. No idea if there's a necessity for having the signal/notification.

[DarkKilauea]

Rebased to resolve conflicts with DisplayServerEnum refactor.

[allenwp]

This is really great!! Thanks! I believe this would be extremely valuable to developers and it would be great to have this for 4.7.

I haven't tested this as thoroughly as I normally would, but this seems like something that's pretty simple and will be easy to bug fix if there are any edge cases that are discovered after merging.

I've left a couple of comments about the documentation and signal/constant naming.

No idea if there's a necessity for having the signal/notification.

@bruvzg Currently the developer must poll this value every frame, even though it may never change (if the player is using an SDR screen or an HDR screen that has a stable brightness setting). An example of the current approach is shown in the documentation here: https://docs.godotengine.org/en/latest/tutorials/rendering/hdr_output.html#using-output-max-linear-value

So I think the signal is valuable in addition to the other events?? You might want to hook a shader uniform up to this value... does a signal help with this? I really am not the one to make the call on this, so if someone else wants to jump in who is more well versed in what justifies a signal and what doesn't, please do so.

[allenwp]

Current usage of this signal looks something like this in most any use case I can think of:

func _ready() -> void:
	get_window().hdr_output_changed.connect(_on_hdr_output_changed)
	_on_hdr_output_changed()


func _on_hdr_output_changed() -> void:
	var output_max_linear_value = get_window().get_output_max_linear_value()
	...

It might be nice to have the output_max_linear_value passed as an argument of the signal? This way the user doesn't need to retrieve it from the Window every time the signal is sent. Combined with a signal name change to output_max_linear_value_changed, this probably makes it more intuitive.

func _ready() -> void:
	var window: Window = get_window()
	window.output_max_linear_value_changed.connect(_on_output_max_linear_value_changed)
	_on_output_max_linear_value_changed(window.get_output_max_linear_value())


func _on_output_max_linear_value_changed(output_max_linear_value: float) -> void:
	...

[DarkKilauea]

It might be nice to have the output_max_linear_value passed as an argument of the signal? This way the user doesn't need to retrieve it from the Window every time the signal is sent. Combined with a signal name change to output_max_linear_value_changed, this probably makes it more intuitive.

func _ready() -> void:
	var window: Window = get_window()
	window.output_max_linear_value_changed.connect(_on_output_max_linear_value_changed)
	_on_output_max_linear_value_changed(window.get_output_max_linear_value())


func _on_output_max_linear_value_changed(output_max_linear_value: float) -> void:
	...

Done! The signal will now emit the current value of Window.output_max_linear_value.

[allenwp]

This is looking great to me! Scripts that used to be:

func _process(_delta: float) -> void:
	var output_max_linear_value = get_window().get_output_max_linear_value()
	# Do something with output_max_linear_value

Can now be written as:

func _enter_tree() -> void:
	var window: Window = get_window()
	window.output_max_linear_value_changed.connect(_on_output_max_linear_value_changed)
	_on_output_max_linear_value_changed(window.get_output_max_linear_value())


func _exit_tree() -> void:
	get_window().output_max_linear_value_changed.disconnect(_on_output_max_linear_value_changed)


func _on_output_max_linear_value_changed(output_max_linear_value: float) -> void:
	# Do something with output_max_linear_value

This seems like a good improvement to me.

I'm adding this to the agenda for tomorrow's Core meeting so we can make sure the public API is appropriate before porting this to the other platforms.

[allenwp]

This looks great to me!

I'm approving now, but this shouldn't be merged until we have this feature ready to merge for other platforms that support HDR output.

[allenwp]

I created a followup PR to this one that adds the events for Apple and Wayland: #118076

[Repiteo]

Thanks!