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 <[email protected]> > --- > 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 > [email protected] > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
