[PATCH 0/2] MR63: Test and fix an out-of-order RECT in RedrawWindow()
Signed-off-by: Pedro Perdigão <pperdigao(a)codeweavers.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/63
From: Pedro Perdigão <pperdigao(a)codeweavers.com> Signed-off-by: Pedro Perdigão <pperdigao(a)codeweavers.com> --- dlls/user32/tests/msg.c | 65 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 3 deletions(-) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index c0a74d8edbe..84833e8b1be 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -8174,8 +8174,9 @@ void dump_region(HRGN hrgn) HeapFree( GetProcessHeap(), 0, data ); } -#define check_update_rgn( hwnd, hrgn ) check_update_rgn_( __LINE__, hwnd, hrgn ) -static void check_update_rgn_( int line, HWND hwnd, HRGN hrgn ) +#define todo_check_update_rgn( hwnd, hrgn ) check_update_rgn_( __LINE__, TRUE, hwnd, hrgn ) +#define check_update_rgn( hwnd, hrgn ) check_update_rgn_( __LINE__, FALSE, hwnd, hrgn ) +static void check_update_rgn_( int line, BOOL todo, HWND hwnd, HRGN hrgn ) { INT ret; RECT r1, r2; @@ -8186,7 +8187,10 @@ static void check_update_rgn_( int line, HWND hwnd, HRGN hrgn ) ok( ret != ERROR, "GetUpdateRgn failed\n" ); if (ret == NULLREGION) { - ok_(__FILE__,line)( !hrgn, "Update region shouldn't be empty\n" ); + if(todo) + todo_wine ok_(__FILE__,line)( !hrgn, "Update region shouldn't be empty\n" ); + else + ok_(__FILE__,line)( !hrgn, "Update region shouldn't be empty\n" ); } else { @@ -8393,6 +8397,61 @@ static void test_paint_messages(void) ok(ret, "RedrawWindow returned %d instead of TRUE\n", ret); check_update_rgn( hwnd, 0 ); + /* test a zeroed rectangle */ + RedrawWindow( hwnd, NULL, 0, RDW_VALIDATE|RDW_NOFRAME|RDW_NOERASE ); + SetRect( &rect, 0, 0, 0, 0 ); + ret = RedrawWindow( hwnd, &rect, NULL, RDW_INVALIDATE ); + ok(ret, "RedrawWindow returned %d instead of TRUE\n", ret); + check_update_rgn( hwnd, 0 ); + + /* a well ordered rectangle */ + RedrawWindow( hwnd, NULL, 0, RDW_VALIDATE|RDW_NOFRAME|RDW_NOERASE ); + SetRect( &rect, 10, 5, 17, 21 ); + ret = RedrawWindow( hwnd, &rect, NULL, RDW_INVALIDATE ); + ok(ret, "RedrawWindow returned %d instead of TRUE\n", ret); + SetRectRgn( hrgn, 10, 5, 17, 21 ); + check_update_rgn( hwnd, hrgn ); + + /* empty rectangle, top and bottom are swapped but left and right have + the same value */ + RedrawWindow( hwnd, NULL, 0, RDW_VALIDATE|RDW_NOFRAME|RDW_NOERASE ); + SetRect( &rect, 5, 30, 5, 10 ); + ret = RedrawWindow( hwnd, &rect, NULL, RDW_INVALIDATE ); + ok(ret, "RedrawWindow returned %d instead of TRUE\n", ret); + check_update_rgn( hwnd, 0 ); + + /* empty rectangle, left and right are swapped but top and bottom have + the same value */ + RedrawWindow( hwnd, NULL, 0, RDW_VALIDATE|RDW_NOFRAME|RDW_NOERASE ); + SetRect( &rect, 17, 10, 5, 10 ); + ret = RedrawWindow( hwnd, &rect, NULL, RDW_INVALIDATE ); + ok(ret, "RedrawWindow returned %d instead of TRUE\n", ret); + check_update_rgn( hwnd, 0 ); + + /* Left and right are swapped */ + RedrawWindow( hwnd, NULL, 0, RDW_VALIDATE|RDW_NOFRAME|RDW_NOERASE ); + SetRect( &rect, 21, 12, 7, 30 ); + ret = RedrawWindow( hwnd, &rect, NULL, RDW_INVALIDATE ); + ok(ret, "RedrawWindow returned %d instead of TRUE\n", ret); + SetRectRgn( hrgn, 7, 12, 21, 30 ); + todo_check_update_rgn( hwnd, hrgn ); + + /* Top and bottom are swapped */ + RedrawWindow( hwnd, NULL, 0, RDW_VALIDATE|RDW_NOFRAME|RDW_NOERASE ); + SetRect( &rect, 7, 30, 21, 12 ); + ret = RedrawWindow( hwnd, &rect, NULL, RDW_INVALIDATE ); + ok(ret, "RedrawWindow returned %d instead of TRUE\n", ret); + SetRectRgn( hrgn, 7, 12, 21, 30 ); + todo_check_update_rgn( hwnd, hrgn ); + + /* both reference points are swapped */ + RedrawWindow( hwnd, NULL, 0, RDW_VALIDATE|RDW_NOFRAME|RDW_NOERASE ); + SetRect( &rect, 21, 30, 7, 12 ); + ret = RedrawWindow( hwnd, &rect, NULL, RDW_INVALIDATE ); + ok(ret, "RedrawWindow returned %d instead of TRUE\n", ret); + SetRectRgn( hrgn, 7, 12, 21, 30 ); + todo_check_update_rgn( hwnd, hrgn ); + /* flush pending messages */ flush_events(); flush_sequence(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/63
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=114529 Your paranoid android. === w10pro64_zh_CN (64 bit report) === user32: msg.c:12865: Test failed: message time not advanced: 151a5 151a5 msg.c:12866: Test failed: coords not changed: (101 101) (101 101) msg.c:12883: Test failed: message time not advanced: 151a5 151a5 msg.c:12884: Test failed: coords not changed: (101 101) (101 101)
On 5/12/22 07:51, Pedro Perdigão wrote:
@@ -8186,7 +8187,10 @@ static void check_update_rgn_( int line, HWND hwnd, HRGN hrgn ) ok( ret != ERROR, "GetUpdateRgn failed\n" ); if (ret == NULLREGION) { - ok_(__FILE__,line)( !hrgn, "Update region shouldn't be empty\n" ); + if(todo) + todo_wine ok_(__FILE__,line)( !hrgn, "Update region shouldn't be empty\n" ); + else + ok_(__FILE__,line)( !hrgn, "Update region shouldn't be empty\n" ); } else {
This was already committed, but for future reference, there's a "todo_wine_if()" macro for this.
From: Pedro Perdigão <pperdigao(a)codeweavers.com> Signed-off-by: Pedro Perdigão <pperdigao(a)codeweavers.com> --- dlls/user32/tests/msg.c | 16 ++++++---------- dlls/win32u/dce.c | 7 +++++-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 84833e8b1be..2705914d5e5 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -8174,9 +8174,8 @@ void dump_region(HRGN hrgn) HeapFree( GetProcessHeap(), 0, data ); } -#define todo_check_update_rgn( hwnd, hrgn ) check_update_rgn_( __LINE__, TRUE, hwnd, hrgn ) -#define check_update_rgn( hwnd, hrgn ) check_update_rgn_( __LINE__, FALSE, hwnd, hrgn ) -static void check_update_rgn_( int line, BOOL todo, HWND hwnd, HRGN hrgn ) +#define check_update_rgn( hwnd, hrgn ) check_update_rgn_( __LINE__, hwnd, hrgn ) +static void check_update_rgn_( int line, HWND hwnd, HRGN hrgn ) { INT ret; RECT r1, r2; @@ -8187,10 +8186,7 @@ static void check_update_rgn_( int line, BOOL todo, HWND hwnd, HRGN hrgn ) ok( ret != ERROR, "GetUpdateRgn failed\n" ); if (ret == NULLREGION) { - if(todo) - todo_wine ok_(__FILE__,line)( !hrgn, "Update region shouldn't be empty\n" ); - else - ok_(__FILE__,line)( !hrgn, "Update region shouldn't be empty\n" ); + ok_(__FILE__,line)( !hrgn, "Update region shouldn't be empty\n" ); } else { @@ -8434,7 +8430,7 @@ static void test_paint_messages(void) ret = RedrawWindow( hwnd, &rect, NULL, RDW_INVALIDATE ); ok(ret, "RedrawWindow returned %d instead of TRUE\n", ret); SetRectRgn( hrgn, 7, 12, 21, 30 ); - todo_check_update_rgn( hwnd, hrgn ); + check_update_rgn( hwnd, hrgn ); /* Top and bottom are swapped */ RedrawWindow( hwnd, NULL, 0, RDW_VALIDATE|RDW_NOFRAME|RDW_NOERASE ); @@ -8442,7 +8438,7 @@ static void test_paint_messages(void) ret = RedrawWindow( hwnd, &rect, NULL, RDW_INVALIDATE ); ok(ret, "RedrawWindow returned %d instead of TRUE\n", ret); SetRectRgn( hrgn, 7, 12, 21, 30 ); - todo_check_update_rgn( hwnd, hrgn ); + check_update_rgn( hwnd, hrgn ); /* both reference points are swapped */ RedrawWindow( hwnd, NULL, 0, RDW_VALIDATE|RDW_NOFRAME|RDW_NOERASE ); @@ -8450,7 +8446,7 @@ static void test_paint_messages(void) ret = RedrawWindow( hwnd, &rect, NULL, RDW_INVALIDATE ); ok(ret, "RedrawWindow returned %d instead of TRUE\n", ret); SetRectRgn( hrgn, 7, 12, 21, 30 ); - todo_check_update_rgn( hwnd, hrgn ); + check_update_rgn( hwnd, hrgn ); /* flush pending messages */ flush_events(); diff --git a/dlls/win32u/dce.c b/dlls/win32u/dce.c index c41067af464..fbde3f3d0ec 100644 --- a/dlls/win32u/dce.c +++ b/dlls/win32u/dce.c @@ -1457,8 +1457,11 @@ BOOL WINAPI NtUserRedrawWindow( HWND hwnd, const RECT *rect, HRGN hrgn, UINT fla if (rect && !hrgn) { - if (IsRectEmpty( rect )) rect = ∅ - ret = redraw_window_rects( hwnd, flags, rect, 1 ); + RECT ordered = *rect; + + order_rect( &ordered ); + if (IsRectEmpty( &ordered )) ordered = empty; + ret = redraw_window_rects( hwnd, flags, &ordered, 1 ); } else if (!hrgn) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/63
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=114530 Your paranoid android. === w10pro64_ar (64 bit report) === user32: msg.c:12861: Test failed: message time not advanced: 1531c 1531c msg.c:12862: Test failed: coords not changed: (101 101) (101 101) msg.c:12879: Test failed: message time not advanced: 1531c 1531c msg.c:12880: Test failed: coords not changed: (101 101) (101 101)
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/63
participants (4)
-
Huw Davies (＠huw) -
Marvin -
Pedro Perdigão
-
Zebediah Figura