date:20170421

Re: [PATCH xserver 1/3] Rewrite the byte swapping macros.

2017-04-21 Thread Eric Anholt

Keith Packard  writes:

> [ Unknown signature status ]
> Eric Anholt  writes:
>
>>  #define swapll(x) do { \
>> +uint64_t temp; \
>>  if (sizeof(*(x)) != 8) \
>>  wrong_size(); \
>> -swap_uint64((uint64_t *)(x));   \
>> +memcpy(, x, 8); \
>> +temp = bswap_64(temp); \
>> +memcpy(x, , 8); \
>>  } while (0)
>
>>  #define swapl(x) do { \
>> +uint32_t temp; \
>>  if (sizeof(*(x)) != 4) \
>>  wrong_size(); \
>> -if (__builtin_constant_p((uintptr_t)(x) & 3) && ((uintptr_t)(x) 
>> & 3) == 0) \
>> -*(x) = lswapl(*(x)); \
>> -else \
>> -swap_uint32((uint32_t *)(x)); \
>> +memcpy(, x, 4); \
>> +temp = bswap_32(temp); \
>> +memcpy(x, , 4); \
>>  } while (0)
>
>>  #define swaps(x) do { \
>> +uint16_t temp; \
>>  if (sizeof(*(x)) != 2) \
>>  wrong_size(); \
>> -if (__builtin_constant_p((uintptr_t)(x) & 1) && ((uintptr_t)(x) 
>> & 1) == 0) \
>> -*(x) = lswaps(*(x)); \
>> -else \
>> -swap_uint16((uint16_t *)(x)); \
>> +memcpy(, x, 2); \
>> +temp = bswap_16(temp); \
>> +memcpy(x, , 2); \
>>  } while (0)
>
> Any particular reason to think that x might be unaligned (and hence need
> the memcpys? Yeah, I know they're probably 'free' when things are
> aligned.
>
> The rest of this is
>
> Reviewed-by: Keith Packard 

The reason to think so is that the previous code had this weird stuff to
explicitly handle unaligned.  I rip out the unaligned support in patch
3, because I think it's not needed.


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 app/xdpyinfo] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2017-04-21 Thread Pali Rohár

On Thursday 13 April 2017 12:58:52 Michel Dänzer wrote:
> On 13/04/17 12:52 PM, Keith Packard wrote:
> > Pali Rohár  writes:
> > 
> >> Current usage of DisplayWidthMM() and DisplayHeightMM() does not make sense
> >> for multi-monitor configuration. In most cases DPI is set to 96 as there is
> >> no sane value.
> >>
> >> Instead when XRANDR 1.2 extension is supported, report dimensions and
> >> resolution information per XRANDR monitor output. It should provide
> >> correct DPI value.
> > 
> > I'd be happy for this to be reported as additional information, but I
> > suspect there are numerous shell scripts which parse the old information
> > which will get confused by any change in the format.
> 
> In which case it should probably only be printed with -ext RANDR, to be
> consistent with other extensions.

I do not agree as xdpyinfo on Xservers with XRANDR 1.2 support output
invalid resolution and dimension information. When XRANDR 1.2 support
then those information are available only via XRANDR extension.

-- 
Pali Rohár
pali.ro...@gmail.com
___
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 xproto] Add XF86XK_Keyboard

2017-04-21 Thread Christian Kellner


> We are not XFree86, and are not adding new XF86 keysyms as the note
> at the top of XF86keysym.h says. Instead keysyms should be added to
> keysymdef.h and the files listed in the big comment at the top of it.

I did indeed see the 'big comment' while making the change, but when I 
looked for a good place in keysymdef.h to put this key nothing obvious
seemed like the right place. The "Misc functions" group doesn't seem to
have any values left to use. Maybe a new group in the "XK_XKB_KEYS"
section?

While discussing this issue (on IRC) with Daniel Stone and a few others
it was pointed out that the intention to not add symbols toXF86keysym.h
  might be a relict from era where XFree86 was still "more active" (to
put it nicely). So still in doubt after all the discussions I decided
to put it in XF86keysym.h was the easiest and despite "the big comment"
maybe still the best option; mainly I wanted to get advice where to
exactly put this keysym (and future ones as there might be more like
this key in the near future since there are quite a few umapped input
key events).

Sorry, I meant to add all this to a cover letter to the initial patch,
somehow that got swallowed.

Thanks,
CK 

-- 
Dr. Christian J. Kellner
Desktop Hardware Enablement
Red Hat
___
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 1/3] Rewrite the byte swapping macros.

2017-04-21 Thread Keith Packard

Eric Anholt  writes:

>  #define swapll(x) do { \
> + uint64_t temp; \
>   if (sizeof(*(x)) != 8) \
>   wrong_size(); \
> -swap_uint64((uint64_t *)(x));   \
> + memcpy(, x, 8); \
> + temp = bswap_64(temp); \
> + memcpy(x, , 8); \
>   } while (0)

>  #define swapl(x) do { \
> + uint32_t temp; \
>   if (sizeof(*(x)) != 4) \
>   wrong_size(); \
> - if (__builtin_constant_p((uintptr_t)(x) & 3) && ((uintptr_t)(x) 
> & 3) == 0) \
> - *(x) = lswapl(*(x)); \
> - else \
> - swap_uint32((uint32_t *)(x)); \
> + memcpy(, x, 4); \
> + temp = bswap_32(temp); \
> + memcpy(x, , 4); \
>   } while (0)

>  #define swaps(x) do { \
> + uint16_t temp; \
>   if (sizeof(*(x)) != 2) \
>   wrong_size(); \
> - if (__builtin_constant_p((uintptr_t)(x) & 1) && ((uintptr_t)(x) 
> & 1) == 0) \
> - *(x) = lswaps(*(x)); \
> - else \
> - swap_uint16((uint16_t *)(x)); \
> + memcpy(, x, 2); \
> + temp = bswap_16(temp); \
> + memcpy(x, , 2); \
>   } while (0)

Any particular reason to think that x might be unaligned (and hence need
the memcpys? Yeah, I know they're probably 'free' when things are
aligned.

The rest of this is

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

Re: [PATCH xserver 1/2] xorg: Change XCONFIGFILE to XCONFIGFILE (and DIR) to fix scan.c.

2017-04-21 Thread Keith Packard

Eric Anholt  writes:

> parser/scan.c was checking for #ifdef XCONFIGFILE and XCONFIGDIR and
> defaulting to "xorg.conf", and "xorg.conf.d", so if you had changed
> __XCONFIGFILE__ to anything else, it would have got out of sync.
> Settle on the name without gratuitous underscores.
>
> Signed-off-by: Eric Anholt 

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

Re: [PATCH xserver 2/2] Remove default defines of some directories.

2017-04-21 Thread Keith Packard

Eric Anholt  writes:

> The build defines these, so having the defaults is just a way for the
> build system's configuration to get out of sync with the code.

>  #ifndef XCONFIGSUFFIX
>  #define XCONFIGSUFFIX".conf"
>  #endif

I think we should get rid of the #ifndef here; as this is not set by the
config system. Alternatively, perhaps this should instead be computed
From the XCONFIGFILE value?


>  #ifndef XCONFENV
>  #define XCONFENV "XORGCONFIG"
>  #endif

Just get rid of the #ifndef here.

The rest of this is

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

[PATCH xserver 3/3] Remove support for unaligned swaps.

2017-04-21 Thread Eric Anholt

The previous misc.h code went out of its way to allow swapping of
unaligned pointers to values.  However, the members of an X
request/response are always naturally aligned within the struct, and
the buffers containing a request/response will also be aligned to at
least 8 bytes, so we can just drop it.

text  data   bssdec  hexfilename
before: 2215167   51552  132016 2398735  249a0f hw/xfree86/Xorg
after:  2214919   51552  132016 2398487  249917 hw/xfree86/Xorg

Signed-off-by: Eric Anholt 
---
 include/misc.h | 15 +++
 test/misc.c| 25 ++---
 2 files changed, 9 insertions(+), 31 deletions(-)

diff --git a/include/misc.h b/include/misc.h
index a75eb617c642..38af70ff9e89 100644
--- a/include/misc.h
+++ b/include/misc.h
@@ -310,12 +310,9 @@ bswap_64(uint64_t x)
 }
 
 #define swapll(x) do { \
-   uint64_t temp; \
if (sizeof(*(x)) != 8) \
wrong_size(); \
-   memcpy(, x, 8); \
-   temp = bswap_64(temp); \
-   memcpy(x, , 8); \
+   *(x) = bswap_64(*(x));  \
} while (0)
 
 static inline uint32_t
@@ -328,12 +325,9 @@ bswap_32(uint32_t x)
 }
 
 #define swapl(x) do { \
-   uint32_t temp; \
if (sizeof(*(x)) != 4) \
wrong_size(); \
-   memcpy(, x, 4); \
-   temp = bswap_32(temp); \
-   memcpy(x, , 4); \
+   *(x) = bswap_32(*(x)); \
} while (0)
 
 static inline uint16_t
@@ -344,12 +338,9 @@ bswap_16(uint16_t x)
 }
 
 #define swaps(x) do { \
-   uint16_t temp; \
if (sizeof(*(x)) != 2) \
wrong_size(); \
-   memcpy(, x, 2); \
-   temp = bswap_16(temp); \
-   memcpy(x, , 2); \
+   *(x) = bswap_16(*(x)); \
} while (0)
 
 /* copy 32-bit value from src to dst byteswapping on the way */
diff --git a/test/misc.c b/test/misc.c
index c10a2b935bc4..3c669b6776ff 100644
--- a/test/misc.c
+++ b/test/misc.c
@@ -204,34 +204,21 @@ bswap_test(void)
 uint16_t result_16;
 uint32_t result_32;
 uint64_t result_64;
-unsigned buffer[sizeof(test_64) + 4];
-void *unaligned = [1];
 
 assert(bswap_16(test_16) == expect_16);
 assert(bswap_32(test_32) == expect_32);
 assert(bswap_64(test_64) == expect_64);
 
-/* Test the swapping-in-a-pointer functions, with unaligned
- * addresses (the functions shouldn't cause traps in that case).
- */
-for (int i = 0; i < 2; i++) {
-unaligned = buffer + i;
-if (((uintptr_t)unaligned & 1) == 1)
-break;
-}
-memcpy(unaligned, _16, sizeof(test_16));
-swaps((uint16_t *)unaligned);
-memcpy(_16, unaligned, sizeof(result_16));
+result_16 = test_16;
+swaps(_16);
 assert(result_16 == expect_16);
 
-memcpy(unaligned, _32, sizeof(test_32));
-swapl((uint32_t *)unaligned);
-memcpy(_32, unaligned, sizeof(result_32));
+result_32 = test_32;
+swapl(_32);
 assert(result_32 == expect_32);
 
-memcpy(unaligned, _64, sizeof(test_64));
-swapll((uint64_t *)unaligned);
-memcpy(_64, unaligned, sizeof(result_64));
+result_64 = test_64;
+swapll(_64);
 assert(result_64 == expect_64);
 }
 
-- 
2.11.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 xserver 2/3] Add unit tests for the byte swapping macros.

2017-04-21 Thread Eric Anholt

Peter noted a weirdness in my new bswap code, which could use some
tests to justify it.

Signed-off-by: Eric Anholt 
Reviewed-by: Peter Hutterer 
---
 test/misc.c | 43 +++
 1 file changed, 43 insertions(+)

diff --git a/test/misc.c b/test/misc.c
index ae46bab9..c10a2b935bc4 100644
--- a/test/misc.c
+++ b/test/misc.c
@@ -192,6 +192,48 @@ dix_request_size_checks(void)
 assert(rc == Success);
 }
 
+static void
+bswap_test(void)
+{
+const uint16_t test_16 = 0xaabb;
+const uint16_t expect_16 = 0xbbaa;
+const uint32_t test_32 = 0xaabbccdd;
+const uint32_t expect_32 = 0xddccbbaa;
+const uint64_t test_64 = 0x11223344aabbccddull;
+const uint64_t expect_64 = 0xddccbbaa44332211ull;
+uint16_t result_16;
+uint32_t result_32;
+uint64_t result_64;
+unsigned buffer[sizeof(test_64) + 4];
+void *unaligned = [1];
+
+assert(bswap_16(test_16) == expect_16);
+assert(bswap_32(test_32) == expect_32);
+assert(bswap_64(test_64) == expect_64);
+
+/* Test the swapping-in-a-pointer functions, with unaligned
+ * addresses (the functions shouldn't cause traps in that case).
+ */
+for (int i = 0; i < 2; i++) {
+unaligned = buffer + i;
+if (((uintptr_t)unaligned & 1) == 1)
+break;
+}
+memcpy(unaligned, _16, sizeof(test_16));
+swaps((uint16_t *)unaligned);
+memcpy(_16, unaligned, sizeof(result_16));
+assert(result_16 == expect_16);
+
+memcpy(unaligned, _32, sizeof(test_32));
+swapl((uint32_t *)unaligned);
+memcpy(_32, unaligned, sizeof(result_32));
+assert(result_32 == expect_32);
+
+memcpy(unaligned, _64, sizeof(test_64));
+swapll((uint64_t *)unaligned);
+memcpy(_64, unaligned, sizeof(result_64));
+assert(result_64 == expect_64);
+}
 
 int
 misc_test(void)
@@ -199,6 +241,7 @@ misc_test(void)
 dix_version_compare();
 dix_update_desktop_dimensions();
 dix_request_size_checks();
+bswap_test();
 
 return 0;
 }
-- 
2.11.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 xserver 1/3] Rewrite the byte swapping macros.

2017-04-21 Thread Eric Anholt

The clever pointer tricks were actually not working, and we were doing
the byte-by-byte moves in general.  By just doing the memcpy and
obvious byte swap code, we end up generating actual byte swap
instructions, thanks to optimizing compilers.

 text  data bss dec hex filename
before: 2240807   51552  132016 2424375  24fe37 hw/xfree86/Xorg
after:  2215167   51552  132016 2398735  249a0f hw/xfree86/Xorg

Signed-off-by: Eric Anholt 
---
 doc/Xserver-spec.xml |  2 +-
 glx/glxbyteorder.h   | 21 
 include/misc.h   | 97 ++--
 os/io.c  |  4 +--
 4 files changed, 37 insertions(+), 87 deletions(-)

diff --git a/doc/Xserver-spec.xml b/doc/Xserver-spec.xml
index 7867544e48a9..3dde65178b7f 100644
--- a/doc/Xserver-spec.xml
+++ b/doc/Xserver-spec.xml
@@ -600,7 +600,7 @@ are: REQUEST, REQUEST_SIZE_MATCH, REQUEST_AT_LEAST_SIZE,
 REQUEST_FIXED_SIZE, LEGAL_NEW_RESOURCE, and
 VALIDATE_DRAWABLE_AND_GC. Useful byte swapping macros can be found
 in Xserver/include/dix.h: WriteReplyToClient and WriteSwappedDataToClient; and
-in Xserver/include/misc.h: lswapl, lswaps, LengthRestB, LengthRestS,
+in Xserver/include/misc.h: bswap_64, bswap_32, bswap_16, LengthRestB, 
LengthRestS,
 LengthRestL, SwapRestS, SwapRestL, swapl, swaps, cpswapl, and cpswaps.
 
 
diff --git a/glx/glxbyteorder.h b/glx/glxbyteorder.h
index 5e94e8626e66..8f0cd8a4b0d7 100644
--- a/glx/glxbyteorder.h
+++ b/glx/glxbyteorder.h
@@ -37,25 +37,4 @@
 
 #include "misc.h"
 
-static inline uint16_t
-bswap_16(uint16_t val)
-{
-swap_uint16();
-return val;
-}
-
-static inline uint32_t
-bswap_32(uint32_t val)
-{
-swap_uint32();
-return val;
-}
-
-static inline uint64_t
-bswap_64(uint64_t val)
-{
-swap_uint64();
-return val;
-}
-
 #endif  /* !defined(__GLXBYTEORDER_H__) */
diff --git a/include/misc.h b/include/misc.h
index 01747fd38339..a75eb617c642 100644
--- a/include/misc.h
+++ b/include/misc.h
@@ -128,21 +128,6 @@ typedef struct _xReq *xReqPtr;
 #define USE_BACKGROUND_PIXEL 3
 #define USE_BORDER_PIXEL 3
 
-/* byte swap a 32-bit literal */
-static inline uint32_t
-lswapl(uint32_t x)
-{
-return ((x & 0xff) << 24) |
-((x & 0xff00) << 8) | ((x & 0xff) >> 8) | ((x >> 24) & 0xff);
-}
-
-/* byte swap a 16-bit literal */
-static inline uint16_t
-lswaps(uint16_t x)
-{
-return (uint16_t)((x & 0xff) << 8) | ((x >> 8) & 0xff);
-}
-
 #undef min
 #undef max
 
@@ -311,88 +296,74 @@ __builtin_constant_p(int x)
 }
 #endif
 
-/* byte swap a 64-bit value */
-static inline void
-swap_uint64(uint64_t *x)
+static inline uint64_t
+bswap_64(uint64_t x)
 {
-char n;
-
-n = ((char *) x)[0];
-((char *) x)[0] = ((char *) x)[7];
-((char *) x)[7] = n;
-
-n = ((char *) x)[1];
-((char *) x)[1] = ((char *) x)[6];
-((char *) x)[6] = n;
-
-n = ((char *) x)[2];
-((char *) x)[2] = ((char *) x)[5];
-((char *) x)[5] = n;
-
-n = ((char *) x)[3];
-((char *) x)[3] = ((char *) x)[4];
-((char *) x)[4] = n;
+return (((x & 0xFF00ull) >> 56) |
+((x & 0x00FFull) >> 40) |
+((x & 0xFF00ull) >> 24) |
+((x & 0x00FFull) >>  8) |
+((x & 0xFF00ull) <<  8) |
+((x & 0x00FFull) << 24) |
+((x & 0xFF00ull) << 40) |
+((x & 0x00FFull) << 56));
 }
 
 #define swapll(x) do { \
+   uint64_t temp; \
if (sizeof(*(x)) != 8) \
wrong_size(); \
-swap_uint64((uint64_t *)(x));   \
+   memcpy(, x, 8); \
+   temp = bswap_64(temp); \
+   memcpy(x, , 8); \
} while (0)
 
-/* byte swap a 32-bit value */
-static inline void
-swap_uint32(uint32_t * x)
+static inline uint32_t
+bswap_32(uint32_t x)
 {
-char n = ((char *) x)[0];
-
-((char *) x)[0] = ((char *) x)[3];
-((char *) x)[3] = n;
-n = ((char *) x)[1];
-((char *) x)[1] = ((char *) x)[2];
-((char *) x)[2] = n;
+return (((x & 0xFF00) >> 24) |
+((x & 0x00FF) >> 8) |
+((x & 0xFF00) << 8) |
+((x & 0x00FF) << 24));
 }
 
 #define swapl(x) do { \
+   uint32_t temp; \
if (sizeof(*(x)) != 4) \
wrong_size(); \
-   if (__builtin_constant_p((uintptr_t)(x) & 3) && ((uintptr_t)(x) 
& 3) == 0) \
-   *(x) = lswapl(*(x)); \
-   else \
-   swap_uint32((uint32_t *)(x)); \
+   memcpy(, x, 4); \
+   temp = bswap_32(temp); \
+   memcpy(x, , 4); \
} while (0)
 
-/* byte swap a 16-bit value */
-static inline void
-swap_uint16(uint16_t * x)
+static inline uint16_t
+bswap_16(uint16_t x)
 {
-char n = ((char *) x)[0];
-
-((char *) x)[0] = ((char *) x)[1];
-

[PATCH xserver 1/2] xorg: Change XCONFIGFILE to XCONFIGFILE (and DIR) to fix scan.c.

2017-04-21 Thread Eric Anholt

parser/scan.c was checking for #ifdef XCONFIGFILE and XCONFIGDIR and
defaulting to "xorg.conf", and "xorg.conf.d", so if you had changed
__XCONFIGFILE__ to anything else, it would have got out of sync.
Settle on the name without gratuitous underscores.

Signed-off-by: Eric Anholt 
---
 configure.ac | 4 ++--
 hw/xfree86/common/xf86Init.c | 6 +++---
 include/xorg-config.h.in | 4 ++--
 include/xorg-server.h.in | 2 +-
 manpages.am  | 2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index 07c5520940a1..4ee43d2d064b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2024,9 +2024,9 @@ if test "x$XORG" = xyes; then
AC_DEFINE(NEED_XF86_TYPES, 1, [Need XFree86 typedefs])
AC_DEFINE(NEED_XF86_PROTOTYPES, 1, [Need XFree86 helper functions])
AC_DEFINE(__XSERVERNAME__, "Xorg", [Name of X server])
-   AC_DEFINE_DIR(__XCONFIGFILE__, XF86CONFIGFILE, [Name of configuration 
file])
+   AC_DEFINE_DIR(XCONFIGFILE, XF86CONFIGFILE, [Name of configuration file])
AC_DEFINE_DIR(XF86CONFIGFILE, XF86CONFIGFILE, [Name of configuration 
file])
-   AC_DEFINE_DIR(__XCONFIGDIR__, XF86CONFIGDIR, [Name of configuration 
directory])
+   AC_DEFINE_DIR(XCONFIGDIR, XF86CONFIGDIR, [Name of configuration 
directory])
AC_DEFINE_DIR(DEFAULT_MODULE_PATH, moduledir, [Default module search 
path])
AC_DEFINE_DIR(DEFAULT_LIBRARY_PATH, libdir, [Default library install 
path])
AC_DEFINE_DIR(DEFAULT_LOGDIR, logdir, [Default log location])
diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
index 88f99ac5ecc0..d3c7c47b01e2 100644
--- a/hw/xfree86/common/xf86Init.c
+++ b/hw/xfree86/common/xf86Init.c
@@ -1359,17 +1359,17 @@ ddxUseMsg(void)
 ErrorF("-modulepath paths  specify the module search path\n");
 ErrorF("-logfile file  specify a log file name\n");
 ErrorF("-configure probe for devices and write an "
-   __XCONFIGFILE__ "\n");
+   XCONFIGFILE "\n");
 ErrorF
 ("-showopts  print available options for all installed 
drivers\n");
 }
 ErrorF
 ("-config file   specify a configuration file, relative to 
the\n");
-ErrorF("   " __XCONFIGFILE__
+ErrorF("   " XCONFIGFILE
" search path, only root can use absolute\n");
 ErrorF
 ("-configdir dir specify a configuration directory, relative 
to the\n");
-ErrorF("   " __XCONFIGDIR__
+ErrorF("   " XCONFIGDIR
" search path, only root can use absolute\n");
 ErrorF("-verbose [n]   verbose startup messages\n");
 ErrorF("-logverbose [n]verbose log messages\n");
diff --git a/include/xorg-config.h.in b/include/xorg-config.h.in
index a7d80b5afbf6..b8d6a87a3363 100644
--- a/include/xorg-config.h.in
+++ b/include/xorg-config.h.in
@@ -34,10 +34,10 @@
 #undef XF86CONFIGFILE
 
 /* Path to configuration file. */
-#undef __XCONFIGFILE__
+#undef XCONFIGFILE
 
 /* Name of configuration directory. */
-#undef __XCONFIGDIR__
+#undef XCONFIGDIR
 
 /* Path to loadable modules. */
 #undef DEFAULT_MODULE_PATH
diff --git a/include/xorg-server.h.in b/include/xorg-server.h.in
index dafc27f4ae79..570893e34bb1 100644
--- a/include/xorg-server.h.in
+++ b/include/xorg-server.h.in
@@ -180,7 +180,7 @@
 #undef __VENDORDWEBSUPPORT__
 
 /* Location of configuration file */
-#undef __XCONFIGFILE__
+#undef XCONFIGFILE
 
 /* Name of X server */
 #undef __XSERVERNAME__
diff --git a/manpages.am b/manpages.am
index 648210b4efbd..124bb557551f 100644
--- a/manpages.am
+++ b/manpages.am
@@ -22,7 +22,7 @@ MAN_SUBSTS += -e 's|__logdir__|$(logdir)|g' \
-e 's|__datadir__|$(datadir)|g' \
-e 's|__mandir__|$(mandir)|g' \
-e 's|__sysconfdir__|$(sysconfdir)|g' \
-   -e 's|__xconfigdir__|$(__XCONFIGDIR__)|g' \
+   -e 's|__xconfigdir__|$(XCONFIGDIR)|g' \
-e 's|__xkbdir__|$(XKB_BASE_DIRECTORY)|g' \
-e 's|__XKB_DFLT_RULES__|$(XKB_DFLT_RULES)|g' \
-e 's|__XKB_DFLT_MODEL__|$(XKB_DFLT_MODEL)|g' \
-- 
2.11.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 xserver 2/2] Remove default defines of some directories.

2017-04-21 Thread Eric Anholt

The build defines these, so having the defaults is just a way for the
build system's configuration to get out of sync with the code.

Signed-off-by: Eric Anholt 
---
 hw/xfree86/parser/Makefile.am |  1 -
 hw/xfree86/parser/scan.c  | 15 ---
 xkb/ddxLoad.c | 12 
 3 files changed, 28 deletions(-)

diff --git a/hw/xfree86/parser/Makefile.am b/hw/xfree86/parser/Makefile.am
index 9aa8cfefbd2b..2e4c6afdb21e 100644
--- a/hw/xfree86/parser/Makefile.am
+++ b/hw/xfree86/parser/Makefile.am
@@ -24,7 +24,6 @@ libxf86config_la_SOURCES = \
$(INTERNAL_SOURCES)
 
 AM_CFLAGS = $(DIX_CFLAGS) $(XORG_CFLAGS) \
-   -DSYSCONFDIR=\"$(sysconfdir)\" \
-DDATADIR=\"$(datadir)\"
 
 EXTRA_DIST = \
diff --git a/hw/xfree86/parser/scan.c b/hw/xfree86/parser/scan.c
index 3356224ce061..64a6274634ea 100644
--- a/hw/xfree86/parser/scan.c
+++ b/hw/xfree86/parser/scan.c
@@ -542,24 +542,9 @@ xf86pathIsSafe(const char *path)
  *%%%
  */
 
-#ifndef XCONFIGFILE
-#define XCONFIGFILE"xorg.conf"
-#endif
-#ifndef XCONFIGDIR
-#define XCONFIGDIR "xorg.conf.d"
-#endif
 #ifndef XCONFIGSUFFIX
 #define XCONFIGSUFFIX  ".conf"
 #endif
-#ifndef PROJECTROOT
-#define PROJECTROOT"/usr/X11R6"
-#endif
-#ifndef SYSCONFDIR
-#define SYSCONFDIR PROJECTROOT "/etc"
-#endif
-#ifndef DATADIR
-#define DATADIRPROJECTROOT "/share"
-#endif
 #ifndef XCONFENV
 #define XCONFENV   "XORGCONFIG"
 #endif
diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c
index f71815aa814b..a1a0fd3a28be 100644
--- a/xkb/ddxLoad.c
+++ b/xkb/ddxLoad.c
@@ -45,18 +45,6 @@ THE USE OR PERFORMANCE OF THIS SOFTWARE.
 #include 
 #include "xkb.h"
 
-/*
- * If XKM_OUTPUT_DIR specifies a path without a leading slash, it is
- * relative to the top-level XKB configuration directory.
- * Making the server write to a subdirectory of that directory
- * requires some work in the general case (install procedure
- * has to create links to /var or somesuch on many machines),
- * so we just compile into /usr/tmp for now.
- */
-#ifndef XKM_OUTPUT_DIR
-#defineXKM_OUTPUT_DIR  "compiled/"
-#endif
-
 #definePRE_ERROR_MSG "\"The XKEYBOARD keymap compiler (xkbcomp) 
reports:\""
 #defineERROR_PREFIX"\"> \""
 #definePOST_ERROR_MSG1 "\"Errors from xkbcomp are not fatal to the X 
server\""
-- 
2.11.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] glamor: an FBO is not needed for Xv pixmaps

2017-04-21 Thread Eric Anholt

Olivier Fourdan  writes:

> It appears that on some hardware/diver combo such as nv30/nouveau, using
> GL_ALPHA as format for 8-bit depth will cause an incomplete attachment
> error (GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT) when trying to bind the
> texture.
>
> As a result, the FBO is NULL and glamor segfaults when trying to access
> the FBO width/height in pixmap_priv_get_scale() in glamor_xv_render().
>
> This happens with glamor-xv which uses 8-bit pixmaps, meaning that on
> such hardware/driver, trying to play a video using Xv will lead to a
> crash of the Xserver. This affects Xwayland, Xephyr, modesetting driver
> with glamor accel.
>
> But the use of an FBO is not actually needed for glamox-xv, so by
> disabling FBO at pixmap creation, we can avoid the issue entirely.
>
> Fix suggested by Eric Anholt 
>
> Signed-off-by: Olivier Fourdan 
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=100710
> Fixes: https://bugzilla.redhat.com/1412814

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 1/2] Introduce keyboard grabbing protocol for Xwayland

2017-04-21 Thread Olivier Fourdan


Hi Jonas,

> > [...]
> > For that last point, I'd rather use:
> > 
> > * does not guarantee that events sent to this client are continuous,
> >   a compositor may change and reroute keyboard events while the grab
> >   is nominally active.
> > 
> > > hmm, and thinking about the last point: do we need to specify what the
> > > focus
> > > behaviour is if a grab is active? I'm assuming 'no' because there is no
> > > notification whatsoever whether it ever activates anyway, but...
> > 
> > No, indeed, that's precisely the main difference with the shortcuts
> > inhibitor logic for the wayland native apps.
> > 
> > Here, keyboards events are routed to the surface regardless of the focus,
> > just like an X11 grab.
> 
> Should this part really be respected though? In what circumstances does
> it make sense to route input events to Xwayland when a Wayland client is
> the one focused?

Yes, that's precisely the whole point of this protocol (and grabs in general) :)

Take the attached sample code for example, it maps a single override redirect 
window (one that no X11 window manager would focus, because it's an O-R) and 
issues an active keyboard grab to get all keyboard events.


While that works fine in X11 native, that won't work in Wayland/Xwayland 
without this protocol and the ability to route keyboard events even when the 
surface is not focused.

> > > Last question: how will you deal with XGrabKeyboard() requests on already
> > > grabbed keyboards? I can send that request twice with different grab
> > > windows
> > > and it'll change the grab over (iirc). Xwayland will destroy the current
> > > grab and request a new one?
> > 
> > Yeap, good point, "XGrabKeyboard overrides any active keyboard grab by the
> > client" so Xwayland needs to destroy any current grab before setting a new
> > one in this case.
> >  
> 
> FWIW, this will create a minor race, where any key presses between the
> .destroy and .grab requests will be ungrabbed.

Yeah, you're right, but I reckon it's acceptable.

Cheers,
Olivier

/*
 * Compile with :
 *
 * gcc -Wall -g grab.c -o grab `pkg-config --cflags gtk+-2.0` `pkg-config --libs gtk+-2.0`
 * 
 */
#include 


static gboolean
on_map_event_cb (GtkWidget *widget,
 GdkEvent  *event,
 gpointer   user_data)
{
GdkWindow *gdk_window = gtk_widget_get_window (widget);

gdk_keyboard_grab (gdk_window, TRUE, GDK_CURRENT_TIME);
g_message ("Widget %p is mapped!", widget);

return FALSE;
}

static void
destroy (GtkWidget *widget, gpointer data)
{
gtk_main_quit ();
}

static gboolean
on_key_press_event (GtkWidget *widget, GdkEventKey *event, gpointer data)
{
g_message ("Key 0x%x pressed, widget %p", event->keyval, widget);

return FALSE;
}

static gboolean
on_key_release_event (GtkWidget *widget, GdkEventKey *event, gpointer data)
{
g_message ("Key 0x%x released, widget %p", event->keyval, widget);

return FALSE;
}

int
main (int argc, char *argv[])
{
GtkWidget *window1;
GtkWidget *button;
GtkWidget *vbox;
GtkWidget *label;
GtkWidget *entry;
gchar* str;

gtk_init (, );

/* Popup window (override redirect on X11) */
window1 = gtk_window_new (GTK_WINDOW_POPUP);
gtk_window_set_keep_above (GTK_WINDOW (window1), TRUE);
gtk_window_set_position (GTK_WINDOW (window1), GTK_WIN_POS_CENTER_ALWAYS);
gtk_container_set_border_width (GTK_CONTAINER (window1), 25);
g_signal_connect (G_OBJECT (window1), "destroy", G_CALLBACK (destroy), NULL);
g_signal_connect_after (G_OBJECT (window1), "map-event", G_CALLBACK (on_map_event_cb), NULL);

vbox = gtk_vbox_new (FALSE, 10);
gtk_container_add (GTK_CONTAINER (window1), vbox);

label = gtk_label_new (NULL);
gtk_label_set_use_markup (GTK_LABEL (label), TRUE);
str = g_markup_printf_escaped ("This is widget %p", window1);
gtk_label_set_markup (GTK_LABEL (label), str);
g_free (str);
gtk_box_pack_start (GTK_BOX (vbox), label, TRUE, TRUE, 0);

entry = gtk_entry_new ();
gtk_entry_set_text (GTK_ENTRY (entry), "Enter some text here...");
gtk_box_pack_start (GTK_BOX (vbox), entry, TRUE, TRUE, 0);

button = gtk_toggle_button_new_with_label ("Done");
gtk_box_pack_start (GTK_BOX (vbox), button, TRUE, TRUE, 0);
g_signal_connect (G_OBJECT (button), "clicked", G_CALLBACK (gtk_main_quit), NULL);

g_signal_connect (G_OBJECT (window1), "key-press-event", G_CALLBACK(on_key_press_event), NULL);
g_signal_connect (G_OBJECT (window1), "key-release-event", G_CALLBACK(on_key_release_event), NULL);

gtk_widget_show_all (window1);

gtk_main ();

return 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 xserver] glamor: an FBO is not needed for Xv pixmaps

2017-04-21 Thread Olivier Fourdan

It appears that on some hardware/diver combo such as nv30/nouveau, using
GL_ALPHA as format for 8-bit depth will cause an incomplete attachment
error (GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT) when trying to bind the
texture.

As a result, the FBO is NULL and glamor segfaults when trying to access
the FBO width/height in pixmap_priv_get_scale() in glamor_xv_render().

This happens with glamor-xv which uses 8-bit pixmaps, meaning that on
such hardware/driver, trying to play a video using Xv will lead to a
crash of the Xserver. This affects Xwayland, Xephyr, modesetting driver
with glamor accel.

But the use of an FBO is not actually needed for glamox-xv, so by
disabling FBO at pixmap creation, we can avoid the issue entirely.

Fix suggested by Eric Anholt 

Signed-off-by: Olivier Fourdan 
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=100710
Fixes: https://bugzilla.redhat.com/1412814
---
 glamor/glamor_xv.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/glamor/glamor_xv.c b/glamor/glamor_xv.c
index 3bcf909..31320d1 100644
--- a/glamor/glamor_xv.c
+++ b/glamor/glamor_xv.c
@@ -430,11 +430,14 @@ glamor_xv_put_image(glamor_port_private *port_priv,
 glamor_destroy_pixmap(port_priv->src_pix[i]);
 
 port_priv->src_pix[0] =
-glamor_create_pixmap(pScreen, width, height, 8, 0);
+glamor_create_pixmap(pScreen, width, height, 8,
+ GLAMOR_CREATE_FBO_NO_FBO);
 port_priv->src_pix[1] =
-glamor_create_pixmap(pScreen, width >> 1, height >> 1, 8, 0);
+glamor_create_pixmap(pScreen, width >> 1, height >> 1, 8,
+ GLAMOR_CREATE_FBO_NO_FBO);
 port_priv->src_pix[2] =
-glamor_create_pixmap(pScreen, width >> 1, height >> 1, 8, 0);
+glamor_create_pixmap(pScreen, width >> 1, height >> 1, 8,
+ GLAMOR_CREATE_FBO_NO_FBO);
 port_priv->src_pix_w = width;
 port_priv->src_pix_h = height;
 
-- 
2.9.3

___
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: [RFC PATCH xserver 0/5] Check glamor-xv usability

2017-04-21 Thread Olivier Fourdan


Hi Michel,

> > [...]
> 
> This means that external drivers calling glamor_xv_init would also have
> to be changed accordingly, which is a bit unfortunate. Also, I'm not
> sure deferring xf86XVScreenInit to CreateScreenResources is a good idea.
> 
> So, I hope one of Eric's suggestions will work out.

Yeah, let's forget that series, GLAMOR_CREATE_FBO_NO_FBO works :)

Cheers,
Olivier
___
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 5/5] glamor: Use GL_RGBA format for 8-bit depth

2017-04-21 Thread Olivier Fourdan


Hi Eric,

> Olivier Fourdan  writes:
> 
> > For some hardware, GL_RGBA is the safest option for textures to bind to
> > an FBO for depth 8 as GL_ALPHA might be unsupported.
> >
> > This allows using glamor-xv on hardware that don't support GL_ALPHA
> > textures attaching to an FBO, such as nouveau on nv30.
> >
> > Newer hardware use GL_RED and texture swizzle anyway, so this change is
> > of limited impact.
> 
> Does nv30 not support *any* 8-bit render target?

Tried with GL_INTENSITY, didn't work either.

> Also, blowing up our XV textures to 32-bit can't be good for its
> performance.  Could we just use GLAMOR_CREATE_FBO_NO_FBO (worst name) to
> have our XV pixmaps not actually create an FBO?  We already use
> glamor_upload_boxes(), so I think that would work.

Blimey, you're absolutely correct, I took for granted that the FBO was needed 
(also because pixmap_priv_get_scale() uses pixmap_priv->fbo size) but it is not 
and it works just fine with no FBO (failing to attach to the FBO releases the 
fbo entirely whereas GLAMOR_CREATE_FBO_NO_FBO creates the structure but not the 
actual framebuffer object so it's just fine)

Trivial patch to follow shortly.

Cheers,
Olivier
___
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 1/3] Rewrite the byte swapping macros.

Re: [PATCH app/xdpyinfo] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

Re: [PATCH xproto] Add XF86XK_Keyboard

Re: [PATCH xserver 1/3] Rewrite the byte swapping macros.

Re: [PATCH xserver 1/2] xorg: Change XCONFIGFILE to XCONFIGFILE (and DIR) to fix scan.c.

Re: [PATCH xserver 2/2] Remove default defines of some directories.

[PATCH xserver 3/3] Remove support for unaligned swaps.

[PATCH xserver 2/3] Add unit tests for the byte swapping macros.

[PATCH xserver 1/3] Rewrite the byte swapping macros.

[PATCH xserver 1/2] xorg: Change XCONFIGFILE to XCONFIGFILE (and DIR) to fix scan.c.

[PATCH xserver 2/2] Remove default defines of some directories.

Re: [PATCH xserver] glamor: an FBO is not needed for Xv pixmaps

Re: [PATCH 1/2] Introduce keyboard grabbing protocol for Xwayland

[PATCH xserver] glamor: an FBO is not needed for Xv pixmaps

Re: [RFC PATCH xserver 0/5] Check glamor-xv usability

Re: [PATCH xserver 5/5] glamor: Use GL_RGBA format for 8-bit depth

16 matches

Site Navigation

Mail list logo

Footer information