The region as passed in is in the source pixmap's coordinate space, so
intersecting it with the clipList (which is in screen space) resulted in
disappointment.
Fixes Firefox popups such as the hamburger menu when using the EGL
backend.
v2:
* Drop vblank->x/y_off from RegionTranslate call, since they're always
0 here (present_wnmd_check_flip rejects flips for x/y_off != 0).
Reported-by: Robert Mader <robert.mader@posteo.de>
Tested-by: Robert Mader <robert.mader@posteo.de>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
Tested-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> # v1
present_wnmd_toplvl_pixmap_window returns a window with the same window
pixmap, so the check could never fail.
Reviewed-by: Roman Gilg <subdiff@gmail.com>
Noticed this was missing while working on the following fix.
v2:
* Dropped present_wnmd_can_window_flip hunk (that function is never
called, will be cleaned up in a follow-up MR).
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com> # v1
Reviewed-by: Roman Gilg <subdiff@gmail.com>
It always takes one update cycle for the copy to become visible on the
host windowing system, so waiting for the target MSC resulted in 1 cycle
delay.
We re-use the idle list for copies which were executed but need their
completion event sent.
Fixes black seams when resizing the "Builder" sub-window of
GDK_BACKEND=x11 gtk4-demo
on Xwayland (see
https://gitlab.gnome.org/GNOME/mutter/-/issues/1290#note_873557).
Unfortunately, this cannot completely fix the seams with apps which
queue up multiple frames in advance, since there's always at least one
queued frame corresponding to the old window size. But it should at
least help a little in that case as well.
v2:
* Bug fix: Don't update exec_msc in present_wnmd_check_flip_window.
(Roman Gilg)
* Use exec_msc = target_msc - 1 instead of exec_msc--, and add a
comment, for clarity.
v3:
* Drop exec_msc = target_msc again in present_wnmd_execute.
* present_execute_copy should never set vblank->queued in
present_wnmd_execute now, so replace that branch with an assertion.
(Roman Gilg)
Reviewed-by: Roman Gilg <subdiff@gmail.com>
Tested-by: Roman Gilg <subdiff@gmail.com>
Allowing it to be called from more functions than before. No functional
change.
Reviewed-by: Roman Gilg <subdiff@gmail.com>
Tested-by: Roman Gilg <subdiff@gmail.com>
For tracking the MSC when the present can be executed separately from
the target MSC.
Allows removing the requeue field instead, plus more later.
v2:
* Rename wait_msc → exec_msc (Roman Gilg)
* Use exec_msc = target_msc instead of exec_msc++, for clarity.
* Bug fix: Set exec_msc = target_msc also if present_flip returned
false in present_execute.
v3:
* Set exec_msc = target_msc also if present_wnmd_flip returned
false in present_wnmd_execute, for consistency.
v4:
* Specifically check for exec_msc == crtc_msc + 1 in
present_execute_wait/copy, to avoid re-introducing
https://bugs.freedesktop.org/show_bug.cgi?id=94596 .
Reviewed-by: Roman Gilg <subdiff@gmail.com>
Tested-by: Roman Gilg <subdiff@gmail.com>
Preparation for different handling between SCMD & WNMD. No functional
change intended.
Reviewed-by: Roman Gilg <subdiff@gmail.com>
Tested-by: Roman Gilg <subdiff@gmail.com>
For Pixmap flips to have well defined outcomes the window must be contained by
the valid region if such region was specified.
The valid region is inserted as an argument to the check in window mode.
Setting this argument is missing in screen mode as well but we ignore it for now
and only add it to window mode.
It seems there are none or only very few clients actually making use of valid
regions at the moment. For simplicity we therefore just check if a valid region
was set by the client and in this case do never flip, independently of the
window being contained by the region or not.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
The function is never called from present_screen.c in contrast to the behavior
in screen mode.
In present_wnmd.c we can simply remove the function which does an unnecessary
check of the property before setting it and directly set the property at the
two locations the function was called previously.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
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>
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>
Most (but not all) of these were found by using
codespell --builtin clear,rare,usage,informal,code,names
but not everything reported by that was fixed.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
When present_wnmd_clear_window_flip is done, present_destroy_window
frees struct present_window_priv, and the events in the flip queue
become unreachable. So if we don't free them first, they're leaked.
Also drop the call to present_wnmd_set_abort_flip, which just sets a
flag in struct present_window_priv and thus can't have any observable
effect after present_destroy_window.
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1042
Reviewed-by: Dave Airlie <airlied@redhat.com>
The comment was incorrect: Any reference held by the window (see
present_wnmd_execute) is in addition to the one in struct present_vblank
(see present_vblank_create). So if we don't drop the latter, the pixmap
will be leaked.
Reviewed-by: Dave Airlie <airlied@redhat.com>
The value is not the current msc of the window, but the target value
the client sets independently of the window speicific msc offset. Make
this clearer.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Move the code portion down. That way it is at a similar position as in
the window mode file.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Make the code more readable by going through some logical abort
conditions. Also make the function only about updating the crtc
msc value and not about also returning the next target msc.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Unfold and extensively annotate the target-msc adjustment function, to make
it easier to understand what's happening and why.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
We can use value arguments instead of pointers when adjusting the timings
by returning the adjusted value. This improves the readability.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Currently, the function `present_wnmd_abort_vblank()` would fail if the
given `crtc` is NULL.
However, `xwl_present_get_crtc()` can return `NULL` under some
circumstances, which would cause an unexpected termination of Xwayland
in such a case, caused by the assertion failure being triggered.
Remove the assertion, considering that the `crtc` isn't actually used in
neither `present_wnmd_abort_vblank()` nor `xwl_present_abort_vblank()`.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
Closes: https://gitlab.freedesktop.org/xorg/xserver/issues/937
This reverts commit ef91da2757.
I thought this couldn't work under any circumstances, but I was wrong,
and drivers are already checking for cases that really cannot work.
Fixes issue #839.
The PresentOptionSuboptimal support code is not optional (once you've
enabled building Present at all), so require a protocol package version
that defines what we need.
Fixes: xorg/xserver#821
This cleans up some code duplication. No functional change.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Instead of testing window->redirectDraw.
With Xwayland, the toplevel window is always redirected, so this would
unnecessarily preclude flipping there in some cases, e.g. with wlroots
based Wayland compositors or with fullscreen X11 windows in weston.
Fixes issue #631.
This has never been actually implemented, do not tempt people to use it.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Daenzer <michel.daenzer@amd.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Xwayland will add and remove CRTCs as Wayland outputs are added or
removed.
If there is a pending flip when this occurs, the
`xwl_present_sync_callback()` will be triggered after the Xwayland
output's RRCtrcPtr has been destroyed, hence causing a crash in Xwayland
while trying to use freed memory:
#1 abort ()
#2 OsAbort () at utils.c:1350
#3 AbortServer () at log.c:877
#4 FatalError () at log.c:1015
#5 OsSigHandler () at osinit.c:156
#6 <signal handler called>
#7 dixGetPrivate () at ../include/privates.h:122
#8 dixLookupPrivate () at ../include/privates.h:166
#9 present_screen_priv () at present_priv.h:198
#10 present_wnmd_flip () at present_wnmd.c:358
#11 present_wnmd_execute () at present_wnmd.c:466
#12 present_wnmd_re_execute () at present_wnmd.c:80
#13 xwl_present_sync_callback () at xwayland-present.c:287
#14 ffi_call_unix64 () from /lib64/libffi.so.6
#15 ffi_call () from /lib64/libffi.so.6
#16 wl_closure_invoke () at src/connection.c:1006
#17 dispatch_event () at src/wayland-client.c:1427
#18 dispatch_queue () at src/wayland-client.c:1573
#19 wl_display_dispatch_queue_pending () at src/wayland-client.c:1815
#20 wl_display_dispatch_pending () at src/wayland-client.c:1878
#21 xwl_read_events () at xwayland.c:814
#22 ospoll_wait () at ospoll.c:651
#23 WaitForSomething () at WaitFor.c:208
#24 Dispatch () at ../include/list.h:220
#25 dix_main () at main.c:276
To avoid the issue, get the `ScreenPtr` from the window instead of the
CRTC that might have been just freed, `xwl_present_flip()` has no use
for the CRTC anyway.
Bugzilla: https://bugs.freedesktop.org/108249
Suggested-by: Michel Daenzer <michel.daenzer@amd.com>
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Daenzer <michel.daenzer@amd.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
When a vblank has been marked as aborted, it's going to be free in the
flip_notify function when stopped. We can't notify it after it's
stopped because the pointer is invalid.
Valgrind backtrace:
==5331== Invalid read of size 8
==5331== at 0x212B4D: present_vblank_notify (present_vblank.c:34)
==5331== by 0x21439B: present_wnmd_flip_notify (present_wnmd.c:194)
==5331== by 0x21439B: present_wnmd_event_notify (present_wnmd.c:228)
==5331== by 0x156216: xwl_present_sync_callback (xwayland-present.c:282)
==5331== by 0x6570FCD: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
==5331== by 0x657093E: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
==5331== by 0x4DDB183: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0)
==5331== by 0x4DD79D8: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0)
==5331== by 0x4DD8EA3: wl_display_dispatch_queue_pending (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0)
==5331== by 0x14BCCA: xwl_read_events (xwayland.c:814)
==5331== by 0x2AC0D0: ospoll_wait (ospoll.c:651)
==5331== by 0x2A5322: WaitForSomething (WaitFor.c:208)
==5331== by 0x27574B: Dispatch (dispatch.c:421)
==5331== Address 0x1b44dc98 is 40 bytes inside a block of size 184 free'd
==5331== at 0x48369EB: free (vg_replace_malloc.c:530)
==5331== by 0x213B0A: present_wnmd_free_idle_vblanks (present_wnmd.c:118)
==5331== by 0x213B0A: present_wnmd_flips_stop (present_wnmd.c:161)
==5331== by 0x2143EF: present_wnmd_flip_notify (present_wnmd.c:192)
==5331== by 0x2143EF: present_wnmd_event_notify (present_wnmd.c:228)
==5331== by 0x156216: xwl_present_sync_callback (xwayland-present.c:282)
==5331== by 0x6570FCD: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
==5331== by 0x657093E: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
==5331== by 0x4DDB183: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0)
==5331== by 0x4DD79D8: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0)
==5331== by 0x4DD8EA3: wl_display_dispatch_queue_pending (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0)
==5331== by 0x14BCCA: xwl_read_events (xwayland.c:814)
==5331== by 0x2AC0D0: ospoll_wait (ospoll.c:651)
==5331== by 0x2A5322: WaitForSomething (WaitFor.c:208)
==5331== Block was alloc'd at
==5331== at 0x48377D5: calloc (vg_replace_malloc.c:711)
==5331== by 0x212D9F: present_vblank_create (present_vblank.c:69)
==5331== by 0x214014: present_wnmd_pixmap (present_wnmd.c:610)
==5331== by 0x21576C: proc_present_pixmap (present_request.c:150)
==5331== by 0x27599D: Dispatch (dispatch.c:479)
==5331== by 0x279945: dix_main (main.c:276)
==5331== by 0x633AB16: (below main) (libc-start.c:310)
v2: Still notify aborted flips (Roman)
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107314
Reviewed-by: Roman Gilg <subdiff@gmail.com>
Tested-by: Roman Gilg <subdiff@gmail.com>
The incorrect values could result in the new pixmap's contents
getting corrupted down the line.
v2:
* Guard screen_x/y lines by #ifdef COMPOSITE
Bugzilla: https://bugs.freedesktop.org/106841
Fixes: 029608dd80 "present: Add window flip mode"
Reviewed-by: Adam Jackson <ajax@redhat.com> # v1
Reviewed-by: Keith Packard <keithp@keithp.com> # v1
Reviewed-by: Roman Gilg <subdiff@gmail.com>
Tested-by: Olivier Fourdan <ofourdan@redhat.com> # v1
The code would fall through to the PresentIdleNotify case, and nothing
good would come of it.
Signed-off-by: Adam Jackson <ajax@redhat.com>
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Instead of getting the current msc value from the window, which might be
different to old one directly take the last saved msc value saved in
the window_priv struct.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Install missing headers to the SDK directory to allow external modules
to properly build against the SDK. After this commit, the list of files
installed in the SDK include directory is the same as the list of files
installed by the autotools-based build.
Reviewed-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
Make sure that vblanks and windows get cleaned up correctly
in window flip mode.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Calculate damage before trying to flip and report it to the driver.
This allows drivers to optimize their rendering.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
In contrast to screen flip mode this mode:
* supports flips per windows (these windows currently need to have the same
size as their parent windows with the same pixmap),
* sends pixmap idle signals to the client only after the driver has given
an additional event notify.
This patch only introduces the new mode as a stub. It additionally needs a
driver hook, such that it can get initialized and appropriate cleanup
functions.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
To enable special functionality of window flips introduce for window flips
a separate set of driver facing function hooks.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Flipping pixmaps per window needs additional arguments in the
flip mode API. Add these as preperation for window flip mode.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Introduce vblank property for flip modes, that demand explicite
allowance by the driver for vblanks to become idle.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
For window flip mode data about flips needs to be stored per window.
Add properties to 'present_window_priv' and initialize them on creation.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
To initialize easily different flip modes, refactor
'present_screen_init'.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Add hooks to query caps, get crtcs, abort vblanks and destroy
a flip.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
To reduce future code duplication refactor timings adjustment out
as a separate function.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Flip modes can now have different implementations of
present_can_window_flip.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Michel Dänzer
Roman Gilg
Alan Coopersmith
Olivier Fourdan
Michel Dänzer
Adam Jackson
Lionel Landwerlin
Thierry Reding