lv2 / pugl

A minimal portable API for embeddable GUIs
https://gitlab.com/lv2/pugl/
ISC License
174 stars 34 forks source link

puglGetScaleFactor on win32 returns 0 if window is not yet realized #80

Closed falkTX closed 1 year ago

falkTX commented 2 years ago

Found a little regression from my previous code when updating to latest pugl. Typically we query the scale factor before realizing the view, otherwise the view is created with the wrong size and needs to be resized right after. On Windows, the scaleFactor is cached in pugl internals and only set during realize.

An option for this is to simply fetch the scale factor using the parent window, if one is has been set. IMO puglGetScaleFactor should always return a valid value. If there is no view/window yet, simply return the value from the default screen, which is already the case on macOS.

The following patch fixes the issue for me:

diff --git a/src/win.c b/src/win.c
index 2cf2781..8a4d092 100644
--- a/src/win.c
+++ b/src/win.c
@@ -115,7 +115,7 @@ puglWinGetWindowExFlags(const PuglView* const view)
 }

 static double
-puglWinGetViewScaleFactor(const PuglView* const view)
+puglWinGetViewScaleFactor(HWND hwnd)
 {
   const HMODULE shcore = LoadLibrary("Shcore.dll");
   if (!shcore) {
@@ -135,7 +135,7 @@ puglWinGetViewScaleFactor(const PuglView* const view)
   if (GetProcessDpiAwareness && GetScaleFactorForMonitor &&
       !GetProcessDpiAwareness(NULL, &dpiAware) && dpiAware) {
     GetScaleFactorForMonitor(
-      MonitorFromWindow(view->impl->hwnd, MONITOR_DEFAULTTOPRIMARY),
+      MonitorFromWindow(hwnd, MONITOR_DEFAULTTOPRIMARY),
       &scaleFactor);
   }

@@ -245,7 +245,7 @@ puglRealize(PuglView* view)
     puglSetWindowTitle(view, view->title);
   }

-  view->impl->scaleFactor = puglWinGetViewScaleFactor(view);
+  view->impl->scaleFactor = puglWinGetViewScaleFactor(view->impl->hwnd);
   view->impl->cursor      = LoadCursor(NULL, IDC_ARROW);

   puglSetFrame(view, view->frame);
@@ -637,7 +637,7 @@ handleMessage(PuglView* view, UINT message, WPARAM wParam, LPARAM lParam)
     }
     break;
   case WM_DISPLAYCHANGE:
-    view->impl->scaleFactor = puglWinGetViewScaleFactor(view);
+    view->impl->scaleFactor = puglWinGetViewScaleFactor(view->impl->hwnd);
     break;
   case WM_WINDOWPOSCHANGED:
     handleConfigure(view, &event);
@@ -1040,7 +1040,14 @@ adjustedWindowRect(PuglView* const view,
 double
 puglGetScaleFactor(const PuglView* const view)
 {
-  return view->impl->scaleFactor;
+  if (view->impl->scaleFactor) {
+    return view->impl->scaleFactor;
+  }
+  return puglWinGetViewScaleFactor(view->parent
+    ? (HWND)view->parent
+    : view->transientParent
+    ? (HWND)view->transientParent
+    : NULL);
 }

 PuglStatus

I can open a PR for the patch if you want.

PS: I did not test under macOS yet.

drobilla commented 2 years ago

I don't think using the transientParent here is appropriate (they aren't really related in terms of window hierarchy, and a transient child can be on another display).

Otherwise, seems reasonable enough, although this function is inherently kludgey and will always be potentially broken in certain situations if the scale factor is used before realizing. Doing this properly needs more/different API.

drobilla commented 1 year ago

Fixed in 604dbd1, thanks.

I went with something roughly the same as your solution, except it tries a little harder to find some monitor and corresponding scale factor. It will always return a "reasonable" scale factor, although it can still be wrong with multiple displays (always the case in this situation).