[PATCH v8 0/3] MR231: Draft: SetWindowPos() parent window exposure fixes (#53153)
Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> -- v8: server: Don't invalidate the newly exposed child region on the parent if it doesn't clip children. https://gitlab.winehq.org/wine/wine/-/merge_requests/231
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> Skip redrawing the composited child window when the window rect and visible region stays the same, since we're taking the union of the old and new visible regions. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53153 Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/user32/tests/msg.c | 4 ++-- server/class.c | 5 +++++ server/region.c | 17 +++++++++++++++++ server/user.h | 9 +++++++++ server/window.c | 25 +++++++++++++++++++++++-- 5 files changed, 56 insertions(+), 4 deletions(-) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 76c30b7dfe5..9b4680bbcc1 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -9303,7 +9303,7 @@ static void subtest_swp_paint_regions_( int line, int wrap_toplevel, LPCSTR pare else { todo_wine_if( EqualRect( &rect_old, &rect_new ) ? is_zorder_redraw : - ((extest->style & WS_CLIPCHILDREN) == 0 || is_composited) ) + ((extest->style & WS_CLIPCHILDREN) == 0 && !is_composited) ) ok( !!rgn_ok, "Parent update region shall match expected region\n" ); } @@ -9366,7 +9366,7 @@ static void subtest_swp_paint_regions_( int line, int wrap_toplevel, LPCSTR pare else { todo_wine_if( EqualRect( &rect_old, &rect_new ) ? is_zorder_redraw : - ((extest->style & WS_CLIPCHILDREN) == 0 || is_composited) ) + ((extest->style & WS_CLIPCHILDREN) == 0 && !is_composited) ) ok( !!rgn_ok, "Child update region shall match expected region\n" ); } diff --git a/server/class.c b/server/class.c index e1e180bd97c..3231f366b26 100644 --- a/server/class.c +++ b/server/class.c @@ -141,6 +141,11 @@ int is_hwnd_message_class( struct window_class *class ) return (!class->local && class->atom == find_global_atom( NULL, &name )); } +int get_class_style( struct window_class *class ) +{ + return class->style; +} + atom_t get_class_atom( struct window_class *class ) { return class->base_atom; diff --git a/server/region.c b/server/region.c index 9f377ee05d3..d169f1ac2b4 100644 --- a/server/region.c +++ b/server/region.c @@ -684,6 +684,23 @@ int is_region_empty( const struct region *region ) } +/* checks if two regions are identical */ +int is_region_equal( const struct region *region1, const struct region *region2 ) +{ + int i; + + if (region1->num_rects != region2->num_rects) return 0; + if (region1->num_rects == 0) return 1; + if (!is_rect_equal( ®ion1->extents, ®ion2->extents )) return 0; + for (i = 0; i < region1->num_rects; i++) + { + if (!is_rect_equal( ®ion1->rects[i], ®ion2->rects[i] )) return 0; + } + + return 1; +} + + /* get the extents rect of a region */ void get_region_extents( const struct region *region, rectangle_t *rect ) { diff --git a/server/user.h b/server/user.h index 55a0d35feff..280da454d07 100644 --- a/server/user.h +++ b/server/user.h @@ -132,6 +132,7 @@ extern rectangle_t *get_region_data( const struct region *region, data_size_t ma extern rectangle_t *get_region_data_and_free( struct region *region, data_size_t max_size, data_size_t *total_size ); extern int is_region_empty( const struct region *region ); +extern int is_region_equal( const struct region *region1, const struct region *region2 ); extern void get_region_extents( const struct region *region, rectangle_t *rect ); extern void offset_region( struct region *region, int x, int y ); extern void mirror_region( const rectangle_t *client_rect, struct region *region ); @@ -175,6 +176,7 @@ extern struct window_class *grab_class( struct process *process, atom_t atom, extern void release_class( struct window_class *class ); extern int is_desktop_class( struct window_class *class ); extern int is_hwnd_message_class( struct window_class *class ); +extern int get_class_style( struct window_class *class ); extern atom_t get_class_atom( struct window_class *class ); extern client_ptr_t get_class_client_ptr( struct window_class *class ); @@ -191,6 +193,13 @@ extern void close_process_desktop( struct process *process ); extern void set_thread_default_desktop( struct thread *thread, struct desktop *desktop, obj_handle_t handle ); extern void release_thread_desktop( struct thread *thread, int close ); +/* checks if two rectangles are identical */ +static inline int is_rect_equal( const rectangle_t *rect1, const rectangle_t *rect2 ) +{ + return (rect1->left == rect2->left && rect1->right == rect2->right && + rect1->top == rect2->top && rect1->bottom == rect2->bottom); +} + static inline int is_rect_empty( const rectangle_t *rect ) { return (rect->left >= rect->right || rect->top >= rect->bottom); diff --git a/server/window.c b/server/window.c index 7675cd1103d..e3f21f2b76d 100644 --- a/server/window.c +++ b/server/window.c @@ -789,6 +789,16 @@ int is_window_transparent( user_handle_t window ) return (win->ex_style & (WS_EX_LAYERED|WS_EX_TRANSPARENT)) == (WS_EX_LAYERED|WS_EX_TRANSPARENT); } +static int is_window_using_parent_dc( struct window *win ) +{ + return (win->style & (WS_POPUP|WS_CHILD)) == WS_CHILD && (get_class_style( win->class ) & CS_PARENTDC) != 0; +} + +static int is_window_composited( struct window *win ) +{ + return (win->ex_style & WS_EX_COMPOSITED) != 0 && !is_window_using_parent_dc(win); +} + /* check if point is inside the window, and map to window dpi */ static int is_point_in_window( struct window *win, int *x, int *y, unsigned int dpi ) { @@ -1712,12 +1722,22 @@ static struct region *expose_window( struct window *win, const rectangle_t *old_ struct region *old_vis_rgn ) { struct region *new_vis_rgn, *exposed_rgn; + int is_composited = win->parent && is_window_composited( win->parent ); if (!(new_vis_rgn = get_visible_region( win, DCX_WINDOW ))) return NULL; + if (is_composited && + is_rect_equal( old_window_rect, &win->window_rect ) && + is_region_equal( old_vis_rgn, new_vis_rgn )) + { + return NULL; + } + if ((exposed_rgn = create_empty_region())) { - if (subtract_region( exposed_rgn, new_vis_rgn, old_vis_rgn ) && !is_region_empty( exposed_rgn )) + if ((is_composited ? union_region( exposed_rgn, new_vis_rgn, old_vis_rgn ) + : subtract_region( exposed_rgn, new_vis_rgn, old_vis_rgn )) && + !is_region_empty( exposed_rgn )) { /* make it relative to the new client area */ offset_region( exposed_rgn, win->window_rect.left - win->client_rect.left, @@ -1736,7 +1756,8 @@ static struct region *expose_window( struct window *win, const rectangle_t *old_ offset_region( new_vis_rgn, win->window_rect.left - old_window_rect->left, win->window_rect.top - old_window_rect->top ); - if ((win->parent->style & WS_CLIPCHILDREN) ? + if (is_composited ? union_region( new_vis_rgn, old_vis_rgn, new_vis_rgn ) : + (win->parent->style & WS_CLIPCHILDREN) ? subtract_region( new_vis_rgn, old_vis_rgn, new_vis_rgn ) : xor_region( new_vis_rgn, old_vis_rgn, new_vis_rgn )) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/231
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53153 Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/user32/tests/msg.c | 8 ++++---- server/window.c | 18 ++++++++++++------ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 9b4680bbcc1..fb22f6042fe 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -9302,8 +9302,8 @@ static void subtest_swp_paint_regions_( int line, int wrap_toplevel, LPCSTR pare } else { - todo_wine_if( EqualRect( &rect_old, &rect_new ) ? is_zorder_redraw : - ((extest->style & WS_CLIPCHILDREN) == 0 && !is_composited) ) + todo_wine_if( !EqualRect( &rect_old, &rect_new ) && + (extest->style & WS_CLIPCHILDREN) == 0 && !is_composited ) ok( !!rgn_ok, "Parent update region shall match expected region\n" ); } @@ -9365,8 +9365,8 @@ static void subtest_swp_paint_regions_( int line, int wrap_toplevel, LPCSTR pare } else { - todo_wine_if( EqualRect( &rect_old, &rect_new ) ? is_zorder_redraw : - ((extest->style & WS_CLIPCHILDREN) == 0 && !is_composited) ) + todo_wine_if( !EqualRect( &rect_old, &rect_new ) && + (extest->style & WS_CLIPCHILDREN) == 0 && !is_composited ) ok( !!rgn_ok, "Child update region shall match expected region\n" ); } diff --git a/server/window.c b/server/window.c index e3f21f2b76d..714fffdc8a1 100644 --- a/server/window.c +++ b/server/window.c @@ -1719,14 +1719,14 @@ static unsigned int get_window_update_flags( struct window *win, struct window * /* expose the areas revealed by a vis region change on the window parent */ /* returns the region exposed on the window itself (in client coordinates) */ static struct region *expose_window( struct window *win, const rectangle_t *old_window_rect, - struct region *old_vis_rgn ) + struct region *old_vis_rgn, int zorder_changed ) { struct region *new_vis_rgn, *exposed_rgn; int is_composited = win->parent && is_window_composited( win->parent ); if (!(new_vis_rgn = get_visible_region( win, DCX_WINDOW ))) return NULL; - if (is_composited && + if (is_composited && !zorder_changed && is_rect_equal( old_window_rect, &win->window_rect ) && is_region_equal( old_vis_rgn, new_vis_rgn )) { @@ -1787,6 +1787,7 @@ static void set_window_pos( struct window *win, struct window *previous, rectangle_t rect; int client_changed, frame_changed; int visible = (win->style & WS_VISIBLE) || (swp_flags & SWP_SHOWWINDOW); + int zorder_changed = 0; if (win->parent && !is_visible( win->parent )) visible = 0; @@ -1798,7 +1799,12 @@ static void set_window_pos( struct window *win, struct window *previous, win->visible_rect = *visible_rect; win->surface_rect = *surface_rect; win->client_rect = *client_rect; - if (!(swp_flags & SWP_NOZORDER) && win->parent) link_window( win, previous ); + if (!(swp_flags & SWP_NOZORDER) && win->parent) + { + struct list *old_prev = win->is_linked ? win->entry.prev : NULL; + link_window( win, previous ); + if (old_prev != win->entry.prev) zorder_changed = 1; + } if (swp_flags & SWP_SHOWWINDOW) win->style |= WS_VISIBLE; else if (swp_flags & SWP_HIDEWINDOW) win->style &= ~WS_VISIBLE; @@ -1827,7 +1833,7 @@ static void set_window_pos( struct window *win, struct window *previous, /* expose anything revealed by the change */ if (!(swp_flags & SWP_NOREDRAW)) - exposed_rgn = expose_window( win, &old_window_rect, old_vis_rgn ); + exposed_rgn = expose_window( win, &old_window_rect, old_vis_rgn, zorder_changed ); if (!(win->style & WS_VISIBLE)) { @@ -1958,7 +1964,7 @@ static void set_window_region( struct window *win, struct region *region, int re win->win_region = region; /* expose anything revealed by the change */ - if (old_vis_rgn && ((exposed_rgn = expose_window( win, &win->window_rect, old_vis_rgn )))) + if (old_vis_rgn && ((exposed_rgn = expose_window( win, &win->window_rect, old_vis_rgn, 0 )))) { redraw_window( win, exposed_rgn, 1, RDW_INVALIDATE | RDW_ERASE | RDW_FRAME | RDW_ALLCHILDREN ); free_region( exposed_rgn ); @@ -1983,7 +1989,7 @@ void free_window_handle( struct window *win ) win->style &= ~WS_VISIBLE; if (vis_rgn) { - struct region *exposed_rgn = expose_window( win, &win->window_rect, vis_rgn ); + struct region *exposed_rgn = expose_window( win, &win->window_rect, vis_rgn, 0 ); if (exposed_rgn) free_region( exposed_rgn ); free_region( vis_rgn ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/231
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This partly reverts e42eaaaf3a873d29b14f8f3a3617df2d0d4210ca. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/comctl32/tests/edit.c | 2 +- dlls/user32/tests/msg.c | 4 ---- server/window.c | 6 ++---- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/dlls/comctl32/tests/edit.c b/dlls/comctl32/tests/edit.c index 5f23fff2c9c..be3de53cdb5 100644 --- a/dlls/comctl32/tests/edit.c +++ b/dlls/comctl32/tests/edit.c @@ -3450,7 +3450,7 @@ static void test_change_focus(void) flush_sequences(sequences, NUM_MSG_SEQUENCES); SetFocus(hwnd); while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); - ok_sequence(sequences, COMBINED_SEQ_INDEX, setfocus_combined_seq, "Set focus", TRUE); + ok_sequence(sequences, COMBINED_SEQ_INDEX, setfocus_combined_seq, "Set focus", FALSE); flush_sequences(sequences, NUM_MSG_SEQUENCES); SetFocus(parent_wnd); diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index fb22f6042fe..e36eb7125e6 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -9302,8 +9302,6 @@ static void subtest_swp_paint_regions_( int line, int wrap_toplevel, LPCSTR pare } else { - todo_wine_if( !EqualRect( &rect_old, &rect_new ) && - (extest->style & WS_CLIPCHILDREN) == 0 && !is_composited ) ok( !!rgn_ok, "Parent update region shall match expected region\n" ); } @@ -9365,8 +9363,6 @@ static void subtest_swp_paint_regions_( int line, int wrap_toplevel, LPCSTR pare } else { - todo_wine_if( !EqualRect( &rect_old, &rect_new ) && - (extest->style & WS_CLIPCHILDREN) == 0 && !is_composited ) ok( !!rgn_ok, "Child update region shall match expected region\n" ); } diff --git a/server/window.c b/server/window.c index 714fffdc8a1..d76950f7801 100644 --- a/server/window.c +++ b/server/window.c @@ -1756,10 +1756,8 @@ static struct region *expose_window( struct window *win, const rectangle_t *old_ offset_region( new_vis_rgn, win->window_rect.left - old_window_rect->left, win->window_rect.top - old_window_rect->top ); - if (is_composited ? union_region( new_vis_rgn, old_vis_rgn, new_vis_rgn ) : - (win->parent->style & WS_CLIPCHILDREN) ? - subtract_region( new_vis_rgn, old_vis_rgn, new_vis_rgn ) : - xor_region( new_vis_rgn, old_vis_rgn, new_vis_rgn )) + if (is_composited ? union_region( new_vis_rgn, old_vis_rgn, new_vis_rgn ) + : subtract_region( new_vis_rgn, old_vis_rgn, new_vis_rgn )) { if (!is_region_empty( new_vis_rgn )) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/231
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=125576 Your paranoid android. === w10pro64 (32 bit report) === comctl32: edit.c:3635: Test failed: WM_IME_COMPOSITION: the msg sequence is not complete: expected 0000 - actual 0084 edit.c:3656: Test failed: WM_IME_COMPOSITION with EIMES_GETCOMPSTRATONCE: the msg sequence is not complete: expected 0000 - actual 0084 === w10pro64 (64 bit report) === comctl32: edit.c:3635: Test failed: WM_IME_COMPOSITION: the msg sequence is not complete: expected 0000 - actual 0084 === w10pro64_en_AE_u8 (64 bit report) === comctl32: edit.c:3635: Test failed: WM_IME_COMPOSITION: the msg sequence is not complete: expected 0000 - actual 0084 === w10pro64_ar (64 bit report) === comctl32: edit.c:3635: Test failed: WM_IME_COMPOSITION: the msg sequence is not complete: expected 0000 - actual 0084 === w10pro64_ja (64 bit report) === comctl32: edit.c:3635: Test failed: WM_IME_COMPOSITION: the msg sequence is not complete: expected 0000 - actual 0084 === w10pro64_zh_CN (64 bit report) === comctl32: edit.c:3635: Test failed: WM_IME_COMPOSITION: the msg sequence is not complete: expected 0000 - actual 0084 edit.c:3641: Test failed: WM_IME_CHAR: the msg sequence is not complete: expected 0000 - actual 0084 === debian11 (32 bit ar:MA report) === comctl32: edit.c:3453: Test failed: Set focus: the msg 0x000f was expected, but got msg 0x0084 instead edit.c:3453: Test failed: Set focus: the msg sequence is not complete: expected 0000 - actual 0020
Jacek Caban (@jacek) commented about server/window.c:
struct region *old_vis_rgn ) { struct region *new_vis_rgn, *exposed_rgn; + int is_composited = win->parent && is_window_composited( win->parent );
if (!(new_vis_rgn = get_visible_region( win, DCX_WINDOW ))) return NULL;
+ if (is_composited && + is_rect_equal( old_window_rect, &win->window_rect ) && + is_region_equal( old_vis_rgn, new_vis_rgn )) + { + return NULL;
You leak new_vis_rgn here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/231#note_12983
Jacek Caban (@jacek) commented about server/window.c:
struct region *old_vis_rgn ) { struct region *new_vis_rgn, *exposed_rgn; + int is_composited = win->parent && is_window_composited( win->parent );
Maybe win->parent check could go to `is_window_composited()` as well? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/231#note_12987
Jacek Caban (@jacek) commented about server/window.c:
win->visible_rect = *visible_rect; win->surface_rect = *surface_rect; win->client_rect = *client_rect; - if (!(swp_flags & SWP_NOZORDER) && win->parent) link_window( win, previous ); + if (!(swp_flags & SWP_NOZORDER) && win->parent) + { + struct list *old_prev = win->is_linked ? win->entry.prev : NULL; + link_window( win, previous ); + if (old_prev != win->entry.prev) zorder_changed = 1;
I think that it would be cleaner to return if z-order changed from `link_window()` would be cleaner. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/231#note_12988
I'm not really familiar with this code, but patches look mostly fine to me (with some comments). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/231#note_12989
On Mon Oct 31 16:47:17 2022 +0000, **** wrote:
Marvin replied on the mailing list: ``` Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=125576 Your paranoid android. === w10pro64 (32 bit report) === comctl32: edit.c:3635: Test failed: WM_IME_COMPOSITION: the msg sequence is not complete: expected 0000 - actual 0084 edit.c:3656: Test failed: WM_IME_COMPOSITION with EIMES_GETCOMPSTRATONCE: the msg sequence is not complete: expected 0000 - actual 0084 === w10pro64 (64 bit report) === comctl32: edit.c:3635: Test failed: WM_IME_COMPOSITION: the msg sequence is not complete: expected 0000 - actual 0084 === w10pro64_en_AE_u8 (64 bit report) === comctl32: edit.c:3635: Test failed: WM_IME_COMPOSITION: the msg sequence is not complete: expected 0000 - actual 0084 === w10pro64_ar (64 bit report) === comctl32: edit.c:3635: Test failed: WM_IME_COMPOSITION: the msg sequence is not complete: expected 0000 - actual 0084 === w10pro64_ja (64 bit report) === comctl32: edit.c:3635: Test failed: WM_IME_COMPOSITION: the msg sequence is not complete: expected 0000 - actual 0084 === w10pro64_zh_CN (64 bit report) === comctl32: edit.c:3635: Test failed: WM_IME_COMPOSITION: the msg sequence is not complete: expected 0000 - actual 0084 edit.c:3641: Test failed: WM_IME_CHAR: the msg sequence is not complete: expected 0000 - actual 0084 === debian11 (32 bit ar:MA report) === comctl32: edit.c:3453: Test failed: Set focus: the msg 0x000f was expected, but got msg 0x0084 instead edit.c:3453: Test failed: Set focus: the msg sequence is not complete: expected 0000 - actual 0020 ``` Previously unregistered flaky tests, likely fixed in https://gitlab.winehq.org/wine/wine/-/merge_requests/1245.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/231#note_12995
participants (4)
-
Jacek Caban (＠jacek) -
Jinoh Kang
-
Jinoh Kang (＠iamahuman)
-
Marvin