[PATCH v2 0/2] MR10235: gdiplus: Validate window handle in has_gdi_dc
graphics->owndc being TRUE indicates that graphics is created using GdipCreateFromHWND. In has_gdi_dc, it is necessary to check whether the window has already been destroyed. -- v2: gdiplus: Validate window handle in GdipMeasureString https://gitlab.winehq.org/wine/wine/-/merge_requests/10235
From: YeshunYe <yeyeshun@uniontech.com> Add a test case to verify the behavior of GdipMeasureString when the window associated with the graphics object is destroyed before the operation. Signed-off-by: YeshunYe <yeyeshun@uniontech.com> --- dlls/gdiplus/tests/graphics.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index 96ef52ce935..bba7338973f 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -35,6 +35,7 @@ static GpStatus (WINAPI *pGdipGraphicsSetAbort)(GpGraphics*,GdiplusAbort*); static const REAL mm_per_inch = 25.4; static const REAL point_per_inch = 72.0; static HWND hwnd; +static HWND hwnd2; static void set_rect_empty(RectF *rc) { @@ -4629,6 +4630,7 @@ static void test_measure_string(void) CharacterRange range; GpRegion *region; GpGraphics *graphics; + GpGraphics *graphics2; GpFontFamily *family; GpFont *font; GpStatus status; @@ -4649,6 +4651,7 @@ static void test_measure_string(void) hdc = CreateCompatibleDC(0); status = GdipCreateFromHDC(hdc, &graphics); + status = GdipCreateFromHWND(hwnd2, &graphics2); status = GdipCreateFont(family, 20, FontStyleRegular, UnitPixel, &font); expect(Ok, status); @@ -5088,6 +5091,21 @@ static void test_measure_string(void) expect(3, glyphs); expect(1, lines); + DestroyWindow(hwnd2); + hwnd2 = NULL; + rect.X = 5.0; + rect.Y = 5.0; + rect.Width = width_M_M; + rect.Height = 32000.0; + status = GdipMeasureString(graphics2, string2, -1, font, &rect, format_no_wrap, &bounds, &glyphs, &lines); + todo_wine expect(Ok, status); + if (status == Ok) + { + expectf_(width_M_M, bounds.Width, 0.1); + expect(3, glyphs); + expect(1, lines); + } + status = GdipDeleteFont(font); expect(Ok, status); @@ -7553,6 +7571,9 @@ START_TEST(graphics) hwnd = CreateWindowA( "gdiplus_test", "graphics test", WS_OVERLAPPEDWINDOW | WS_VISIBLE, CW_USEDEFAULT, CW_USEDEFAULT, 200, 200, 0, 0, GetModuleHandleA(0), 0 ); ok(hwnd != NULL, "Expected window to be created\n"); + hwnd2 = CreateWindowA( "gdiplus_test", "test graphics after window destroy", WS_OVERLAPPEDWINDOW | WS_VISIBLE, + CW_USEDEFAULT, CW_USEDEFAULT, 200, 200, 0, 0, GetModuleHandleA(0), 0 ); + ok(hwnd2 != NULL, "Expected window to be created\n"); gdiplusStartupInput.GdiplusVersion = 1; gdiplusStartupInput.DebugEventCallback = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10235
From: YeshunYe <yeyeshun@uniontech.com> graphics->owndc being TRUE indicates that graphics is created using `GdipCreateFromHWND`. In `GdipMeasureString`, it is necessary to check whether the window has already been destroyed. Signed-off-by: YeshunYe <yeyeshun@uniontech.com> --- dlls/gdiplus/graphics.c | 2 +- dlls/gdiplus/tests/graphics.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c index 2419eea94db..b1a1d4d76bd 100644 --- a/dlls/gdiplus/graphics.c +++ b/dlls/gdiplus/graphics.c @@ -5999,7 +5999,7 @@ GpStatus WINGDIPAPI GdipMeasureString(GpGraphics *graphics, if(!graphics || !string || !font || !rect || !bounds) return InvalidParameter; - if(!has_gdi_dc(graphics)) + if(!has_gdi_dc(graphics) || (graphics->owndc && !IsWindow(graphics->hwnd))) { hdc = temp_hdc = CreateCompatibleDC(0); if (!temp_hdc) return OutOfMemory; diff --git a/dlls/gdiplus/tests/graphics.c b/dlls/gdiplus/tests/graphics.c index bba7338973f..3195698deaf 100644 --- a/dlls/gdiplus/tests/graphics.c +++ b/dlls/gdiplus/tests/graphics.c @@ -5098,7 +5098,7 @@ static void test_measure_string(void) rect.Width = width_M_M; rect.Height = 32000.0; status = GdipMeasureString(graphics2, string2, -1, font, &rect, format_no_wrap, &bounds, &glyphs, &lines); - todo_wine expect(Ok, status); + expect(Ok, status); if (status == Ok) { expectf_(width_M_M, bounds.Width, 0.1); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10235
If we can implement this function correctly without a window DC (and I see no reason we can't), then I see no reason to use one even if the window still exists. We can probably get away with a temporary HDC, but if not, any screen DC would probably work. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10235#note_131540
On Mon Mar 9 01:01:47 2026 +0000, Esme Povirk wrote:
If we can implement this function correctly without a window DC (and I see no reason we can't), then I see no reason to use one even if the window still exists. We can probably get away with a temporary HDC, but if not, any screen DC would probably work. You're absolutely right. When calculating, `GdipMeasureString` can likely complete the computation based solely on the parameters and the data recorded inside `graphics`. So, in essence, it could indeed uniformly use a temporary HDC to handle all scenarios. Should I modify the patch to uniformly use a temporary HDC in `GdipMeasureString`?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10235#note_131562
On Tue Mar 10 03:19:34 2026 +0000, Yeshun Ye wrote:
There is a minor issue with using a temporary HDC directly. The `gdi_transform_acquire` function also contains a `has_gdi_dc` check and a call to `gdi_dc_acquire`, which will cause an error and output an ERR message in `gdi_transform_release`. I see. We only really need `gdi_transform_acquire` and `gdi_transform_release` if we're using an HDC from the Graphics object, so in theory we could skip them or pass some flag around. It feels messy, though.
I had been wondering if we could use a compatible DC for all cases, but wineps and winex11 both override some font-related functions. I was worried that hinting/anti-aliasing might change measurements slightly depending on the device. But it would mean we don't have to call either variety of acquire/release function, which would simplify things a lot. @huw Would you expect text measurements to change depending on the HDC device, assuming identical HFONT objects and transforms? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10235#note_131863
participants (3)
-
Esme Povirk (＠madewokherd) -
Yeshun Ye (＠yeyeshun)
-
YeshunYe