Add "Background scale mode" setting #36111
Conversation
|
Reminder that this setting should also be in First run setup as well. |
| fillMode = config.GetBindable<BackgroundFillMode>(OsuSetting.BackgroundFillMode); | ||
| fillMode.BindValueChanged(_ => loadBeatmapBackground(beatmap)); |
There was a problem hiding this comment.
Why is this change handling done here as opposed to inside BeatmapBackground?
Also, we never put BindValueChanged inside BDL methods as it is thread unsafe.
There was a problem hiding this comment.
it's really strange, don't remember why i did this, maybe something didn't work before that
should i use BindValueChanged inside LoadComplete instead?
There was a problem hiding this comment.
Yes, but also consider not reloading the whole beatmap to update the mode. I think the can be removed and handled exclusively in BeatmapBackground.
| { | ||
| base.LoadComplete(); | ||
|
|
||
| fillMode.ValueChanged += _ => updateBackgroundFillMode(); |
There was a problem hiding this comment.
now, BindValueChanged doesn't seem to work properly during and after playing beatmap
@peppy what could be the reason for this?
2025-12-26.13-53-45.mp4
No one in their right mind would want this.
|
@bdach if you want to give this 5 minutes that would be appreciated, since i changed over 50%. |
diquoks
changed the title
didn't want to cause problems, i'm just not very familiar with the codebase yet 🙌 |
it's fine, didn't mean it like that. just a standard practice that we get an extra review in such cases. |
|
This looks wonky in song select when progressing from main menu: Screen.Recording.2025-12-29.at.12.12.23.movNote left side of screen, where the old background from main menu is visible where the "letterbox" would be until it insta-fades out. |
|
Right, probably need to invoke some fake letterboxing boxes to fix that, which is going to be less fun. |
| this.textureName = textureName; | ||
| RelativeSizeAxes = Axes.Both; | ||
|
|
||
| AddInternal(Letterbox = new Box |
There was a problem hiding this comment.
This results in a performance regression, due to overdrawing of layers.
- Check something like
LetterboxOverlayfor how this needs to be done. - Should not result in any boxes unless required (ie. for fill screen it shouldn't be required).
4 participants
a slightly better version with localisation and code quality