FixesSelectSelectionInput
A recent "security fix" in ProcXFixesSelectSelectionInput hamstrings
this request in the event that no ownership has yet been asserted over
the selection.
The proximate cause is thus: dixLookupSelection returns error
indications when no selection data exists, which case is identified by
remaining unaltered, but is erroneously interpreted as an
access control denial, with the important consequence that gnome-shell
crashes on startup having received a BadMatch.
I don't believe it's possible to control access to still-nonexistent
selections through XACE.
diff --git a/xfixes/select.c b/xfixes/select.c
index 660eed210..11ddc4939 100644
--- a/xfixes/select.c
+++ b/xfixes/select.c
@@ -128,7 +128,9 @@ XFixesSelectSelectionInput(ClientPtr pClient,
Selection *selection;
rc = dixLookupSelection(, selection_name, pClient,
DixGetAttrAccess);
-if (rc != Success)
+if (rc != Success
+/* Selection exists, but access control handlers were triggered. */
+&& selection)
return rc;
for (prev = (e = *prev); prev = >next) {
Re: Xlib contract for XEvent up/down-casting
Jeremy Huddleston Sequoia writes:
> I've been running XQuartz with ASan+UBSan to try to catch some issues
> some users have reported, and I stumbled across something below GLUT
> (specifically, freeglut 2.8.1), which does:
>
> XConfigureEvent fakeEvent = {0};
> ...
> XPutBackEvent(fgDisplay.Display, (XEvent*));
>
> and XPutBackEvent eventually does:
>
> XEvent store = *event;
>
> which overflows the stack on read because:
>
> sizeof(XConfigureEvent) == 88
> sizeof(XEvent) == 192
>
> So the problem is clear, but I'm not sure which side needs to change.
>
> What is the contract for Xlib's APIs that take XEvent *? Is Xlib
> expected to handle any XEvent "subtype", or does it need to be exactly
> an XEvent (ie: is it the client's responsibility to pad it)?
It needs to be an XEvent, since the event ends up back on the event
queue. The client is supposed to pad it.
Re: vblank-synchronized Present copy
Matthieu Herrb writes: > On Thu, Nov 24, 2022 at 06:35:07PM +0800, Po Lu wrote: >> Matthieu Herrb writes: >> >> > Not all systems supported by X.Org have timerfd. So please make this >> > feàture conditional it's presence, >> >> Sure, but isn't modesetting recent-Linux-only? > > No, the version in Xserver 21.1.4 works perferctly on OpenBSD with > Mesa 21.1 and the intel / radeon and amdgpu kernel DRM drivers. Right, I will do that. Does anyone know why the 2 ms delay is necessary? Thanks.
Re: vblank-synchronized Present copy
Matthieu Herrb writes: > Not all systems supported by X.Org have timerfd. So please make this > feàture conditional it's presence, Sure, but isn't modesetting recent-Linux-only?
vblank-synchronized Present copy
Here is a change to xf86-video-modesetting that should let PresentPixmap
copy within the vertical blanking period, as opposed to immediately
after. I'm not sure whether or not it's ok to depend on timerfd, and to
also send the PresentCompleteNotify event at vblank start time (as
opposed to the first-pixel-out time, which is when it's sent in response
to a page flip.)
There's also a 2000 us delay in AddVblankTimer which should not be
there. If anyone knows why it is necessary, I'd be very grateful.
Thanks.
diff --git a/hw/xfree86/drivers/modesetting/vblank.c
b/hw/xfree86/drivers/modesetting/vblank.c
index ea9e7a88c..09622753c 100644
--- a/hw/xfree86/drivers/modesetting/vblank.c
+++ b/hw/xfree86/drivers/modesetting/vblank.c
@@ -32,9 +32,32 @@
#include
#include
#include
+#include
+
#include "driver.h"
#include "drmmode_display.h"
+typedef struct _VblankTimerRec {
+struct _VblankTimerRec *next, *last;
+struct timespec timespec;
+uint64_t msc;
+uint32_t seq;
+} VblankTimerRec, *VblankTimerPtr;
+
+/**
+ * List of software vblank timers used to schedule copying. Vblank
+ * events are sent after the blanking period on some drivers, so they
+ * cannot be reliably used to schedule copying. In fact, a more
+ * reliable method is to just ask for the time with drmWaitVBlank, and
+ * to schedule a software timer for that time.
+ */
+
+static VblankTimerRec vblank_timers;
+
+/** timerfd descriptor used for this. */
+
+static int vblank_timer_fd;
+
/**
* Tracking for outstanding events queued to the kernel.
*
@@ -260,6 +283,167 @@ ms_get_kernel_ust_msc(xf86CrtcPtr crtc,
}
}
+static void ms_drm_sequence_handler(int, uint64_t, uint64_t, Bool, uint64_t);
+
+static void
+RunVblankTimer (VblankTimerPtr timer, struct timespec time)
+{
+timer->last->next = timer->next;
+timer->next->last = timer->last;
+ms_drm_sequence_handler (-1, timer->msc, (time.tv_sec * 10
+ + time.tv_nsec),
+xFalse, timer->seq);
+free (timer);
+}
+
+static void
+CheckVblank (void)
+{
+VblankTimerPtr current, last;
+struct timespec now, prime;
+struct itimerspec new_value;
+static Bool inside;
+
+if (inside)
+ return;
+
+clock_gettime (CLOCK_MONOTONIC, );
+
+prime.tv_sec = 0;
+prime.tv_nsec = 0;
+
+current = vblank_timers.next;
+
+/* This function is reentrant after this point! */
+inside = xTrue;
+
+while (current != _timers) {
+ last = current;
+ current = last->next;
+
+ if (last->timespec.tv_sec < now.tv_sec
+ || (last->timespec.tv_sec == now.tv_sec
+ && last->timespec.tv_nsec <= now.tv_nsec))
+ RunVblankTimer (last, now);
+ else {
+ /* Now, find the earliest vblank and prime the timerfd for
+* that. */
+ if ((!prime.tv_sec && !prime.tv_nsec)
+ || (last->timespec.tv_sec < prime.tv_sec)
+ || (last->timespec.tv_sec == prime.tv_sec
+ && last->timespec.tv_nsec < prime.tv_nsec))
+ prime = last->timespec;
+ }
+}
+
+new_value.it_interval.tv_sec = 0;
+new_value.it_interval.tv_nsec = 0;
+new_value.it_value = prime;
+
+timerfd_settime (vblank_timer_fd, TFD_TIMER_ABSTIME,
+_value, NULL);
+inside = xFalse;
+}
+
+static void
+HandleVblankTimerReadable (int fd, int ready, void *data)
+{
+uint64_t nexpired;
+
+if (read (fd, , sizeof nexpired) < sizeof nexpired)
+ return;
+
+/* nexpired is ignored from here onwards. */
+CheckVblank ();
+}
+
+static Bool
+AddVblankTimer(xf86CrtcPtr crtc, drmVBlankReply *reply,
+ uint32_t seq, uint32_t msc)
+{
+VblankTimerPtr timer;
+drmmode_crtc_private_ptr msCrtc;
+drmModeModeInfoPtr mode_info;
+int64_t value, vblank_duration_ns, refresh_duration_ns;
+
+/* Compute the vertical blanking period. */
+msCrtc = crtc->driver_private;
+mode_info = >mode_crtc->mode;
+
+if (mode_info->htotal <= 0 || mode_info->vtotal <= 0)
+ return xFalse;
+
+timer = malloc (sizeof (VblankTimerRec));
+
+if (!timer)
+ return xFalse;
+
+timer->timespec.tv_sec = reply->tval_sec;
+timer->timespec.tv_nsec = reply->tval_usec * 1000;
+
+/* Get to the specified frame by repeatedly adding the refresh
+ * period until the desired sequence is reached. */
+value = mode_info->vtotal * mode_info->htotal;
+
+if (mode_info->flags & DRM_MODE_FLAG_DBLSCAN)
+ value += value;
+
+value = (value * 1000 + mode_info->clock - 1) / mode_info->clock;
+refresh_duration_ns = value * 1000 * min (1, (msc - reply->sequence));
+
+timer->timespec.tv_nsec += refresh_duration_ns;
+
+while (timer->timespec.tv_nsec >= 10) {
+ timer->timespec.tv_sec++;
+ timer->timespec.tv_nsec -= 10;
+}
+
+/* Now, subtract the vblank period. */
+
Re: Fwd: XInitThreads in library constructor breaks Motif!
Matthieu Herrb writes: > My french input method has eaten the URL. here the unmangled one: > > https://marc.info/?l=openbsd-tech=166742060513284=2 My crystal ball says it's probably that before XInitThreads was called, pthread stubs somehow found their way into the executable image. If you link the crashing program with -lpthread, does the crash magically go away?
Re: Fwd: XInitThreads in library constructor breaks Motif!
On November 2, 2022 1:25:06 AM GMT+08:00, Adam Jackson wrote: >On Mon, Oct 31, 2022 at 8:46 PM Po Lu wrote: >Again, I proposed a workaround that preserves the thread-safety guarantee I >need, and wrote the patch implementing it. Sure would be nice if someone >tested it. I did say it works for the programs that broke for me.
Re: XInitThreads in library constructor breaks Motif!
Carsten Haitzler writes: > Sorry - but no. Motif being non-threadsafe and Xlib being threadsafe or not > are > different matters. A higher level lib that is not threadsafe makes use of a > lower level one (xlib) and the latter can be threadsafe if desired. Being thread-safe is one thing. Deadlocking clients is another. If Xlib calls XInitThreads in a library constructor without consequences to programs > They man xinitthreads() then use motif from a single thread. Xlib can > be used then from multiple threads (e.g. one motif handled window and > another xlib handled window which is drawn to in another thread but > the same xlib display and connect below it all - as long as you are > not consuming events from both threads). Then any program doing that cannot use Motif. How simple is that? > ??? Did you not read what I wrote? You are saying I said the opposite of what > I > clearly wrote - xcb has nothing to do with discussing xinitthreads being > validly > enabled or not by default. this really has nothing to do with xcb. xlib being > built on xcb is just a happenstance of how xlib is built today and modern code > can actually mix and match xcb and xlib, but it's not part of the core issue. Evidently, the author of the change in question disagrees. Motif is not safe to use after calling XInitThreads, and the Motif applications on my desktop don't do that. Yet, without doing so, they were frozen, as I found out trying to make a calendar appointment. And if you think no application today can avoid pulling in EGL or GLX, you're just wrong. > And X11R6 added it. Behind XInitThreads. The old behavior of the library was intentionally kept for clients that did not use threads. (anecdotally, Emacs also did not require changes to work on R6. Despite being a big and complex program that heavily mixed Xt and Xlib code) > various things broke/changed between x11r5 and x11r6. Certainly parts > of ICCCM broke that required minor code changes in WMs and some > clients. the ICCCM has a nice list of them if you want to look it > up. And which of those changes caused clients to freeze? In fact, which of those changes affected Xlib more than "new fields added to XSizeHints, which was documented as subject to extension"? > They are not "new features to make use of voluntarily". They are > hard "must change your code" changes. No, they were "new fields added to structs" changes. Changes to the ICCCM naturally affected clients and window managers, but only if you used clients written for the new version of the ICCCM with those written for the old one. Motif programs, OTOH, were simply broken by Xlib 1.8.0, regardless of what other libraries they used. > I remember dealing with this back then as there were still some x11r5 > systems floating about that had some subtle differences. You're now > hitting another subtle change only really found now decades > later. There was obvious appetite for making small code breakages back > then and this here falls into that category. Deadlocking Motif programs is not a "small change", and you cannot seriously compare it to the ICCCM 2.0.
Re: XInitThreads in library constructor breaks Motif!
Carsten Haitzler writes: > no it doesn't. the contract is simply to not use motif from multiple threads. > it has nothing to do with xlib itself becoming threadsafe. i think the PR/MR > for xlib has all the details you want listed in it. No, it doesn't, see below: > it has nothing to do with it. xcb is an artefact of how xlib is implemented > today - it absolutely is not part of the spec or design or requirements. If my program does not use XCB (or even link with X11-xcb), why should it be subject to constraints placed by XCB? XCB is just one transport for the X library. > has nothing to do with it. having xlib just be threadsafe always when it is as > easy as xinitthreads being the default anyway is nice. it doesn't violate the > xlib contract with users of it if they are correctly implemented. Nonsense. The X library has worked how I (and Motif) expect it to since at least the early 90s. The X11R5 manual page says: The XIfEvent function completes only when the specified predicate procedure returns True for an event, which indicates an event in the queue matches. XIfEvent flushes the output buffer if it blocks waiting for additional events. XIfEvent removes the matching event from the queue and copies the structure into the client-supplied XEvent structure. When the predicate procedure finds a match, XCheckIfEvent copies the matched event into the client-supplied XEvent structure and returns True. (This event is removed from the queue.) If the predicate procedure finds no match, XCheckIfEvent returns False, and the output buffer will have been flushed. All earlier events stored in the queue are not discarded. The XPeekIfEvent function returns only when the specified predicate procedure returns True for an event. After the predicate procedure finds a match, XPeekIfEvent copies the matched event into the client-supplied XEvent structure without removing the event from the queue. XPeekIfEvent flushes the output buffer if it blocks waiting for additional events. Notice how there is no mention of calling Xlib functions being unsafe, as at that point the X library did not have thread support at all.
Re: XInitThreads in library constructor breaks Motif!
Adam Jackson writes: > Because XInitThreads only works if it is called before XOpenDisplay. So why not fix that? I think the simple fix would be to call _XInitDisplayLock in XGetXCBConnection.
Re: XInitThreads in library constructor breaks Motif!
Carsten Haitzler writes: > using motif and using xlib are different things. you are still free to use > xlib > separately to motif and thus use it from multiple threads... as long as you > use > motif and only motif from a single thread, you agree to the non-threadsafe > contract. Which Xlib breaks, by initializing threads. > the solution to this is... use a recursive lock in xlib or change the > locking around xcheckifevent and friends to unlock when going into the > callback (which is the current proposed solution in gitlab). the point of > locking is to keep xlib working with multiple threads as it would with a > single > one and this would honor that goal :) XCB afaik doesn't start multiple threads itself, so I don't see the problem at all. If there is really a big problem with clients using XCB, then why not call XInitThreads in XGetXCBConnection instead?
Re: XInitThreads in library constructor breaks Motif!
Keith Packard writes: > As currently described, the fix is untested. I'd encourage you to apply > it and see if it works; that would at least provide some evidence that > it helps. It helps for Motif programs. But what about other X extensions that may want to run callbacks inside LockDisplay?
Re: XInitThreads in library constructor breaks Motif!
On October 31, 2022 7:08:50 AM GMT+08:00, Alan Coopersmith wrote: >On 10/29/22 19:00, Po Lu wrote: >> Motif assumes that you can call functions that lock the display inside >> XCheckIfEvent as it never calls XInitThreads itself. It also never uses >> threads. > >It was never up to Motif to decide whether or not to call XInitThreads() >- that decision was up to the applications, and if Motif assumed no >application would ever do that, it was buggy and incompatible with such >applications. Okay, so what about the countless Motif *applications* that don't use threads? Motif is not a thread-safe library, and is documented as such, so Motif applications (mine included) deliberately do not call XInitThreads. >https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/150 is >a proposed solution to the problem, but has not generated any interest >in testing it or moving it forward, suggesting either everyone is >building with the --disable-thread-safety-constructor option or is >using correctly written software that does not make such assumptions. Or most people are on Xlib 1.7.x. Anyway, the documentation for X*IfEvent says it is okay to call functions that take the display lock inside the predicate given that it does not change the event queue. I have written such software, and clearly the Open Group has too, along with XFCE, etc. Besides, how to move it forward? If it works, wouldn't the obvious solution be to install the change? If all that is needed is for someone to try it out, I'd be happy to do it, but I'm pretty sure the better solution is just to remove the call to XInitThreads in the first place. So please don't make arbitrary changes to documented behavior!! There are people who pay for Motif, and such things did not happen when both Motif and Xlib were being developed under the auspices of the Open Group. Thanks.
XInitThreads in library constructor breaks Motif!
Motif assumes that you can call functions that lock the display inside XCheckIfEvent as it never calls XInitThreads itself. It also never uses threads. Please revert the following change to Xlib: commit afcdb6fb0045c6186aa83d9298f327a7ec1b2cb9 Author: Adam Jackson Date: Tue Mar 22 18:24:29 2022 -0400 global: call XInitThreads() from the library's constructor There is really no point in not being thread safe, I measured, all you can see happen is noop performance gets like twice as slow and you have thread safety bugs. And we're using xcb as the transport which means we should expect threads in our clients anyway. Just do it. This assumes your compiler understands __attribute__((constructor)). If this is not your compiler, you can disable this with the appropriate configure flag, but be aware you're asking for bugs. Signed-off-by: Adam Jackson as it stops Motif from working (this is easily seen by dragging something onto a Motif program and observing the ensuing hang.)
How does glamor upload picture contents to a texture or fbo?
Is there documentation anywhere on glamor internals? And if there isn't, does anyone on this list know how glamor decides whether or not to upload pixmap contents to a texture (and/or fbo) when used as the source Picture to a CompositePictures request? This might end up being an X/Y question, so I'll say why I'm asking. I'm writing a Wayland compositor that runs on X, and so far the "fast path" of using PresentPixmap to display the contents of a buffer is working fine. However, XRender is used to composite buffer contents to the window when there are multiple subsurfaces attached, and I'm running into a problem where partially drawn contents from shared memory buffers end up displayed by the X server while glamor is enabled. exa and the Render acceleration in xf86-video-intel work fine, but the former is slow. The only way this can make sense is if wl_buffer.release is sent before the buffer contents are uploaded to the fbo, resulting in the Wayland client scribbling over the contents of the buffer before the X server has a chance to display them. So what I need to know is whether or not the pixmap contents have been uploaded (or copied somewhere) by the time an XSync after the Composite request completes, or if not, when the pixmap contents will have been uploaded. Thanks.
Additions to the Composite extension
Recently, I've found that in order to obey _NET_WM_SYNC_REQUEST events
use presentation at the same time in a reasonable fashion, one must be
able to determine whether or not the compositing manager has actually
redirected the window. Polling with an error trap and
XCompositeRedirectWindow is horribly inefficient, and there is no other
way for a client to be notified when redirection has changed.
These additions to the Composite extension constitute a minor revision
of that extension, and provide a mechanism for clients to listen to
redirection changes on a given window. The patches consist of additions
to xorgproto defining the new version of the extension, an
implementation in the X.Org server, and stubs in libXcomposite.
Since I've totally lost track of how X server development works today,
I've copied in the maintainer of libXcomposite, as this list does not
seem to get much attention. I hope that isn't a problem.
If these changes look good, I'd like for them to be installed. If
they're not, please tell me why, and I will try my best to fix them.
Thanks in advance.
>From 8c72d0297c1993cb2f8f121cd5bc2f5d7702bfd9 Mon Sep 17 00:00:00 2001
From: Po Lu
Date: Wed, 19 Oct 2022 15:43:38 +0800
Subject: [PATCH xorgproto] Add composite redirection notification events
Applications may want to monitor whether or not their windows are
redirected, in order to decide whether or not to wait for protocols
such as _NET_WM_FRAME_DRAWN when the compositor can choose to
unredirect them. This update to the Composite protocol allows
applications to select for events that are emitted upon changes to the
redirection state of arbitrary windows, and to query their state.
---
compositeproto.pc.in| 2 +-
compositeproto.txt | 145
configure.ac| 2 +-
include/X11/extensions/composite.h | 25 +++-
include/X11/extensions/compositeproto.h | 77 +
meson.build | 2 +-
6 files changed, 247 insertions(+), 6 deletions(-)
diff --git a/compositeproto.pc.in b/compositeproto.pc.in
index 91f2422..f053a3a 100644
--- a/compositeproto.pc.in
+++ b/compositeproto.pc.in
@@ -3,5 +3,5 @@ includedir=@includedir@
Name: CompositeExt
Description: Composite extension headers
-Version: 0.4.2
+Version: 0.5.0
Cflags: -I${includedir}
diff --git a/compositeproto.txt b/compositeproto.txt
index c1a7878..febdde3 100644
--- a/compositeproto.txt
+++ b/compositeproto.txt
@@ -303,3 +303,148 @@ operations other than QueryVersion.
Composite Overlay Window on the screen specified by the
argument 'window'. A screen's Composite Overlay Window is
unmapped when there are no longer any clients using it.
+
+12. Composite Input (0.5)
+
+Applications may wish to monitor whether or not their windows are redirected.
+The CompositeSelectInput request paried with the RedirectNotify event allows
+monitoring whether or not a window is redirected, and why it was redirected or
+unredirected.
+
+12.1 Types
+
+ COMPOSITEEVENT { RedirectNotify,
+ RedirectStateNotify, }
+ REDIRECTREASON { WindowRedirected,
+ ParentRedirected,
+ ChildCreated,
+ ChildReparented, }
+
+12.2 Events
+
+RedirectNotify
+
+ reason:REDIRECTREASON
+ update:SETofUPDATETYPE or Unredirected
+ window:Window
+ timestamp: Timestamp
+
+ This event is emitted when a client has selected for RedirectNotify
+ events on a window through the CompositeSelectInput request, and the
+ storage for its contents may have changed.
+
+If 'type' is Unredirected, then the window contents are no longer
+ directed to off-screen storage.
+
+ Otherwise, 'type' is a mask containing the update modes given to all
+ RedirectWindow or RedirectSubwindows requests that are currently causing
+ the window contents to be redirected. 'reason' specifies why the event
+ was sent, and has the following meanings:
+
+ WindowRedirected
+
+ The window may have been redirected or unredirected as
+ the result of being specified as the 'window' argument
+ to a RedirectWindow, UnredirectWindow,
+ UnredirectSubwindows, or RedirectSubwindows request, or
+ its equivalent, such as client destruction.
+
+ ParentRedirected
+
+ The window may have been redirected or unredirected as
+ the result of being a child of the 'window' argument to
+ a RedirectSubwindows or UnredirectSubwindows request at
+ the time the request was made, or its equivalent, such
+ as client destruction.
+
+ ChildCreated
+
+ The window was redirected as the result of being created
+ as a child of the 'window' argument to a
+ RedirectSubwindows request, after that request was made.
+
+ ChildReparented
+
+ The window was redirected or unredirected as a result of
+ being reparented to or away from the 'window' argument
+ of a RedirectSubwindows request, either by a
+ ReparentWindow request or save-set process
Re: Questions about XPresent
Keith Packard writes: > I got started creating a 'semi-automatic compositing' design where the > compositing manager could tell the X server "Hey, just composite this > window to the screen in the 'obvious' way and don't bother me with the > details". Wouldn't that just be RedirectWindow with automatic updates, except without using CopyArea when the depths are the same and the window is not opaque, instead using CompositePicture with PictOpOver synchronized with vblank? I think it would have to composite the contents of windows underneath too in some cases. There would also have to be a way for clients using visuals with an alpha channels to tell the X server which parts of their windows are opaque, similar to _NET_WM_OPAQUE_REGION in the wm-spec. That sounds surprisingly simple, as long you only "automatically composite" windows whose clip lists are the same as their bounds.
Re: Questions about XPresent
Michel Dänzer writes: > I'm thinking of this kind of scenario with a direct rendering compositor: > > 1. Compositor retrieves window pixmap, creates OpenGL texture object for it. > 2. Client sends PresentPixmap request, which results in replacing the window > pixmap. > 3. Client starts drawing new frame to the old window pixmap buffer. > 4. Compositor samples from texture object created in step 1. > > > I did actually try this a while ago, and saw artifacts due to this issue. Hmm, okay. I don't want to work out yet another ugly synchronization protocol, so how about this: the compositing manager makes a new "CompositeRedirectSubwindowPresentation" request on the root window, and receives every PresentPixmap request made by clients to a subwindow of the root window. It must then present the pixmap and send completion/idle notify events itself. That being said, there are definitely more details to work out here. For example, as I said earlier, what if a client draws to the window after PresentCompleteNotify? In that case, a damage notification will be generated, but the drawing will probably have been done to outdated display contents, so the compositor must also tell the X server the the pixmap that was presented in order for future drawing to work correctly. Does that make any sense? I'm thinking of a control flow that looks like this, when the client draws to the window after a pixmap has been presented by the compositor: Client Server Compositing manager PresentPixmap > PresentRequest ---> actually takes the pixmap specified and composites it to display. (window damage between PresentRequest and PresentCompleteNotify is ignored by the compositor.) SetPresentedPixmap <--- compositing manager completes PresentCompleteNotify <- presentation by "flipping". XDrawRectangle > X server copies from the presented pixmap to the window's off screen storage, and then draws the rectangle > DamageNotify PresentIdleNotify <- compositing manager composites the window pixmap and sends PresentIdleNotify for previously flipped pixmap.
Re: Questions about XPresent
Michel Dänzer writes: > You don't need to do anything special. The effect of PresentPixmap is > essentially the same as XCopyArea as far as the destination window > contents are concerned, regardless of how the presentation is > effectively performed. Thanks. > The problem with this is that there's no explicit transfer of > ownership of the window pixmap between the compositor and other > entities. So the compositor may end up reading from a stale window > pixmap after another presentation has already flipped in a new pixmap, > and the client may already be drawing a new frame to the pixmap used > by the compositor, resulting in visual artifacts. If the compositor issues a Composite request, then by the time the request arrives on the X server, won't the pixmap resource point to the data of the pixmap that is currently busy? Or are you saying that the X server keeps references to pixmap data that it has sent IdleNotify events for, and may use those pixmaps in response to a future request from the compositor? Thanks.
Re: Yet another leak in Xlib
Po Lu writes:
> Doesn't this leak found by Valgrind sound like a bug?
> For reference, here's how I'm calling XRegisterIMInstantiateCallback:
>
> XSetLocaleModifiers ("");
> XRegisterIMInstantiateCallback (compositor.display,
> XrmGetDatabase (compositor.display),
> (char *) compositor.resource_name,
> (char *) compositor.app_name,
> IMInstantiateCallback, NULL);
>
> and XMODIFIERS is:
>
> @im=ibus
>
> and the leak is:
>
> ==67186== 408 bytes in 1 blocks are definitely lost in loss record 272 of 344
> ==67186==at 0x484A464: calloc (vg_replace_malloc.c:1328)
> ==67186==by 0x490935F: _XimOpenIM (in /usr/lib64/libX11.so.6.4.0)
> ==67186==by 0x490F386: _XimRegisterIMInstantiateCallback (in
> /usr/lib64/libX11.so.6.4.0)
> ==67186==by 0x48FBDBD: XRegisterIMInstantiateCallback (in
> /usr/lib64/libX11.so.6.4.0)
> ==67186==by 0x4456B5: tiInitTextInput (text_input.c:837)
> ==67186==by 0x4079C5: dlMain (loader.c:205)
> ==67186==by 0x4079F2: main (loader.c:214)
>
> Thanks.
Please ignore this message. I wasn't subscribed when I sent it, so it
got stuck in moderation for 2 weeks.
Yet another leak in Xlib
Doesn't this leak found by Valgrind sound like a bug?
For reference, here's how I'm calling XRegisterIMInstantiateCallback:
XSetLocaleModifiers ("");
XRegisterIMInstantiateCallback (compositor.display,
XrmGetDatabase (compositor.display),
(char *) compositor.resource_name,
(char *) compositor.app_name,
IMInstantiateCallback, NULL);
and XMODIFIERS is:
@im=ibus
and the leak is:
==67186== 408 bytes in 1 blocks are definitely lost in loss record 272 of 344
==67186==at 0x484A464: calloc (vg_replace_malloc.c:1328)
==67186==by 0x490935F: _XimOpenIM (in /usr/lib64/libX11.so.6.4.0)
==67186==by 0x490F386: _XimRegisterIMInstantiateCallback (in
/usr/lib64/libX11.so.6.4.0)
==67186==by 0x48FBDBD: XRegisterIMInstantiateCallback (in
/usr/lib64/libX11.so.6.4.0)
==67186==by 0x4456B5: tiInitTextInput (text_input.c:837)
==67186==by 0x4079C5: dlMain (loader.c:205)
==67186==by 0x4079F2: main (loader.c:214)
Thanks.
pixman rectangle banding guarantee
Does pixman guarantee that pixman_region16_rectangles will return rectangles in the same X/Y banded format used by the X server?
Restore PictStandardA4
Emacs uses the PictStandardA4 picture format to display certain kinds of images. A "painfully slow" software fallback is not a problem, since the important thing is for the picture to be displayed in the first place. So there are "users in the wild", and in any case the X.Org server should hold itself to its own protocol specifications. Thus, as an Emacs developer, I'd like to ask for: https://gitlab.freedesktop.org/xorg/xserver/-/commit/436fd7e8b4966c305ea9c43f3c14c2ca04c35539 to be reverted, or for a major version bump of the rendering extension. That commit came as a nasty surprise when I updated my local X server. Backwards incompatible changes should *NOT* be made out of the blue. No matter how many applications you look at, you will always miss one or two. And there may be private code, or code someone will write in the future. "All the major toolkits and libXft" do *NOT* cover every application in the wild, especially not Emacs, whose X Windows code predates all of those libraries and toolkits and refrains from relying on any them. I cannot stress that enough. Emacs has never needed to deal with X servers breaking backwards compatibility in the past and it would be a shame for that to start now. They should be made in accordance with what renderproto.txt says: Major versions changes can introduce incompatibilities in existing functionality, minor version changes introduce only backward compatible changes.
Questions about XPresent
Hello. I have recently been working on a Wayland compositor that runs on X, and have several questions about the presentation extension. My compositor utilizes XRender for compositing subsurfaces with the contents of a toplevel window, and _NET_WM_SYNC_REQUEST to handle frame synchronization. However, I would like to use the presentation extension to efficiently copy surface contents provided by Wayland clients to the X toplevel window when there is no need for any compositing at all. Since _NET_WM_SYNC_REQUEST is being used to provide frame synchronization functionality, there is no need to have XPresent wait for vblank before copying or flipping the pixmap contents onto the toplevel window. That is ensured by specifying PresentOptionAsync and a target-msc of 0, which brings me to my first question. The documentation for PresentPixmap says: If 'options' contains PresentOptionAsync, and the 'target-msc' is less than or equal to the current msc for 'window', then the operation will be performed as soon as possible, not necessarily waiting for the next vertical blank interval. What does the "necessarily" here mean? That the presentation might still wait for vblank, even if PresentOptionAsync is specified, and the target MSC is less than the MSC of the window? What will a following SyncSetCounter do? Will the counter always be set to the new value after the presentation happens and the window damage is sent to the compositor? My second question is about drawing to the window after presentation happens. When direct presentation becomes impossible (by, for example, the Wayland client attaching a subsurface), then the results of any previously made presentation must be overwritten by the following Composite requests. What happens if a Composite request is made, targeting a window whose contents have been previously specified with PresentPixmap, especially if the pixmap has been flipped and not copied? Do I have to present an empty pixmap, wait for any PresentIdleNotify for the previously attached pixmap, or what? Or is there simply no safe way to draw to a window after presenting a pixmap onto it? That brings me to the third question. Wouldn't a good optimization be to "flip" the presented pixmap, if possible, into the contents of the window pixmap if the window were to be redirected? Compositing managers would then be able to read from the window immediately after presentation, without needing the contents of the presented pixmap to be copied to the window pixmap first. I also have another question not quite related to XPresent: is there a way to be notified once a window is unredirected by the compositing manager, because it is fullscreen and has no opaque region? Thanks in advance.
Re: Yet another leak in Xlib
Thomas Dickey writes: > looks okay reading the library code (src/xlibi18n/XDefaultIMIF.c, _CloseIM). > > xterm doesn't free that 'xim' value (and the XCloseIM manual page doesn't > say who's responsible for that -- though it's possible that some other > application developer read the library source code and is freeing it). XCloseIM (in IMWrap.c) frees the XIM value itself after calling close to deinitialize the input method. So I think the patch should be fine, and I've been running it for a while with no ill effect. Could it be installed? Thanks.
Re: Yet another leak in Xlib
Po Lu writes:
> ==67186== 408 bytes in 1 blocks are definitely lost in loss record 272 of 344
> ==67186==at 0x484A464: calloc (vg_replace_malloc.c:1328)
> ==67186==by 0x490935F: _XimOpenIM (in /usr/lib64/libX11.so.6.4.0)
> ==67186==by 0x490F386: _XimRegisterIMInstantiateCallback (in
> /usr/lib64/libX11.so.6.4.0)
> ==67186==by 0x48FBDBD: XRegisterIMInstantiateCallback (in
> /usr/lib64/libX11.so.6.4.0)
> ==67186==by 0x4456B5: tiInitTextInput (text_input.c:837)
> ==67186==by 0x4079C5: dlMain (loader.c:205)
> ==67186==by 0x4079F2: main (loader.c:214)
diff --git a/modules/im/ximcp/imInsClbk.c b/modules/im/ximcp/imInsClbk.c
index 95b379cb..c10e347f 100644
--- a/modules/im/ximcp/imInsClbk.c
+++ b/modules/im/ximcp/imInsClbk.c
@@ -212,6 +212,9 @@ _XimRegisterIMInstantiateCallback(
if( xim ) {
lock = True;
xim->methods->close( (XIM)xim );
+ /* XIMs must be freed manually after being opened; close just
+ does the protocol to deinitialize the IM. */
+ XFree( xim );
lock = False;
icb->call = True;
callback( display, client_data, NULL );
Does this seem right?
Re: incompatible change in XIMPreeditCallbacks
Alan Coopersmith writes: > Unfortunately, there is no one left reviewing Xlib patches who knows how > input methods work or what might be a breaking change, or who could write > documentation of such changes. We'd love for someone to step up and > take on such work. How about this? diff --git a/specs/libX11/CH13.xml b/specs/libX11/CH13.xml index 5ca59790..6be4f1c2 100644 --- a/specs/libX11/CH13.xml +++ b/specs/libX11/CH13.xml @@ -8045,7 +8045,11 @@ set to XIMPreeditPosition. When specified to any input method other than XIMPreeditPosition, -this XIC value is ignored. +this XIC value is ignored. Some Xlib implementations +will allow this to be set when +XNInputStyle is set to +XIMPreeditCallbacks. Behavior in that case is +implementation defined.
Yet another leak in Xlib
(Resending after subscribing as the list moderator seems to be
unresponsive.)
Doesn't this leak found by Valgrind from libX11 sound like a bug? For
reference, here's how I'm calling XRegisterIMInstantiateCallback:
XSetLocaleModifiers ("");
XRegisterIMInstantiateCallback (compositor.display,
XrmGetDatabase (compositor.display),
(char *) compositor.resource_name,
(char *) compositor.app_name,
IMInstantiateCallback, NULL);
and XMODIFIERS is:
@im=ibus
and the leak is:
==67186== 408 bytes in 1 blocks are definitely lost in loss record 272 of 344
==67186==at 0x484A464: calloc (vg_replace_malloc.c:1328)
==67186==by 0x490935F: _XimOpenIM (in /usr/lib64/libX11.so.6.4.0)
==67186==by 0x490F386: _XimRegisterIMInstantiateCallback (in
/usr/lib64/libX11.so.6.4.0)
==67186==by 0x48FBDBD: XRegisterIMInstantiateCallback (in
/usr/lib64/libX11.so.6.4.0)
==67186==by 0x4456B5: tiInitTextInput (text_input.c:837)
==67186==by 0x4079C5: dlMain (loader.c:205)
==67186==by 0x4079F2: main (loader.c:214)
Thanks.
BTW, what about the keycode name leak that I found earlier? The fix
involving the resource name quark table seems like it would actually
work.
Also, there has been at least one recent change to Xlib that (if my
memory of the documentation is correct) goes against the documentation,
notably allowing the client to send XNSpotLocation when
XIMPreeditCallbacks is specified in the input method style:
The XNSpotLocation argument specifies to the input method the
coordinates of the spot to be used by an input method executing with
XNInputStyle set to XIMPreeditPosition. When specified to any input
method other than XIMPreeditPosition, this XIC value is ignored. <=
The x coordinate specifies the position where the next character would
be inserted. The y coordinate is the position of the baseline used by
the current text line in the focus window. The x and y coordinates are
relative to the focus window, if it has been set; otherwise, they are
relative to the client window. If neither the focus window nor the
client window has been set, the results are undefined.
The value of the argument is a pointer to a structure of type XPoint.
That is an incompatible change and should at least be documented. I
remember having written input servers and clients in the past that
expect things to work the old way.
Re: Leak in XKeysymToString
Keith Packard writes: > Maybe something like this? That would work, yes. Thanks.
Re: Leak in XKeysymToString
Alan Coopersmith writes: > Thanks - while gitlab is our preferred method, when that's not possible, > we prefer using the xorg-devel mailing list (cc'ed) instead of trying to > guess which individual developer to contact. Thanks for explaining. I thought xorg-devel was shut down and replaced by Discourse, but I'm probably confusing it with GNOME lists. > This bug has been previously reported, but no one has developed a good > fix yet - I don't know if many XKeysymToString callers keep references to > the returned pointers and would be broken if those pointers suddenly had a > different string or were invalid due to a realloc() call. FWIW, libxkbfile (which triggers this bug) does not. I haven't seen code do that myself either. > If they do, then perhaps a safer fix would be to keep a list of the > returned strings and reuse the pointer when the same keysym is seen again > instead of continuing to allocate new memory each time. Probably in an assoc table or similar structure, right? Using a plain list seems horribly inefficient once XKeysymToString is called for enough keysyms. > The safest fix would be to just have makekeys generate these values > when building the keysym tables, but that would increase the memory > usage for those tables. That could work too, yes. > If not, and we decided overwriting the string each time was acceptable, > I'd just go with "static char s[10];" instead of malloc/realloc at all. Sure.
