Skip to content

[GEN][ZH] Center Game App Window on startup and resolution change #541

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

Merged
merged 16 commits into from
May 16, 2025

Conversation

helmutbuhler
Copy link

@helmutbuhler helmutbuhler commented Mar 29, 2025

This PR centers the window after loading on the screen (not including taskbar).
It also adds code to pump window messages during startup to avoid "(Not responding)" in the title. (That was necessary because Windows otherwise ignores the Move Call in debug builds, and maybe also on slower machines)

Also note that I needed to #define WINVER 0x0500, because the Monitor API didn't exist in Win 95 yet. This causes the compiler to emit a message when compiling dx8wrapper.cpp, but generates no error or warning. The game will also no longer run on Win95, but I think we can live with that.

Change list

  • fix: In windowed mode, the game now starts centered on the screen.

TODO

  • Replicate in Generals

@helmutbuhler helmutbuhler changed the title Center Window on Startup [ZH] Center Window on Startup Mar 29, 2025
@xezon xezon added Enhancement Is new feature or request Minor Severity: Minor < Major < Critical < Blocker labels Mar 29, 2025
@DevGeniusCode DevGeniusCode added the ZH Relates to Zero Hour label Mar 29, 2025
@tintinhamans
Copy link

While this is neat, how do we want to handle cases where the user does not want this functionality.

@tomsons26
Copy link

tomsons26 commented Apr 3, 2025

I'm gonna also ruin this and say, you will likely only center the initial window, the game makes two, one to show splash other for game,
we tried this in Thyme but if the splash window doesn't match the res of the game, result was only splash window got centered

@helmutbuhler
Copy link
Author

While this is neat, how do we want to handle cases where the user does not want this functionality.

The current behavior is that the window starts in the bottom right corner. The bottom right part for the window is usually not visible. Tell me who wants this behavior?
Also, Gentool changes the behavior to what this PR implements, did anyone complain about that?

I'm gonna also ruin this and say, you will likely only center the initial window, the game makes two, one to show splash other for game, we tried this in Thyme but if the splash window doesn't match the res of the game, result was only splash window got centered

Are you saying that this PR is wrong because you didn't get this feature to work in Thyme? If you look at the code or try out the build, you'll see that it works.

@xezon
Copy link

xezon commented Apr 4, 2025

Also, Gentool changes the behavior to what this PR implements, did anyone complain about that?

No complaints over 15 years. Many users use the "Full" window preset in GenTool together with windowed mode.

@tintinhamans
Copy link

Something doesn't seem to be quite right.. there seems to be a gap present below the window despite the correct height of 1440 being set in options.ini.

NVIDIA_Overlay_4NI2GDvNVx(1)

@tintinhamans
Copy link

tintinhamans commented Apr 4, 2025

No complaints over 15 years. Many users use the "Full" window preset in GenTool together with windowed mode.

I can't use Full, it pulls the window to the left of my monitor 😢 :

image

Also, I can think of a use case for other ultrawide users that may actually want the window on the left so that they can have another full-size window (Discord, OBS, etc.) on the right.

@tomsons26
Copy link

While this is neat, how do we want to handle cases where the user does not want this functionality.

The current behavior is that the window starts in the bottom right corner. The bottom right part for the window is usually not visible. Tell me who wants this behavior? Also, Gentool changes the behavior to what this PR implements, did anyone complain about that?

I'm gonna also ruin this and say, you will likely only center the initial window, the game makes two, one to show splash other for game, we tried this in Thyme but if the splash window doesn't match the res of the game, result was only splash window got centered

Are you saying that this PR is wrong because you didn't get this feature to work in Thyme? If you look at the code or try out the build, you'll see that it works.

I'm saying there may be unexpected behavior that may not always manifest due to the game making and destroying multiple windows during its startup

@willherzog
Copy link

The new official patch (1.05) adds window centering. I have no idea whether it's done in the same way as this PR, but certainly for me it's much appreciated after having to deal for so long with the game window starting out not only off-centered but also partially off-screen.

Technically what appeared to happen originally is that the splash screen was centered but then the actual game window used the exact same top+left coordinates as the splash screen - so any game resolution besides 800x600 (which would be the same as the splash screen) necessarily resulted in it being off-centered.

@helmutbuhler
Copy link
Author

Something doesn't seem to be quite right.. there seems to be a gap present below the window despite the correct height of 1440 being set in options.ini.

NVIDIA_Overlay_4NI2GDvNVx(1)

This does indeed look wrong, but sadly I cannot reproduce it. But I also don't have such a wide monitor or Windows 11.

Did you disable DPI Scaling in the Compatibility Settings?
Can you please select a smaller resolution ingame and make another screenshot? Select a resolution such that the titlebar of the window is visible.

Thanks!

@helmutbuhler
Copy link
Author

Never mind, I found the issue. I center the window in the workarea (area not occupied by taskbar and similar), but if the resolution is bigger than the workarea, it's better to center in the whole monitorarea. I just pushed a commit to fix this.
While I was at it, I also changed the codepath that repositions the window after changing the resolution to do the same. I also added some more comments.

@tintinhamans Can you check if the behavior is now fixed?

Copy link

@OmniBlade OmniBlade left a comment

Choose a reason for hiding this comment

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

If the previous issues reported are confirmed fixed I'm happy to approve this?

@helmutbuhler
Copy link
Author

Yes, please.

@aliendroid1
Copy link

I think the main reason for the issue is that the position is set based on the launcher image resolution but then not updated when resolution is updated

@helmutbuhler
Copy link
Author

I think the main reason for the issue is that the position is set based on the launcher image resolution but then not updated when resolution is updated

Yes, and this PR fixes it

Copy link

@xezon xezon left a comment

Choose a reason for hiding this comment

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

I tested this change and the behaviour this adds is very good. It automatically centers or fullscreens the game window depending on the resolution.

It also updates the position when changing the resolution from the Options menu. The window reposition does look a bit buggy then, but that is unrelated to this change. For reference, with GenTool the window reposition after resolution change looks a bit better.

I do not understand what bug the serviceWindowsOS call fixes. I cannot reproduce it.

This change needs to be replicated in Generals.

// on slower computers and in debug build.
// This also ensures that the window is correctly positioned, because Windows
// apparently ignores the SetWindowPos call when the window is not responding.
serviceWindowsOS();
Copy link

Choose a reason for hiding this comment

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

How can this problem be reproduced? I tried this on my machine with Debug configuration and I cannot get the Window to be non responding during boot with and without this change.

Copy link
Author

Choose a reason for hiding this comment

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

Hm, are you perhaps having a debugger attached? The buggy behavior will only occur if there is no debugger attached.
You can also try to insert a Sleep(5000); call instead of serviceWindowsOS(); When I do this, even the Release build shows buggy behavior.

Copy link

Choose a reason for hiding this comment

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

I cannot confirm that this avoids "(Not Responding)" in the window title. I tested this with

serviceWindowsOS();
Sleep(10000);
serviceWindowsOS();

But it does fix the window positioning as you say.

Copy link
Author

Choose a reason for hiding this comment

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

Well, of course it shows the "(Not Responding)" if you put a Sleep in there :)
The original issue arises when you use Debug build without optimizations and without a debugger attached. In that case the startup is so slow that this "(Not Responding)" shenanigans arises without a Sleep call.

In case you are interested. MS has a little bit of documentation about this feature:
https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-ishungappwindow?redirectedfrom=MSDN
https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-disableprocesswindowsghosting
This undocumented function might also be of interest: HungWindowFromGhostWindow

// Pump messages during startup to avoid "(Not responding)" in the window title
// on slower computers and in debug build.
// This also ensures that the window is correctly positioned, because Windows
// apparently ignores the SetWindowPos call when the window is not responding.
Copy link

Choose a reason for hiding this comment

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

This comment says "apparently". What does that mean? Does it ignore the SetWindowPos or not? If it factually ignores the SetWindowPos, then there should be no word of doubt in the comment.

Copy link
Author

Choose a reason for hiding this comment

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

Well, Windows behaves as if it ignores the call. Does it actually ignore it, or does the GhostWindow implementation have bugs that ignore that call, or is it some other bug? I don't know, I don't have the code. All I know is that Windows appears to ignore the call, so I think the comment is accurate.

Copy link

Choose a reason for hiding this comment

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

Ok if we do not know what Windows really does, then how about we just describe the observed issue? This way there will be no speculation.

Example: "Windows behaves as if the call to SetWindowPos never occurred".

Choose a reason for hiding this comment

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

Ok if we do not know what Windows really does, then how about we just describe the observed issue? This way there will be no speculation.

Example: "Windows behaves as if the call to SetWindowPos never occurred".

I think the mistake causing this issue is inside the d3d8 wrapper setRenderDevice function. Even when passed the argument to resize, inside it's using flags for no resize and no reposition

Copy link
Author

Choose a reason for hiding this comment

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

@xezon In my opinion the comment is fine as it is and means the same as your suggestion. If you feel the comment needs to be changed, can you do it?

@aliendroid1 No, the flags are correct.

@xezon xezon changed the title [ZH] Center Window on Startup [ZH] Center Game Window on startup and resolution change May 12, 2025
@xezon xezon added this to the Important features milestone May 12, 2025
@xezon xezon added Critical Severity: Minor < Major < Critical < Blocker and removed Minor Severity: Minor < Major < Critical < Blocker labels May 12, 2025
@xezon xezon removed the ZH Relates to Zero Hour label May 12, 2025
@helmutbuhler
Copy link
Author

Thanks for the review! I believe everything is resolved now. I can also sync with Generals once approved.

@xezon
Copy link

xezon commented May 15, 2025

Comments simplified. Two new debug logs added. Replicated in Generals. Check if good.

@xezon xezon changed the title [ZH] Center Game Window on startup and resolution change [GEN][ZH] Center Game App Window on startup and resolution change May 15, 2025
@xezon xezon added Gen Relates to Generals ZH Relates to Zero Hour labels May 16, 2025
@helmutbuhler
Copy link
Author

Thanks! Looks all good!

I left a comment above about that "Not responding" things, in case you are curious.

Slightly related to this PR is also this: #595 Maybe you can approve that, too? (I already reviewed it)

@xezon
Copy link

xezon commented May 16, 2025

Appended a few more minor word tweaks.

@xezon xezon merged commit d44d901 into TheSuperHackers:main May 16, 2025
18 checks passed
@xezon xezon deleted the center_window_ea branch May 16, 2025 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Critical Severity: Minor < Major < Critical < Blocker Enhancement Is new feature or request Gen Relates to Generals ZH Relates to Zero Hour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants