[PATCH] modesetting: Update fb_id from shadow allocate and destroy if not set

2018-02-05 Thread Tony Lindgren
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

2018-02-05 Thread Tony Lindgren
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

2018-02-05 Thread Giuseppe Bilotta
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

2018-02-05 Thread walter harms


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.

2018-02-05 Thread Mario Kleiner
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

2018-02-05 Thread 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”).

-- 
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.

2018-02-05 Thread Thomas Hellstrom

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

2018-02-05 Thread walter harms


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

2018-02-05 Thread Michel Dänzer
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

2018-02-05 Thread Michel Dänzer
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)

2018-02-05 Thread Adam Jackson
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)

2018-02-05 Thread Kyle Brenneman

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)

2018-02-05 Thread Adam Jackson
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

2018-02-05 Thread Keith Packard
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

2018-02-05 Thread Keith Packard
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)

2018-02-05 Thread Matthieu Herrb
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)

2018-02-05 Thread Alan Coopersmith
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)

2018-02-05 Thread Adam Jackson
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)

2018-02-05 Thread Adam Jackson
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

2018-02-05 Thread Adam Jackson
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?

2018-02-05 Thread Adam Jackson
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

2018-02-05 Thread Keith Packard
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]

2018-02-05 Thread Keith Packard
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]

2018-02-05 Thread Keith Packard
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

2018-02-05 Thread Keith Packard
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

2018-02-05 Thread Adam Jackson
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]

2018-02-05 Thread Adam Jackson
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?

2018-02-05 Thread Adam Jackson
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

2018-02-05 Thread Lyude Paul
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]

2018-02-05 Thread Keith Packard
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

2018-02-05 Thread Keith Packard
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

2018-02-05 Thread Giuseppe Bilotta
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

2018-02-05 Thread Giuseppe Bilotta
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

2018-02-05 Thread Giuseppe Bilotta
---
 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

2018-02-05 Thread Keith Packard
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

2018-02-05 Thread Keith Packard
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

2018-02-05 Thread Keith Packard
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

2018-02-05 Thread Keith Packard
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