Make themes DPI aware #376
Conversation
Ported pull request from WiX 4 wixtoolset/wix#46 (wixtoolset/wix#46)
|
Thanks for the pull request. Although this bug was triaged for v3.x, the scope of this means that we need to get it into v4 first (this is reminding us that the Burn engine has GDI+ in v4 and we might not have fully tested that for the DLL hijacking issue, plus marking the exe as high-DPI aware might cause display issues in existing BA's). Please submit a new pull request for this in the wix4 repo. A quick glance over the code shows a couple of style nits. According to the style guide, all local variables should be declared and initialized at the top of the function and there should be only one return statement from a function. |
|
I'd rather not take GDI+ into the engine in v3. Other than splash screen, is there any hidpi functionality we lose without it? The ThmUtil change is great; I'll review that part soon(ish). |
|
The high DPI works even without that change to splash screen so this should not require GDI+. |
|
Any updates on this pull request? It would be great to have high DPI support on wix v3, given the fast increasing number of devices that make use of it. Given that apparently no GDI+ support is needed, is there anything else holding this back? |
There was a problem hiding this comment.
Either of us can remove Heath's commit to avoid GDI+ in the engine. The other changes are fairly trivial. Sorry for the delay; this one required way too much spelunking in GDI to understand the code for getting per-monitor DPI in a backward-compatible manner. It sucks that's the easiest way and all the good functions were added in Windows 8.1 and 10. :)
| GetDpiForMonitor - returns the X and Y DPI values for specified monitor. | ||
|
|
||
| ********************************************************************/ | ||
| BOOL DAPI GetDpiForMonitor( |
There was a problem hiding this comment.
This doesn't really belong in GdipUtil. How about a new DpiUtil? Make this DpiGetForWindow?
| GetScaleFactorForDpi - returns the scale factor for given DPI resolution. | ||
|
|
||
| ********************************************************************/ | ||
| FLOAT DAPI GetScaleFactorForDpi( |
| @@ -550,6 +554,11 @@ DAPI_(void) ThemeSetFocus( | |||
| __in DWORD dwControl | |||
| ); | |||
|
|
|||
| DAPI_(int) ScaleByFactor( | |||
There was a problem hiding this comment.
Does this need to be public? If so, needs a Thm prefix.
|
Oh, and can you add a |
|
@barnson ,What version of wixtoolset should one use to consume this fix ? Thanks. |
|
It hasn't been merged. |
|
Is there a timeline planned to merge this fix? |
|
pinging on this thread. When will we have a wixtoolset version with this fix in it? |
|
The engine changes are already present in WiX v4. The ThmUtil changes can be merged in WiX v4 when the open issues are addressed. |
|
cc @merriemcgaw |
|
@robmen And I see that someone has indeed stepped up: When can we see Hi-DPI support in WiX 3 or WiX 4? Can we get a general ETA so we can plan accordingly. |
|
As I mentioned before, there are still open issues so it hasn't been merged. This PR is tagged |
|
hey @barnson thanks for getting back to us. Where can I find information regarding when WiX v4.0 is shipping? Also, if there is something we can do to help address some of those issues, would love to help. |
|
@livarcocc, WiX v4 is still in active development, so we don't have useful dates yet beyond "this year." If you wanted to port the change in this PR to https://github.com/wixtoolset/dutil and address the issues I raised above, they'd be available sooner than "when I have the spare time." |
I've updated theme loader to scale UI elements at load time to the DPI level of primary monitor.
I've ported the splashcreen code from wixtoolset/wix#46
The burn.exe is marked as DPI aware. Loading the DPI of primary monitor should be sufficient for general use. I could not make the rescaling based on current monitor. I tested the built-in themes and they worked correctly.
I also updated thmviewer.exe to support high DPI monitors.
There may be some quirks in the implementation. This fixes the wixtoolset/issues#3920