Thanks for the testing Olivier. That's great news because the code
change was written rather hastily and I hadn't tested it extensively
yet. I just wanted to get some early feedback on the overall design.
But using separate persistent window structs is just more robust than
using xwl_window, which always get discarded on unmap. So that the
change directly worked for you without regressions is probably a
consequence of that.
On Fri, May 4, 2018 at 10:05 AM, Olivier Fourdan <ofour...@redhat.com> wrote:
> Hi Roman,
>
> On Fri, May 4, 2018 at 3:07 AM, Roman Gilg <subd...@gmail.com> wrote:
>>
>> Instead of reusing xwl_window introduce a persistent window struct for
>> every
>> window, that asks for Present flips.
>>
>> This struct saves all relevant data and is only freed on window destroy.
>>
>> Signed-off-by: Roman Gilg <subd...@gmail.com>
>> ---
>> hw/xwayland/xwayland-present.c | 277
>> ++++++++++++++++++++++++-----------------
>> hw/xwayland/xwayland.c | 50 ++++----
>> hw/xwayland/xwayland.h | 37 +++---
>> 3 files changed, 212 insertions(+), 152 deletions(-)
>>
>> diff --git a/hw/xwayland/xwayland-present.c
>> b/hw/xwayland/xwayland-present.c
>> index 66bfaae..ae9f47e 100644
>> --- a/hw/xwayland/xwayland-present.c
>> +++ b/hw/xwayland/xwayland-present.c
>> @@ -36,11 +36,49 @@
>> #define TIMER_LEN_COPY 17 // ~60fps
>> #define TIMER_LEN_FLIP 1000 // 1fps
>>
>> +static DevPrivateKeyRec xwl_present_window_private_key;
>> +
>> +static struct xwl_present_window *
>> +xwl_present_window_priv(WindowPtr window)
>> +{
>> + return dixGetPrivate(&window->devPrivates,
>> + &xwl_present_window_private_key);
>> +}
>> +
>> +static struct xwl_present_window *
>> +xwl_present_window_get_priv(WindowPtr window)
>> +{
>> + struct xwl_present_window *xwl_present_window =
>> xwl_present_window_priv(window);
>> +
>> + if (xwl_present_window == NULL) {
>> + ScreenPtr screen = window->drawable.pScreen;
>> +
>> + xwl_present_window = calloc (1, sizeof (struct
>> xwl_present_window));
>> + if (!xwl_present_window)
>> + return NULL;
>> +
>> + xwl_present_window->crtc_fake = RRCrtcCreate(screen,
>> + xwl_present_window);
>> + xwl_present_window->window = window;
>> + xwl_present_window->msc = 1;
>> + xwl_present_window->ust = GetTimeInMicros();
>> +
>> + xorg_list_init(&xwl_present_window->event_list);
>> + xorg_list_init(&xwl_present_window->release_queue);
>> +
>> + dixSetPrivate(&window->devPrivates,
>> + &xwl_present_window_private_key,
>> + xwl_present_window);
>> + }
>> +
>> + return xwl_present_window;
>> +}
>> +
>> static void
>> -xwl_present_free_timer(struct xwl_window *xwl_window)
>> +xwl_present_free_timer(struct xwl_present_window *xwl_present_window)
>> {
>> - TimerFree(xwl_window->present_timer);
>> - xwl_window->present_timer = NULL;
>> + TimerFree(xwl_present_window->frame_timer);
>> + xwl_present_window->frame_timer = NULL;
>> }
>>
>> static CARD32
>> @@ -49,57 +87,73 @@ xwl_present_timer_callback(OsTimerPtr timer,
>> void *arg);
>>
>> static inline Bool
>> -xwl_present_has_events(struct xwl_window *xwl_window)
>> +xwl_present_has_events(struct xwl_present_window *xwl_present_window)
>> {
>> - return !xorg_list_is_empty(&xwl_window->present_event_list) ||
>> - !xorg_list_is_empty(&xwl_window->present_release_queue);
>> + return !xorg_list_is_empty(&xwl_present_window->event_list) ||
>> + !xorg_list_is_empty(&xwl_present_window->release_queue);
>> +}
>> +
>> +static inline Bool
>> +xwl_present_is_flipping(WindowPtr window, struct xwl_window *xwl_window)
>> +{
>> + return xwl_window && xwl_window->present_window == window;
>> }
>>
>> static void
>> -xwl_present_reset_timer(struct xwl_window *xwl_window)
>> +xwl_present_reset_timer(struct xwl_present_window *xwl_present_window)
>> {
>> - if (xwl_present_has_events(xwl_window)) {
>> - uint32_t timer_len = xwl_window->present_window ? TIMER_LEN_FLIP
>> :
>> - TIMER_LEN_COPY;
>> -
>> - xwl_window->present_timer = TimerSet(xwl_window->present_timer,
>> - 0,
>> - timer_len,
>> - &xwl_present_timer_callback,
>> - xwl_window);
>> + if (xwl_present_has_events(xwl_present_window)) {
>> + WindowPtr present_window = xwl_present_window->window;
>> + Bool is_flipping = xwl_present_is_flipping(present_window,
>> +
>> xwl_window_from_window(present_window));
>> +
>> + xwl_present_window->frame_timer =
>> TimerSet(xwl_present_window->frame_timer,
>> + 0,
>> + is_flipping ?
>> TIMER_LEN_FLIP :
>> +
>> TIMER_LEN_COPY,
>> +
>> &xwl_present_timer_callback,
>> + xwl_present_window);
>> } else {
>> - xwl_present_free_timer(xwl_window);
>> + xwl_present_free_timer(xwl_present_window);
>> }
>> }
>>
>> void
>> -xwl_present_cleanup(struct xwl_window *xwl_window, WindowPtr window)
>> +xwl_present_cleanup(WindowPtr window)
>> {
>> + struct xwl_window *xwl_window = xwl_window_from_window(window);
>> + struct xwl_present_window *xwl_present_window =
>> xwl_present_window_priv(window);
>> struct xwl_present_event *event, *tmp;
>>
>> - if (xwl_window->present_window != window && xwl_window->window !=
>> window)
>> + if (!xwl_present_window)
>> return;
>>
>> - if (xwl_window->present_frame_callback) {
>> - wl_callback_destroy(xwl_window->present_frame_callback);
>> - xwl_window->present_frame_callback = NULL;
>> + if (xwl_window && xwl_window->present_window == window)
>> + xwl_window->present_window = NULL;
>> +
>> + RRCrtcDestroy(xwl_present_window->crtc_fake);
>> +
>> + if (xwl_present_window->frame_callback) {
>> + wl_callback_destroy(xwl_present_window->frame_callback);
>> + xwl_present_window->frame_callback = NULL;
>> }
>> - xwl_window->present_window = NULL;
>>
>> /* Clear remaining events */
>> - xorg_list_for_each_entry_safe(event, tmp,
>> &xwl_window->present_event_list, list) {
>> + xorg_list_for_each_entry_safe(event, tmp,
>> &xwl_present_window->event_list, list) {
>> xorg_list_del(&event->list);
>> free(event);
>> }
>>
>> /* Clear remaining buffer releases and inform Present about free
>> ressources */
>> - xorg_list_for_each_entry_safe(event, tmp,
>> &xwl_window->present_release_queue, list) {
>> + xorg_list_for_each_entry_safe(event, tmp,
>> &xwl_present_window->release_queue, list) {
>> xorg_list_del(&event->list);
>> event->abort = TRUE;
>> }
>>
>> /* Clear timer */
>> - xwl_present_free_timer(xwl_window);
>> + xwl_present_free_timer(xwl_present_window);
>> +
>> + free(xwl_present_window);
>> }
>>
>> static void
>> @@ -113,11 +167,10 @@ static void
>> xwl_present_buffer_release(void *data, struct wl_buffer *buffer)
>> {
>> struct xwl_present_event *event = data;
>> -
>> if (!event)
>> return;
>> - wl_buffer_set_user_data(buffer, NULL);
>>
>> + wl_buffer_set_user_data(buffer, NULL);
>> event->buffer_released = TRUE;
>>
>> if (event->abort) {
>> @@ -127,10 +180,10 @@ xwl_present_buffer_release(void *data, struct
>> wl_buffer *buffer)
>> }
>>
>> if (!event->pending) {
>> - present_wnmd_event_notify(event->present_window,
>> + present_wnmd_event_notify(event->xwl_present_window->window,
>> event->event_id,
>> - event->xwl_window->present_ust,
>> - event->xwl_window->present_msc);
>> + event->xwl_present_window->ust,
>> + event->xwl_present_window->msc);
>> xwl_present_free_event(event);
>> }
>> }
>> @@ -140,18 +193,18 @@ static const struct wl_buffer_listener
>> xwl_present_release_listener = {
>> };
>>
>> static void
>> -xwl_present_events_notify(struct xwl_window *xwl_window)
>> +xwl_present_events_notify(struct xwl_present_window *xwl_present_window)
>> {
>> - uint64_t msc = xwl_window->present_msc;
>> + uint64_t msc = xwl_present_window->msc;
>> struct xwl_present_event *event, *tmp;
>>
>> xorg_list_for_each_entry_safe(event, tmp,
>> - &xwl_window->present_event_list,
>> + &xwl_present_window->event_list,
>> list) {
>> if (event->target_msc <= msc) {
>> - present_wnmd_event_notify(event->present_window,
>> + present_wnmd_event_notify(xwl_present_window->window,
>> event->event_id,
>> - xwl_window->present_ust,
>> + xwl_present_window->ust,
>> msc);
>> xwl_present_free_event(event);
>> }
>> @@ -163,21 +216,23 @@ xwl_present_timer_callback(OsTimerPtr timer,
>> CARD32 time,
>> void *arg)
>> {
>> - struct xwl_window *xwl_window = arg;
>> + struct xwl_present_window *xwl_present_window = arg;
>> + WindowPtr present_window = xwl_present_window->window;
>> + struct xwl_window *xwl_window =
>> xwl_window_from_window(present_window);
>>
>> - xwl_window->present_timer_firing = TRUE;
>> - xwl_window->present_msc++;
>> - xwl_window->present_ust = GetTimeInMicros();
>> + xwl_present_window->frame_timer_firing = TRUE;
>> + xwl_present_window->msc++;
>> + xwl_present_window->ust = GetTimeInMicros();
>>
>> - xwl_present_events_notify(xwl_window);
>> + xwl_present_events_notify(xwl_present_window);
>>
>> - if (xwl_present_has_events(xwl_window)) {
>> + if (xwl_present_has_events(xwl_present_window)) {
>> /* Still events, restart timer */
>> - return xwl_window->present_window ? TIMER_LEN_FLIP :
>> - TIMER_LEN_COPY;
>> + return xwl_present_is_flipping(present_window, xwl_window) ?
>> TIMER_LEN_FLIP :
>> +
>> TIMER_LEN_COPY;
>> } else {
>> /* No more events, do not restart timer and delete it instead */
>> - xwl_present_free_timer(xwl_window);
>> + xwl_present_free_timer(xwl_present_window);
>> return 0;
>> }
>> }
>> @@ -187,25 +242,25 @@ xwl_present_frame_callback(void *data,
>> struct wl_callback *callback,
>> uint32_t time)
>> {
>> - struct xwl_window *xwl_window = data;
>> + struct xwl_present_window *xwl_present_window = data;
>>
>> - wl_callback_destroy(xwl_window->present_frame_callback);
>> - xwl_window->present_frame_callback = NULL;
>> + wl_callback_destroy(xwl_present_window->frame_callback);
>> + xwl_present_window->frame_callback = NULL;
>>
>> - if (xwl_window->present_timer_firing) {
>> + if (xwl_present_window->frame_timer_firing) {
>> /* If the timer is firing, this frame callback is too late */
>> return;
>> }
>>
>> - xwl_window->present_msc++;
>> - xwl_window->present_ust = GetTimeInMicros();
>> + xwl_present_window->msc++;
>> + xwl_present_window->ust = GetTimeInMicros();
>>
>> - xwl_present_events_notify(xwl_window);
>> + xwl_present_events_notify(xwl_present_window);
>>
>> /* we do not need the timer anymore for this frame,
>> * reset it for potentially the next one
>> */
>> - xwl_present_reset_timer(xwl_window);
>> + xwl_present_reset_timer(xwl_present_window);
>> }
>>
>> static const struct wl_callback_listener xwl_present_frame_listener = {
>> @@ -218,7 +273,7 @@ xwl_present_sync_callback(void *data,
>> uint32_t time)
>> {
>> struct xwl_present_event *event = data;
>> - struct xwl_window *xwl_window = event->xwl_window;
>> + struct xwl_present_window *xwl_present_window =
>> event->xwl_present_window;
>>
>> event->pending = FALSE;
>>
>> @@ -230,17 +285,17 @@ xwl_present_sync_callback(void *data,
>> return;
>> }
>>
>> - present_wnmd_event_notify(event->present_window,
>> + present_wnmd_event_notify(xwl_present_window->window,
>> event->event_id,
>> - xwl_window->present_ust,
>> - xwl_window->present_msc);
>> + xwl_present_window->ust,
>> + xwl_present_window->msc);
>>
>> if (event->buffer_released)
>> /* If the buffer was already released, send the event now again
>> */
>> - present_wnmd_event_notify(event->present_window,
>> + present_wnmd_event_notify(xwl_present_window->window,
>> event->event_id,
>> - xwl_window->present_ust,
>> - xwl_window->present_msc);
>> + xwl_present_window->ust,
>> + xwl_present_window->msc);
>> }
>>
>> static const struct wl_callback_listener xwl_present_sync_listener = {
>> @@ -250,33 +305,24 @@ static const struct wl_callback_listener
>> xwl_present_sync_listener = {
>> static RRCrtcPtr
>> xwl_present_get_crtc(WindowPtr present_window)
>> {
>> - struct xwl_window *xwl_window =
>> xwl_window_from_window(present_window);
>> - if (xwl_window == NULL)
>> + struct xwl_present_window *xwl_present_window =
>> xwl_present_window_get_priv(present_window);
>> + if (xwl_present_window == NULL)
>> return NULL;
>>
>> - return xwl_window->present_crtc_fake;
>> + return xwl_present_window->crtc_fake;
>> }
>>
>> static int
>> xwl_present_get_ust_msc(WindowPtr present_window, uint64_t *ust, uint64_t
>> *msc)
>> {
>> - struct xwl_window *xwl_window =
>> xwl_window_from_window(present_window);
>> - if (!xwl_window)
>> + struct xwl_present_window *xwl_present_window =
>> xwl_present_window_get_priv(present_window);
>> + if (!xwl_present_window)
>> return BadAlloc;
>> - *ust = xwl_window->present_ust;
>> - *msc = xwl_window->present_msc;
>>
>> - return Success;
>> -}
>> -
>> -static void
>> -xwl_present_set_present_window(struct xwl_window *xwl_window,
>> - WindowPtr present_window)
>> -{
>> - if (xwl_window->present_window)
>> - return;
>> + *ust = xwl_present_window->ust;
>> + *msc = xwl_present_window->msc;
>>
>> - xwl_window->present_window = present_window;
>> + return Success;
>> }
>>
>> /*
>> @@ -290,12 +336,13 @@ xwl_present_queue_vblank(WindowPtr present_window,
>> uint64_t msc)
>> {
>> struct xwl_window *xwl_window =
>> xwl_window_from_window(present_window);
>> + struct xwl_present_window *xwl_present_window =
>> xwl_present_window_get_priv(present_window);
>> struct xwl_present_event *event;
>>
>> if (!xwl_window)
>> return BadMatch;
>>
>> - if (xwl_window->present_crtc_fake != crtc)
>> + if (xwl_present_window->crtc_fake != crtc)
>> return BadRequest;
>>
>> if (xwl_window->present_window &&
>> @@ -307,14 +354,13 @@ xwl_present_queue_vblank(WindowPtr present_window,
>> return BadAlloc;
>>
>> event->event_id = event_id;
>> - event->present_window = present_window;
>> - event->xwl_window = xwl_window;
>> + event->xwl_present_window = xwl_present_window;
>> event->target_msc = msc;
>>
>> - xorg_list_append(&event->list, &xwl_window->present_event_list);
>> + xorg_list_append(&event->list, &xwl_present_window->event_list);
>>
>> - if (!xwl_window->present_timer)
>> - xwl_present_reset_timer(xwl_window);
>> + if (!xwl_present_window->frame_timer)
>> + xwl_present_reset_timer(xwl_present_window);
>>
>> return Success;
>> }
>> @@ -329,13 +375,13 @@ xwl_present_abort_vblank(WindowPtr present_window,
>> uint64_t event_id,
>> uint64_t msc)
>> {
>> - struct xwl_window *xwl_window =
>> xwl_window_from_window(present_window);
>> + struct xwl_present_window *xwl_present_window =
>> xwl_present_window_priv(present_window);
>> struct xwl_present_event *event, *tmp;
>>
>> - if (!xwl_window)
>> + if (!xwl_present_window)
>> return;
>>
>> - xorg_list_for_each_entry_safe(event, tmp,
>> &xwl_window->present_event_list, list) {
>> + xorg_list_for_each_entry_safe(event, tmp,
>> &xwl_present_window->event_list, list) {
>> if (event->event_id == event_id) {
>> xorg_list_del(&event->list);
>> free(event);
>> @@ -343,7 +389,7 @@ xwl_present_abort_vblank(WindowPtr present_window,
>> }
>> }
>>
>> - xorg_list_for_each_entry(event, &xwl_window->present_release_queue,
>> list) {
>> + xorg_list_for_each_entry(event, &xwl_present_window->release_queue,
>> list) {
>> if (event->event_id == event_id) {
>> event->abort = TRUE;
>> return;
>> @@ -378,15 +424,6 @@ xwl_present_check_flip2(RRCrtcPtr crtc,
>> xwl_window->present_window != present_window)
>> return FALSE;
>>
>> - if (!xwl_window->present_crtc_fake)
>> - return FALSE;
>> - /*
>> - * Make sure the client doesn't try to flip to another crtc
>> - * than the one created for 'xwl_window'.
>> - */
>> - if (xwl_window->present_crtc_fake != crtc)
>> - return FALSE;
>> -
>> /*
>> * We currently only allow flips of windows, that have the same
>> * dimensions as their xwl_window parent window. For the case of
>> @@ -408,35 +445,45 @@ xwl_present_flip(WindowPtr present_window,
>> RegionPtr damage)
>> {
>> struct xwl_window *xwl_window =
>> xwl_window_from_window(present_window);
>> + struct xwl_present_window *xwl_present_window =
>> xwl_present_window_priv(present_window);
>> BoxPtr present_box, damage_box;
>> Bool buffer_created;
>> struct wl_buffer *buffer;
>> struct xwl_present_event *event;
>>
>> + if (!xwl_window)
>> + return FALSE;
>> +
>> + /*
>> + * Make sure the client doesn't try to flip to another crtc
>> + * than the one created for 'xwl_window'.
>> + */
>> + if (xwl_present_window->crtc_fake != crtc)
>> + return FALSE;
>> +
>> present_box = RegionExtents(&present_window->winSize);
>> damage_box = RegionExtents(damage);
>>
>> - xwl_present_set_present_window(xwl_window, present_window);
>> -
>> event = malloc(sizeof *event);
>> if (!event)
>> return FALSE;
>>
>> + xwl_window->present_window = present_window;
>> +
>> buffer = xwl_glamor_pixmap_get_wl_buffer(pixmap,
>> present_box->x2 -
>> present_box->x1,
>> present_box->y2 -
>> present_box->y1,
>> &buffer_created);
>>
>> event->event_id = event_id;
>> - event->present_window = present_window;
>> - event->xwl_window = xwl_window;
>> + event->xwl_present_window = xwl_present_window;
>> event->buffer = buffer;
>> - event->target_msc = xwl_window->present_msc;
>> + event->target_msc = xwl_present_window->msc;
>> event->pending = TRUE;
>> event->abort = FALSE;
>> event->buffer_released = FALSE;
>>
>> - xorg_list_add(&event->list, &xwl_window->present_release_queue);
>> + xorg_list_add(&event->list, &xwl_present_window->release_queue);
>>
>> if (buffer_created)
>> wl_buffer_add_listener(buffer, &xwl_present_release_listener,
>> NULL);
>> @@ -445,18 +492,18 @@ xwl_present_flip(WindowPtr present_window,
>> /* We can flip directly to the main surface (full screen window
>> without clips) */
>> wl_surface_attach(xwl_window->surface, buffer, 0, 0);
>>
>> - if (!xwl_window->present_timer ||
>> - xwl_window->present_timer_firing) {
>> + if (!xwl_present_window->frame_timer ||
>> + xwl_present_window->frame_timer_firing) {
>> /* Realign timer */
>> - xwl_window->present_timer_firing = FALSE;
>> - xwl_present_reset_timer(xwl_window);
>> + xwl_present_window->frame_timer_firing = FALSE;
>> + xwl_present_reset_timer(xwl_present_window);
>> }
>>
>> - if (!xwl_window->present_frame_callback) {
>> - xwl_window->present_frame_callback =
>> wl_surface_frame(xwl_window->surface);
>> - wl_callback_add_listener(xwl_window->present_frame_callback,
>> + if (!xwl_present_window->frame_callback) {
>> + xwl_present_window->frame_callback =
>> wl_surface_frame(xwl_window->surface);
>> + wl_callback_add_listener(xwl_present_window->frame_callback,
>> &xwl_present_frame_listener,
>> - xwl_window);
>> + xwl_present_window);
>> }
>>
>> wl_surface_damage(xwl_window->surface, 0, 0,
>> @@ -465,8 +512,8 @@ xwl_present_flip(WindowPtr present_window,
>>
>> wl_surface_commit(xwl_window->surface);
>>
>> - xwl_window->present_sync_callback =
>> wl_display_sync(xwl_window->xwl_screen->display);
>> - wl_callback_add_listener(xwl_window->present_sync_callback,
>> + xwl_present_window->sync_callback =
>> wl_display_sync(xwl_window->xwl_screen->display);
>> + wl_callback_add_listener(xwl_present_window->sync_callback,
>> &xwl_present_sync_listener,
>> event);
>>
>> @@ -478,6 +525,7 @@ static void
>> xwl_present_flips_stop(WindowPtr window)
>> {
>> struct xwl_window *xwl_window = xwl_window_from_window(window);
>> + struct xwl_present_window *xwl_present_window =
>> xwl_present_window_priv(window);
>>
>> if (!xwl_window)
>> return;
>> @@ -487,7 +535,7 @@ xwl_present_flips_stop(WindowPtr window)
>> xwl_window->present_window = NULL;
>>
>> /* Change back to the fast refresh rate */
>> - xwl_present_reset_timer(xwl_window);
>> + xwl_present_reset_timer(xwl_present_window);
>> }
>>
>> static present_wnmd_info_rec xwl_present_info = {
>> @@ -518,5 +566,8 @@ xwl_present_init(ScreenPtr screen)
>> if (xwl_screen->egl_backend.post_damage != NULL)
>> return FALSE;
>>
>> + if (!dixRegisterPrivateKey(&xwl_present_window_private_key,
>> PRIVATE_WINDOW, 0))
>> + return FALSE;
>> +
>> return present_wnmd_screen_init(screen, &xwl_present_info);
>> }
>> diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
>> index f7e2ce9..f5b03b2 100644
>> --- a/hw/xwayland/xwayland.c
>> +++ b/hw/xwayland/xwayland.c
>> @@ -531,17 +531,6 @@ xwl_realize_window(WindowPtr window)
>> wl_region_destroy(region);
>> }
>>
>> -#ifdef GLAMOR_HAS_GBM
>> - if (xwl_screen->present) {
>> - xwl_window->present_crtc_fake = RRCrtcCreate(xwl_screen->screen,
>> xwl_window);
>> - xwl_window->present_msc = 1;
>> - xwl_window->present_ust = GetTimeInMicros();
>> -
>> - xorg_list_init(&xwl_window->present_event_list);
>> - xorg_list_init(&xwl_window->present_release_queue);
>> - }
>> -#endif
>> -
>> wl_display_flush(xwl_screen->display);
>>
>> send_surface_id_event(xwl_window);
>> @@ -607,12 +596,6 @@ xwl_unrealize_window(WindowPtr window)
>>
>> compUnredirectWindow(serverClient, window, CompositeRedirectManual);
>>
>> -#ifdef GLAMOR_HAS_GBM
>> - xwl_window = xwl_window_from_window(window);
>> - if (xwl_window && xwl_screen->present)
>> - xwl_present_cleanup(xwl_window, window);
>> -#endif
>> -
>> screen->UnrealizeWindow = xwl_screen->UnrealizeWindow;
>> ret = (*screen->UnrealizeWindow) (window);
>> xwl_screen->UnrealizeWindow = screen->UnrealizeWindow;
>> @@ -629,11 +612,6 @@ xwl_unrealize_window(WindowPtr window)
>> if (xwl_window->frame_callback)
>> wl_callback_destroy(xwl_window->frame_callback);
>>
>> -#ifdef GLAMOR_HAS_GBM
>> - if (xwl_window->present_crtc_fake)
>> - RRCrtcDestroy(xwl_window->present_crtc_fake);
>> -#endif
>> -
>> free(xwl_window);
>> dixSetPrivate(&window->devPrivates, &xwl_window_private_key, NULL);
>>
>> @@ -661,6 +639,31 @@ static const struct wl_callback_listener
>> frame_listener = {
>> frame_callback
>> };
>>
>> +static Bool
>> +xwl_destroy_window(WindowPtr window)
>> +{
>> + ScreenPtr screen = window->drawable.pScreen;
>> + struct xwl_screen *xwl_screen = xwl_screen_get(screen);
>> + Bool ret;
>> +
>> +#ifdef GLAMOR_HAS_GBM
>> + if (xwl_screen->present)
>> + xwl_present_cleanup(window);
>> +#endif
>> +
>> + screen->DestroyWindow = xwl_screen->DestroyWindow;
>> +
>> + if (screen->DestroyWindow)
>> + ret = screen->DestroyWindow (window);
>> + else
>> + ret = TRUE;
>> +
>> + xwl_screen->DestroyWindow = screen->DestroyWindow;
>> + screen->DestroyWindow = xwl_destroy_window;
>> +
>> + return ret;
>> +}
>> +
>> static void
>> xwl_window_post_damage(struct xwl_window *xwl_window)
>> {
>> @@ -1114,6 +1117,9 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char
>> **argv)
>> xwl_screen->UnrealizeWindow = pScreen->UnrealizeWindow;
>> pScreen->UnrealizeWindow = xwl_unrealize_window;
>>
>> + xwl_screen->DestroyWindow = pScreen->DestroyWindow;
>> + pScreen->DestroyWindow = xwl_destroy_window;
>> +
>> xwl_screen->CloseScreen = pScreen->CloseScreen;
>> pScreen->CloseScreen = xwl_close_screen;
>>
>> diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
>> index 985ba9d..ce290d4 100644
>> --- a/hw/xwayland/xwayland.h
>> +++ b/hw/xwayland/xwayland.h
>> @@ -77,6 +77,7 @@ struct xwl_screen {
>> CloseScreenProcPtr CloseScreen;
>> RealizeWindowProcPtr RealizeWindow;
>> UnrealizeWindowProcPtr UnrealizeWindow;
>> + DestroyWindowProcPtr DestroyWindow;
>> XYToWindowProcPtr XYToWindow;
>>
>> struct xorg_list output_list;
>> @@ -172,26 +173,29 @@ struct xwl_window {
>> struct xorg_list link_damage;
>> struct wl_callback *frame_callback;
>> Bool allow_commits;
>> -#ifdef GLAMOR_HAS_GBM
>> - /* present */
>> - RRCrtcPtr present_crtc_fake;
>> - struct xorg_list present_link;
>> WindowPtr present_window;
>> - uint64_t present_msc;
>> - uint64_t present_ust;
>> +};
>> +
>> +#ifdef GLAMOR_HAS_GBM
>> +struct xwl_present_window {
>> + struct xwl_screen *xwl_screen;
>> + WindowPtr window;
>> + struct xorg_list link;
>>
>> - OsTimerPtr present_timer;
>> - Bool present_timer_firing;
>> + RRCrtcPtr crtc_fake;
>> + uint64_t msc;
>> + uint64_t ust;
>>
>> - struct wl_callback *present_frame_callback;
>> - struct wl_callback *present_sync_callback;
>> + OsTimerPtr frame_timer;
>> + Bool frame_timer_firing;
>>
>> - struct xorg_list present_event_list;
>> - struct xorg_list present_release_queue;
>> -#endif
>> + struct wl_callback *frame_callback;
>> + struct wl_callback *sync_callback;
>> +
>> + struct xorg_list event_list;
>> + struct xorg_list release_queue;
>> };
>>
>> -#ifdef GLAMOR_HAS_GBM
>> struct xwl_present_event {
>> uint64_t event_id;
>> uint64_t target_msc;
>> @@ -200,8 +204,7 @@ struct xwl_present_event {
>> Bool pending;
>> Bool buffer_released;
>>
>> - WindowPtr present_window;
>> - struct xwl_window *xwl_window;
>> + struct xwl_present_window *xwl_present_window;
>> struct wl_buffer *buffer;
>>
>> struct xorg_list list;
>> @@ -433,7 +436,7 @@ Bool xwl_glamor_allow_commits(struct xwl_window
>> *xwl_window);
>>
>> #ifdef GLAMOR_HAS_GBM
>> Bool xwl_present_init(ScreenPtr screen);
>> -void xwl_present_cleanup(struct xwl_window *xwl_window, WindowPtr
>> window);
>> +void xwl_present_cleanup(WindowPtr window);
>> #endif
>>
>> void xwl_screen_release_tablet_manager(struct xwl_screen *xwl_screen);
>> --
>> 2.7.4
>>
>> _______________________________________________
>> xorg-devel@lists.x.org: X.Org development
>> Archives: http://lists.x.org/archives/xorg-devel
>> Info: https://lists.x.org/mailman/listinfo/xorg-devel
>
>
> I reckon this is much better/nicer than changing the API/ABI :)
>
> I am not familiar enough with the internals of the present implementation to
> comment on the correctness of the patch itself, but I tested it and it works
> well (it seems it works better actually, I don't see any black window being
> shown on remap at all now, although this might be subjective).
>
> I also ran it in valgrind and tested the build without glamor just to make
> sure we didn't regress on this front either. No problem found, so, fwiw:
>
> Tested-by: Olivier Fourdan <ofour...@redhat.com>
>
> As Pekka pointed out on irc, Xwayland would be better off using the Wayland
> presentation time protocol, but mutter doesn't implement it (yet) so there
> will be room for future improvement, but imho we would be in a much better
> state wrt 1.20 with your patch here.
>
> Cheers,
> Olivier
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
