On Tue Nov 28 21:12:43 2023 +0000, Paul Gofman wrote:
I think after 'XReparentWindow( gdi_display, ...)' we always need XSync(gdi_display, False). I know that as far as this patch is concerned all the uses of the helper do that XSync after anyway. But it looks like the following patches which use detach_client_window (directly or through attach_client_window added later) miss that. Maybe worth considering putting XSync() here and rearranging the calling code so it doesn't call it the second time (doing the other ops on gdi_context before this call)?
I don't really see why it would need an additional `XSync` if the current synchronization is correct. The only synchronization points are the same as before or after this MR:
1) when client_window is created using the gdi_display, we need to make sure gdi_display knows about its whole_window parent beforehand. After the creation we also need a XFlush so to make sure the creation requests will be seen by other displays.
2) before XSelectInput is called with thread display, we need to make sure thread display know about the client window.
3) when whole_window is destroyed, through the thread display, we need to make sure any client window has been fully reparented to a different window. The reparenting request is made on the gdi_display, so we could do `XFlush( gdi_display ); XSync( thread->display )` but as this is only causing errors after the whole window request is sent and processed, a `XSync( gdi_display )` should be enough.
Re-parenting alone should not need any synchronization as whole_window is still valid at that point, and you can reparent windows as many times as you like, as long as both windows stay valid.