[RFC xserver] xwayland: persistent window struct on present

Olivier Fourdan ofourdan at redhat.com
Fri May 4 08:05:31 UTC 2018


Hi Roman,

On Fri, May 4, 2018 at 3:07 AM, Roman Gilg <subdiff at 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 <subdiff at 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 at 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 <ofourdan at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.x.org/archives/xorg-devel/attachments/20180504/7bac342a/attachment-0001.html>


More information about the xorg-devel mailing list