It may be counterintuitive, but every version of Windows returns this.
Signed-off-by: Zebediah Figura z.figura12@gmail.com --- dlls/gdi32/tests/dc.c | 52 ++++++++++++------------------------- dlls/gdi32/tests/metafile.c | 48 ++++++++++++---------------------- 2 files changed, 32 insertions(+), 68 deletions(-)
diff --git a/dlls/gdi32/tests/dc.c b/dlls/gdi32/tests/dc.c index 9739b8f61d..e9fdb12f68 100644 --- a/dlls/gdi32/tests/dc.c +++ b/dlls/gdi32/tests/dc.c @@ -40,27 +40,6 @@
static DWORD (WINAPI *pSetLayout)(HDC hdc, DWORD layout);
-static void dump_region(HRGN hrgn) -{ - DWORD i, size; - RGNDATA *data = NULL; - RECT *rect; - - if (!hrgn) - { - printf( "(null) region\n" ); - return; - } - if (!(size = GetRegionData( hrgn, 0, NULL ))) return; - if (!(data = HeapAlloc( GetProcessHeap(), 0, size ))) return; - GetRegionData( hrgn, size, data ); - printf( "%d rects:", data->rdh.nCount ); - for (i = 0, rect = (RECT *)data->Buffer; i < data->rdh.nCount; i++, rect++) - printf( " (%d,%d)-(%d,%d)", rect->left, rect->top, rect->right, rect->bottom ); - printf( "\n" ); - HeapFree( GetProcessHeap(), 0, data ); -} - static void test_dc_values(void) { HDC hdc = CreateDCA("DISPLAY", NULL, NULL, NULL); @@ -111,6 +90,8 @@ static void test_dc_values(void)
static void test_savedc_2(void) { + char buffer[100]; + RGNDATA *rgndata = (RGNDATA *)buffer; HWND hwnd; HDC hdc; HRGN hrgn; @@ -130,13 +111,15 @@ static void test_savedc_2(void) ok(hdc != NULL, "GetDC failed\n");
ret = GetClipBox(hdc, &rc_clip); - ok(ret == SIMPLEREGION || broken(ret == COMPLEXREGION), "GetClipBox returned %d instead of SIMPLEREGION\n", ret); + ok(ret == SIMPLEREGION, "wrong region type %d\n", ret); ret = GetClipRgn(hdc, hrgn); ok(ret == 0, "GetClipRgn returned %d instead of 0\n", ret); ret = GetRgnBox(hrgn, &rc); ok(ret == NULLREGION, "GetRgnBox returned %d %s instead of NULLREGION\n", ret, wine_dbgstr_rect(&rc)); - /*dump_region(hrgn);*/ + ret = GetRegionData(hrgn, sizeof(buffer), rgndata); + ok(ret == sizeof(RGNDATAHEADER), "got %u\n", ret); + ok(!rgndata->rdh.nCount, "got %u rectangles\n", rgndata->rdh.nCount); SetRect(&rc, 0, 0, 100, 100); ok(EqualRect(&rc, &rc_clip), "rects are not equal: %s - %s\n", wine_dbgstr_rect(&rc), wine_dbgstr_rect(&rc_clip)); @@ -145,20 +128,17 @@ static void test_savedc_2(void) ok(ret == 1, "ret = %d\n", ret);
ret = IntersectClipRect(hdc, 0, 0, 50, 50); - if (ret == COMPLEXREGION) - { - /* XP returns COMPLEXREGION although dump_region reports only 1 rect */ - trace("Windows BUG: IntersectClipRect returned %d instead of SIMPLEREGION\n", ret); - /* let's make sure that it's a simple region */ - ret = GetClipRgn(hdc, hrgn); - ok(ret == 1, "GetClipRgn returned %d instead of 1\n", ret); - dump_region(hrgn); - } - else - ok(ret == SIMPLEREGION, "IntersectClipRect returned %d instead of SIMPLEREGION\n", ret); + todo_wine ok(ret == COMPLEXREGION, "wrong region type %d\n", ret); + ret = GetClipRgn(hdc, hrgn); + ok(ret == 1, "GetClipRgn returned %d instead of 1\n", ret); + ret = GetRegionData(hrgn, sizeof(buffer), rgndata); + ok(ret == sizeof(RGNDATAHEADER) + sizeof(RECT), "got %u\n", ret); + ok(rgndata->rdh.nCount == 1, "got %u rectangles\n", rgndata->rdh.nCount); + SetRect(&rc, 0, 0, 50, 50); + ok(EqualRect((RECT *)rgndata->Buffer, &rc), "got rect %s\n", wine_dbgstr_rect((RECT *)rgndata->Buffer));
ret = GetClipBox(hdc, &rc_clip); - ok(ret == SIMPLEREGION || broken(ret == COMPLEXREGION), "GetClipBox returned %d instead of SIMPLEREGION\n", ret); + ok(ret == SIMPLEREGION, "wrong region type %d\n", ret); SetRect(&rc, 0, 0, 50, 50); ok(EqualRect(&rc, &rc_clip), "rects are not equal: %s - %s\n", wine_dbgstr_rect(&rc), wine_dbgstr_rect(&rc_clip)); @@ -167,7 +147,7 @@ static void test_savedc_2(void) ok(ret, "ret = %d\n", ret);
ret = GetClipBox(hdc, &rc_clip); - ok(ret == SIMPLEREGION || broken(ret == COMPLEXREGION), "GetClipBox returned %d instead of SIMPLEREGION\n", ret); + ok(ret == SIMPLEREGION, "wrong region type %d\n", ret); SetRect(&rc, 0, 0, 100, 100); ok(EqualRect(&rc, &rc_clip), "rects are not equal: %s - %s\n", wine_dbgstr_rect(&rc), wine_dbgstr_rect(&rc_clip)); diff --git a/dlls/gdi32/tests/metafile.c b/dlls/gdi32/tests/metafile.c index 231d6a4d35..d1c18cdb29 100644 --- a/dlls/gdi32/tests/metafile.c +++ b/dlls/gdi32/tests/metafile.c @@ -60,20 +60,6 @@ static void init_function_pointers(void) GDI_GET_PROC(SetDCPenColor); }
-static DWORD rgn_rect_count(HRGN hrgn) -{ - DWORD size; - RGNDATA *data; - - if (!hrgn) return 0; - if (!(size = GetRegionData(hrgn, 0, NULL))) return 0; - if (!(data = HeapAlloc(GetProcessHeap(), 0, size))) return 0; - GetRegionData(hrgn, size, data); - size = data->rdh.nCount; - HeapFree(GetProcessHeap(), 0, data); - return size; -} - static int CALLBACK eto_emf_enum_proc(HDC hdc, HANDLETABLE *handle_table, const ENHMETARECORD *emr, int n_objs, LPARAM param) { @@ -2716,6 +2702,8 @@ static int CALLBACK clip_emf_enum_proc(HDC hdc, HANDLETABLE *handle_table,
static void test_emf_clipping(void) { + char buffer[100]; + RGNDATA *rgndata = (RGNDATA *)buffer; static const RECT rc = { 0, 0, 100, 100 }; RECT rc_clip = { 100, 100, 1024, 1024 }; HWND hwnd; @@ -2790,15 +2778,13 @@ static void test_emf_clipping(void) wine_dbgstr_rect(&rc_res));
ret = IntersectClipRect(hdc, 0, 0, 100, 100); - ok(ret == SIMPLEREGION || broken(ret == COMPLEXREGION) /* XP */, "got %d\n", ret); - if (ret == COMPLEXREGION) - { - /* XP returns COMPLEXREGION although region contains only 1 rect */ - ret = GetClipRgn(hdc, hrgn); - ok(ret == 1, "expected 1, got %d\n", ret); - ret = rgn_rect_count(hrgn); - ok(ret == 1, "expected 1, got %d\n", ret); - } + todo_wine ok(ret == COMPLEXREGION, "wrong region type %d\n", ret); + ret = GetClipRgn(hdc, hrgn); + ok(ret == 1, "expected 1, got %d\n", ret); + ret = GetRegionData(hrgn, sizeof(buffer), rgndata); + ok(ret == sizeof(RGNDATAHEADER) + sizeof(RECT), "got %u\n", ret); + ok(rgndata->rdh.nCount == 1, "got %u rectangles\n", rgndata->rdh.nCount); + ok(EqualRect((RECT *)rgndata->Buffer, &rc), "got rect %s\n", wine_dbgstr_rect((RECT *)rgndata->Buffer)); SetRect(&rc_res, -1, -1, -1, -1); ret = GetClipBox(hdc, &rc_res); ok(ret == SIMPLEREGION, "got %d\n", ret); @@ -2807,15 +2793,13 @@ static void test_emf_clipping(void)
SetRect(&rc_sclip, 0, 0, 100, 50); ret = ExcludeClipRect(hdc, 0, 50, 100, 100); - ok(ret == SIMPLEREGION || broken(ret == COMPLEXREGION) /* XP */, "got %d\n", ret); - if (ret == COMPLEXREGION) - { - /* XP returns COMPLEXREGION although region contains only 1 rect */ - ret = GetClipRgn(hdc, hrgn); - ok(ret == 1, "expected 1, got %d\n", ret); - ret = rgn_rect_count(hrgn); - ok(ret == 1, "expected 1, got %d\n", ret); - } + todo_wine ok(ret == COMPLEXREGION, "wrong region type %d\n", ret); + ret = GetClipRgn(hdc, hrgn); + ok(ret == 1, "expected 1, got %d\n", ret); + ret = GetRegionData(hrgn, sizeof(buffer), rgndata); + ok(ret == sizeof(RGNDATAHEADER) + sizeof(RECT), "got %u\n", ret); + ok(rgndata->rdh.nCount == 1, "got %u rectangles\n", rgndata->rdh.nCount); + ok(EqualRect((RECT *)rgndata->Buffer, &rc_sclip), "got rect %s\n", wine_dbgstr_rect((RECT *)rgndata->Buffer)); SetRect(&rc_res, -1, -1, -1, -1); ret = GetClipBox(hdc, &rc_res); ok(ret == SIMPLEREGION, "got %d\n", ret);
Addresses test failures with multi-monitor setups.
Signed-off-by: Zebediah Figura z.figura12@gmail.com --- dlls/gdi32/tests/dc.c | 102 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 99 insertions(+), 3 deletions(-)
diff --git a/dlls/gdi32/tests/dc.c b/dlls/gdi32/tests/dc.c index e9fdb12f68..f70f73baba 100644 --- a/dlls/gdi32/tests/dc.c +++ b/dlls/gdi32/tests/dc.c @@ -409,9 +409,6 @@ static void test_device_caps( HDC hdc, HDC ref_dc, const char *descr, int scale } else ok( ret || broken(!ret) /* NT4 */, "GetDeviceGammaRamp failed on %s (type %d), error %u\n", descr, GetObjectType( hdc ), GetLastError() ); - type = GetClipBox( hdc, &rect ); - todo_wine_if (GetObjectType( hdc ) == OBJ_ENHMETADC) - ok( type == SIMPLEREGION, "GetClipBox returned %d on memdc for %s\n", type, descr );
type = GetBoundsRect( hdc, &rect, 0 ); ok( type == DCB_RESET || broken(type == DCB_SET) /* XP */, @@ -1549,6 +1546,104 @@ static void test_pscript_printer_dc(void) DeleteDC(hdc); }
+static void test_clip_box(void) +{ + HBITMAP bitmap = CreateBitmap(10, 10, 1, 1, NULL); + HDC dc, desktop_dc, printer_dc; + RECT rect, expect, screen_rect; + int type, screen_type; + DEVMODEA scale_mode; + + EnumDisplaySettingsA(NULL, ENUM_CURRENT_SETTINGS, &scale_mode); + scale_mode.dmFields |= DM_SCALE; + scale_mode.u1.s1.dmScale = 200; + + SetRect(&screen_rect, GetSystemMetrics(SM_XVIRTUALSCREEN), GetSystemMetrics(SM_YVIRTUALSCREEN), + GetSystemMetrics(SM_XVIRTUALSCREEN) + GetSystemMetrics(SM_CXVIRTUALSCREEN), + GetSystemMetrics(SM_YVIRTUALSCREEN) + GetSystemMetrics(SM_CYVIRTUALSCREEN)); + screen_type = GetSystemMetrics(SM_CMONITORS) > 1 ? COMPLEXREGION : SIMPLEREGION; + + dc = CreateDCA("DISPLAY", NULL, NULL, NULL); + type = GetClipBox(dc, &rect); + todo_wine_if(screen_type == COMPLEXREGION) + ok(type == screen_type, "wrong region type %d\n", type); + ok(EqualRect(&rect, &screen_rect), "expected %s, got %s\n", + wine_dbgstr_rect(&screen_rect), wine_dbgstr_rect(&rect)); + DeleteDC(dc); + + dc = CreateDCA("DISPLAY", NULL, NULL, &scale_mode); + type = GetClipBox(dc, &rect); + todo_wine_if(screen_type == COMPLEXREGION) + ok(type == screen_type, "wrong region type %d\n", type); + ok(EqualRect(&rect, &screen_rect), "expected %s, got %s\n", + wine_dbgstr_rect(&screen_rect), wine_dbgstr_rect(&rect)); + ResetDCA(dc, &scale_mode); + type = GetClipBox(dc, &rect); + todo_wine_if(screen_type == COMPLEXREGION) + ok(type == screen_type, "wrong region type %d\n", type); + ok(EqualRect(&rect, &screen_rect), "expected %s, got %s\n", + wine_dbgstr_rect(&screen_rect), wine_dbgstr_rect(&rect)); + DeleteDC(dc); + + dc = CreateCompatibleDC(NULL); + type = GetClipBox(dc, &rect); + ok(type == SIMPLEREGION, "wrong region type %d\n", type); + SetRect(&expect, 0, 0, 1, 1); + ok(EqualRect(&rect, &expect), "got %s\n", wine_dbgstr_rect(&rect)); + SelectObject(dc, bitmap); + type = GetClipBox(dc, &rect); + ok(type == SIMPLEREGION, "wrong region type %d\n", type); + SetRect(&expect, 0, 0, 10, 10); + ok(EqualRect(&rect, &expect), "got %s\n", wine_dbgstr_rect(&rect)); + DeleteDC(dc); + + desktop_dc = GetDC(0); + type = GetClipBox(desktop_dc, &rect); + todo_wine_if(screen_type == COMPLEXREGION) + ok(type == screen_type, "wrong region type %d\n", type); + ok(EqualRect(&rect, &screen_rect), "expected %s, got %s\n", + wine_dbgstr_rect(&screen_rect), wine_dbgstr_rect(&rect)); + + dc = CreateEnhMetaFileA(desktop_dc, NULL, NULL, NULL); + ok(!!dc, "CreateEnhMetaFile() failed\n"); + type = GetClipBox(dc, &rect); + todo_wine ok(type == SIMPLEREGION, "wrong region type %d\n", type); + if (type != ERROR) + ok(EqualRect(&rect, &screen_rect), "expected %s, got %s\n", + wine_dbgstr_rect(&screen_rect), wine_dbgstr_rect(&rect)); + DeleteEnhMetaFile(CloseEnhMetaFile(dc)); + + ReleaseDC(0, desktop_dc); + + dc = CreateMetaFileA(NULL); + ok(!!dc, "CreateEnhMetaFile() failed\n"); + type = GetClipBox(dc, &rect); + ok(type == ERROR, "wrong region type %d\n", type); + DeleteMetaFile(CloseMetaFile(dc)); + + if ((printer_dc = create_printer_dc(100, FALSE))) + { + type = GetClipBox(printer_dc, &rect); + ok(type == SIMPLEREGION, "wrong region type %d\n", type); + + dc = CreateCompatibleDC(printer_dc); + type = GetClipBox(dc, &rect); + ok(type == SIMPLEREGION, "wrong region type %d\n", type); + SetRect(&expect, 0, 0, 1, 1); + ok(EqualRect(&rect, &expect), "got %s\n", wine_dbgstr_rect(&rect)); + DeleteDC(dc); + + dc = CreateEnhMetaFileA(printer_dc, NULL, NULL, NULL); + type = GetClipBox(dc, &rect); + ok(type == SIMPLEREGION, "wrong region type %d\n", type); + DeleteEnhMetaFile(CloseEnhMetaFile(dc)); + + DeleteDC(printer_dc); + } + + DeleteObject(bitmap); +} + START_TEST(dc) { pSetLayout = (void *)GetProcAddress( GetModuleHandleA("gdi32.dll"), "SetLayout"); @@ -1565,4 +1660,5 @@ START_TEST(dc) test_gamma(); test_printer_dc(); test_pscript_printer_dc(); + test_clip_box(); }
Zebediah Figura z.figura12@gmail.com wrote:
ret = IntersectClipRect(hdc, 0, 0, 50, 50);
- if (ret == COMPLEXREGION)
- {
/* XP returns COMPLEXREGION although dump_region reports only 1 rect */
trace("Windows BUG: IntersectClipRect returned %d instead of SIMPLEREGION\n", ret);
/* let's make sure that it's a simple region */
ret = GetClipRgn(hdc, hrgn);
ok(ret == 1, "GetClipRgn returned %d instead of 1\n", ret);
dump_region(hrgn);
- }
- else
ok(ret == SIMPLEREGION, "IntersectClipRect returned %d instead of SIMPLEREGION\n", ret);
- todo_wine ok(ret == COMPLEXREGION, "wrong region type %d\n", ret);
- ret = GetClipRgn(hdc, hrgn);
- ok(ret == 1, "GetClipRgn returned %d instead of 1\n", ret);
- ret = GetRegionData(hrgn, sizeof(buffer), rgndata);
- ok(ret == sizeof(RGNDATAHEADER) + sizeof(RECT), "got %u\n", ret);
- ok(rgndata->rdh.nCount == 1, "got %u rectangles\n", rgndata->rdh.nCount);
Even if every Windows version returns broken result doesn't make it legitimate to return COMPLEXREGION when the region contains only 1 rectangle. This is clearly broken.
On 12/30/19 8:12 PM, Dmitry Timoshkov wrote:
Zebediah Figura z.figura12@gmail.com wrote:
ret = IntersectClipRect(hdc, 0, 0, 50, 50);
- if (ret == COMPLEXREGION)
- {
/* XP returns COMPLEXREGION although dump_region reports only 1 rect */
trace("Windows BUG: IntersectClipRect returned %d instead of SIMPLEREGION\n", ret);
/* let's make sure that it's a simple region */
ret = GetClipRgn(hdc, hrgn);
ok(ret == 1, "GetClipRgn returned %d instead of 1\n", ret);
dump_region(hrgn);
- }
- else
ok(ret == SIMPLEREGION, "IntersectClipRect returned %d instead of SIMPLEREGION\n", ret);
- todo_wine ok(ret == COMPLEXREGION, "wrong region type %d\n", ret);
- ret = GetClipRgn(hdc, hrgn);
- ok(ret == 1, "GetClipRgn returned %d instead of 1\n", ret);
- ret = GetRegionData(hrgn, sizeof(buffer), rgndata);
- ok(ret == sizeof(RGNDATAHEADER) + sizeof(RECT), "got %u\n", ret);
- ok(rgndata->rdh.nCount == 1, "got %u rectangles\n", rgndata->rdh.nCount);
Even if every Windows version returns broken result doesn't make it legitimate to return COMPLEXREGION when the region contains only 1 rectangle. This is clearly broken.
Windows does many things that are nonsensical, buggy, or contradicting their own documentation or other parts of the code. This seems nothing new. If sufficiently motivated one could even argue that a simple region is a special case of a complex region (I wouldn't actually be that surprised if such reasoning led a Windows programmer to just always return COMPLEXREGION because it was easier for them). Are we abandoning the idea of bug-for-bug compatibility now?
I'm not arguing that we have to return COMPLEXREGION to satisfy the test (I certainly have no intention of writing such a patch), but our tests document the behaviour of Windows functions, not their documentation; that's why they exist.
Zebediah Figura z.figura12@gmail.com wrote:
Even if every Windows version returns broken result doesn't make it legitimate to return COMPLEXREGION when the region contains only 1 rectangle. This is clearly broken.
Windows does many things that are nonsensical, buggy, or contradicting their own documentation or other parts of the code. This seems nothing new. If sufficiently motivated one could even argue that a simple region is a special case of a complex region (I wouldn't actually be that surprised if such reasoning led a Windows programmer to just always return COMPLEXREGION because it was easier for them).
That's a pretty flawed argumentation, according to the tests the APIs that return region type are not limited to COMPLEXREGION, they also return SIMPLEREGION when appropriate. In any case adding more convincing tests is always welcome if you really think that IntersectClipRect() should always return COMPLEXREGION regardless of rectangle count in the resulting region and type of the DC.
Are we abandoning the idea of bug-for-bug compatibility now?
Bug for bug compatibility makes sence only if there's an application that depends on particular behaviour, as far as I know that's not the case here.
I'm not arguing that we have to return COMPLEXREGION to satisfy the test (I certainly have no intention of writing such a patch), but our tests document the behaviour of Windows functions, not their documentation; that's why they exist.
Current tests perfectly follow this documentating purpose rule, if they don't adding more comments or tests is the right thing to do instead.
On 12/31/19 10:53 PM, Dmitry Timoshkov wrote:
Zebediah Figura z.figura12@gmail.com wrote:
Even if every Windows version returns broken result doesn't make it legitimate to return COMPLEXREGION when the region contains only 1 rectangle. This is clearly broken.
Windows does many things that are nonsensical, buggy, or contradicting their own documentation or other parts of the code. This seems nothing new. If sufficiently motivated one could even argue that a simple region is a special case of a complex region (I wouldn't actually be that surprised if such reasoning led a Windows programmer to just always return COMPLEXREGION because it was easier for them).
That's a pretty flawed argumentation, according to the tests the APIs that return region type are not limited to COMPLEXREGION, they also return SIMPLEREGION when appropriate. In any case adding more convincing tests is always welcome if you really think that IntersectClipRect() should always return COMPLEXREGION regardless of rectangle count in the resulting region and type of the DC.
Are we abandoning the idea of bug-for-bug compatibility now?
Bug for bug compatibility makes sence only if there's an application that depends on particular behaviour, as far as I know that's not the case here.
I'm not arguing that we have to return COMPLEXREGION to satisfy the test (I certainly have no intention of writing such a patch), but our tests document the behaviour of Windows functions, not their documentation; that's why they exist.
Current tests perfectly follow this documentating purpose rule, if they don't adding more comments or tests is the right thing to do instead.
I still disagree, but I have no intention of furthering this argument, it's clearly counterproductive and gets nowhere. I'll leave it up to Huw and/or Alexandre to decide.
On Wed, Jan 01, 2020 at 11:34:37AM -0600, Zebediah Figura wrote:
On 12/31/19 10:53 PM, Dmitry Timoshkov wrote:
Zebediah Figura z.figura12@gmail.com wrote:
I'm not arguing that we have to return COMPLEXREGION to satisfy the test (I certainly have no intention of writing such a patch), but our tests document the behaviour of Windows functions, not their documentation; that's why they exist.
Current tests perfectly follow this documentating purpose rule, if they don't adding more comments or tests is the right thing to do instead.
I still disagree, but I have no intention of furthering this argument, it's clearly counterproductive and gets nowhere. I'll leave it up to Huw and/or Alexandre to decide.
It's clearly a Windows bug, so I'd mark it as broken(). If we need to minic Windows' behaviour at some point we can review it again then.
There is benefit to always testing the region's rectangle count and ensuring IntersectClipRect()'s retval isn't anything other than SIMPLEREGION or COMPLEXREGION, so the other changes in the patch would be good.
Huw.
On 1/2/20 5:50 AM, Huw Davies wrote:
On Wed, Jan 01, 2020 at 11:34:37AM -0600, Zebediah Figura wrote:
On 12/31/19 10:53 PM, Dmitry Timoshkov wrote:
Zebediah Figura z.figura12@gmail.com wrote:
I'm not arguing that we have to return COMPLEXREGION to satisfy the test (I certainly have no intention of writing such a patch), but our tests document the behaviour of Windows functions, not their documentation; that's why they exist.
Current tests perfectly follow this documentating purpose rule, if they don't adding more comments or tests is the right thing to do instead.
I still disagree, but I have no intention of furthering this argument, it's clearly counterproductive and gets nowhere. I'll leave it up to Huw and/or Alexandre to decide.
It's clearly a Windows bug, so I'd mark it as broken(). If we need to minic Windows' behaviour at some point we can review it again then.
There is benefit to always testing the region's rectangle count and ensuring IntersectClipRect()'s retval isn't anything other than SIMPLEREGION or COMPLEXREGION, so the other changes in the patch would be good.
Okay, thanks. I'll resend with the return check changed.
Huw.