[PATCH wayland v3 4/5] Add support for signed 24.8 decimal numbers

Kristian Høgsberg hoegsberg at gmail.com
Mon May 7 09:14:16 PDT 2012


On Fri, May 04, 2012 at 11:21:22AM +0100, Daniel Stone wrote:
> signed_24_8 is a signed decimal type which offers a sign bit, 23 bits of
> integer precision, and 8 bits of decimal precision.  This is converted
> to double on the C API side, as passing through varargs involves an
> implicit type promotion to double anyway.

I'd like to follow cairo's convention here and just call it
fixed/wl_fixed_t.  Signed is implicit like for 'int', and if we need
an unsigned fixed we can call it ufixed/wl_ufixed_t.

Just few more comments below.

Kristian

> Signed-off-by: Daniel Stone <daniel at fooishbar.org>
> ---
>  src/Makefile.am         |    4 ++--
>  src/connection.c        |   20 +++++++++++++++++++-
>  src/scanner.c           |    9 +++++++++
>  src/wayland-server.h    |    1 +
>  src/wayland-util.h      |    7 +++++++
>  tests/connection-test.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 80 insertions(+), 3 deletions(-)
> 
> v3: Use a new typedef instead of double.
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index c685885..f93954e 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -18,7 +18,7 @@ libwayland_util_la_SOURCES =			\
>  	wayland-os.h				\
>  	wayland-private.h
>  
> -libwayland_server_la_LIBADD = $(FFI_LIBS) libwayland-util.la -lrt
> +libwayland_server_la_LIBADD = $(FFI_LIBS) libwayland-util.la -lrt -lm
>  libwayland_server_la_SOURCES =			\
>  	wayland-protocol.c			\
>  	wayland-server.c			\
> @@ -26,7 +26,7 @@ libwayland_server_la_SOURCES =			\
>  	data-device.c				\
>  	event-loop.c
>  
> -libwayland_client_la_LIBADD = $(FFI_LIBS) libwayland-util.la -lrt
> +libwayland_client_la_LIBADD = $(FFI_LIBS) libwayland-util.la -lrt -lm
>  libwayland_client_la_SOURCES =			\
>  	wayland-protocol.c			\
>  	wayland-client.c
> diff --git a/src/connection.c b/src/connection.c
> index 06cc66f..8d16dc4 100644
> --- a/src/connection.c
> +++ b/src/connection.c
> @@ -22,6 +22,7 @@
>  
>  #define _GNU_SOURCE
>  
> +#include <math.h>
>  #include <stdlib.h>
>  #include <stdint.h>
>  #include <string.h>
> @@ -428,6 +429,13 @@ wl_closure_vmarshal(struct wl_closure *closure,
>  
>  	for (i = 2; i < count; i++) {
>  		switch (message->signature[i - 2]) {
> +		case 'f':
> +			closure->types[i] = &ffi_type_sint32;
> +			closure->args[i] = p;
> +			if (end - p < 1)
> +				goto err;
> +			*p++ = va_arg(ap, wl_signed_24_8_t);
> +			break;
>  		case 'u':
>  			closure->types[i] = &ffi_type_uint32;
>  			closure->args[i] = p;
> @@ -611,6 +619,10 @@ wl_connection_demarshal(struct wl_connection *connection,
>  			closure->types[i] = &ffi_type_sint32;
>  			closure->args[i] = p++;
>  			break;
> +		case 'f':
> +			closure->types[i] = &ffi_type_sint32;
> +			closure->args[i] = p++;
> +			break;
>  		case 's':
>  			closure->types[i] = &ffi_type_pointer;
>  			length = *p++;
> @@ -812,6 +824,7 @@ void
>  wl_closure_print(struct wl_closure *closure, struct wl_object *target, int send)
>  {
>  	union wl_value *value;
> +	int32_t si;
>  	int i;
>  	struct timespec tp;
>  	unsigned int time;
> @@ -835,7 +848,12 @@ wl_closure_print(struct wl_closure *closure, struct wl_object *target, int send)
>  			fprintf(stderr, "%u", value->uint32);
>  			break;
>  		case 'i':
> -			fprintf(stderr, "%d", value->uint32);
> +			si = (int32_t) value->uint32;
> +			fprintf(stderr, "%d", si);
> +			break;
> +		case 'f':
> +			si = (int32_t) value->uint32;
> +			fprintf(stderr, "%f", wl_signed_24_8_to_double(si));
>  			break;
>  		case 's':
>  			fprintf(stderr, "\"%s\"", value->string);
> diff --git a/src/scanner.c b/src/scanner.c
> index 3fba0ad..a13b943 100644
> --- a/src/scanner.c
> +++ b/src/scanner.c
> @@ -82,6 +82,7 @@ enum arg_type {
>  	NEW_ID,
>  	INT,
>  	UNSIGNED,
> +	SIGNED_24_8,
>  	STRING,
>  	OBJECT,
>  	ARRAY,
> @@ -296,6 +297,8 @@ start_element(void *data, const char *element_name, const char **atts)
>  			arg->type = INT;
>  		else if (strcmp(type, "uint") == 0)
>  			arg->type = UNSIGNED;
> +		else if (strcmp(type, "signed_24_8") == 0)
> +			arg->type = SIGNED_24_8;
>  		else if (strcmp(type, "string") == 0)
>  			arg->type = STRING;
>  		else if (strcmp(type, "array") == 0)
> @@ -446,6 +449,9 @@ emit_type(struct arg *a)
>  	case UNSIGNED:
>  		printf("uint32_t ");
>  		break;
> +	case SIGNED_24_8:
> +		printf("wl_signed_24_8_t ");
> +		break;
>  	case STRING:
>  		printf("const char *");
>  		break;
> @@ -953,6 +959,9 @@ emit_messages(struct wl_list *message_list,
>  			case UNSIGNED:
>  				printf("u");
>  				break;
> +			case SIGNED_24_8:
> +				printf("f");
> +				break;
>  			case STRING:
>  				printf("s");
>  				break;
> diff --git a/src/wayland-server.h b/src/wayland-server.h
> index 9d4f58e..73c6f57 100644
> --- a/src/wayland-server.h
> +++ b/src/wayland-server.h
> @@ -281,6 +281,7 @@ struct wl_input_device {
>   * The variable arguments' types are:
>   * - type=uint: 	uint32_t
>   * - type=int:		int32_t
> + * - type=signed_24_8:	wl_signed_24_8_t
>   * - type=string:	(const char *) to a nil-terminated string
>   * - type=array:	(struct wl_array *)
>   * - type=fd:		int, that is an open file descriptor
> diff --git a/src/wayland-util.h b/src/wayland-util.h
> index 1b8fd4b..ebc60b5 100644
> --- a/src/wayland-util.h
> +++ b/src/wayland-util.h
> @@ -165,6 +165,13 @@ void wl_array_release(struct wl_array *array);
>  void *wl_array_add(struct wl_array *array, size_t size);
>  void wl_array_copy(struct wl_array *array, struct wl_array *source);
>  
> +
> +typedef int32_t wl_signed_24_8_t;
> +#define wl_signed_24_8_to_double(x) ((double) (x) / 256.0)
> +#define wl_signed_24_8_from_double(x) ((int32_t) round(x * 256.0))
> +#define wl_signed_24_8_to_int(x) (x / 256)
> +#define wl_signed_24_8_from_int(x) (x * 256)

Let's do these as static inline functions and #include math.h.

>  #ifdef  __cplusplus
>  }
>  #endif
> diff --git a/tests/connection-test.c b/tests/connection-test.c
> index 54ac423..608c7f6 100644
> --- a/tests/connection-test.c
> +++ b/tests/connection-test.c
> @@ -20,6 +20,7 @@
>   * OF THIS SOFTWARE.
>   */
>  
> +#include <math.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <stdarg.h>
> @@ -288,6 +289,17 @@ validate_demarshal_h(struct marshal_data *data,
>  }
>  
>  static void
> +validate_demarshal_f(struct marshal_data *data,
> +		     struct wl_object *object, wl_signed_24_8_t f)
> +{
> +	double in = wl_signed_24_8_to_double(data->value.i);
> +	double wire = wl_signed_24_8_to_double(f);
> +
> +	assert(data->value.i == f);
> +	assert(fabs(in - wire) <= 1.0 / 256.0);

This last assert is just testing the fixed/double conversion macros.
We should do that, but let's not mix it up with the demarshal tests.

> +}
> +
> +static void
>  demarshal(struct marshal_data *data, const char *format,
>  	  uint32_t *msg, void (*func)(void))
>  {
> @@ -335,6 +347,24 @@ TEST(connection_demarshal)
>  	memcpy(&msg[3], data.value.s, msg[2]);
>  	demarshal(&data, "s", msg, (void *) validate_demarshal_s);
>  
> +	data.value.i = wl_signed_24_8_from_double(5678.1234);
> +	msg[0] = 400200;
> +	msg[1] = 12;
> +	msg[2] = data.value.i;
> +	demarshal(&data, "f", msg, (void *) validate_demarshal_f);
> +
> +	data.value.i = wl_signed_24_8_from_double(-90000.2390);
> +	msg[0] = 400200;
> +	msg[1] = 12;
> +	msg[2] = data.value.i;
> +	demarshal(&data, "f", msg, (void *) validate_demarshal_f);
> +
> +	data.value.i = wl_signed_24_8_from_int((1 << 23) - 1);
> +	msg[0] = 400200;
> +	msg[1] = 12;
> +	msg[2] = data.value.i;
> +	demarshal(&data, "f", msg, (void *) validate_demarshal_f);
> +
>  	release_marshal_data(&data);
>  }
>  
> @@ -400,6 +430,18 @@ TEST(connection_marshal_demarshal)
>  	marshal_demarshal(&data, (void *) validate_demarshal_h,
>  			  8, "h", data.value.h);
>  
> +	data.value.i = wl_signed_24_8_from_double(1234.5678);
> +	marshal_demarshal(&data, (void *) validate_demarshal_f,
> +	                  12, "f", data.value.i);
> +
> +	data.value.i = wl_signed_24_8_from_double(-90000.2390);
> +	marshal_demarshal(&data, (void *) validate_demarshal_f,
> +	                  12, "f", data.value.i);
> +
> +	data.value.i = wl_signed_24_8_from_double((1 << 23) - 1 + 0.0941);
> +	marshal_demarshal(&data, (void *) validate_demarshal_f,
> +	                  12, "f", data.value.i);
> +
>  	release_marshal_data(&data);
>  }
>  
> -- 
> 1.7.10
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel


More information about the wayland-devel mailing list