[PATCH] modesetting: Update fb_id from shadow allocate and destroy if not set
Looks like drmModeDirtyFB() stopped working at some point and we now call it with fb_id of zero for rotated displays. This will stop displays relying on DRM_IOCTL_MODE_DIRTYFB ioctl to display anything. This regression probably with commit 3dcd591fa9b7 ("modesetting: Add support for using RandR shadow buffers") that inroduced rotate_fb_id. Let's fix this issue by copying rotate_fb_id to fb_id if zero and then reset it back to zero after we're done with the rotation. After the fix we then call drmModeDirtyFB() we then have the following fb_id changes: $ startx drmmode_get_default_bpp: fb_id: 51 drmmode_get_default_bpp: cleared fb_id dispatch_dirty: ms->drmmode.fb_id: 0 fb_id: 0 drmmode_set_mode_major: drmmode->fb_id: 52 dispatch_dirty: ms->drmmode.fb_id: 52 fb_id: 52 ... $ xrandr --output DSI-1 --rotate right drmmode_xf86crtc_resize: cleared old_fb_id dispatch_dirty: ms->drmmode.fb_id: 0 fb_id: 0 drmmode_shadow_allocate: drmmode_crtc->rotate_fb_id: 50 dispatch_dirty: ms->drmmode.fb_id: 50 fb_id: 50 ... $ xrandr --output DSI-1 --rotate normal drmmode_shadow_destroy: cleared drmmode_crtc->rotate_fb_id dispatch_dirty: ms->drmmode.fb_id: 0 fb_id: 0 dispatch_dirty: ms->drmmode.fb_id: 0 fb_id: 0 dispatch_dirty: ms->drmmode.fb_id: 0 fb_id: 0 drmmode_set_mode_major: drmmode->fb_id: 51 dispatch_dirty: ms->drmmode.fb_id: 51 fb_id: 51 ... Cc: Hans De Goede Cc: Jason Ekstrand Cc: Laurent Pinchart Cc: Sebastian Reichel Cc: Tomi Valkeinen Signed-off-by: Tony Lindgren --- hw/xfree86/drivers/modesetting/drmmode_display.c | 10 ++ hw/xfree86/drivers/modesetting/drmmode_display.h | 1 + 2 files changed, 11 insertions(+) diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c --- a/hw/xfree86/drivers/modesetting/drmmode_display.c +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c @@ -997,6 +997,12 @@ drmmode_shadow_allocate(xf86CrtcPtr crtc, int width, int height) return NULL; } +/* Must have proper drmmode->fb_id for drmModeDirtyFB() */ +if (!drmmode->fb_id) { +drmmode->fb_id = drmmode_crtc->rotate_fb_id; +drmmode_crtc->reset_fb_id = TRUE; +} + #ifdef GLAMOR_HAS_GBM if (drmmode->gbm) return drmmode_crtc->rotate_bo.gbm; @@ -1084,6 +1090,10 @@ drmmode_shadow_destroy(xf86CrtcPtr crtc, PixmapPtr rotate_pixmap, void *data) if (data) { drmModeRmFB(drmmode->fd, drmmode_crtc->rotate_fb_id); +if (drmmode_crtc->reset_fb_id && +drmmode->fb_id == drmmode_crtc->rotate_fb_id) +drmmode->fb_id = 0; +drmmode_crtc->reset_fb_id = FALSE; drmmode_crtc->rotate_fb_id = 0; drmmode_bo_destroy(drmmode, &drmmode_crtc->rotate_bo); diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.h b/hw/xfree86/drivers/modesetting/drmmode_display.h --- a/hw/xfree86/drivers/modesetting/drmmode_display.h +++ b/hw/xfree86/drivers/modesetting/drmmode_display.h @@ -98,6 +98,7 @@ typedef struct { drmmode_bo rotate_bo; unsigned rotate_fb_id; +Bool reset_fb_id; PixmapPtr prime_pixmap; PixmapPtr prime_pixmap_back; -- 2.16.1 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] modesetting: Update fb_id from shadow allocate and destroy if not set
Hi, * Keith Packard [180203 20:33]: > Tony Lindgren writes: > > > Looks like drmModeDirtyFB() stopped working at some point and we now > > call it with fb_id of zero for rotated displays. This will stop displays > > relying on DRM_IOCTL_MODE_DIRTYFB ioctl to display anything. > > This bug just breaks rotated displays, right? Yes it only happens after rotating the LCD. > > This regression probably with commit 3dcd591fa9b7 ("modesetting: Add > > support for using RandR shadow buffers") that inroduced rotate_fb_id. > > > > Let's fix this issue by copying rotate_fb_id to fb_id if zero and then > > reset it back to zero after we're done with the rotation. > > I think this fix assumes that there is only one CRTC running; if you've > got two, one scanning from the regular fb_id and the other from a > rotated fb_id, then you want to dirty each separately. Hmm yes I noticed we're doing dirtyfb flushes also on the HDMI on the same device in addition to the LCD, which seems unnecessary. > I think the right place to fix this is in dispatch_dirty_regions, which > should be walking the set of active CTRCs and damaging the appropriate > fb_id for each one? OK, Jason, any comments on this? Regards, Tony ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: Making Composite better for interactive apps
On Mon, Feb 5, 2018 at 4:27 AM, Keith Packard wrote: > Giuseppe Bilotta writes: > >> I'm not sure if this could be implemented without making the server >> more susceptible to DOS attacks, though (basically the same issue I >> see in the blocking AutoList geometry change notification). > > In that case, the Window Manager can already perform DoS attacks by > simply never mapping the window. Similarly, the Compositing Manager can > DoS the system by never painting anything. Given that one generally > starts a Window Manager/Compositing Manager at the start of the session, > and that only one of either can be running at a time, there shouldn't be > any way for such a DoS to occur. Eh, sometimes I miss the obvious 8-D. So the only difference between the two approaches is that in the current async way the server can cover the latency that would come from waiting for the WM to complete the mapping (or in general other such requests) by processing other events, but this comes at the cost of the complexity inevitably deriving from the asynchronicity itself, both in terms of protocol and in things like trying to guarantee consistency between disparate events affecting the same windows (such as the geometry changes' interaction with the presentation that we discussed so far). I must say I'm a bit conflicted about this. I actually like the mostly asynchronous nature of the X11 protocol (but that might just be a perversion of mine developed over years of HPC). In fact, the thing I like the least about it is that the C language doesn't have something that maps naturally to it (while the C++11 std::future actually do). I do agree however that having some better ways to enforce sequencing in some circumstances would be preferrable. > In any case, we're not likely to change how window management works at > this point; toolkits have dealt with re-directed configuration for over > thirty years at this point. Well, obviously, these would be more along general thoughts about how things could be different for X12 ;-) Or at least design principles that may be kept in mind for future extensions. -- Giuseppe "Oblomov" Bilotta ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xrandr v4 2/5] xrandr: stricter --scale argument parsing
Am 05.02.2018 02:47, schrieb Giuseppe Bilotta: > We used to accept something like --scale 2x3junk as a valid input > (scaling x by 2 and y by 3), even though this isn't really a valid > scaling factor. > > Fix by making sure there is nothing after the parsed number(s). > > Signed-off-by: Giuseppe Bilotta > --- > xrandr.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/xrandr.c b/xrandr.c > index 1085a95..6a38cf2 100644 > --- a/xrandr.c > +++ b/xrandr.c > @@ -3014,11 +3014,12 @@ main (int argc, char **argv) > if (!strcmp ("--scale", argv[i])) > { > double sx, sy; > + char junk; > if (!config_output) argerr ("%s must be used after --output\n", > argv[i]); > if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]); > - if (sscanf (argv[i], "%lfx%lf", &sx, &sy) != 2) > + if (sscanf (argv[i], "%lfx%lf%c", &sx, &sy, &junk) != 2) > { > - if (sscanf (argv[i], "%lf", &sx) != 1) > + if (sscanf (argv[i], "%lf%c", &sx, &junk) != 1) > argerr ("failed to parse '%s' as a scaling factor\n", > argv[i]); > sy = sx; > } can the scanf be converted to strtod ? there you get an endpointer by default. re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] glx: Only assign 8 bpc fbconfigs for composite visuals.
Commit 91c42093b248 ("glx: Duplicate relevant fbconfigs for compositing visuals") adds many new depth 32 fbconfigs as composite visuals. On a X-Screen running at depth 24, this also adds bgra 10-10-10-2 fbconigs, as they also have config.rgbBits == 32, but these are not displayable on a depth 24 screen, leading to visually corrupted desktops under some compositors, e.g., fdo bug 104597 "Compton weird colors" when running compton with "compton --backend glx". Be more conservative for now and only select fbconfigs with 8 bpc red, green, blue components for composite visuals. Fixes: 91c42093b248 ("glx: Duplicate relevant fbconfigs for compositing visuals") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104597 Signed-off-by: Mario Kleiner Cc: Thomas Hellstrom Cc: Adam Jackson --- glx/glxdricommon.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/glx/glxdricommon.c b/glx/glxdricommon.c index d3136e8..dbf199c 100644 --- a/glx/glxdricommon.c +++ b/glx/glxdricommon.c @@ -218,6 +218,9 @@ createModeFromConfig(const __DRIcoreExtension * core, if (duplicateForComp && (render_type_is_pbuffer_only(renderType) || config->config.rgbBits != 32 || + config->config.redBits != 8 || + config->config.greenBits != 8 || + config->config.blueBits != 8 || config->config.visualRating != GLX_NONE || config->config.sampleBuffers != 0)) { free(config); -- 2.7.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xrandr v4 2/5] xrandr: stricter --scale argument parsing
On Mon, Feb 5, 2018 at 10:44 AM, walter harms wrote: > > Am 05.02.2018 02:47, schrieb Giuseppe Bilotta: >> { >> double sx, sy; >> + char junk; >> if (!config_output) argerr ("%s must be used after --output\n", >> argv[i]); >> if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]); >> - if (sscanf (argv[i], "%lfx%lf", &sx, &sy) != 2) >> + if (sscanf (argv[i], "%lfx%lf%c", &sx, &sy, &junk) != 2) >> { >> - if (sscanf (argv[i], "%lf", &sx) != 1) >> + if (sscanf (argv[i], "%lf%c", &sx, &junk) != 1) >> argerr ("failed to parse '%s' as a scaling factor\n", >> argv[i]); >> sy = sx; >> } > > can the scanf be converted to strtod ? there you get an endpointer by default. I'm not a big fan of strtod because with it it's impossible to know if a conversion actually happened. xrandr --scale ' ' would actually be accepted (resulting in a scale value of 0), while the scanf catches it. For the same reason I use two sscanf instead of a single one (because that wouldn't be able to catch something like xrandr --scale 1j). Of course there's also to be said that we could reject a scale factor of 0, regardless of whether it comes from a correct parsing of the string '0.0' or from the parse of an empty string (but of course then we couldn't customize the error message to differentiate between “incorrect parse” and “value out of range”). -- Giuseppe "Oblomov" Bilotta ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] glx: Only assign 8 bpc fbconfigs for composite visuals.
On 02/05/2018 11:20 AM, Mario Kleiner wrote: Commit 91c42093b248 ("glx: Duplicate relevant fbconfigs for compositing visuals") adds many new depth 32 fbconfigs as composite visuals. On a X-Screen running at depth 24, this also adds bgra 10-10-10-2 fbconigs, as they also have config.rgbBits == 32, but these are not displayable on a depth 24 screen, leading to visually corrupted desktops under some compositors, e.g., fdo bug 104597 "Compton weird colors" when running compton with "compton --backend glx". Be more conservative for now and only select fbconfigs with 8 bpc red, green, blue components for composite visuals. Fixes: 91c42093b248 ("glx: Duplicate relevant fbconfigs for compositing visuals") Bugzilla: https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D104597&d=DwIBAg&c=uilaK90D4TOVoH58JNXRgQ&r=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ&m=IbtkCrmjzJVhB0PdaE0y9A3Zqx2CEYhUPvtI6PeGSEo&s=6MOlztrQC3tRtcJvqesPVJ1ri_ILRWLMh-iZbrs7NJ0&e= Signed-off-by: Mario Kleiner Cc: Thomas Hellstrom Cc: Adam Jackson --- glx/glxdricommon.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/glx/glxdricommon.c b/glx/glxdricommon.c index d3136e8..dbf199c 100644 --- a/glx/glxdricommon.c +++ b/glx/glxdricommon.c @@ -218,6 +218,9 @@ createModeFromConfig(const __DRIcoreExtension * core, if (duplicateForComp && (render_type_is_pbuffer_only(renderType) || config->config.rgbBits != 32 || + config->config.redBits != 8 || + config->config.greenBits != 8 || + config->config.blueBits != 8 || config->config.visualRating != GLX_NONE || config->config.sampleBuffers != 0)) { free(config); LGTM. Reviewed-by: Thomas Hellstrom ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xrandr v4 2/5] xrandr: stricter --scale argument parsing
Am 05.02.2018 11:24, schrieb Giuseppe Bilotta: > On Mon, Feb 5, 2018 at 10:44 AM, walter harms wrote: >> >> Am 05.02.2018 02:47, schrieb Giuseppe Bilotta: >>> { >>> double sx, sy; >>> + char junk; >>> if (!config_output) argerr ("%s must be used after --output\n", >>> argv[i]); >>> if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]); >>> - if (sscanf (argv[i], "%lfx%lf", &sx, &sy) != 2) >>> + if (sscanf (argv[i], "%lfx%lf%c", &sx, &sy, &junk) != 2) >>> { >>> - if (sscanf (argv[i], "%lf", &sx) != 1) >>> + if (sscanf (argv[i], "%lf%c", &sx, &junk) != 1) >>> argerr ("failed to parse '%s' as a scaling factor\n", >>> argv[i]); >>> sy = sx; >>> } >> >> can the scanf be converted to strtod ? there you get an endpointer by >> default. > > I'm not a big fan of strtod because with it it's impossible to know if > a conversion actually happened. xrandr --scale ' ' would actually be > accepted (resulting in a scale value of 0), while the scanf catches > it. For the same reason I use two sscanf instead of a single one > (because that wouldn't be able to catch something like xrandr --scale > 1j). > > Of course there's also to be said that we could reject a scale factor > of 0, regardless of whether it comes from a correct parsing of the > string '0.0' or from the parse of an empty string (but of course then > we couldn't customize the error message to differentiate between > “incorrect parse” and “value out of range”). > Ok, i understand. My guess is that a scale factor of 0 would be rejected or interessting things will happen re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 14/19] present: Add window flip mode
On 2018-02-02 09:56 PM, Keith Packard wrote: > Michel Dänzer writes: > >> On 2018-02-02 10:42 AM, Roman Gilg wrote: >>> It's because of what you made me aware of in the previous patch set: >>> the window original pixmap needs to have the updated content from the >>> flip Pixmap, otherwise for example screenshot applications won't work >>> anymore. I tested it with xwd. >> >> FWIW, my concerns were about using sub-surfaces in cases where the >> presented pixmap dimensions don't match the window pixmap dimensions. > > And this is the main reason I didn't really worry about flipping window > pixmaps -- the application window was generally contained within a > window manager frame, which (typically) also contains window management > decorations. So the application window is generally smaller than the > frame, so the pixmap can't just be flipped. With client-side > decorations, or other techniques that make the client geometry match > that of the redirected window, figuring out how to do this makes good > sense. The motivation for this work is fullscreen applications with Xwayland. >>> I also tried to not copy, but set the window pixmap to the flip >>> pixmap. The problem I encountered here, is that the window original >>> pixmap can be controlled by the client. >> >> How? > > If it's a DRM buffer passed to the server, then the client has a direct > handle to the object? Above, Roman wrote the client can control *the original window pixmap*. I'm asking how it can do that. >> The inability to queue a presentation to the next MSC is more of a step >> back compared to the status quo. > > I'm about to go write up some ideas I'm working on that will make it > possible to more regularly display redirected windows at the target MSC, > and to reliably report which MSC the window contents were displayed at. > > I mention this because I think there are some parallels between that > work and this. Not really seeing that. Composite isn't involved with Xwayland. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 14/19] present: Add window flip mode
On 2018-02-02 11:11 PM, Keith Packard wrote: > Michel Dänzer writes: > >> But this seems irrelevant for per-window flips. In this case, the window >> pixmap isn't used for anything else after flipping, so having direct >> access to the pixmap doesn't allow the client to do anything it couldn't >> do anyway using the X11 protocol. (There might be exceptions to this if >> the window wasn't created by the presenting client, and the Security >> extension comes into play. But that would be a rather exotic scenario, >> so I don't think we need to consider it here.) > > Agreed. Discarding the window pixmap during per-window flips seems like > it should be fine. Of course, you need to replace the window pixmap for > all windows sharing the original redirection, and that might include the > parent. That's mildly concerning as now the client has control over the > contents of another application's window outside of the X protocol. Ignoring the Security extension, the client has the same control over the contents of another application's window *using* the X protocol, doesn't it? If this is a concern, Present could try allocating a new window pixmap to unflip to, and just leave the last flip pixmap as the window pixmap if that fails (which it "never" will, in basically any scenario where the X server can otherwise continue working normally). -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
xorgproto build system (was Re: [PATCH xserver 00/10] Implement RandR 1.6)
On Sat, 2018-02-03 at 14:28 -0800, Keith Packard wrote: > Adam Jackson writes: > > > Got impatient and hacked on this a bit more: > > > > https://cgit.freedesktop.org/xorg/proto/xorgproto/ > > Thanks for taking this on. It looks great. > > I have a question -- is there any place which we care about that doesn't > have meson support yet? If not, then might I gently suggest that we > simply never release this with autotools support? I'm likely to want new protocol bits in a future RHEL7 update, yeah. I don't know how many such updates, but I'd bet at least one. If it were just this one package I'd be fine with backporting stuff to the old split modules in 7, but if that assumption bubbled out to xserver's build system - that needing RANDR 1.6 implied already having meson - that'd be a bit more annoying. Now, can we call the meson build official and do the releases from that, probably yes with some more work. util/modular/release.sh doesn't know about meson yet, which is straightforward to add. The tarballs would be .tar.xz. xz has been optionally supported since 2013, this would make it mandatory unless we created gz/bz2 ourselves. Would anyone have a problem with only making xz tarballs? > Also, is there any way we could have the 'autogen.sh' file (or similar) > do all of the necessary meson bits so I don't have to remember how to > run that command? I bet we can! gnome-shell has an adaptor script for the gnome build API which expects a ./configure, and it's even BSD licensed. We could stash a copy of that in util/modular for future reference. - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 2/4] glx: Use vnd layer for dispatch (v3)
On 02/02/2018 12:15 PM, Adam Jackson wrote: The big change here is MakeCurrent and context tag tracking. We now delegate context tags entirely to the vnd layer, and simply store a pointer to the context state as the tag data. If a context is deleted while it's current, we allocate a fake ID for the context and move the context state there, so the tag data still points to a real context. As a result we can stop trying so hard to detach the client from contexts at disconnect time and just let resource destruction handle it. Since vnd handles all the MakeCurrent protocol now, our request handlers for it can just be return BadImplementation. We also remove a bunch of LEGAL_NEW_RESOURCE, because now by the time we're called vnd has already allocated its tracking resource on that XID. v2: Update to match v2 of the vnd import, and remove more redundant work like request length checks. v3: Add/remove the XID map from the vendor private thunk, not the backend. (Kyle Brenneman) Signed-off-by: Adam Jackson --- configure.ac | 2 +- glx/createcontext.c| 2 - glx/glxcmds.c | 212 + glx/glxcmdsswap.c | 98 +--- glx/glxext.c | 348 + glx/glxext.h | 4 + glx/glxscreens.h | 1 + glx/glxserver.h| 5 - glx/xfont.c| 2 - hw/kdrive/ephyr/ephyr.c| 2 +- hw/kdrive/ephyr/meson.build| 1 + hw/kdrive/src/kdrive.c | 3 + hw/vfb/InitOutput.c| 2 + hw/vfb/meson.build | 3 +- hw/xfree86/Makefile.am | 5 + hw/xfree86/common/xf86Init.c | 2 +- hw/xfree86/dixmods/glxmodule.c | 1 + hw/xfree86/meson.build | 1 + hw/xquartz/darwin.c| 4 +- hw/xwayland/Makefile.am| 1 + hw/xwayland/meson.build| 1 + hw/xwayland/xwayland.c | 2 + include/glx_extinit.h | 5 +- 23 files changed, 328 insertions(+), 379 deletions(-) In __glXDisp_DestroyContext, doesn't it need to record the fake XID that it generates? If I'm reading it right, if the client deletes a current context and later unbinds it, then xorgGlxMakeCurrent will call FreeResourceByType with the original (already freed) XID, not with the fake one. In xorgGlxThunkRequest, it needs to call glxServer.removeXIDMap to remove the XID for a GLXDestroyGLXPbufferSGIX request. The handling for created XID's in xorgGlxThunkRequest looks correct. As a minor note, you could set the "resource" variable in the switch statement, rather than in a separate if/else block. Also, I think the "if (!vendor)" block after the switch is dead code. Every branch in the switch assigns something to vendor and returns an error if it gets NULL. -Kyle ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: xorgproto build system (was Re: [PATCH xserver 00/10] Implement RandR 1.6)
On Mon, 2018-02-05 at 10:38 -0500, Adam Jackson wrote: > Now, can we call the meson build official and do the releases from > that, probably yes with some more work. Update: "some more work" is turning out to be more than I want to spend on this at the moment. release.sh's use of git worktrees is confusing meson, such that I can't easily make 'ninja -C _build dist' work. I'm sure that's fixable but we're already way too far down far too boring of a rabbit hole here. 2018.1 release will be built from autotools, we can switch later. > Would anyone have a problem with only making xz tarballs? This would still be nice to know the answer to. > I bet we can! gnome-shell has an adaptor script for the gnome build API > which expects a ./configure, and it's even BSD licensed. We could stash > a copy of that in util/modular for future reference. I've done this, and added some trivial compatibility for --enable-foo flags. It's not hard to wire this up in autogen.sh to magically redirect to the meson build, but until that has a working 'make distcheck' that's not really an option as otherwise you could never release it. - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 14/19] present: Add window flip mode
Michel Dänzer writes: > Ignoring the Security extension, the client has the same control over > the contents of another application's window *using* the X protocol, > doesn't it? Yeah, good point -- it could easily call DRI3BufferFromPixmap for the window pixmap and have exactly the same access. > If this is a concern, Present could try allocating a new window pixmap > to unflip to, and just leave the last flip pixmap as the window pixmap > if that fails (which it "never" will, in basically any scenario where > the X server can otherwise continue working normally). Given your point above, I can't think of any reason to not just use the provided pixmap as the window pixmap. It does mean we'll be trusting that the application doesn't accidentally re-use the buffer at some later time, but as you say, there's no actual security benefit. -- -keith signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 14/19] present: Add window flip mode
Michel Dänzer writes: > The motivation for this work is fullscreen applications with Xwayland. I think this could be equally useful for windowed applications as well as it should allow them to avoid the copy to the window pixmap and (perhaps) land in an overlay in whatever wayland server is running? Hrm. If the presented-to window is nested within an outer window, and that window is the one with the pixmap, I assume this doesn't still work? I'm not sure this actually matters as I don't think you're reparenting at this point? > Not really seeing that. Composite isn't involved with Xwayland. Ok, just seemed like we were playing in similar areas, avoiding a copy from the flip buffer to the window buffer. -- -keith signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 00/10] Implement RandR 1.6 (leases and non-desktop)
On Sat, Feb 03, 2018 at 02:28:58PM -0800, Keith Packard wrote: > Adam Jackson writes: > > > Got impatient and hacked on this a bit more: > > > > https://cgit.freedesktop.org/xorg/proto/xorgproto/ > > Thanks for taking this on. It looks great. > > I have a question -- is there any place which we care about that doesn't > have meson support yet? If not, then might I gently suggest that we > simply never release this with autotools support? As far as OpenBSD is concerned (and my work as maintainer) dropping autotools in favor of meson in general would be a major PITA. For the proto case, we actually don't use the autotools build system for the split packages version, because of the overhead, and the (relative) simplicity of just writing BSD makefiles for these. I haven't looked that the merged xproto yet though. -- Matthieu Herrb signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: xorgproto build system (was Re: [PATCH xserver 00/10] Implement RandR 1.6)
On 02/ 5/18 11:06 AM, Adam Jackson wrote: > On Mon, 2018-02-05 at 10:38 -0500, Adam Jackson wrote: >> Would anyone have a problem with only making xz tarballs? > > This would still be nice to know the answer to. I wouldn't, since we deal with those for GNOME already. -- -Alan Coopersmith- alan.coopersm...@oracle.com Oracle Solaris Engineering - https://blogs.oracle.com/alanc ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 2/4] glx: Use vnd layer for dispatch (v3)
On Mon, 2018-02-05 at 12:05 -0700, Kyle Brenneman wrote: > In __glXDisp_DestroyContext, doesn't it need to record the fake XID that > it generates? If I'm reading it right, if the client deletes a current > context and later unbinds it, then xorgGlxMakeCurrent will call > FreeResourceByType with the original (already freed) XID, not with the > fake one. Hah, yes it does. What I'd written would have GCd the context at client disconnect but not before. We do look up the context by tag but then free it by its (memory of its) ID, so we need to update that ID when we ghost it. Nice catch! > In xorgGlxThunkRequest, it needs to call glxServer.removeXIDMap to > remove the XID for a GLXDestroyGLXPbufferSGIX request. > > The handling for created XID's in xorgGlxThunkRequest looks correct. As > a minor note, you could set the "resource" variable in the switch > statement, rather than in a separate if/else block. > > Also, I think the "if (!vendor)" block after the switch is dead code. > Every branch in the switch assigns something to vendor and returns an > error if it gets NULL. All quite right. New version in a second, the thunk was also missing some bounds checking. - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 2/5] glx: Use vnd layer for dispatch (v4)
The big change here is MakeCurrent and context tag tracking. We now delegate context tags entirely to the vnd layer, and simply store a pointer to the context state as the tag data. If a context is deleted while it's current, we allocate a fake ID for the context and move the context state there, so the tag data still points to a real context. As a result we can stop trying so hard to detach the client from contexts at disconnect time and just let resource destruction handle it. Since vnd handles all the MakeCurrent protocol now, our request handlers for it can just be return BadImplementation. We also remove a bunch of LEGAL_NEW_RESOURCE, because now by the time we're called vnd has already allocated its tracking resource on that XID. v2: Update to match v2 of the vnd import, and remove more redundant work like request length checks. v3: Add/remove the XID map from the vendor private thunk, not the backend. (Kyle Brenneman) v4: Fix deletion of ghost contexts (Kyle Brenneman) Signed-off-by: Adam Jackson vnd layer fixup --- configure.ac | 2 +- glx/createcontext.c| 2 - glx/glxcmds.c | 213 ++--- glx/glxcmdsswap.c | 98 +--- glx/glxext.c | 349 + glx/glxext.h | 4 + glx/glxscreens.h | 1 + glx/glxserver.h| 5 - glx/xfont.c| 2 - hw/kdrive/ephyr/ephyr.c| 2 +- hw/kdrive/ephyr/meson.build| 1 + hw/kdrive/src/kdrive.c | 3 + hw/vfb/InitOutput.c| 2 + hw/vfb/meson.build | 3 +- hw/xfree86/Makefile.am | 5 + hw/xfree86/common/xf86Init.c | 2 +- hw/xfree86/dixmods/glxmodule.c | 1 + hw/xfree86/meson.build | 1 + hw/xquartz/darwin.c| 4 +- hw/xwayland/Makefile.am| 1 + hw/xwayland/meson.build| 1 + hw/xwayland/xwayland.c | 2 + include/glx_extinit.h | 5 +- 23 files changed, 330 insertions(+), 379 deletions(-) diff --git a/configure.ac b/configure.ac index 98b8ea2ed..dd073f252 100644 --- a/configure.ac +++ b/configure.ac @@ -1295,7 +1295,7 @@ if test "x$GLX" = xyes; then PKG_CHECK_MODULES([GL], $GLPROTO $LIBGL) AC_SUBST(XLIB_CFLAGS) AC_DEFINE(GLXEXT, 1, [Build GLX extension]) - GLX_LIBS='$(top_builddir)/glx/libglx.la' + GLX_LIBS='$(top_builddir)/glx/libglx.la $(top_builddir)/glx/libglxvnd.la' GLX_SYS_LIBS="$GLX_SYS_LIBS $GL_LIBS" else GLX=no diff --git a/glx/createcontext.c b/glx/createcontext.c index 76316db67..00c23fcdd 100644 --- a/glx/createcontext.c +++ b/glx/createcontext.c @@ -123,8 +123,6 @@ __glXDisp_CreateContextAttribsARB(__GLXclientState * cl, GLbyte * pc) if (req->length != expected_size) return BadLength; -LEGAL_NEW_RESOURCE(req->context, client); - /* The GLX_ARB_create_context spec says: * * "* If is not a valid GLXFBConfig, GLXBadFBConfig is diff --git a/glx/glxcmds.c b/glx/glxcmds.c index f4820d8e4..c5d6324d3 100644 --- a/glx/glxcmds.c +++ b/glx/glxcmds.c @@ -47,6 +47,7 @@ #include "indirect_table.h" #include "indirect_util.h" #include "protocol-versions.h" +#include "glxvndabi.h" static char GLXServerVendorName[] = "SGI"; @@ -135,6 +136,10 @@ _X_HIDDEN int validGlxContext(ClientPtr client, XID id, int access_mode, __GLXcontext ** context, int *err) { +/* no ghost contexts */ +if (id & SERVER_BIT) +return FALSE; + *err = dixLookupResourceByType((void **) context, id, __glXContextRes, client, access_mode); if (*err != Success || (*context)->idExists == GL_FALSE) { @@ -240,8 +245,6 @@ DoCreateContext(__GLXclientState * cl, GLXContextID gcId, __GLXcontext *glxc, *shareglxc; int err; -LEGAL_NEW_RESOURCE(gcId, client); - /* ** Find the display list space that we want to share. ** @@ -356,14 +359,11 @@ DoCreateContext(__GLXclientState * cl, GLXContextID gcId, int __glXDisp_CreateContext(__GLXclientState * cl, GLbyte * pc) { -ClientPtr client = cl->client; xGLXCreateContextReq *req = (xGLXCreateContextReq *) pc; __GLXconfig *config; __GLXscreen *pGlxScreen; int err; -REQUEST_SIZE_MATCH(xGLXCreateContextReq); - if (!validGlxScreen(cl->client, req->screen, &pGlxScreen, &err)) return err; if (!validGlxVisual(cl->client, pGlxScreen, req->visual, &config, &err)) @@ -376,14 +376,11 @@ __glXDisp_CreateContext(__GLXclientState * cl, GLbyte * pc) int __glXDisp_CreateNewContext(__GLXclientState * cl, GLbyte * pc) { -ClientPtr client = cl->client; xGLXCreateNewContextReq *req = (xGLXCreateNewContextReq *) pc; __GLXconfig *config; __GLXscreen *pGlxScreen; int err; -REQUEST_SIZE_MATCH(xGLXCreateNewContextReq); - if (!validGlxScreen(cl->client, req
Re: [PATCH 14/19] present: Add window flip mode
On Mon, 2018-02-05 at 12:53 +0100, Michel Dänzer wrote: > > > The inability to queue a presentation to the next MSC is more of a step > > > back compared to the status quo. > > > > I'm about to go write up some ideas I'm working on that will make it > > possible to more regularly display redirected windows at the target MSC, > > and to reliably report which MSC the window contents were displayed at. > > > > I mention this because I think there are some parallels between that > > work and this. > > Not really seeing that. Composite isn't involved with Xwayland. This is not entirely true, I don't think. In rootless mode you only get a wl_surface for manually-redirected windows; if you don't have a compositing window manager running, you don't get X windows. - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Anyone have a backup of XFree86 CVS?
I've asked this a couple of times on IRC, repeating it here for wider distribution. I had thought I had a git import laying around somewhere but I can't seem to find it. cvs.xfree86.org hasn't been alive in a long time, and it'd be a shame if that history just got lost, fragmentary as it was. If anyone has a working git import, or a cvsps mirror or other copy of the cvsroot, or knows someone who does, please let us know so we can preserve some history. - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: Making Composite better for interactive apps
Giuseppe Bilotta writes: > So the only difference between the two approaches is that in the > current async way the server can cover the latency that would come > from waiting for the WM to complete the mapping (or in general other > such requests) by processing other events, but this comes at the cost > of the complexity inevitably deriving from the asynchronicity itself, > both in terms of protocol and in things like trying to guarantee > consistency between disparate events affecting the same windows (such > as the geometry changes' interaction with the presentation that we > discussed so far). Yeah, so the core protocol was designed for this asynchronous operation, and all clients are currently required to handle it. However, we can't exactly change things so that this new Composite behavior could run asynchronously as clients would probably not respond well. > I do agree however that having some better ways to enforce sequencing > in some circumstances would be preferrable. And for things which happen no more often than once per frame, a bit of synchronization seems fine. Ok, I've been mulling this over since I woke up this morning. Let's go back a few steps and think about how we actually want Compositing to work in general. What we want is for the compositing manager to get a clean snapshot of the system and to then be able to present that on the screen each frame. That means disallowing *all* changes for the duration of the construction of the presented image. This is independent of whether some windows are composited within the X server or composited by the Compositing Manager. If a window gets resized between the time the compositing manager decides to start drawing and actually gets around to fetching the bits, then the resulting image will be inconsistent. Which is exactly the same if we have the X server doing that compositing, assuming that the Compositing Manager has control over the geometry in the Auto List. Which means this problem is at least partially separable from my original issue and we can think about it in some isolation. Here's a proposal for this issue: 1) Create a synchronization point within the X server which defines the start of presentation (for a single screen? globally?). This would generally be a time far enough before vblank that all rendering can be performed and still get the buffer queued before vblank so that the swap will work. 2) Between this point and the receipt of the PresentPixmap for this frame, freeze all window drawing and geometry changes. A client which sends a PresentPixmap or any window configuration request which would affect the ongoing presentation will be blocked. 3) Once the presentation is complete, unblock all clients and process their queued requests. One trivial way of implementing this is to have the Compositing Manager grab the X server at the start of the synchronization point, perform a round-trip operation to ensure that all pending updates are local and then run the presentation operation. After sending the PresentPixmap request, it would ungrab the server. This works for the current Manual compositing mechanism where the Compositing Manager is the one doing the drawing. So, I think the trick is simply to figure out how to make sure that the Compositing Manager is doing the drawing whenever a geometry change occurs. To fix windows in the Auto List, is it not sufficient to simply remove those windows from the Auto List and tell the Compositing manager? The Compositing Manager would then be responsible for constructing that region of the screen again. > Well, obviously, these would be more along general thoughts about how > things could be different for X12 ;-) Or at least design principles > that may be kept in mind for future extensions. Yup. Always good to know what doesn't work like we want in case we can avoid that in related operations, or even fix things in the future. -- -keith signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xorgproto 2/2] randr: Add non-desktop output property and behaviors [v3]
non-desktop devices are those to which the normal desktop environment should not be extended. Examples are Head-mounted displays and the Apple Touch Bar. How an output device is set to non-desktop is not part of this proposal; it is expected that the underlying operating system will provide this information and have it reflected to X applications through this extension. v2: fix puncutation and duplicated 'the'. v3: switch to 32-bit property named non-desktop to match Linux Signed-off-by: Keith Packard --- include/X11/extensions/randr.h | 1 + randrproto.txt | 84 -- 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/include/X11/extensions/randr.h b/include/X11/extensions/randr.h index e53cd56..e7caab1 100644 --- a/include/X11/extensions/randr.h +++ b/include/X11/extensions/randr.h @@ -196,6 +196,7 @@ typedef unsigned long XRandrModeFlags; #define RR_PROPERTY_BORDER_DIMENSIONS "BorderDimensions" #define RR_PROPERTY_GUID "GUID" #define RR_PROPERTY_RANDR_TILE "TILE" +#define RR_PROPERTY_NON_DESKTOP"non-desktop" /* roles this device can carry out */ #define RR_Capability_None 0 diff --git a/randrproto.txt b/randrproto.txt index 4c0990a..8d569bf 100644 --- a/randrproto.txt +++ b/randrproto.txt @@ -194,14 +194,23 @@ just mark them as disconnected. 1.6. Introduction to version 1.6 of the extension -Version 1.6 adds resource leasing. +Version 1.6 adds resource leasing and non desktop output management. - • A 'Lease' is a collection of crtcs and outputs which are made + • A “Lease” is a collection of crtcs and outputs which are made available to a client for direct access via kernel KMS and DRM APIs. This is done by passing a suitable file descriptor back to the client which has access to those resources. While leased, those resources aren't used by the X server. + • A “non-desktop” output is a device which should not normally be + considered as part of the desktop environment. Head-mounted + displays and the Apple "Touch Bar" are examples of such + devices. A desktop environment should be able to discover which + outputs are connected to such devices and, by default, not present + normal desktop applications on them. This is done by having + RRGetOutputInfo report such devices as Disconnected while reporting + all other information about the device correctly. + 1.99 Acknowledgments Our thanks to the contributors to the design found on the xpert mailing @@ -772,6 +781,12 @@ dynamic changes in the display environment. monitor in some way; for fixed-pixel devices, this would generally indicate which modes match the resolution of the output device. + Changes in version 1.6 of the protocol: + + When a “non-desktop” device is connected, the 'connection' + field will report Disconnected but the remaining fields will + report information about the connected device. + ┌─── RRListOutputProperties output:OUTPUT @@ -783,6 +798,12 @@ dynamic changes in the display environment. This request returns the atoms of properties currently defined on the output. + Changes in version 1.6 of the protocol: + + When a “non-desktop” device is connected, the property list + will be correct for the device, even though RRGetOutputInfo + reports the device as disconnected. + ┌─── RRQueryOutputProperty output: OUTPUT @@ -814,6 +835,12 @@ dynamic changes in the display environment. changed by clients. Immutable properties are interpreted by the X server. + Changes in version 1.6 of the protocol: + + When a “non-desktop” device is connected, the property information + will be correct for the device, even though RRGetOutputInfo + reports the device as disconnected. + ┌─── RRConfigureOutputProperty output: OUTPUT @@ -932,6 +959,12 @@ dynamic changes in the display environment. is True and the bytes-after is zero, the property is also deleted from the output, and a RROutputPropertyNotify event is generated. + Changes in version 1.6 of the protocol: + + When a “non-desktop” device is connected, the property value + will be correct for the device, even though RRGetOutputInfo + reports the device as disconnected. + ┌─── RRCreateMode window: WINDOW @@ -1821,6 +1854,12 @@ factors, such as re-cabling a monitor, etc. precise change can be detected by examining the new state of the system. + Changes in version 1.6 of the protocol: + + When a “non-desktop” device is connected, this event will be + delivered when the connection status of the output changes, + however the 'connection' value will be set to 'Disconnected'. + ┌─── RROutputPropertyNotify: window: WINDOW window requesting notification @@ -19
[PATCH xorgproto 1/2] randr: Add Leases. [v4]
A "lease" is a set of crtc and output resources granted to another application for use outside of X. These will not be usable through the X protocol until the lease terminates. Leased outputs will be seen as disconnected, leased CRTCs will be seen as not usable with any output. v2: Delete output grabs Add LeaseNotify events Add FreeLease with option to terminate v3: Clarify a couple of lease behaviors: * You can lease an in-use object, it makes the X server stop using it, you don't get an error back. * There's no explicit 'Disabled' state for a crtc, when a crtc is disabled, it just has a set of reported values for GetCrtcInfo. v4: Integrate into merged xorgproto repo Signed-off-by: Keith Packard --- include/X11/extensions/randr.h | 15 -- include/X11/extensions/randrproto.h | 57 +++ randrproto.pc.in| 2 +- randrproto.txt | 93 +++-- 4 files changed, 158 insertions(+), 9 deletions(-) diff --git a/include/X11/extensions/randr.h b/include/X11/extensions/randr.h index 6fcda87..e53cd56 100644 --- a/include/X11/extensions/randr.h +++ b/include/X11/extensions/randr.h @@ -40,11 +40,11 @@ typedef unsigned long XRandrModeFlags; #define RANDR_NAME "RANDR" #define RANDR_MAJOR1 -#define RANDR_MINOR5 +#define RANDR_MINOR6 -#define RRNumberErrors 4 +#define RRNumberErrors 5 #define RRNumberEvents 2 -#define RRNumberRequests 45 +#define RRNumberRequests 47 #define X_RRQueryVersion 0 /* we skip 1 to make old clients fail pretty immediately */ @@ -109,6 +109,10 @@ typedef unsigned long XRandrModeFlags; #define X_RRSetMonitor 43 #define X_RRDeleteMonitor44 +/* v1.6 */ +#define X_RRCreateLease 45 +#define X_RRFreeLease46 + /* Event selection bits */ #define RRScreenChangeNotifyMask (1L << 0) /* V1.2 additions */ @@ -119,6 +123,8 @@ typedef unsigned long XRandrModeFlags; #define RRProviderChangeNotifyMask (1L << 4) #define RRProviderPropertyNotifyMask (1L << 5) #define RRResourceChangeNotifyMask (1L << 6) +/* V1.6 additions */ +#define RRLeaseNotifyMask(1L << 7) /* Event codes */ #define RRScreenChangeNotify 0 @@ -131,6 +137,8 @@ typedef unsigned long XRandrModeFlags; #define RRNotify_ProviderChange3 #define RRNotify_ProviderProperty 4 #define RRNotify_ResourceChange5 +/* V1.6 additions */ +#define RRNotify_Lease 6 /* used in the rotation field; rotation and reflection in 0.1 proto. */ #define RR_Rotate_01 #define RR_Rotate_90 2 @@ -172,6 +180,7 @@ typedef unsigned long XRandrModeFlags; #define BadRRCrtc 1 #define BadRRMode 2 #define BadRRProvider 3 +#define BadRRLease 4 /* Conventional RandR output properties */ diff --git a/include/X11/extensions/randrproto.h b/include/X11/extensions/randrproto.h index 48be7aa..712c8b5 100644 --- a/include/X11/extensions/randrproto.h +++ b/include/X11/extensions/randrproto.h @@ -50,6 +50,7 @@ #define RRCrtc CARD32 #define RRProvider CARD32 #define RRModeFlags CARD32 +#define RRLease CARD32 #define Rotation CARD16 #define SizeID CARD16 @@ -835,6 +836,46 @@ typedef struct { } xRRGetProviderPropertyReply; #define sz_xRRGetProviderPropertyReply 32 +/* + * Additions for V1.6 + */ + +typedef struct { +CARD8 reqType; +CARD8 randrReqType; +CARD16 length B16; +Window window B32; +RRLeaselid B32; +CARD16 nCrtcs B16; +CARD16 nOutputs B16; +} xRRCreateLeaseReq; +#define sz_xRRCreateLeaseReq 16 + +typedef struct { +BYTE type; +CARD8 nfd; +CARD16 sequenceNumber B16; +CARD32 length B32; +CARD32 pad2 B32; +CARD32 pad3 B32; +CARD32 pad4 B32; +CARD32 pad5 B32; +CARD32 pad6 B32; +CARD32 pad7 B32; +} xRRCreateLeaseReply; +#define sz_xRRCreateLeaseReply 32 + +typedef struct { +CARD8 reqType; +CARD8 randrReqType; +CARD16 length B16; +RRLeaselid B32; +BYTE terminate; +CARD8 pad1; +CARD16 pad2 B16; +} xRRFreeLeaseReq; +#define sz_xRRFreeLeaseReq 12 + /* * event */ @@ -948,6 +989,22 @@ typedef struct { } xRRResourceChangeNotifyEvent; #define sz_xRRResourceChangeNotifyEvent32 +typedef struct { +CARD8 type;/* always evBase + RRNotify */ +CARD8 subCode; /* RRNotify_Lease */ +CARD16 sequenceNumber B16; +Time timestamp B32;/* time resource was changed */ +Window window B32; /* window requesting notification */ +RRLease lease B32; +
[PATCH xorgproto 0/2] randr: Add leases and non-desktop property
I've just taken my randrproto patches and cherry-picked them into xorgproto, then cleaned up the bits for the randrproto version number. ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 3/3] xfree86: Only call PreInit handler if it exists for device
On Sun, 2018-02-04 at 23:17 -0600, Jeff Smith wrote: > DoConfigure() attempts to call the PreInit handler on a device without > checking that the handler exists. > > Check that the PreInit handler exists for a device before attempting to > call it. This is a bit strange to me as the PreInit hook is basically required for the driver to work. However it matches what the non-X-dash- configure init code does, so whatever. Merged, thanks: remote: I: patch #202554 updated using rev 1a24a0ae7b1a7400735530a21ac8c0247723223d. remote: I: patch #202555 updated using rev e81031f3fda0f8b4237224b13c016759eaa52449. remote: I: patch #202556 updated using rev fd21b282dc88936043a23baa4ec053a2811319a7. remote: I: 3 patch(es) updated to state Accepted. To ssh://git.freedesktop.org/git/xorg/xserver 1e23f03dd5..fd21b282dc master -> master - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xorgproto 1/2] randr: Add Leases. [v4]
On Mon, 2018-02-05 at 12:39 -0800, Keith Packard wrote: > randrproto.pc.in| 2 +- Please be sure to bump the pc version in meson.build too. (If there's a more convenient way to only bump things once, great, let's do that instead.) > +7.6. Extension Requests added in version 1.6 of the extension. > + > +┌─── > +RRCreateLease > + window : WINDOW > + lid: LEASE > + crtcs: LISTofCRTC > + outputs: LISTofOUTPUT > + ▶ > + nfd: CARD8 > + lease: FD > +└─── > + Errors: IdChoice, Window, Access, Value, CRTC, Output > + > + Creates a new Lease called 'lid' for the specified crtcs and > + outputs from the screen defined by 'window'. Returns a KMS/DRM > + file descriptor which can control the leased objects directly > + through the kernel. While leased, all resources will appear to > + be 'useless' to clients other than the leasing client as > + follows: > + > + • Crtcs are reported as having no 'possible-outputs' and all > + other values reported as if the crtc were disabled. > + > + • Outputs are reported as having no crtcs they can be > + connected to, no clones they can share a crtc with, will > + report a connection status of Disconnected, and will show > + the current crtc as if it were disabled. > + > + The lease remains in effect until the file descriptor is > + closed, even if the client holding the lease disconnects from > + the X server. > + > + Returns an Access error if any of the named resources are > + already leased to another client. This should throw something if it's not possible to pass fds to the client (presumably because it's non-local). BadAccess or BadMatch seem like good ideas. Also I'm a little nervous about just returning a file descriptor without any way to describe what kind of operations you can do with it; if we ever develop an output setup API we hate less than KMS we'll have painted ourselves into a corner. Should this fd come with an atom naming an ioctl protocol? Should we apply that atom to the crtc[s] as well so the client can know the protocol in advance? - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: Anyone have a backup of XFree86 CVS?
On Mon, 2018-02-05 at 15:24 -0500, Adam Jackson wrote: > I've asked this a couple of times on IRC, repeating it here for wider > distribution. I had thought I had a git import laying around somewhere > but I can't seem to find it. cvs.xfree86.org hasn't been alive in a > long time, and it'd be a shame if that history just got lost, > fragmentary as it was. If anyone has a working git import, or a cvsps > mirror or other copy of the cvsroot, or knows someone who does, please > let us know so we can preserve some history. Well that was fast! Got a copy of the cvsroot for 3.x/4.x, hooray. I'll take a crack at importing that to git once xserver 1.20 is out. If anyone has copies of the release tarballs prior to 3.3.6, or of any pre-3.x CVS or RCS history (no idea if that even exists), that would still be very cool to have for the archive. - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] xwayland: Don't process cursor warping without an xwl_seat
Unfortunately, on my machine Xwayland immediately crashes when I try to start it. gdb backtrace: #0 0x774f0e79 in wl_proxy_marshal () from target:/lib64/libwayland-client.so.0 #1 0x00413172 in zwp_confined_pointer_v1_destroy (zwp_confined_pointer_v1=0x7) at hw/xwayland/Xwayland@exe/pointer-constraints-unstable-v1-client-protocol.h:612 #2 0x00418bc0 in xwl_seat_destroy_confined_pointer (xwl_seat=0x8ba2a0) at /home/lyudess/Projects/xserver/hw/xwayland/xwayland-input.c:2839 #3 0x00418c09 in xwl_seat_unconfine_pointer (xwl_seat=0x8ba2a0) at /home/lyudess/Projects/xserver/hw/xwayland/xwayland-input.c:2849 #4 0x00410d97 in xwl_cursor_confined_to (device=0xa5a000, screen=0x8b9d80, window=0x9bdb70) at /home/lyudess/Projects/xserver/hw/xwayland/xwayland.c:328 #5 0x004a8571 in ConfineCursorToWindow (pDev=0xa5a000, pWin=0x9bdb70, generateEvents=1, confineToScreen=0) at /home/lyudess/Projects/xserver/dix/events.c:900 #6 0x004a94b7 in ScreenRestructured (pScreen=0x8b9d80) at /home/lyudess/Projects/xserver/dix/events.c:1387 #7 0x00502386 in RRScreenSizeNotify (pScreen=0x8b9d80) at /home/lyudess/Projects/xserver/randr/rrscreen.c:160 #8 0x0041a83c in update_screen_size (xwl_output=0x8e7670, width=3840, height=2160) at /home/lyudess/Projects/xserver/hw/xwayland/xwayland-output.c:203 #9 0x0041a9f0 in apply_output_change (xwl_output=0x8e7670) at /home/lyudess/Projects/xserver/hw/xwayland/xwayland-output.c:252 #10 0x0041aaeb in xdg_output_handle_done (data=0x8e7670, xdg_output=0x8e7580) at /home/lyudess/Projects/xserver/hw/xwayland/xwayland-output.c:307 #11 0x750e9d1e in ffi_call_unix64 () at ../src/x86/unix64.S:76 #12 0x750e968f in ffi_call (cif=, fn=, rvalue=, avalue=) at ../src/x86/ffi64.c:525 #13 0x774f3d8b in wl_closure_invoke () from target:/lib64/libwayland-client.so.0 #14 0x774f0928 in dispatch_event.isra () from target:/lib64/libwayland-client.so.0 #15 0x774f1be4 in wl_display_dispatch_queue_pending () from target:/lib64/libwayland-client.so.0 #16 0x774f200b in wl_display_roundtrip_queue () from target:/lib64/libwayland-client.so.0 #17 0x00418cad in InitInput (argc=12, argv=0x7fffd9c8) at /home/lyudess/Projects/xserver/hw/xwayland/xwayland-input.c:2867 #18 0x004a20e3 in dix_main (argc=12, argv=0x7fffd9c8, envp=0x7fffda30) at /home/lyudess/Projects/xserver/dix/main.c:250 #19 0x00420cb2 in main (argc=12, argv=0x7fffd9c8, envp=0x7fffda30) at /home/lyudess/Projects/xserver/dix/stubmain.c:34 This appears to be the result of xwl_cursor_confined_to() and xwl_screen_get_default_seat(). xwl_cursor_confined_to() can be called very on during the Xwayland init sequence, well before any seat has actually been created for the server. However, this function doesn't make an attempt to actually check for whether or not there's currently a seat available, and just eagerly assumes that xwl_screen_get_default_seat() will always return a valid seat. Unfortunately, before an xwl_seat is actually initialized the xorg_list will be in a fresh state with no members in it, e.g. list->prev == list->next == &list. Since xwl_screen_get_default_seat() doesn't actually check whether or not the seat list is empty, this causes us to end up returning a pointer to &list instead of an actual xwl_seat struct, which subsequently causes us to crash. So, actually return NULL in xwl_screen_get_default_seat() if the seat list is empty, and skip any pointer confinement processing in xwl_cursor_confined_to() when we don't have a seat setup yet. Signed-off-by: Lyude Paul --- Just a quick note!!! I haven't actually tested at all whether or not this breaks cursor confinement, if you have any demo applications I use to easily do this please let me know. I have at least, tested that this lets me start Xwayland again :). hw/xwayland/xwayland.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c index 19aa14a47..9b1d85674 100644 --- a/hw/xwayland/xwayland.c +++ b/hw/xwayland/xwayland.c @@ -265,6 +265,9 @@ xwl_close_screen(ScreenPtr screen) static struct xwl_seat * xwl_screen_get_default_seat(struct xwl_screen *xwl_screen) { +if (xorg_list_is_empty(&xwl_screen->seat_list)) +return NULL; + return container_of(xwl_screen->seat_list.prev, struct xwl_seat, link); @@ -324,6 +327,10 @@ xwl_cursor_confined_to(DeviceIntPtr device, if (!xwl_seat) xwl_seat = xwl_screen_get_default_seat(xwl_screen); +/* xwl_seat hasn't been setup yet, don't do anything just yet */ +if (!xwl_seat) +return; + if (window == screen->root) { xwl_seat_unconfine_pointer(xwl_seat); return; -- 2.14.3
Re: [PATCH xorgproto 1/2] randr: Add Leases. [v4]
Adam Jackson writes: > On Mon, 2018-02-05 at 12:39 -0800, Keith Packard wrote: > >> randrproto.pc.in| 2 +- > > Please be sure to bump the pc version in meson.build too. (If there's a > more convenient way to only bump things once, great, let's do that > instead.) Thanks, I didn't catch that. Annoying that this version is in two places... >> +7.6. Extension Requests added in version 1.6 of the extension. >> + >> +┌─── >> +RRCreateLease > This should throw something if it's not possible to pass fds to the > client (presumably because it's non-local). BadAccess or BadMatch seem > like good ideas. It returns BadAlloc, just like ShmCreateSegment does. That seems like a inappropriate error to return, although I'm not sure the precise error code actually matters? > Also I'm a little nervous about just returning a file descriptor > without any way to describe what kind of operations you can do with it; > if we ever develop an output setup API we hate less than KMS we'll have > painted ourselves into a corner. Well, Linux ain't going to be changing the interface on /dev/dri/card* anytime soon; existing applications will have to continue to work. > Should this fd come with an atom naming an ioctl protocol? Should we > apply that atom to the crtc[s] as well so the client can know the > protocol in advance? I'd suggest that we burn that bridge when we come to it. If the old DRM interface isn't available at all, this call can certainly fail. If we do create a new kernel interface, we'll have to update the kernel, X server and clients to use it, so I don't see a lot of use in having this interface usable with a new interface. -- -keith signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xrandr v4 2/5] xrandr: stricter --scale argument parsing
Giuseppe Bilotta writes: > I'm not a big fan of strtod because with it it's impossible to know if > a conversion actually happened. xrandr --scale ' ' would actually be > accepted (resulting in a scale value of 0), while the scanf catches > it. strtod takes an 'endptr' argument which can be used for precisely this purpose, but I think your use of sscanf is easier to read as it does both conversions in one call, and lets you pick the two different versions easily (--scale 2 and --scale 2x1.5). One could imagine doing xscale = strtod(string, &endptr); if (*endptr) { if (endptr == string || *endptr != 'x') syntax error; string = endptr + 1; yscale = strtod(string, &endptr); if (endptr == string || *endptr) syntax error; } else { yscale = xscale; That's a lot more code than two calls to sscanf... > Of course there's also to be said that we could reject a scale factor > of 0, regardless of whether it comes from a correct parsing of the > string '0.0' or from the parse of an empty string (but of course then > we couldn't customize the error message to differentiate between > “incorrect parse” and “value out of range”). Probably a good thing to catch. -- -keith signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: Making Composite better for interactive apps
On Mon, Feb 5, 2018 at 9:24 PM, Keith Packard wrote: > What we want is for the compositing manager to get a clean snapshot of > the system and to then be able to present that on the screen each > frame. That means disallowing *all* changes for the duration of the > construction of the presented image. This is independent of whether some > windows are composited within the X server or composited by the > Compositing Manager. While I find this conceptually acceptable, I have an uneasy feeling about its practicality. I'm coming at this from the assumption that a Compositing Manager would try to pump out frames as frequently as possible, in order to minimize latency, which would lead to changes being disabled more often than not. This is particularly damning if you consider the case of multiple monitors with different refresh rates (think 144Hz “gaming” monitor and 60Hz “secondary” monitor), which is something I would recommend be considered in the design of this proposal, because this is already a current reality that even Windows 10 has (or at least has had) issues with (interestingly, apparently not WIndows 7). > Here's a proposal for this issue: > > 1) Create a synchronization point within the X server which defines the > start of presentation (for a single screen? globally?). If we want to take the separate refresh rates into account, I think this should be per-window/virtual root. The rationale for this is that a Compositing Manager which supports mixed refresh rates would create one overlay per monitor, and update each at a separate frequency. > 2) Between this point and the receipt of the PresentPixmap for this > frame, freeze all window drawing and geometry changes. A client > which sends a PresentPixmap or any window configuration request > which would affect the ongoing presentation will be blocked. > > 3) Once the presentation is complete, unblock all clients and process > their queued requests. Ideally, the blockade should only involve the windows within the affected area, but I'm not entirely sure this would be enforceable. > One trivial way of implementing this is to have the Compositing Manager > grab the X server at the start of the synchronization point, perform a > round-trip operation to ensure that all pending updates are local and > then run the presentation operation. After sending the PresentPixmap > request, it would ungrab the server. The question of the century is then: how much time will the server spend in grabbed state? How much does this affect the server behavior? Would it be more lightweight to have a specific message instead? This could be used to carry extra information. For example, the AutoList for this present may be sent with this message. > This works for the current Manual compositing mechanism where the > Compositing Manager is the one doing the drawing. So, I think the trick > is simply to figure out how to make sure that the Compositing Manager is > doing the drawing whenever a geometry change occurs. > > To fix windows in the Auto List, is it not sufficient to simply remove > those windows from the Auto List and tell the Compositing manager? The > Compositing Manager would then be responsible for constructing that > region of the screen again Maybe we don't even need to change the AutoList. What we want is to ensure that the Compositing Manager has _processed_ the notification of the geometry change for the windows in the AutoList. This could be achieved for exampe by having the CM send also the geometry of the AutoList windows when asking for the lock, and the server refusing the lock if the geometry is not up to date, or something like that. -- Giuseppe "Oblomov" Bilotta ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xrandr v4 2/5] xrandr: stricter --scale argument parsing
On Tue, Feb 6, 2018 at 1:01 AM, Keith Packard wrote: > Giuseppe Bilotta writes: > >> I'm not a big fan of strtod because with it it's impossible to know if >> a conversion actually happened. xrandr --scale ' ' would actually be >> accepted (resulting in a scale value of 0), while the scanf catches >> it. > > strtod takes an 'endptr' argument which can be used for precisely this > purpose, Not in the sense I mean above. If the string is a sequences of whitespace characters, strtod would return 0 as value (no conversion), and set endptr to the end of the string, because it would gobble the whitespace. This would be indistinguishable from it successfully parsing a string argument of, say, '0.0'. > but I think your use of sscanf is easier to read as it does > both conversions in one call, and lets you pick the two different > versions easily (--scale 2 and --scale 2x1.5). One could imagine doing > > xscale = strtod(string, &endptr); > if (*endptr) { > if (endptr == string || *endptr != 'x') > syntax error; > string = endptr + 1; > yscale = strtod(string, &endptr); > if (endptr == string || *endptr) > syntax error; > } else { > yscale = xscale; > > That's a lot more code than two calls to sscanf... I interpreted Walter's suggestion as using strtod only in if the sscanf failed to find 2 values, since then we expect a single value. >> Of course there's also to be said that we could reject a scale factor >> of 0, regardless of whether it comes from a correct parsing of the >> string '0.0' or from the parse of an empty string (but of course then >> we couldn't customize the error message to differentiate between >> “incorrect parse” and “value out of range”). > > Probably a good thing to catch. True, we should do this regardless of how the values are parsed. Also ensure that it's not negative. Hm. Since we have to do it both for scale and gamma, I wonder if it's overengineering if I try to refactor this parsing of "N or 1 positive values with separator S" into its own function. -- Giuseppe "Oblomov" Bilotta ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xorgproto] randr: MONITORINFO has outputs, not crtcs
--- randrproto.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/randrproto.txt b/randrproto.txt index c06bc90..f57301d 100644 --- a/randrproto.txt +++ b/randrproto.txt @@ -2363,14 +2363,14 @@ A.1.1 Common Types added in version 1.5 of the protocol 4 ATOMname 1 BOOLprimary 1 BOOLautomatic - 2 CARD16 ncrtcs + 2 CARD16 noutputs 2 INT16 x 2 INT16 y 2 CARD16 width in pixels 2 CARD16 height in pixels 4 CARD32 width in millimeters 4 CARD32 height in millimeters - 4*n CRTCcrtcs + 4*n OUTPUT outputs └─── A.2 Protocol Requests -- 2.14.1.439.g647b9b4702 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: Making Composite better for interactive apps
Giuseppe Bilotta writes: > Maybe we don't even need to change the AutoList. What we want is to > ensure that the Compositing Manager has _processed_ the notification > of the geometry change for the windows in the AutoList. This could be > achieved for exampe by having the CM send also the geometry of the > AutoList windows when asking for the lock, and the server refusing the > lock if the geometry is not up to date, or something like that. As I said, I think the two problems are separable, so I don't think we need to associate the grab/ungrab notion with the Auto List management. Simply removing the window from the auto list when its geometry changes and notifying the Compositing Manager means that in the next frame constructed by the compositing manager, it will either paint the window itself or it will have adapted to the geometry change. This may still leave window contents out of sync with the results of Compositing temporarily, but as long as things eventually correct themselves, that's the same as the current Compositing Manager environment. -- -keith signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: Making Composite better for interactive apps
Giuseppe Bilotta writes: > While I find this conceptually acceptable, I have an uneasy feeling > about its practicality. Yeah, we'd definitely have to experiment with both X server and Compositing Manager changes together to see what actually works. > I'm coming at this from the assumption that a Compositing Manager > would try to pump out frames as frequently as possible, in order to > minimize latency, which would lead to changes being disabled more > often than not. I think that's the wrong assumption. Given that you have monitors at a fixed refresh rate, you want each frame on the monitor to contain the most recent changes for all windows. That means painting the screen after all applications have updated for the current frame. Because we can't really tell when applications are done updating, as some may skip a frame now and then, what we can do is to delay painting until there's just barely enough time left before vblank to paint the frame. We've got lots of information that can be used to help predict when that time should be. One option would be to bury heuristics like this into the X server and have it send an event when it thinks the Compositing Manager should start painting; probably not ideal, but workable. Or even have the Compositing Manager tell the X server how much lead time it needs. In either case, the X server could suspend the other clients and send an event to the Compositing Manager at that time, unblocking when the Compositing Manager was finished. > Ideally, the blockade should only involve the windows within the > affected area, but I'm not entirely sure this would be enforceable. Sure, it would be great to suspend applications only when they try to manipulate resources on the target CRTC. We can do that in X, it's just more work to go through the window system code and stick in checks to suspend clients in relevant requests. Completely enforceable, in terms of geometry and rendering requests. > The question of the century is then: how much time will the server > spend in grabbed state? How much does this affect the server behavior? From the users' perspective, there is no effect at all -- you can't see anything that clients do until the Compositing Manager draws the screen. And, the server should spend very little time in the grabbed state; only long enough for the Compositing Manager to draw the frame. A Direct Rendered application would probably see no impact at all; if triple buffered, it will have all of the necessary resources to continue drawing without waiting for the PresentPixmap to finish. > Would it be more lightweight to have a specific message instead? This > could be used to carry extra information. For example, the AutoList > for this present may be sent with this message. For this to work without changing existing applications, I think the only thing we can do is stop executing requests from them. -- -keith signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xrandr v4 2/5] xrandr: stricter --scale argument parsing
Giuseppe Bilotta writes: > Not in the sense I mean above. If the string is a sequences of > whitespace characters, strtod would return 0 as value (no conversion), > and set endptr to the end of the string, because it would gobble the > whitespace. This would be indistinguishable from it successfully > parsing a string argument of, say, '0.0'. No, strtod requires a non-empty sequence of digits. #include #include int main(int argc, char **argv) { char*blanks = ""; char*endptr; double x; x = strtod(blanks, &endptr); printf("x: %f endptr: \"%s\"\n", x, endptr); } $ ./a.out x: 0.00 endptr: "" strtod is actually not a terrible function (surprising for libc, I know) > Hm. Since we have to do it both for scale and gamma, I wonder if it's > overengineering if I try to refactor this parsing of "N or 1 positive > values with separator S" into its own function. Yes. I'd just use sscanf or strtod directly as you please. -- -keith signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xorgproto] randr: MONITORINFO has outputs, not crtcs
Giuseppe Bilotta writes: > - 2 CARD16 ncrtcs > + 2 CARD16 noutputs Reviewed-by: Keith Packard -- -keith signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel