Commit Graph

121 Commits

Author SHA1 Message Date
Michel Dänzer fc53e3c536 xwaland/present: Drop flip_pending member of struct xwl_present_window
Use the first element of the flip_queue list for the same purpose.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:17:23 +02:00
Michel Dänzer c592c66625 xwayland/present: Fold xwl_present_flip_notify into its callers
No need for them to be separate anymore.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:17:15 +02:00
Michel Dänzer 4230176080 xwayland/present: Embed present_vblank_rec in xwl_present_event
This allows for various simplifications.

Use the pointer to the struct memory as the event ID. In contrast to
the SCMD code for Xorg (where pending DRM events cannot be cancelled),
this is safe here, because we can destroy pending Wayland callbacks. So
we can't get a callback with a stale pointer to freed memory.

Remove xwl_present_window::release_list in favour of
present_vblank_rec::window_list.

Remove xwl_present_event::xwl_present_window in favour of
present_vblank_rec::window.

xwl_present_free_event is never called for a NULL pointer anymore, no
need to check.

v2:
* Restore DestroyWindow wrapping order to make sure
  present_destroy_window doesn't call xwl_present_abort_vblank.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:15:50 +02:00
Michel Dänzer 35f173ddb6 xwayland/present: Drop abort member of struct xwl_present_event
We can call xwl_present_free_event unconditionally from
xwl_present_abort_vblank, since the sync_callback is already destroyed
in xwl_present_cleanup.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:15:10 +02:00
Michel Dänzer 0517460301 xwayland/present: Simplify calls to Xwayland-private functions
Change parameter types to what's really needed, or just fold the
function into its only caller.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:14:56 +02:00
Michel Dänzer 490248ea70 xwayland/present: Rename present_wnmd_* functions to xwl_present_*
The WNMD indirection is gone.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:14:47 +02:00
Michel Dänzer 0c0cbbc7cb present: Remove present_wnmd_info_rec
Doesn't serve any purpose anymore.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:14:38 +02:00
Michel Dänzer 561c63d0f1 xwayland/present: Merge present_wnmd_flips_stop & xwl_present_flips_stop
Just use the latter instead of the former elsewhere. No need for them
to be separate anymore.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:14:28 +02:00
Michel Dänzer a67f16fde1 xwayland/present: Fold present_wnmd_get_ust_msc into its callers
Just use xwl_present_get_ust_msc directly. No need for the indirection
anymore.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:14:19 +02:00
Michel Dänzer 080c1ca3f5 xwayland/present: Fold present_wnmd_queue_vblank into its callers
Just use xwl_present_queue_vblank directly. No need for the indirection
anymore.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:14:09 +02:00
Michel Dänzer c22887bc7a xwayland/present: Fold present_wnmd_get_crtc into present_wnmd_pixmap
And use xwl_present_get_crtc directly.

No need for them to be separate anymore.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:13:59 +02:00
Michel Dänzer cb35ff596e xwayland/present: Fold present_wnmd_check_flip into its callers
Mainly into xwl_present_check_flip, and call that from
present_wnmd_check_flip_window.

No need for them to be separate anymore.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:13:48 +02:00
Michel Dänzer 7b78cf6b3a xwayland/present: Simplify query_capabilities
No need for the WNMD indirection anymore.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:13:39 +02:00
Michel Dänzer 244403ec87 xwayland/present: Fold present_wnmd_abort_vblank into its only caller
No need for them to be separate anymore.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:13:32 +02:00
Michel Dänzer f7adbc2166 xwayland/present: Drop present_wnmd_flush in favour of xwl_present_flush
No need for the indirection anymore.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:13:24 +02:00
Michel Dänzer 7fd114365d xwayland/present: Fold present_wnmd_flip into present_wnmd_execute
No need for the indirection anymore.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:13:15 +02:00
Michel Dänzer 2e1dcd731f xwayland/present: Fold present_wnmd_screen_init into xwl_present_init
No need for them to be separate anymore.

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:13:04 +02:00
Michel Dänzer b6419359b6 present: Move present_wnmd.c contents to hw/xwayland/xwayland-present.c
This will allow eliminating indirections and making the Xwayland Present
code more efficient and easier to follow.

While this technically changes the Xorg video driver ABI, I don't know
of any drivers using the dropped present_wnmd_* symbols, and I doubt a
Xorg driver could make use of them as is anyway.

(As a bonus, Xorg no longer links any Xwayland specific Present code)

v2:
* Wrap DestroyWindow before initializing Present, so that
  present_destroy_window runs before xwl_present_cleanup. Avoids crash
  due to present_destroy_window calling xwl_present_* functions when
  xwl_present_window was already freed. (Olivier Fourdan)

Acked-by: Olivier Fourdan <ofourdan@redhat.com>
2021-07-09 16:11:22 +02:00
Michel Dänzer 104c7c5048 xwayland/present: Move wl_buffer check into xwl_glamor_check_flip
Keeps the glamor specific code together more.

Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
2021-06-14 10:44:03 +02:00
Simon Ser f3eb1684fa xwayland: enable MIT-SHM shared pixmaps
Allow X11 clients to create shared pixmaps via the MIT-SHM
extension under Xwayland. Tested with a wlroots patch [1].

Also add a few assertions to make sure we have wl_buffers where we
need them.

[1]: https://github.com/swaywm/wlroots/pull/2875

Signed-off-by: Simon Ser <contact@emersion.fr>
Acked-by: Michel Dänzer <mdaenzer@redhat.com>
2021-05-18 15:19:55 +00:00
Olivier Fourdan 25d2f4948f xwayland: Check buffer prior to attaching it
If the buffer is NULL, do not even try to attach it, and risk a Wayland
protocol error which would be fatal to us.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Martin Peres <martin.peres@mupuf.org>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
https://gitlab.freedesktop.org/xorg/xserver/-/issues/1156
2021-05-11 14:08:58 +02:00
Erik Kurzinger 38e875904b xwayland: implement pixmap_from_buffers for the eglstream backend
Provides an implementation for the pixmap_from_buffers DRI3 function for
xwayland's eglstream backend. This will be used by the NVIDIA GLX driver
to pass buffers from client applications to the server. These can then
be presented using the PRESENT extension.

To hopefully make this less error-prone, we also introduce a "type"
field for this struct to distinguish between xwl_pixmaps for the new
DRI3-created pixmaps and those for the existing glamor-created pixmaps.

Additionally, the patch enables wnmd present mode with the eglstream backend.
This involves creating a wl_buffer for the provided dma-buf before importing it
into EGL and passing this to the compositor so it can be scanned out directly
if possible.

Since both backends now support this present mode, the HAS_PRESENT_FLIP flag is
no longer needed, so it can be removed.

Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
Acked-by: Olivier Fourdan <ofourdan@redhat.com>
Signed-off-by: Erik Kurzinger <ekurzinger@nvidia.com>
2021-04-09 14:00:01 +00:00
Erik Kurzinger a9269808f3 xwayland: remove unused parameter of xwl_glamor_pixmap_get_wl_buffer
There are currently no callers that make use of the "created" output parameter
of xwl_glamor_pixmap_get_wl_buffer. Remove it, along with the corresponding
argument of the associated EGL backend entrypoint.
2021-01-08 06:17:51 -08:00
Olivier Fourdan ae84f14fb5 xwayland: Add a flag to expose EGL backend features
The present flip does not work with the EGLStream backend. Similarly,
the EGLStream backend does not require the buffer to be flushed as
eglSwapBuffers() should take care of this.

Instead of actually checking the backend in use in the present code,
add a flag in the form of a bitfield to the EGL backend to indicate
its features and requirements.

This should not introduce any functional change.

v2: Fix logical test (Adam Jackson <ajax@redhat.com>)

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2020-09-22 17:59:42 +02:00
Michel Dänzer 4c25356d6c xwayland: Check window pixmap in xwl_present_check_flip2
We can only flip if the window pixmap matches that of the toplevel
window. Doing so regardless could cause the toplevel window pixmap to
get destroyed while it was still referenced by the window, resulting in
use-after-free and likely a crash.

Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1033
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Roman Gilg <subdiff@gmail.com>
2020-09-07 17:55:12 +02:00
Roman Gilg c24eb7e31e xwayland: Simplify Present event handling code
Instead of optionally return early when an event is aborted and potentially
clean it up in there we can only optionally inform Present if not aborted and
afterwards clean it up if required.

Saves some lines of code and conditional branches.

Signed-off-by: Roman Gilg <subdiff@gmail.com>
2020-07-30 09:28:20 +00:00
Roman Gilg ab880b8b9e present: Idle vblanks any time in window mode
With the newly introduced separate API method for idling a presented Pixmap in
window mode we can simplify the logic by allowing calls to it at any point in
time.

This is done by setting the flip_idler flag if the Pixmap was idled before
being presented.

Signed-off-by: Roman Gilg <subdiff@gmail.com>
2020-07-22 11:10:17 +02:00
Roman Gilg 932c6baca2 present: Notify via distinct API functions in window mode
Notifying Present about events' states was done prior with the single function
present_wnmd_event_notify just like in screen mode. But it is more intelligible
if at least in window mode we make use of three different functions with names
that directly indicate what their purpose is:

* present_wnmd_event_notify only for queued events feedback.
* present_wnmd_flip_notify for when a presentation occured (flip).
* present_wnmd_idle_notify for when the Pixmap of the event can be reused.

This is an API-breaking change in regards to window mode. DDX written against
the previous version won't work anymore. It is assumed that there only exists
the XWayland DDX at the moment using the window mode such that this is not an
issue for the overall ecosystem.

Signed-off-by: Roman Gilg <subdiff@gmail.com>
2020-07-22 11:09:50 +02:00
Roman Gilg f8211095c3 xwayland: Rename present event lists
Rename the lists release_queue to release_list and event_list to
wait_list.

The prior names release_queue and event_list were ambiguous: in both are event-
like vblanks which can be removed from the lists in random order. In the
release_queue can be flips that are already released but still wait for the
sync or frame callback but normally the release comes later. In the event_list
are queued events waiting for a later msc.

Signed-off-by: Roman Gilg <subdiff@gmail.com>
2020-07-22 11:01:42 +02:00
Roman Gilg 85a6fd11c7 xwayland: Damage surface in surface-relative coordinates
In 9141196d positional coordinates were added to the damage call of pixmap
flips. The damage box coordinates are in screen space though and we need
to convert them first to surface-relative ones by substracting the origin
of the window.

Signed-off-by: Roman Gilg <subdiff@gmail.com>
2020-07-14 08:21:03 +00:00
Michel Dänzer 12af425acd xwayland: Rename xwl_pixmap_cb → xwl_buffer_release_cb
Seems clearer.

While we're at it, also drop the unused pixmap parameter.

Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
2020-07-07 13:47:11 +00:00
Michel Dänzer 9eb0b4f731 xwayland: Remove xwl_present_event::buffer_released in favor of ::pixmap
No need for the separate boolean.

Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
2020-07-07 13:47:11 +00:00
Michel Dänzer 9141196d31 xwayland: Propagate damage x1/y1 coordinates in xwl_present_flip
This couldn't have worked correctly for non-0 x1/y1.

Noticed by inspection.

Reviewed-by: Simon Ser <contact@emersion.fr>
2020-07-07 15:37:23 +02:00
Michel Dänzer b670527429 xwayland: Free all remaining events in xwl_present_cleanup
These events aren't reachable after xwl_present_cleanup, so they're
leaked if we don't free them first.

This requires storing the pixmap pointer in struct xwl_present_window.
Luckily, the buffer pointer isn't used for anything, so just replace
that.

v2:
* Bump pixmap reference count in xwl_present_flip and drop it in
  xwl_present_free_event, fixes use-after-free in the latter due to the
  pixmap already being destroyed.

Reviewed-by: Dave Airlie <airlied@redhat.com>
2020-06-25 17:32:19 +02:00
Michel Dänzer 1beffba699 xwayland: Always use xwl_present_free_event for freeing Present events
Minor cleanup, and will make the next change simpler. No functional
change intended.

Reviewed-by: Dave Airlie <airlied@redhat.com>
2020-06-25 17:31:06 +02:00
Michel Dänzer 5e91587302 xwayland: Delete all frame_callback_list nodes in xwl_unrealize_window
We were only calling xwl_present_unrealize_window for the toplevel
window, but the list can contain entries from child windows as well,
in which case we were leaving dangling pointers to freed memory.

Closes: https://gitlab.freedesktop.org/xorg/xserver/issues/1000
Fixes: c5067feaee "xwayland: Use single frame callback for Present
                     flips and normal updates"
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
Tested-by: Olivier Fourdan <ofourdan@redhat.com>
2020-03-17 11:45:22 +01:00
Olivier Fourdan 58155baeac xwayland: Cleanup and remove `xwayland.h`
Now that each source and header should be in order, we can safely cleaup
the last remaining bits from the main `xwayland.h` which is not needed
anymore and can be removed.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
2019-12-20 16:19:01 +01:00
Olivier Fourdan 0617c635fa xwayland: Separate Xwayland screen code
Move Xwayland screen related code to a separate source file and header.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
2019-12-20 16:19:01 +01:00
Olivier Fourdan e23d2223d8 xwayland: Move Xwayland present declarations
Move the Xwayland Present declarations to their own header file.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
2019-12-20 16:19:01 +01:00
Olivier Fourdan d780bdc2fd xwayland: Separate Xwayland pixmap code
Move Xwayland generic pixmap code to a separate source file and header.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
2019-12-20 16:19:01 +01:00
Olivier Fourdan 89e32d00f6 xwayland: Move Xwayland windows to its own sources
Over time, Xwayland main source file `xwayland.c` has grown in size
which makes it look cluttered and harder to read.

Move the code dealing with Xwayland window to its own source and header
files.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
2019-12-20 16:19:01 +01:00
Michel Dänzer 9b31358c52 xwayland: Use frame callbacks for Present vblank events
Instead of only the fallback timer.

Fixes https://gitlab.freedesktop.org/xorg/xserver/issues/854

v2:
* Drop unused frame_callback member of struct xwl_present_window
  (Olivier Fourdan)

Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
2019-12-17 10:31:22 +01:00
Michel Dänzer c5067feaee xwayland: Use single frame callback for Present flips and normal updates
Using a list of Present windows that need to be called back.

This prepares for the following change, there should be no change in
observed behaviour.

v2:
* Use xwl_window_create_frame_callback instead of making the
  frame_listener struct non-static (Olivier Fourdan)

Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
2019-12-17 10:28:33 +01:00
Michel Dänzer 2a2234ad1a xwayland: Do flush GPU work in xwl_present_flush
The Present code sends the idle notification event to the client after
xwl_present_flush returns. If we don't flush our GPU work here, the
client may race to draw another frame to the same buffer, so we may end
up copying (parts of) that new frame instead of the one we meant to.

Fixes https://gitlab.freedesktop.org/xorg/xserver/issues/835

Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
2019-12-03 18:24:34 +01:00
Olivier Fourdan 7765874186 xwayland: Add buffer release callback
The API `wl_buffer_add_listener` is misleading in the sense that there
can be only one `wl_buffer` release callback, and trying to add a new
listener when once is already in place will lead to a protocol error.

The Xwayland EGL backends may need to set up their own `wl_buffer`
release listener, meaning that there is no way to our own `wl_buffer`
release callback.

To avoid the problem, add our own callback API to be notified when the
`wl_buffer` associated with an `xwl_pixmap` is released, triggered from
the different `xwl_pixmap` implementations.

Also update the Present code to use the new buffer release callback API.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
2019-11-28 17:32:44 +01:00
Robert Mader 7c6f17790d xwayland: Use buffer_damage instead of surface damage if available
When a viewport is set, damage will only work properly when using
wl_surface_damage_buffer instead of wl_surface_damage.

When no viewport is set, there should be no difference between
surface and buffer damage.

This is a preparation patch for using viewport to add support for fake
mode-changes through xrandr for apps which want to change the resolution
when going fullscreen.

Changes by Hans de Goede <hdegoede@redhat.com>:
-Split the damage changes out into their own patch
-Add xwl_surface_damage helper
-Also use buffer_damage / the new helper for the present and cursor code

Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
Acked-by: Michel Dänzer <mdaenzer@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
2019-10-12 12:19:14 +02:00
Michel Dänzer 2e18eec6f0 xwayland/present: Destroy sync_callback in xwl_present_cleanup
xwl_present_cleanup frees the struct xwl_present_window memory,
so if there's a pending callback, we have to destroy it to prevent
use-after-free in xwl_present_sync_callback.

Should fix issue #645.

Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
2019-03-13 10:36:11 +01:00
Olivier Fourdan e8295c5020 xwayland: handle case without any crtc
Xwayland creates and destroys the CRTC along with the Wayland outputs,
so there is possibly a case where the number of CRTC drops to 0.

However, `xwl_present_get_crtc()` always return `crtcs[0]` which is
invalid when `numCrtcs` is 0.

That leads to crash if a client queries the Present capabilities when
there is no CRTC, the backtrace looks like:

  #0  raise() from libc.so
  #1  abort() from libc.so
  #2  OsAbort() at utils.c:1350
  #3  AbortServer() at log.c:879
  #4  FatalError() at log.c:1017
  #5  OsSigHandler() at osinit.c:156
  #6  OsSigHandler() at osinit.c:110
  #7  <signal handler called>
  #8  main_arena() from libc.so
  #9  proc_present_query_capabilities() at present_request.c:236
  #10 Dispatch() at dispatch.c:478
  #11 dix_main() at main.c:276

To avoid returning an invalid pointer (`crtcs[0]`) in that case, simply
check for `numCrtcs` being 0 and return `NULL` in that case.

Thanks to Michel Dänzer <michel.daenzer@amd.com> for pointing this as a
possible cause of the crash.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Bugzilla: https://bugzilla.redhat.com/1609181
2019-01-09 14:08:04 +01:00
Michel Dänzer e6cd1c9bde xwayland: Don't take buffer release queue into account for frame timer
The buffer release queue has two kinds of entries:

* Pending async flips.
* Completed flips waiting for their buffer to be released by the Wayland
  compositor.

xwl_present_timer_callback neither completes async flips nor releases
buffers, so the timer isn't needed for the buffer release queue.
2018-11-19 20:57:15 +00:00
Michel Dänzer f541615342 xwayland: Don't need xwl_window anymore in xwl_present_queue_vblank
Fixes issue #12. Presumably the problem was that Present operations on
unmapped windows were executed immediately instead of only when reaching
the target MSC.
2018-11-19 20:57:15 +00:00
Michel Dänzer 8c9538573c xwayland: Add xwl_present_unrealize_window
When a window is unrealized, a pending frame callback may never be
called, which could result in repeatedly freezing until the frame timer
fires after a second.

Fixes these symptoms when switching from fullscreen to windowed mode in
sauerbraten.
2018-11-19 20:57:15 +00:00
Michel Dänzer 6b016d58d2 xwayland: Replace xwl_window::present_window with ::present_flipped
There's no need to keep track of the window which last performed a
Present flip. This fixes crashes due to the assertion in
xwl_present_flips_stop failing. Fixes issue #10.

The damage generated by a flip only needs to be ignored once, then
xwl_window::present_flipped can be cleared. This may fix freezing in
the (hypothetical) scenario where Present flips are performed on a
window, followed by other drawing requests using the window as the
destination, but nothing triggering xwl_present_flips_stop. The damage
from the latter drawing requests would continue being ignored.
2018-11-19 20:57:15 +00:00
Michel Dänzer ace551d8a2 xwayland: Complete "synchronous" Present flips from xwl_present_msc_bump
Completing them from xwl_present_sync_callback had at least two issues:

* It was before the MSC was incremented in xwl_present_frame_callback,
  so the MSC value in the completion event could be lower than the
  target specified by the client. This could cause hangs with the Mesa
  Vulkan drivers.
* It allowed clients to run at a frame-rate higher than the Wayland
  compositor's frame-rate, wasting energy on generating frames which
  were never displayed. This isn't expected to happen unless the client
  specified PresentOptionAsync (in which case flips are still completed
  from xwl_present_sync_callback, allowing higher frame-rates).

v2:
* Make xwl_present_has_events return true when there's a pending
  "synchronous" flip, so those complete after at most ~1 second even if
  the Wayland server doesn't send a frame event.

Bugzilla: https://bugs.freedesktop.org/106713
2018-10-25 15:35:30 +00:00
Michel Dänzer 2bfc46d414 xwayland: Rename xwl_present_events_notify to xwl_present_msc_bump
And consolidate more code from xwl_present_timer_callback and
xwl_present_frame_callback in it.
2018-10-25 15:35:30 +00:00
Michel Dänzer 5e8b9a3a56 xwayland: Use xwl_present_reset_timer in xwl_present_timer_callback
Apart from simplifying the code, this should also prevent a condition
(which might only be possible with the following fix) reported in
https://gitlab.freedesktop.org/wayland/weston/issues/115#note_52467:

1. xwl_present_timer_callback indirectly calls xwl_present_reset_timer
   -> xwl_present_free_timer
2. xwl_present_timer_callback then returns a non-0 value, so DoTimer
   calls TimerSet with the old xwl_present_window->frame_timer pointer
   which was freed in step 1 => use after free

Calling xwl_present_reset_timer explicitly passes NULL to TimerSet if
step 1 freed xwl_present_window->frame_timer, and it will allocate a new
one.
2018-10-25 15:35:30 +00:00
Michel Dänzer cb0de153bf xwayland: Plug leaks in xwl_present_sync_callback
xwl_present_window->sync_callback was leaked.

The event memory was leaked if the corresponding buffer had already been
released.
2018-10-24 09:53:36 +00:00
Olivier Fourdan a2d188c7db xwayland: keep `xwl_present_timer_callback()` private
`xwl_present_timer_callback()` is initially marked a private and later
implemented as public.

Let's keep that private, shall we.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
2018-10-18 17:59:08 +00:00
Olivier Fourdan 3f31f56929 xwayland: Remove xwl_present_window from privates on cleanup
Xwayland's `xwl_destroy_window()` invokes `xwl_present_cleanup()`
before the common `DestroyWindow()`.

But then `DestroyWindow()` calls `present_destroy_window()` which will
possibly end up in `xwl_present_abort_vblank()` which will try to access
data that was previously freed by `xwl_present_cleanup()`:

  Invalid read of size 8
     at 0x434184: xwl_present_abort_vblank (xwayland-present.c:378)
     by 0x53785B: present_wnmd_abort_vblank (present_wnmd.c:651)
     by 0x53695A: present_free_window_vblank (present_screen.c:87)
     by 0x53695A: present_destroy_window (present_screen.c:152)
     by 0x42A90D: xwl_destroy_window (xwayland.c:653)
     by 0x584298: compDestroyWindow (compwindow.c:613)
     by 0x53CEE3: damageDestroyWindow (damage.c:1570)
     by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326)
     by 0x46F7F6: FreeWindowResources (window.c:1031)
     by 0x472847: DeleteWindow (window.c:1099)
     by 0x46B54C: doFreeResource (resource.c:880)
     by 0x46C706: FreeClientResources (resource.c:1146)
     by 0x446ADE: CloseDownClient (dispatch.c:3473)
   Address 0x182abde0 is 80 bytes inside a block of size 112 free'd
     at 0x4C2FDAC: free (vg_replace_malloc.c:530)
     by 0x42A937: xwl_destroy_window (xwayland.c:647)
     by 0x584298: compDestroyWindow (compwindow.c:613)
     by 0x53CEE3: damageDestroyWindow (damage.c:1570)
     by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326)
     by 0x46F7F6: FreeWindowResources (window.c:1031)
     by 0x472847: DeleteWindow (window.c:1099)
     by 0x46B54C: doFreeResource (resource.c:880)
     by 0x46C706: FreeClientResources (resource.c:1146)
     by 0x446ADE: CloseDownClient (dispatch.c:3473)
     by 0x446DA5: ProcKillClient (dispatch.c:3279)
     by 0x4476AF: Dispatch (dispatch.c:479)
   Block was alloc'd at
     at 0x4C30B06: calloc (vg_replace_malloc.c:711)
     by 0x433F46: xwl_present_window_get_priv (xwayland-present.c:54)
     by 0x434228: xwl_present_get_crtc (xwayland-present.c:302)
     by 0x539728: proc_present_query_capabilities (present_request.c:227)
     by 0x4476AF: Dispatch (dispatch.c:479)
     by 0x44B5B5: dix_main (main.c:276)
     by 0x75F611A: (below main) (libc-start.c:308)

This is because `xwl_present_cleanup()` frees the memory but does not
remove it from the window's privates, and `xwl_present_abort_vblank()`
will still find it and hence try to access that freed memory...

Remove `xwl_present_window` from window's privates on cleanup so that no
other function can find and reuse that data once it's freed.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1616269
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
2018-09-10 15:05:44 +00:00
Olivier Fourdan 792359057b xwayland: simplify xwl_glamor_pixmap_get_wl_buffer()
When retrieving the Wayland buffer from a pixmap, if the buffer already
exists, the GBM backend will return that existing buffer.

However, as seen with the Present issues, if the call had previously
passed a wrong size, that buffer will remain at the wrong size for as
long as the buffer exists, which is error prone.

Considering that the width/height passed to get_wl_buffer() is always the
actual pixmap  drawable size, and considering that the EGLStream backend
makes no use of the size either, there is really no point in passing the
width/height around.

Simplify the xwl_glamor_pixmap_get_wl_buffer() and EGL backends API by
removing the pixmap size, and use the drawable size instead.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
2018-06-21 10:54:10 -04:00
Olivier Fourdan 5d843f6947 xwayland: check for EGLStream backend explicitly
Now that we have separate backends for EGLStream and GBM, we can
explicitly check for the EGLStream backend to disable present support
in that case.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
2018-06-21 10:54:10 -04:00
Olivier Fourdan d7185a84b6 xwayland: refactor EGL backends for wayland registry
To be able to check for availability of the Wayland interfaces required
to run a given EGL backend (either GBM or EGLStream for now), we need
to have each backend structures and vfuncs in place before we enter the
Wayland registry dance.

That basically means that we should init all backends at first, connect
to the Wayland compositor and query the available interfaces and then
decide which backend is available and should be used (or none if either
the Wayland interfaces or the EGL extensions are not available).

For this purpose, hold an egl_backend struct for each backend we are to
consider prior to connect to the Wayland display so that, when we get to
query the Wayland interfaces, everything is in place for each backend to
handle the various Wayland interfaces.

Eventually, when we need to chose which EGL backend to use for glamor,
the available Wayland interfaces and EGL extensions available are all
known to Xwayland.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
2018-06-21 10:54:10 -04:00
Olivier Fourdan 1993f147d0 xwayland: use pixmap size on present flip
If the pixmap size does not match the present box size, flickering
occurs.

This can happen when the client changes its size (e.g. switching to
fullscreen), and since the buffer is kept as long as the pixmap is
valid, once the buffer is created, it remains at the wrong (old) size
and causes continuous flickering.

Use the actual pixmap's drawable size instead of the present box to
create the buffer so that it's sized appropriately.

Bugzilla: https://bugs.freedesktop.org/106841
Fixes: 0fb2cca193 "xwayland: Preliminary support for Present's new
                     window flip mode"
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Reviewed-by: Roman Gilg <subdiff@gmail.com>
2018-06-11 18:27:33 +02:00
Adam Jackson cc66777d85 xwayland: Don't create a "fake" crtc for Present
We probably don't want a fake crtc to be visible to clients, and we
definitely don't want to generate events every time we create such a
fake (which would happen as a side effect from RRCrtcCreate hitting
RRTellChanged). As it happens we're not actually using that crtc for
anything because xwayland doesn't store any state on the crtc object,
so it suffices to use the real crtc for the screen.

Signed-off-by: Adam Jackson <ajax@redhat.com>
Tested-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Roman Gilg <subdiff@gmail.com>
2018-05-08 12:15:29 -04:00
Roman Gilg cf838f5ca8 xwayland: persistent window struct on present
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@gmail.com>
Tested-by: Olivier Fourdan <ofourdan@redhat.com>
2018-05-07 15:24:23 -04:00
Roman Gilg a843c61456 xwayland: restrict present cleanup to presenting and top parent window
Clean up only if the request points to the presenting window or its top
parent window.

Since in this case all events are removed unconditionally, always stop
the timer.

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Roman Gilg <subdiff@gmail.com>
2018-04-30 15:52:16 -04:00
Lyude Paul 54ac09717c xwayland: Add glamor egl_backend for EGLStreams
This adds initial support for displaying Xwayland applications through
the use of EGLStreams and nvidia's custom wayland protocol by adding
another egl_backend driver. This also adds some additional egl_backend
hooks that are required to make things work properly.

EGLStreams work a lot differently then the traditional way of handling
buffers with wayland. Unfortunately, there are also a LOT of various
pitfalls baked into it's design that need to be explained.

This has a very large and unfortunate implication: direct rendering is,
for the time being at least, impossible to do through EGLStreams. The
main reason being that the EGLStream spec mandates that we lose the
entire color buffer contents with each eglSwapBuffers(), which goes
against X's requirement of not losing data with pixmaps.  no way to use
an allocated EGLSurface as the storage for glamor rendering like we do
with GBM, we have to rely on blitting each pixmap to it's respective
EGLSurface producer each frame. In order to pull this off, we add two
different additional egl_backend hooks that GBM opts out of
implementing:

- egl_backend.allow_commits for holding off displaying any EGLStream
  backed pixmaps until the point where it's stream is completely
  initialized and ready for use
- egl_backend.post_damage for blitting the content of the EGLStream
  surface producer before Xwayland actually damages and commits the
  wl_surface to the screen.

The other big pitfall here is that using nvidia's wayland-eglstreams
helper library is also not possible for the most part. All of it's API
for creating and destroying streams rely on being able to perform a
roundtrip in order to bring each stream to completion since the wayland
compositor must perform it's job of connecting a consumer to each
EGLstream. Because Xwayland has to potentially handle both responding to
the wayland compositor and it's own X clients, the situation of the
wayland compositor being one of our X clients must be considered. If we
perform a roundtrip with the Wayland compositor, it's possible that the
wayland compositor might currently be connected to us as an X client and
thus hang while both Xwayland and the wayland compositor await responses
from eachother. To avoid this, we work directly with the wayland
protocol and use wl_display_sync() events along with release() events to
set up and destroy EGLStreams asynchronously alongside handling X
clients.

Additionally, since setting up EGLStreams is not an atomic operation we
have to take into consideration the fact that an EGLStream can
potentially be created in response to a window resize, then immediately
deleted due to another pending window resize in the same X client's
pending reqests before Xwayland hits the part of it's event loop where
we read from the wayland compositor. To make this even more painful, we
also have to take into consideration that since EGLStreams are not
atomic that it's possible we could delete wayland resources for an
EGLStream before the compositor even finishes using them and thus run
into errors. So, we use quite a bit of tracking logic to keep EGLStream
objects alive until we know the compositor isn't using them (even if
this means the stream outlives the pixmap it backed).

While the default backend for glamor remains GBM, this patch exists for
users who have had to deal with the reprecussion of their GPU
manufacturers ignoring the advice of upstream and the standardization of
GBM across most major GPU manufacturers. It is not intended to be a
final solution to the GBM debate, but merely a baindaid so our users
don't have to suffer from the consequences of companies avoiding working
upstream. New drivers are strongly encouraged not to use this as a
backend, and use GBM like everyone else. We even spit this out as an
error from Xwayland when using the eglstream backend.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Acked-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2018-04-24 16:51:18 -04:00
Olivier Fourdan 3b4671f9e9 xwayland: Clean up all frame callbacks
Regardless of the order we un-realize windows.

Suggested-by: Roman Gilg <subdiff@gmail.com>
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Roman Gilg <subdiff@gmail.com>
2018-04-19 16:15:44 -04:00
Olivier Fourdan 8b8f9007cc xwayland: avoid using freed xwl_window on unrealize
xwl_unrealize_window() would use freed xwl_window which can lead to
various memory corruption and crashes, as reported by valgrind:

 Invalid read of size 8
    at 0x42C802: xwl_present_cleanup (xwayland-present.c:84)
    by 0x42BA67: xwl_unrealize_window (xwayland.c:601)
    by 0x541EE9: compUnrealizeWindow (compwindow.c:285)
    by 0x57E1FA: UnrealizeTree (window.c:2816)
    by 0x581189: UnmapWindow (window.c:2874)
    by 0x54EB26: ProcUnmapWindow (dispatch.c:879)
    by 0x554B7D: Dispatch (dispatch.c:479)
    by 0x558BE5: dix_main (main.c:276)
    by 0x7C4B1BA: (below main) (libc-start.c:308)
  Address 0xf520f60 is 96 bytes inside a block of size 184 free'd
    at 0x4C2EDAC: free (vg_replace_malloc.c:530)
    by 0x42B9FB: xwl_unrealize_window (xwayland.c:624)
    by 0x541EE9: compUnrealizeWindow (compwindow.c:285)
    by 0x57E1FA: UnrealizeTree (window.c:2816)
    by 0x581189: UnmapWindow (window.c:2874)
    by 0x54EB26: ProcUnmapWindow (dispatch.c:879)
    by 0x554B7D: Dispatch (dispatch.c:479)
    by 0x558BE5: dix_main (main.c:276)
    by 0x7C4B1BA: (below main) (libc-start.c:308)
  Block was alloc'd at
    at 0x4C2FB06: calloc (vg_replace_malloc.c:711)
    by 0x42B307: xwl_realize_window (xwayland.c:488)
    by 0x541E59: compRealizeWindow (compwindow.c:268)
    by 0x57DA40: RealizeTree (window.c:2617)
    by 0x580B28: MapWindow (window.c:2694)
    by 0x54EA2A: ProcMapWindow (dispatch.c:845)
    by 0x554B7D: Dispatch (dispatch.c:479)
    by 0x558BE5: dix_main (main.c:276)
    by 0x7C4B1BA: (below main) (libc-start.c:308)

This is because UnrealizeTree() traverses the tree from top to bottom,
which invalidates the assumption that if the Window doesn't feature an
xwl_window on its own, it's the xwl_window of its first ancestor with
one.

This reverts commit 82df2ce3

Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
2018-04-18 10:47:15 -04:00
Roman Gilg 07750ff3c0 xwayland: Implement queuing present vblanks
Queue present events to msc values. Fake msc events with a refresh rate of
about 60fps when flips are not possible. When flips are executed rely on
frame callbacks with a slow updating timer as fallback.

This is important for applications, that want to limit their framerate.

Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2018-03-28 14:36:59 -04:00
Roman Gilg 86df366973 xwayland: Add fallback timer for msc counting
When the compositor is not sending frame callbacks while we still wait
on buffer release events fake a continuous msc counter with a timer.

Having this timer is a prerequisite for queuing events.

Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2018-03-28 14:36:55 -04:00
Roman Gilg 0fb2cca193 xwayland: Preliminary support for Present's new window flip mode
Introduce support for Present's window flip mode. The support is not yet
complete, but works reasonable well for the most important use case, that
is fullscreen applications.

We take a Present flip and if the xwl_window->window has the same dimensions
as the presenting window, the flip is represented by a wl_buffer and attached
to the main wl_surface of the xwl_window.

After commit we are listening for the sync callback in order to tell Present,
that the pixmap flip is not longer pending, for the frame callback in order
to update the msc counter and for the buffer release callback in order to tell
Present that the pixmap is idle again.

The following functionality is missing from this patch:
* (slowed down) flips in case the compositor is not sending frame callbacks,
* queuing events to MSC times,
* per window flips for child windows with smaller size than the xwl_window.

To make use of this functionality Xwayland must run rootless and with
Glamor/GBM.

Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
2018-03-28 14:36:51 -04:00