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

win7 support #86

Open
wants to merge 1 commit into
base: debug
Choose a base branch
from
Open

Conversation

xaevman
Copy link
Contributor

@xaevman xaevman commented May 15, 2022

Both .NET6 and WebView2 provide support for windows versions all the way back to Win7 SP1. Currently, we can't compile for Win7 due to the use of the newer DPI-awareness APIs which are only available in Win10+

This is a proposal to dynamically load those APIs only on platforms that support them, allowing us to run without dpi support on older win7/win8 installations. Some future work could be done to wire up the primitive dpi support that earlier versions of windows did provide, though that is not yet done and tested in this PR.

The proposed changes are enabled by defining COMPAT_DPI within the _WIN32 section of Photino.h. Without COMPAT_DPI defined, the previous code structure is maintained as-is.

Thoughts or opinions on this?

@MikeYeager MikeYeager added enhancement New feature or request Windows Only on Windows labels May 19, 2022
@MikeYeager
Copy link
Collaborator

@xaevman It looks like it will always compile for Win7 compatibility with the code provided, which is a different default than previous versions. Since this is a compile-time decision, we have to decide if it makes sense to change this default and if it will cause any grief to those running on Win10. Or we could create a new target and compile Linux, macOS, Win7 and Win10 versions and put them into the NuGet package using the target RID win7-x64 instead of win-x64 in the .NET .csproj to specify that you want the win7/8 compatible version. Thoughts?

@xaevman
Copy link
Contributor Author

xaevman commented May 19, 2022

Yeah, that's true. I would propose we always compile with the compat flags

#define WINVER _WIN32_WINNT_WIN7
#define _WIN32_WINNT _WIN32_WINNT_WIN7

so that new features are not introduced with static linking which break the old platforms. This change still makes all the same Dpi-aware api calls when running on a platform where they are available via GetProcAddress from user32.dll, so the runtime effect on Win10 users should be none. The vast majority of our players use Win10, so just wanted to be clear that these proposed changes still keep the Win10 experience front-of-mind. I'm not specifically developing for old platforms, just trying to extend our reach as much as possible :D

There is undeniably more "grief" in supporting the old platforms as a programmer, but 1) If we support win7+8 at all, we'll have that grief to deal with regardless of what the default is because we'd still have to validate both build and/or runtime paths, and 2) most of the codebase is already loading newer APIs via dynamic calls, so this would really just be formalizing that standard.

If we did decide this was the way to go, the COMPAT_DPI ifdef and all the old call sites should just be deleted, I think

@philippjbauer
Copy link
Member

@xaevman We're struggling to see how the proposal alleviates the need to compile two versions. Can you explain a little bit further and deliver an example / finalize the current code you have provided?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Windows Only on Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants