Re: [RFC] DeepColor Visual Class Extension

[Alex Goins]

We brainstormed all of the suggestions amongst ourselves, and have a revised
version of the spec. It also includes some changes of our own.

Unfortunately, we weren't able to bottom out on everything (most notably the
masquerading DeepColor/TrueColor visuals), but the suggestions we haven't yet
implemented in the spec are included in the issues section, along with our
outstanding concerns. We will likely need more discussion over email or at XDC
to make sure that these suggestions are fully fleshed out.

Thanks,
Alex

--

   DeepColor Extension
   Version X.X
   2017-XX-XX

   Alex Goins
ago...@nvidia.com
NVIDIA Corporation


1. Introduction

The DeepColor Extension provides a new visual class, DeepColor, designed for
handling visuals that are incompatible with the existing core X visual classes:
StaticGray, StaticColor, TrueColor, GrayScale, PseudoColor, or DirectColor.

These visual classes provided by the core X11 protocol are insufficient for
visuals that require a greater than 32 bit depth, or non-integer formats. As
such, they are not suitable for many HDR formats.

In order to support a variety of HDR formats or any other formats that are
incompatible with the core X11 visual classes, DeepColor doesn't rely on a
colormap to define the relationship between pixel values and colors. Instead,
windows with DeepColor visuals will rely on a window property to define a
colorspace/encoding, chosen from an enumerated set of colorspaces/encodings
associated with a dedicated child of the root window. Compositors may detect
changes in the colorspace/encoding of redirected windows by listening for
PropertyNotify events from those windows.

In addition to supporting a variety of colorspaces/encodings, DeepColor visuals
must also support a variety of pixel formats, e.g. FP16 RGBA. The information
provided by the existing XVisualInfo structure is insufficient for
differentiating such formats, so a mechanism must be provided for clients to
determine the pixel formats of DeepColor visuals.

1.1. Acknowledgements

Zach Angold,NVIDIA Corporation
Adam Jackson,   Red Hat, Inc. 
James Jones,NVIDIA Corporation
Robert Morell,  NVIDIA Corporation
Keith Packard,  Hewlett-Packard Company   
Aaron Plattner, NVIDIA Corporation
Andy Ritger,NVIDIA Corporation

2. DeepColor Visual Class

The DeepColor extension defines a new visual class, DeepColor.

DeepColor visuals do not make use of the red_mask, green_mask, blue_mask, or
colormap_size fields of the XVisualInfo structure. Colormap size is defined to
be 0.

Information pertaining to the pixel format and colorspace/encoding of DeepColor
visuals and associated windows, respectively, must be queried using the
mechanisms defined below.

3. Colorspace/Encoding Window Properties

Windows with DeepColor visuals must rely on window properties, as opposed to
colormaps, to determine the relationship between pixel values and colors. These
properties must specify constants that correspond to colorspace/encoding pairs.

Possible colorspace/encoding constants are defined to be:

// Undefined:
// Undefined colorspace and encoding
#define _DEEPCOLOR_UNDEFINED0

// scRGB Linear:
// scRGB colorspace with linear OETF
#define _DEEPCOLOR_SCRGB_LINEAR 1

// BT.2020 Linear:
// BT.2020 colorspace with linear OETF
#define _DEEPCOLOR_BT2020_LINEAR2

// BT.2020 PQ:
// BT.2020 colorspace with SMPTE ST.2084 Perceptual Quantizer (PQ) OETF,
// also known as HDR10
#define _DEEPCOLOR_BT2020_PQ3

// BT.2020 HLG:
// BT.2020 colorspace with ARIB STD-B67 Hybrid Log-Gamma (HLG) OETF, 
also
// known as HLG10
#define _DEEPCOLOR_BT2020_HLG   4

// DCI-P3 Linear:
// DCI-P3 colorspace with linear OETF
#define _DEEPCOLOR_DCI_P3_LINEAR5

// DCI-P3 Nonlinear:
// DCI-P3 colorspace with Gamma 2.6 OETF
#define _DEEPCOLOR_DCI_P3_NONLINEAR 6

// Display-P3 Nonlinear:
// DCI-P3 colorspace with sRGB-like OETF
#define _DEEPCOLOR_DISPLAY_P3_NONLINEAR 7

// ACES2065-1:
// ACES colorspace with AP0 primaries and linear OETF
#define _DEEPCOLOR_ACES_2065_1  8

// ACEScg:
// ACES colorspace with AP1 primaries and linear OETF
#define _DEEPCOLOR_ACES_CG  9

// ACEScc:
// ACES colorspace with AP1 primaries and logarithmic OETF
#define _DEEPCOLOR_ACES_CC  10

// ACEScct:
// ACES colorspace with AP1 primaries and modified logarithmic OETF
#define _DEEPCOLOR_ACES_CCT 11

3.1. 

Re: [PATCH xserver v2] meson: Fix epoll detection

[Keith Packard]

Peter Harris  writes:

> HAVE_OSPOLL is only defined inside this file, not by autoconf/meson, so
> it's always defined to 1.
>
> The rest of os/ospoll.c uses #if instead of #ifdef everywhere. I'd be
> inclined to update all of them if I were to paint that particular
> bikeshed.

Sounds good. I was only looking at the patch in isolation.

-- 
-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 v2] meson: Fix epoll detection

[Peter Harris]

On 2017-08-14 4:45 PM, Keith Packard wrote:
> Peter Harris  writes:
>> -#if !HAVE_OSPOLL && HAVE_EPOLL_CREATE1
>> +#if !HAVE_OSPOLL && defined(HAVE_EPOLL_CREATE1)
> 
> Should be using defined for HAVE_OSPOLL as well?

HAVE_OSPOLL is only defined inside this file, not by autoconf/meson, so
it's always defined to 1.

The rest of os/ospoll.c uses #if instead of #ifdef everywhere. I'd be
inclined to update all of them if I were to paint that particular bikeshed.

Peter Harris
-- 
   Open Text Connectivity Solutions Group
Peter Harrishttp://connectivity.opentext.com/
Research and DevelopmentPhone: +1 905 762 6001
phar...@opentext.comToll Free: 1 877 359 4866
___
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 v2] meson: Fix epoll detection

[Eric Anholt]

Peter Harris  writes:

> The epoll code depends on epoll_create1, not epoll_create.

Reviewed and pushed.  Thanks!


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 v2] meson: Fix epoll detection

[Keith Packard]

Peter Harris  writes:

> The epoll code depends on epoll_create1, not epoll_create.
>
> Signed-off-by: Peter Harris 

Thanks!

> -#if !HAVE_OSPOLL && HAVE_EPOLL_CREATE1
> +#if !HAVE_OSPOLL && defined(HAVE_EPOLL_CREATE1)

Should be using defined for HAVE_OSPOLL as well?

-- 
-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 2/3] glamor: Scissor CopyArea to the bounds of the drawing.

[Eric Anholt]

Mark Marshall  writes:

> On 1 August 2017 at 22:59, Eric Anholt  wrote:
>> Like the previous fix to rectangles, this reduces the area drawn on
>> tiled renderers by letting the CPU-side tile setup know what tiles
>> might be drawn at all.
>>
>> Surprisingly, it improves x11perf -copypixwin1 -repeat 1 -reps 1
>> on i965 by 2.93185% +/- 1.5561% (n=90).
>>
>> Signed-off-by: Eric Anholt 
>> ---
>>  glamor/glamor_copy.c  | 27 +++
>>  glamor/glamor_utils.h |  9 +
>>  2 files changed, 32 insertions(+), 4 deletions(-)
>>
>> diff --git a/glamor/glamor_copy.c b/glamor/glamor_copy.c
>> index f7d6eb163fac..3296b7b1bf75 100644
>> --- a/glamor/glamor_copy.c
>> +++ b/glamor/glamor_copy.c
>> @@ -351,6 +351,7 @@ glamor_copy_fbo_fbo_draw(DrawablePtr src,
>>  const glamor_facet *copy_facet;
>>  int n;
>>  Bool ret = FALSE;
>> +BoxRec bounds = glamor_no_rendering_bounds();
>>
>>  glamor_make_current(glamor_priv);
>>
>> @@ -391,11 +392,20 @@ glamor_copy_fbo_fbo_draw(DrawablePtr src,
>>  glVertexAttribPointer(GLAMOR_VERTEX_POS, 2, GL_SHORT, GL_FALSE,
>>2 * sizeof (GLshort), vbo_offset);
>>
>> +if (nbox < 100) {
>> +bounds = glamor_start_rendering_bounds();
>> +for (int i = 0; i < nbox; i++)
>> +glamor_bounds_union_box(, [i]);
>> +}
>> +
>>  for (n = 0; n < nbox; n++) {
>>  v[0] = box->x1; v[1] = box->y1;
>>  v[2] = box->x1; v[3] = box->y2;
>>  v[4] = box->x2; v[5] = box->y2;
>>  v[6] = box->x2; v[7] = box->y1;
>> +
>> +glamor_bounds_union_box(, box);
>> +
>
> I'm only looking at the diff, but aren't you doing
> glamor_bounds_union_box twice in the n < 100 case, and in the n >= 100
> case I guess it does nothing anyway, apart from waste cycles?

Thanks, that was leftover debug code.


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] meson: Fix epoll detection

[Eric Anholt]

Peter Harris  writes:

> On 2017-08-11 5:50 AM, Peter Hutterer wrote:
>> On Tue, Aug 08, 2017 at 11:16:13AM -0400, Peter Harris wrote:
>>> The epoll code depends on epoll_create1, not epoll_create.
>>>
>>> The trinary " ? 1 : false" is used because HAVE_EPOLL_CREATE1 is tested
>>> with #if instead of #ifdef.
>> 
>> might be worth using conf_data.set10(...) then? that should write out 0/1
>> correctly.
>
> ".set10" writes 1 or 0, whereas "? 1 : false" writes 1 or #undef, which
> more closely matches what autoconf does.
>
> I'm fine with .set10 if you don't mind a little divergence. Or I could
> change #if HAVE_EPOLL_CREATE1 to #ifdef HAVE_EPOLL_CREATE1 to match
> (most of) the rest of the server instead, if you prefer.

I like that -- it's what I did to our other #ifs for meson.


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 libX11 1/3] Make _XCloseLC thread safe.

[Jacek Caban]

On 14.08.2017 17:11, Adam Jackson wrote:
> On Thu, 2017-08-10 at 23:04 +0200, Jacek Caban wrote:
>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=55678
>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=68538
>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=69088
>> Signed-off-by: Jacek Caban 
>> ---
>>  src/xlibi18n/lcWrap.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/src/xlibi18n/lcWrap.c b/src/xlibi18n/lcWrap.c
>> index 38242608..43b4d622 100644
>> --- a/src/xlibi18n/lcWrap.c
>> +++ b/src/xlibi18n/lcWrap.c
>> @@ -324,6 +324,8 @@ _XCloseLC(
>>  {
>>  XLCdList cur, *prev;
>>  +_XLockMutex(_Xi18n_lock);
>> +
> This patch is malformed. As is 2/3 of this series, or at least the
> thing I get out of patchwork is. Can you resend these with git-send-
> email instead? I think your user agent may be corrupting things.

Sorry about that, I resent patches.

Thanks,
Jacek
___
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 0/7] xwin: misc small fixes

[Jon Turney]

On 03/08/2017 20:15, Emil Velikov wrote:

Hi all,

Here's a few small cleanups that I had lying around for months.
Patches are dead trivial, although completely untested.

Please review,


Thanks very much for doing this.  Getting rid of those conditionals has 
been on my todo list for a long time...


With the small corrections noted for 4/7 and 5/7,

Reviewed-by: Jon Turney 

___
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 4/7] xwin: remove always true/set XWIN_CLIPBOARD conditional/define

[Jon Turney]

On 03/08/2017 20:15, Emil Velikov wrote:

--- a/hw/xwin/winscrinit.c
+++ b/hw/xwin/winscrinit.c
@@ -264,9 +264,7 @@ winFinishScreenInitFB(int i, ScreenPtr pScreen, int argc, 
char **argv)
  winScreenInfo *pScreenInfo = pScreenPriv->pScreenInfo;
  VisualPtr pVisual = NULL;
  
-#if defined(XWIN_CLIPBOARD) || defined(XWIN_MULTIWINDOW)

  int iReturn;
-#endif
  
  /* Create framebuffer */

  if (!(*pScreenPriv->pwinInitScreen) (pScreen)) {
@@ -504,7 +502,6 @@ winFinishScreenInitFB(int i, ScreenPtr pScreen, int argc, 
char **argv)
  pScreenPriv->CloseScreen = pScreen->CloseScreen;
  pScreen->CloseScreen = pScreenPriv->pwinCloseScreen;
  
-#if defined(XWIN_CLIPBOARD) || defined(XWIN_MULTIWINDOW)

  /* Create a mutex for modules in separate threads to wait for */
  iReturn = pthread_mutex_init(>pmServerStarted, NULL);
  if (iReturn != 0) {


The matching #endif needs to be removed.
___
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 libX11 3/3] Make conv_list thread safe.

[Jacek Caban]

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=55678
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=68538
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=69088
Signed-off-by: Jacek Caban 
---
 src/locking.c | 13 +
 src/xlibi18n/lcConv.c | 25 -
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/src/locking.c b/src/locking.c
index 9f4fe067..3981c0f5 100644
--- a/src/locking.c
+++ b/src/locking.c
@@ -66,6 +66,8 @@ in this Software without prior written authorization from The 
Open Group.
 
 /* in lcWrap.c */
 extern LockInfoPtr _Xi18n_lock;
+/* in lcConv.c */
+extern LockInfoPtr _conv_lock;
 
 #ifdef WIN32
 static DWORD _X_TlsIndex = (DWORD)-1;
@@ -98,6 +100,7 @@ static xthread_t _Xthread_self(void)
 
 static LockInfoRec global_lock;
 static LockInfoRec i18n_lock;
+static LockInfoRec conv_lock;
 
 static void _XLockMutex(
 LockInfoPtr lip
@@ -594,12 +597,22 @@ Status XInitThreads(void)
global_lock.lock = NULL;
return 0;
 }
+if (!(conv_lock.lock = xmutex_malloc())) {
+   xmutex_free(global_lock.lock);
+   global_lock.lock = NULL;
+   xmutex_free(i18n_lock.lock);
+   i18n_lock.lock = NULL;
+   return 0;
+}
 _Xglobal_lock = _lock;
 xmutex_init(_Xglobal_lock->lock);
 xmutex_set_name(_Xglobal_lock->lock, "Xlib global");
 _Xi18n_lock = _lock;
 xmutex_init(_Xi18n_lock->lock);
 xmutex_set_name(_Xi18n_lock->lock, "Xlib i18n");
+_conv_lock = _lock;
+xmutex_init(_conv_lock->lock);
+xmutex_set_name(_conv_lock->lock, "Xlib conv");
 _XLockMutex_fn = _XLockMutex;
 _XUnlockMutex_fn = _XUnlockMutex;
 _XCreateMutex_fn = _XCreateMutex;
diff --git a/src/xlibi18n/lcConv.c b/src/xlibi18n/lcConv.c
index 7d9a4738..32699746 100644
--- a/src/xlibi18n/lcConv.c
+++ b/src/xlibi18n/lcConv.c
@@ -29,6 +29,11 @@
 #include "Xlibint.h"
 #include "XlcPubI.h"
 #include 
+#include "locking.h"
+
+#ifdef XTHREADS
+LockInfoPtr _conv_lock;
+#endif
 
 typedef struct _XlcConverterListRec {
 XLCd from_lcd;
@@ -58,6 +63,9 @@ get_converter(
 XrmQuark to_type)
 {
 XlcConverterList list, prev = NULL;
+XlcConv conv = NULL;
+
+_XLockMutex(_conv_lock);
 
 for (list = conv_list; list; list = list->next) {
if (list->from_lcd == from_lcd && list->to_lcd == to_lcd
@@ -69,13 +77,16 @@ get_converter(
conv_list = list;
}
 
-   return (*list->converter)(from_lcd, list->from, to_lcd, list->to);
+   conv = (*list->converter)(from_lcd, list->from, to_lcd, list->to);
+   break;
}
 
prev = list;
 }
 
-return (XlcConv) NULL;
+_XUnlockMutex(_conv_lock);
+
+return conv;
 }
 
 Bool
@@ -92,18 +103,20 @@ _XlcSetConverter(
 from_type = XrmStringToQuark(from);
 to_type = XrmStringToQuark(to);
 
+_XLockMutex(_conv_lock);
+
 for (list = conv_list; list; list = list->next) {
if (list->from_lcd == from_lcd && list->to_lcd == to_lcd
&& list->from_type == from_type && list->to_type == to_type) {
 
list->converter = converter;
-   return True;
+   goto ret;
}
 }
 
 list = Xmalloc(sizeof(XlcConverterListRec));
 if (list == NULL)
-   return False;
+   goto ret;
 
 list->from_lcd = from_lcd;
 list->from = from;
@@ -115,7 +128,9 @@ _XlcSetConverter(
 list->next = conv_list;
 conv_list = list;
 
-return True;
+ret:
+_XUnlockMutex(_conv_lock);
+return list != NULL;
 }
 
 typedef struct _ConvRec {
-- 
2.13.0

___
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 libX11 2/3] Don't cache last lcd in _XlcCurrentLC.

[Jacek Caban]

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=55678
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=68538
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=69088

The way it's currently cached is not thread safe. As long as locale doesn't 
change, the same object is reused anyway.

Signed-off-by: Jacek Caban 
---
 src/xlibi18n/lcWrap.c | 12 +---
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/src/xlibi18n/lcWrap.c b/src/xlibi18n/lcWrap.c
index 43b4d622..5339dcf3 100644
--- a/src/xlibi18n/lcWrap.c
+++ b/src/xlibi18n/lcWrap.c
@@ -352,17 +352,7 @@ _XCloseLC(
 XLCd
 _XlcCurrentLC(void)
 {
-XLCd lcd;
-static XLCd last_lcd = NULL;
-
-lcd = _XOpenLC((char *) NULL);
-
-if (last_lcd)
-   _XCloseLC(last_lcd);
-
-last_lcd = lcd;
-
-return lcd;
+return _XOpenLC(NULL);
 }
 
 XrmMethods
-- 
2.13.0

___
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 libX11 1/3] Make _XCloseLC thread safe.

[Jacek Caban]

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=55678
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=68538
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=69088
Signed-off-by: Jacek Caban 
---
 src/xlibi18n/lcWrap.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/xlibi18n/lcWrap.c b/src/xlibi18n/lcWrap.c
index 38242608..43b4d622 100644
--- a/src/xlibi18n/lcWrap.c
+++ b/src/xlibi18n/lcWrap.c
@@ -324,6 +324,8 @@ _XCloseLC(
 {
 XLCdList cur, *prev;
 
+_XLockMutex(_Xi18n_lock);
+
 for (prev = _list; (cur = *prev); prev = >next) {
if (cur->lcd == lcd) {
if (--cur->ref_count < 1) {
@@ -339,6 +341,8 @@ _XCloseLC(
_XlcDeInitLoader();
loader_list = NULL;
 }
+
+_XUnlockMutex(_Xi18n_lock);
 }
 
 /*
-- 
2.13.0

___
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] Make PixmapDirtyUpdateRec::src a DrawablePtr

[Adam Jackson]

On Tue, 2017-04-18 at 19:07 +0900, Michel Dänzer wrote:
> From: Michel Dänzer 
> 
> This allows making the master screen's pixmap_dirty_list entries
> explicitly reflect that we're now tracking the root window instead of
> the screen pixmap, in order to allow Present page flipping on master
> outputs while there are active slave outputs.
> 
> Define HAS_DIRTYTRACKING_DRAWABLE_SRC for drivers to check, but leave
> HAS_DIRTYTRACKING_ROTATION defined as well to make things slightly
> easier for drivers.
> 
> Signed-off-by: Michel Dänzer 

Reviewed-by: Adam Jackson 

- 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 libX11 1/3] Make _XCloseLC thread safe.

[Adam Jackson]

On Thu, 2017-08-10 at 23:04 +0200, Jacek Caban wrote:
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=55678
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=68538
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=69088
> Signed-off-by: Jacek Caban 
> ---
>  src/xlibi18n/lcWrap.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/xlibi18n/lcWrap.c b/src/xlibi18n/lcWrap.c
> index 38242608..43b4d622 100644
> --- a/src/xlibi18n/lcWrap.c
> +++ b/src/xlibi18n/lcWrap.c
> @@ -324,6 +324,8 @@ _XCloseLC(
>  {
>  XLCdList cur, *prev;
>  +_XLockMutex(_Xi18n_lock);
> +

This patch is malformed. As is 2/3 of this series, or at least the
thing I get out of patchwork is. Can you resend these with git-send-
email instead? I think your user agent may be corrupting things.

- 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 libX11 1/3] Make _XCloseLC thread safe.

[walter harms]

hi,
can you provide a simple program does needs the patch to work ?

re,
 wh

Am 10.08.2017 23:04, schrieb Jacek Caban:
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=55678
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=68538
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=69088
> Signed-off-by: Jacek Caban 
> ---
>  src/xlibi18n/lcWrap.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/xlibi18n/lcWrap.c b/src/xlibi18n/lcWrap.c
> index 38242608..43b4d622 100644
> --- a/src/xlibi18n/lcWrap.c
> +++ b/src/xlibi18n/lcWrap.c
> @@ -324,6 +324,8 @@ _XCloseLC(
>  {
>  XLCdList cur, *prev;
>  +_XLockMutex(_Xi18n_lock);
> +
>  for (prev = _list; (cur = *prev); prev = >next) {
>   if (cur->lcd == lcd) {
>   if (--cur->ref_count < 1) {
> @@ -339,6 +341,8 @@ _XCloseLC(
>   _XlcDeInitLoader();
>   loader_list = NULL;
>  }
> +
> +_XUnlockMutex(_Xi18n_lock);
>  }
>   /*
___
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] xwayland: Avoid repeatedly looping through window ancestor chain

[Pekka Paalanen]

On Sat, 12 Aug 2017 03:45:39 +0200
Roman Gilg  wrote:

> On Thu, Aug 10, 2017 at 2:49 PM, Pekka Paalanen  wrote:

> > Assuming the theory is sound, how about you kept the old names and
> > semantics of xwl_window_get() and xwl_window_from_window() but just
> > used the stored private in xwl_window_from_window()? You would need to
> > have a xwl_window_find_from_window() for the single use in
> > xwl_realize_window(), but I think that would allow you to put the test
> >
> > if (win != xwl_window->window)
> >
> > in xwl_window_get() rather than open-coding it in the users. I believe
> > keeping the semantics would be less surprising to the reader,
> > especially if they know the old code.
> >  
> 
> It's a good idea to not open-code the condition for the users. I would
> prefer though to risk the short-term confusion for the users, because
> otherwise in the long term the naming is ambiguous:
> 
> A Window has exactly one xwl_window associated (or none): The one in its
> private field. Calling xwl_window_get() therefore is the natural way to
> receive this one xwl_window. The function naming becomes really confusing
> when we call xwl_window_from_window() inside xwl_window_get() to receive
> the private field and afterwards do the test or when we call
> xwl_window_from_window() in xwl_window_find_from_window() on the Window's
> parent Window.

Hi Roman,

are you seeing xwl_window_get() as "get the private" while I think of
it as "get the owned xwl_window"? In that case, and as it has not been
documented which one it really meant before, I think we should just not
use the name xwl_window_get() anymore at all to get rid of the
ambiguity. This is the most important point in my comments and even
that is not too important.

> How do you like this approach:
> * xwl_window_get(WindowPtr) returns the private field
> * New function xwl_window_own(WindowPtr win) calls xwl_window_get and
> forwards the xwl_window if (win == xwl_window->window), otherwise NULL.
> Maybe there is a better name than xwl_window_own though for this function?
> * xwl_window_from_window() is renamed to xwl_window_find() and as it's now
> will only be called in xwl_realize_window.
> 
> But if you prefer the other naming, it's also fine with me.

Yes, that sounds better. Since the semantics of xwl_window_get() change
due to setting the window private for more windows than the one
actually owning the xwl_window, it requires touching all the current
callers. That's why I previously suggested to keep its semantics intact.

Would it be nice to rename xwl_window_get() to
xwl_window_from_priv() to denote this change in semantics and
ensure that no callers go unnoticed? Or to_xwl_window()?

xwl_window_own() would be the replacement for the old xwl_window_get(),
or maybe call it... xwl_window_from_owner()? Returns NULL if the Window
doesn't own any xwl_window.

How does your Present and sub-surface work change the relationships
between Windows and xwl_windows? Does the child Window getting a
sub-surface have its own xwl_window? Are there cases for fetching from
the child Window (or from its descendant) both its own xwl_window and
the respective toplevel Window's xwl_window?

If the above doesn't feel particularly fitting, then let's go with your
suggestion.


Thanks,
pq


pgpTf0C51NLEN.pgp
Description: OpenPGP digital 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: Second Feedback request for my GSoC project to improve Present support in Xwayland

[Michel Dänzer]

On 11/08/17 12:04 AM, Pekka Paalanen wrote:
> On Mon, 7 Aug 2017 17:36:27 +0900
> Michel Dänzer  wrote:
>> On 04/08/17 06:57 PM, Roman Gilg wrote:
>>> On Fri, Aug 4, 2017 at 8:44 AM, Michel Dänzer >> > wrote:
>>>
>>> On 03/08/17 09:11 PM, Roman Gilg wrote:  
> 
>>> > Also there is the following issue, which I encountered: The Present
>>> > extension assumes, that on flip the DDX is not able to flip 
>>> immediately,
>>> > but signals present_even_notify for the flip a little bit later (this
>>> > doesn't mean that's not async, just that the flip is pending for a
>>> > little while). But in the Xwayland  case (or maybe other DDX), we can
>>> > and want flip immediately.  
>>>
>>> Then you can call present_event_notify immediately. :)
>>>
>>> The problem I faced when doing that, was that present_event_notify
>>> directly does some changes to the current and pending flips, while the
>>> present_execute routine assumes that these changes are not yet done.
>>>
>>> For example vblank->pixmap gets nulled on present_event_notify here:
>>> https://cgit.freedesktop.org/xorg/xserver/tree/present/present.c#n501
>>> But it's still needed after the flip here:
>>> https://cgit.freedesktop.org/xorg/xserver/tree/present/present.c#n698
>>>
>>> While I tried in the beginning to simply guard against these changes
>>> individually, for example by setting a local variable vblank_pixmap =
>>> vblank->pixmap in present_execute and then working with the local
>>> variable, I felt that the more clean and secure version is to introduce
>>> the additional hook for DDX' with immediate flips.  
>>
>> Is there such a thing as an "immediate flip" though? All you can do in
>> the flip hook is send the "flip" request to the Wayland compositor,
>> right? The intent of the current interface is that the driver only calls
>> present_event_notify once it receives confirmation that the flip has
>> completed. Are there no appropriate Wayland events that can be used for
>> this?
> 
> Hi,
> 
> depends on what you consider as "flip complete".
> 
> If it is enough that the Wayland server has processed the attach/commit
> and any side-effects that might cause, including a possible release of
> a previous buffer, then you can use wl_display.sync request. When the
> callback it creates sends an event, the Wayland server has processed
> all requests you sent before the wl_display.sync. This is as immediate
> as it can be.

This sounds like it would be useful for the PresentOptionAsync case.


> If "flip complete" means the buffer has actually hit the screen, then
> there is the Wayland Presentation feedback extension that gives you
> exactly that.

That should be what we want in the normal case without PresentOptionAsync.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer



signature.asc
Description: OpenPGP digital 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