diff mbox

[FFmpeg-devel] gdigrab: fix HIDPI support

Message ID 13d761d6-5e68-c01a-ba09-fb565719652f@gmail.com
State Superseded
Headers show

Commit Message

Dilshod Mukhtarov Jan. 26, 2019, 5:53 p.m. UTC 
HI, this is the patch that fixes HIDPI support in gdigrab:

1) Mouse position was not calculated properly in area or window mode
2) In window mode the size of window was not calculated properly (cropped)

Comments

Carl Eugen Hoyos Jan. 27, 2019, 12:45 a.m. UTC | #1 
2019-01-26 18:53 GMT+01:00, Dilshod Mukhtarov <dilshodm@gmail.com>:
> HI, this is the patch that fixes HIDPI support in gdigrab

> +    double h_dpr;   // Horizontal device pixel ratio
> +    double v_dpr;   // Vertical device pixel ratio

I would expect these to be AVRational, if this is not
possible, it should be explained why.

Please put "else" on the same line as "}", no linebreak
between "}" and "else".

> 1) Mouse position was not calculated properly in area or window mode
> 2) In window mode the size of window was not calculated properly (cropped)

This may not apply here, but typically, if a patch says "fixes A and B",
it should be split in two patches to ease review and future debugging.

Carl Eugen
Carl Eugen Hoyos Jan. 27, 2019, 6:51 p.m. UTC | #2 
2019-01-27 1:45 GMT+01:00, Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> 2019-01-26 18:53 GMT+01:00, Dilshod Mukhtarov <dilshodm@gmail.com>:
>> HI, this is the patch that fixes HIDPI support in gdigrab
>
>> +    double h_dpr;   // Horizontal device pixel ratio
>> +    double v_dpr;   // Vertical device pixel ratio
>
> I would expect these to be AVRational, if this is not
> possible, it should be explained why.

Sorry if this was not an ideal comment, it should have been:
All aspect ratio calculations (and all calculations related to
aspect ratio) do not need double (or floats) but should be
done as integer calculations. AVRational structs are often
used, theoretically they may not be needed.

Carl Eugen
diff mbox

Patch

>From 98043face18a5fae93ed01c14a15140a60dd7ad4 Mon Sep 17 00:00:00 2001
From: Dilshod Mukhtarov <dilshodm@gmail.com>
Date: Fri, 25 Jan 2019 11:52:02 +0400
Subject: [PATCH] libavdevice/gdigrab: Fix HIDPI support

In Windows HIDPI mode grigrab mouse was not properly positioned when
using area recording. When using windowed recording the window size was
not correct (recording window cropped)

Signed-off-by: Dilshod Mukhtarov <dilshodm@gmail.com>
---
 libavdevice/gdigrab.c | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/libavdevice/gdigrab.c b/libavdevice/gdigrab.c
index ab08c11788..08e7b927de 100644
--- a/libavdevice/gdigrab.c
+++ b/libavdevice/gdigrab.c
@@ -243,6 +243,8 @@  gdigrab_read_header(AVFormatContext *s1)
     RECT clip_rect;
     BITMAP bmp;
     int ret;
+    double h_dpr;   // Horizontal device pixel ratio
+    double v_dpr;   // Vertical device pixel ratio
 
     if (!strncmp(filename, "title=", 6)) {
         name = filename + 6;
@@ -277,18 +279,25 @@  gdigrab_read_header(AVFormatContext *s1)
     }
     bpp = GetDeviceCaps(source_hdc, BITSPIXEL);
 
+    horzres = GetDeviceCaps(source_hdc, HORZRES);
+    vertres = GetDeviceCaps(source_hdc, VERTRES);
+    desktophorzres = GetDeviceCaps(source_hdc, DESKTOPHORZRES);
+    desktopvertres = GetDeviceCaps(source_hdc, DESKTOPVERTRES);
+    h_dpr = (double) desktophorzres / horzres;
+    v_dpr = (double) desktopvertres / vertres;
+
     if (hwnd) {
         GetClientRect(hwnd, &virtual_rect);
+        virtual_rect.left   *= h_dpr;
+        virtual_rect.right  *= h_dpr;
+        virtual_rect.top    *= v_dpr;
+        virtual_rect.bottom *= v_dpr;
     } else {
         /* desktop -- get the right height and width for scaling DPI */
-        horzres = GetDeviceCaps(source_hdc, HORZRES);
-        vertres = GetDeviceCaps(source_hdc, VERTRES);
-        desktophorzres = GetDeviceCaps(source_hdc, DESKTOPHORZRES);
-        desktopvertres = GetDeviceCaps(source_hdc, DESKTOPVERTRES);
         virtual_rect.left = GetSystemMetrics(SM_XVIRTUALSCREEN);
         virtual_rect.top = GetSystemMetrics(SM_YVIRTUALSCREEN);
-        virtual_rect.right = (virtual_rect.left + GetSystemMetrics(SM_CXVIRTUALSCREEN)) * desktophorzres / horzres;
-        virtual_rect.bottom = (virtual_rect.top + GetSystemMetrics(SM_CYVIRTUALSCREEN)) * desktopvertres / vertres;
+        virtual_rect.right = (virtual_rect.left + GetSystemMetrics(SM_CXVIRTUALSCREEN)) * h_dpr;
+        virtual_rect.bottom = (virtual_rect.top + GetSystemMetrics(SM_CYVIRTUALSCREEN)) * h_dpr;
     }
 
     /* If no width or height set, use full screen/window area */
@@ -455,6 +464,8 @@  static void paint_mouse_pointer(AVFormatContext *s1, struct gdigrab *gdigrab)
         int vertres = GetDeviceCaps(gdigrab->source_hdc, VERTRES);
         int desktophorzres = GetDeviceCaps(gdigrab->source_hdc, DESKTOPHORZRES);
         int desktopvertres = GetDeviceCaps(gdigrab->source_hdc, DESKTOPVERTRES);
+        double h_dpr = (double) desktophorzres / horzres;   // Horizontal device pixel ratio
+        double v_dpr = (double) desktopvertres / vertres;   // Vertical device pixel ratio
         info.hbmMask = NULL;
         info.hbmColor = NULL;
 
@@ -473,24 +484,26 @@  static void paint_mouse_pointer(AVFormatContext *s1, struct gdigrab *gdigrab)
             goto icon_error;
         }
 
-        pos.x = ci.ptScreenPos.x - clip_rect.left - info.xHotspot;
-        pos.y = ci.ptScreenPos.y - clip_rect.top - info.yHotspot;
-
         if (hwnd) {
             RECT rect;
 
             if (GetWindowRect(hwnd, &rect)) {
-                pos.x -= rect.left;
-                pos.y -= rect.top;
+                pos.x = ci.ptScreenPos.x - clip_rect.left - info.xHotspot - rect.left;
+                pos.y = ci.ptScreenPos.y - clip_rect.top - info.yHotspot - rect.top;
+
+                //that would keep the correct location of mouse with hidpi screens
+                pos.x *= h_dpr;
+                pos.y *= v_dpr;
             } else {
                 CURSOR_ERROR("Couldn't get window rectangle");
                 goto icon_error;
             }
         }
-
-        //that would keep the correct location of mouse with hidpi screens
-        pos.x = pos.x * desktophorzres / horzres;
-        pos.y = pos.y * desktopvertres / vertres;
+        else {
+            //that would keep the correct location of mouse with hidpi screens
+            pos.x = ci.ptScreenPos.x * h_dpr - clip_rect.left - info.xHotspot;
+            pos.y = ci.ptScreenPos.y * v_dpr - clip_rect.top - info.yHotspot;
+        }
 
         av_log(s1, AV_LOG_DEBUG, "Cursor pos (%li,%li) -> (%li,%li)\n",
                 ci.ptScreenPos.x, ci.ptScreenPos.y, pos.x, pos.y);
-- 
2.17.1