[RFC xserver] xwayland: persistent window struct on present

Roman Gilg subdiff at gmail.com
Fri May 4 11:10:27 UTC 2018


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 <ofourdan at redhat.com> wrote:
> 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


More information about the xorg-devel mailing list