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 option to disable automatic resize of main window #4011

Merged
merged 12 commits into from
Sep 13, 2024

Conversation

kalimag
Copy link
Contributor

@kalimag kalimag commented Aug 23, 2024

  • Add option to disable the automatic resizing of the main window whenever the framebuffer size changes
    • Window can be resized arbitrarily and stays that size
    • Selecting a standard scale through the menu/keybinds/API still works
  • Restore last window size after restart (if automatic resize is disabled)
  • Minor changes to saved window position
    • Make config values nullable instead of magic -1 value
    • Don't save window position if minimized (this didn't quite work as intended) or maximized

This feature is useful for streamers who want a consistent capture size, as well as for people who want to fit the BizHawk window into a fixed layout with other windows (corner snapping etc.).

This is not the same functionality as the custom resolution in the display settings, which stretches the framebuffer to the specified size and fits the window to that size exactly.

With the feature in this PR, the window/client area stays a consistent size while automatic letterboxing/pillarboxing, integer resizing, 1:1 pixels etc. still work. This is preferable for working with multiple games, resolution changes mid-game, changing DS/WS screen rotation/layouts and similar, where squeezing everything into the same resolution is undesirable. It's also just more convenient than figuring out a resolution in the right aspect ratio and entering that in the display settings all the time.

Check if completed:

resolves #1560, #3039, and #3410

src/BizHawk.Client.EmuHawk/MainForm.Events.cs Outdated Show resolved Hide resolved
src/BizHawk.Client.EmuHawk/MainForm.Events.cs Show resolved Hide resolved
src/BizHawk.Client.EmuHawk/MainForm.cs Show resolved Hide resolved
{
Location = new Point(Config.MainWndx, Config.MainWndy);
if (Config.MainWndx is int x && Config.MainWndy is int y)
Copy link
Member

@YoshiRulz YoshiRulz Sep 12, 2024

Choose a reason for hiding this comment

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

Was it necessary to make these nullable to implement this new feature? (edit: I see it's the last commit, so I'm guessing that's a no.) If you could continue using -1 as the sentinel value instead of null, I think that would be preferable. Otherwise, try Point? instead of 2 int?s.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not necessary to the PR, no. I just made this change because negative values including -1 are valid window positions

Copy link
Member

Choose a reason for hiding this comment

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

Possibly related to #3544?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not directly, tool window position saving is handled elsewhere

src/BizHawk.Client.EmuHawk/MainForm.cs Show resolved Hide resolved
{
Config.MainWndy = Location.Y;
Config.MainWindowPosition = Location;
Config.MainWindowSize = Size;
Copy link
Member

Choose a reason for hiding this comment

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

To clarify for anyone else: This preserves the existing behaviour of not updating the config in the edge case where EmuHawk is closed while already minimised.

@YoshiRulz

This comment was marked as resolved.

@kalimag
Copy link
Contributor Author

kalimag commented Sep 12, 2024

Point and Size serialization will have to be re-tested on #3932

@YoshiRulz YoshiRulz dismissed their stale review September 12, 2024 15:00

Nitpicks resolved

Copy link
Member

@YoshiRulz YoshiRulz left a comment

Choose a reason for hiding this comment

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

LGTM now, so I've made a note to merge this before the upcoming RC.

Thanks for all the contributions recently.

@kalimag
Copy link
Contributor Author

kalimag commented Sep 12, 2024

LGTM now, so I've made a note to merge this before the upcoming RC.

Thanks for all the contributions recently.

Well, I finally had to stop procrastinating to get any of this into a release before 2.10.1 in December 2025 ;)

@kalimag
Copy link
Contributor Author

kalimag commented Sep 13, 2024

I have a few more changes that I didn't PR yet because of the merge conflicts with(out) this PR: kalimag/BizHawk@pr/disable-automatic-resize...kalimag:BizHawk:pr/disable-automatic-resize-plus

Should I wait until this one is merged and open a new PR, or just push them into this PR?

@YoshiRulz
Copy link
Member

I don't follow. Merge conflicts with master? You're welcome to rewrite history and force push, just comment with what you changed compared to the old HEAD.

@kalimag
Copy link
Contributor Author

kalimag commented Sep 13, 2024

What I mean is that those changes are not strictly related to this PR, but the commits touch the same code and are based on this PR's branch rather than master.

So I can't trivially open a separate PR with just those changes without (probably) creating a bunch of merge conflicts between the two PRs, and the easier option is to either have one PR (this one) containing all changes, or wait until this one is merged to open a second PR.

@YoshiRulz YoshiRulz merged commit 2e4d2ce into TASEmulators:master Sep 13, 2024
4 checks passed
@kalimag kalimag mentioned this pull request Sep 13, 2024
2 tasks
@kalimag kalimag deleted the pr/disable-automatic-resize branch September 13, 2024 13:47
CasualPokePlayer added a commit that referenced this pull request Sep 19, 2024
…/height

while this did "work", and mimicked other emulators handling internal upscaling, it was a hack, and ends up causing issue with lua padding. it is also more unneeded now that #4029 (makes the UX of a sudden "large window" more managable) and #4011 (allowing for mimicking the frozen smaller window state) are merged

resolves #4039
@skywind3000
Copy link

Thanks, but I still can't find the new option to keep the window size:

图片

I am using the latest BizHawk-2.10-rc1-win-x64.zip

@YoshiRulz
Copy link
Member

View > Window Size > Static Size

@skywind3000
Copy link

skywind3000 commented Sep 22, 2024

thanks, that’s really helpful. Why not make it enabled by default?

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.

Window is spontaneously moved/recreated during Yoshi's island's opening
3 participants