Skip to content

suggest_fallback_scale_factor() reports whether the value was actually used - #361

Open
podulator wants to merge 1 commit into
RustAudio:nextfrom
podulator:scale-used-enum
Open

podulator wants to merge 1 commit into
RustAudio:nextfrom
podulator:scale-used-enum

Conversation

@podulator

Copy link
Copy Markdown
Contributor

This resolves the first half of #359: Window::suggest_fallback_scale_factor() now returns a Result<bool, Error>.

true means the fallback is now the scale in use, and false means the platform already had its own value, and uses that.

This lets a plugin wrapper answer the host truthfully: CLAP's clap_plugin_gui::set_scale is documented to "override any OS info" and to return false if the call is ignored, but baseview previously always reported success even when its Xft.dpi/system value took priority over the host's suggestion. The bundled CLAP examples are updated to show the intended use (propagate false as an error, since clack's PluginGuiImpl::set_scale has no other way to say "ignored" yet).

It doesn't change the X11/Win32 priority logic itself (the host overriding the platform for embedded windows) - that's the discussion in #359 (comment).

The API change means that ? style callers still compile, but code that returns the call's result directly from a Result<(), _> function won't. So, it is a small breaking change, and it might want to go in a v0.5?

It's tested on X11, but could use a real world Windows and Mac test, really.

@prokopyl

Copy link
Copy Markdown
Member

This PR looks good to me, but you are correct, it is a breaking change, so it'll have to be released in the next major version of baseview.

Would you mind rebasing your work on the next branch instead of main? That way it'll be released as part of 0.5 (which is already being worked on).

Be aware however that the X11 implementation has changed quite a bit (see #357), however it should make your changes simpler since you won't need to send a request to a separate thread anymore. 🙂

@prokopyl prokopyl added this to the v0.5 - New Host API milestone Oct 25, 2026
…y used

This resolves the first half of RustAudio#359: Window::suggest_fallback_scale_factor() now returns a Result<bool, Error>, and so does HostedWindow::suggest_fallback_scale_factor() in baseview-host.

true means the fallback is now the scale in use, and false means the platform already had its own value, and uses that.

This lets a plugin wrapper answer the host truthfully: CLAP's clap_plugin_gui::set_scale is documented to "override any OS info" and to return false if the call is ignored, but baseview previously always reported success even when its Xft.dpi/system value took priority over the host's suggestion.
The bundled CLAP examples are updated to show the intended use (propagate false as an error, since clack's PluginGuiImpl::set_scale has no other way to say "ignored" yet).

It doesn't change the X11/Win32 priority logic itself (the host overriding the platform for embedded windows) - that's the discussion in RustAudio#359 (comment).

The API change means that `?` style callers still compile, but code that returns the call's result directly from a Result<(), _> function won't. So, it is a small breaking change, hence `next`, for 0.5.

It's tested on X11, but could use a real world Windows and Mac test, really.
@podulator
podulator changed the base branch from master to next October 25, 2026 09:58
@podulator

Copy link
Copy Markdown
Contributor Author

Ok, rebased onto next, thanks!
You were right, it was simpler than previous version on main. With the X11 thread gone, the value is simply returned, no extra enum needed.
HostedWindow::suggest_fallback_scale_factor() in baseview-host returns bool too, so both APIs say the same thing.

One thing I noticed, next doesn't have the #358 fix yet (it landed on master after next branched), so on next a second suggested scale still compounds with the first. I've left it out of this PR, since I guess it comes with the next master → next merge, but I'm happy to send it as a separate PR against next if that helps.....

Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants