Add ability to change background dim colour in grayscale

[Uncomfy]

Previously discussed here: #12983

Depends on:

• Add a custom shader for BeatmapBackground to handle background dim colour changes osu-resources#344
• Allow to overwrite BufferedContainer's TextureShader and inherit from its DrawNode osu-framework#6395
• Allow to do DrawNode invalidation on cached BufferedContainer without redrawing the framebuffer osu-framework#6404

Allows to change which colour the background is dimmed to. Currently only supports grayscale.

This is not a full implementation, as it doesn't support storyboards and videos.

The main reason I'm asking for comments so early is that this implementation handles dimming in two different classes, which leads to code duplication, so I wonder if there is a cleaner way to do this.

osu_bg_dim_colour.mp4

Here are also a few examples of how it could look with colour picker instead of a grayscale slider, using SettingsColour, but I'm unsure if it fits the design, as colour pickers weren't used in Settings/Visual Settings before. Also it doesn't work properly in Visual Settings yet, because it doesn't have a PopoverContainer in its hierarchy, and I'm not really sure how to handle it.

[jhk2601]

Just a quick thing, "0%-100%" doesn't really make sense in the context of a slider changing how light or dark the dim is. This would be fixed in the color picker implementation anyway though (which I personally think makes more sense, though maybe hiding the UI element behind an option that's off by default?)

[frenzibyte]

Isn't TextureShader responsible for rendering the framebuffer texture of the buffered container? If so, isn't that pointless? Isn't it enough to apply the custom "BeatmapBackground" shader on the underlying beatmap background sprite? Would that cause issues with blurring? I can't imagine it will.

In other words, do Sprite.TextureShader = "BeatmapBackground" instead (alongside the extra logic for the uniform buffer). That will save exposing TextureShader framework-side and also possibly "save" shader computation because the overall effect will become buffered (don't quote me on this).

[Uncomfy]

Yes, but it will require the redraw of the framebuffer texture on each DimLevel change, which will also force the blur to be redrawn, and I feared that this might introduce unnecessary overhead during (for example) breaks, if player has background blur enabled. If that is not a concern - yeah, keeping the custom shader only on the sprite makes complete sense.

[Uncomfy]

You're right, it's pointless, at least in the current implementation.
Changing the dim parameters requires DrawNode invalidation, which forces the framebuffer to be redrawn, so there are no benefits. This does mean that DimLevel transitions are way more expensive now (at least on my rather old and slow hardware).

Here are the results of my benchmarks for average frametime during different parts of the beatmap.
With user blur set to 0%:

	Gameplay	Breaks	Fade in	Fade out
Baseline	4.25 ms	4.29 ms	4.42 ms	4.08 ms
Coloured dim	4.26 ms	4.33 ms	4.92 ms	4.55 ms
Relative increase	0.2%	0.9%	11.3%	11.5%

With user blur set to 100%:

	Gameplay	Breaks	Fade in	Fade out
Baseline	4.20 ms	4.24 ms	4.39 ms	4.02 ms
Coloured dim	4.22 ms	4.30 ms	5.18 ms	4.66 ms
Relative increase	0.5%	1.4%	18%	15.9%

"Baseline" is osu! 2024.1009.1
"Coloured dim" is commit c5867b6

Details about the benchmark

Made a map with 64 gameplay sections and 63 breaks for this benchmark, total duration is 3:09.

"Fade in" refers to frames rendered during gameplay-to-break transition, "Fade out" refers to break-to-gameplay transition.
"Gameplay" and "Breaks" columns are just for sanity-checking, as they should be same.

Total amount of recorded frames:

	Blur	Gameplay	Breaks	Fade in	Fade out
Baseline	0%	12068	8176	11018	12069
Baseline	100%	12193	8253	11089	12236
Coloured dim	0%	12090	8105	9810	10689
Coloured dim	100%	12154	8137	9448	10512

osu! settings:

• Compiled in release mode
• Single-threaded mode
• "OpenGL (Experimental)" renderer
• Windowed screen mode
• Resolution: 1564x964
• Frame limiter: Basically unlimited

System info:

• OS: Kubuntu 24.04.1 LTS x86_64
• CPU: Intel i3-6100U @ 2.300GHz
• GPU: Intel HD Graphics 520
• RAM: 19907MiB

So I guess now there are two ways to go - ditch the shader replacement on the BufferedContainer and have slower transitions, or modify the BufferedContainer somehow to avoid redrawing the framebuffer when shader parameters are changed.

Which one is more preferable?

[frenzibyte]

Fair enough, this is a bit of a pesky situation. You can control whether the framebuffer gets redrawn by overriding GetDrawVersion() and make that point to a custom invalidation ID that is normally incremented but not when dim properties are changed. See Path in osu!framework, it might actually be doing exactly what you're looking for.

[Uncomfy]

Added a test case for it, but solving it requires a change in the framework: ppy/osu-framework#6404

With that change applied benchmarks look a lot better.

With user blur set to 0%:

Implementation	Gameplay	Breaks	Fade in	Fade out
Baseline	4.15 ms	4.20 ms	4.34 ms	3.99 ms
Coloured dim	4.19 ms	4.23 ms	4.39 ms	4.00 ms
Relative increase	1.0%	0.7%	1.2%	0.3%

With user blur set to 100%:

Implementation	Gameplay	Breaks	Fade in	Fade out
Baseline	4.11 ms	4.16 ms	4.31 ms	3.93 ms
Coloured dim	4.05 ms	4.11 ms	4.25 ms	3.85 ms
Relative increase	-1.5%	-1.2%	-1.4%	-2.0%

"Baseline" is osu! 2024.1009.1
"Coloured dim" is commit 93c3fac with ppy/osu-framework#6404 applied

[Uncomfy]

Just a quick thing, "0%-100%" doesn't really make sense in the context of a slider changing how light or dark the dim is. This would be fixed in the color picker implementation anyway though (which I personally think makes more sense, though maybe hiding the UI element behind an option that's off by default?)

I'm not sure what would be better than 0%-100% for a slider. Or do you mean ditching the slider completely?
Would renaming the setting to background dim brightness make more sense?

[jhk2601]

Just a quick thing, "0%-100%" doesn't really make sense in the context of a slider changing how light or dark the dim is. This would be fixed in the color picker implementation anyway though (which I personally think makes more sense, though maybe hiding the UI element behind an option that's off by default?)

I'm not sure what would be better than 0%-100% for a slider. Or do you mean ditching the slider completely? Would renaming the setting to background dim brightness make more sense?

yes that would be better, if I saw a slider labeled "background dim color" and it was a percent I wouldn't know what the number represented, "brightness" works better.

[peppy]

It might make more sense to rename "Background dim" to "Background opacity" and have 100% be a fully visible image. Although this will go against user expectations coming from stable so maybe it's too late to make a change like this.