[PATCH weston v2 12/21] Introduce a 'double fixed' data type abstraction
Peter Hutterer
peter.hutterer at who-t.net
Thu Jun 11 15:40:02 PDT 2015
On Thu, Jun 11, 2015 at 02:37:38PM -0700, Bill Spitzak wrote:
> Do you actually need 64 bits or are you just trying to get more than 8 bits
> of fraction?
>
> If instead other fixed types were provided, such as 1.30 or 7.24, etc could
> that be used? This could be covered by the existing fixed type by just
> defining the argument as being the desired value multiplied by some power
> of 2.
any 32-bit fixed point will hit the limits in one way our another, so you're
just messing up the space. it's easier to have two data types where one will
definitely be enough.
Cheers,
Peter
> The advantage is that these fit in 32 bits used for the api, and also that
> it is possible to do fast conversions to/from them using double floating
> point hardware (ie adding a large constant so the exponent is known and
> then masking out the correct section of bits). This can't be done if the
> integer is the same size as the double because the exponent takes some of
> the bits.
>
> I personally feel that adding float to the api would not be a bad idea.
>
> On Wed, May 13, 2015 at 4:02 AM, Jonas Ådahl <jadahl at gmail.com> wrote:
>
> > On Wed, May 13, 2015 at 01:44:02PM +0300, Giulio Camuffo wrote:
> > > 2015-05-13 13:26 GMT+03:00 Jonas Ådahl <jadahl at gmail.com>:
> > > > The 'double fixed' value type is a fixed point data type implemented as
> > > > two signed 32 bit integers. It is intended to be sent over the wire and
> > > > used when wl_fixed_t is not detailed enough.
> > > >
> > > > Two helper functions are introduced: wl_double_fixed_to_double and
> > > > wl_double_fixed_from_double that can be used for conversion.
> > > >
> > > > The type is implemented by separating the integral part from the
> > > > fractional part, storing them in individual signed 32 bit integers.
> > > >
> > > > Converting to a double could be implemented with:
> > > >
> > > > double = integral + fraction / 2147483648
> > > >
> > > > Converting to a double could be implemented with:
> > > >
> > > > integral = integral part of double
> > > > fraction = fractional part of double * 2147483648
> > > >
> > > > The implementation provided in this patch is slightly faster than the
> > > > above methods, but assumes that the IEEE 754 double-precision binary
> > > > floating-point format is used internally. It is based on the wl_fixed_t
> > > > conversion functions from wayland-util.h.
> > > >
> > > > Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> > > > ---
> > > > Makefile.am | 6 +++
> > > > shared/util.h | 57 +++++++++++++++++++++
> > > > tests/double-fixed-benchmark.c | 110
> > +++++++++++++++++++++++++++++++++++++++++
> > > > 3 files changed, 173 insertions(+)
> > > > create mode 100644 shared/util.h
> > > > create mode 100644 tests/double-fixed-benchmark.c
> > > >
> > > > diff --git a/Makefile.am b/Makefile.am
> > > > index 2f16fac..0a30cb4 100644
> > > > --- a/Makefile.am
> > > > +++ b/Makefile.am
> > > > @@ -990,6 +990,7 @@ noinst_PROGRAMS += \
> > > > $(shared_tests) \
> > > > $(weston_tests) \
> > > > $(ivi_tests) \
> > > > + double-fixed-benchmark \
> > > > matrix-test
> > > >
> > > > test_module_ldflags = \
> > > > @@ -1102,6 +1103,11 @@ matrix_test_SOURCES =
> > \
> > > > matrix_test_CPPFLAGS = -DUNIT_TEST
> > > > matrix_test_LDADD = -lm -lrt
> > > >
> > > > +double_fixed_benchmark_SOURCES = \
> > > > + tests/double-fixed-benchmark.c \
> > > > + shared/util.h
> > > > +double_fixed_benchmark_LDADD = -lm
> > > > +
> > > > if ENABLE_IVI_SHELL
> > > > module_tests += \
> > > > ivi-layout-internal-test.la \
> > > > diff --git a/shared/util.h b/shared/util.h
> > > > new file mode 100644
> > > > index 0000000..97a6f9b
> > > > --- /dev/null
> > > > +++ b/shared/util.h
> > > > @@ -0,0 +1,57 @@
> > > > +/*
> > > > + * Copyright © 2008 Kristian Høgsberg
> > > > + * Copyright © 2015 Red Hat Inc.
> > > > + *
> > > > + * Permission to use, copy, modify, distribute, and sell this
> > software and its
> > > > + * documentation for any purpose is hereby granted without fee,
> > provided that
> > > > + * the above copyright notice appear in all copies and that both that
> > copyright
> > > > + * notice and this permission notice appear in supporting
> > documentation, and
> > > > + * that the name of the copyright holders not be used in advertising
> > or
> > > > + * publicity pertaining to distribution of the software without
> > specific,
> > > > + * written prior permission. The copyright holders make no
> > representations
> > > > + * about the suitability of this software for any purpose. It is
> > provided "as
> > > > + * is" without express or implied warranty.
> > > > + *
> > > > + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> > SOFTWARE,
> > > > + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS,
> > IN NO
> > > > + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL,
> > INDIRECT OR
> > > > + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
> > LOSS OF USE,
> > > > + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
> > OTHER
> > > > + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
> > PERFORMANCE
> > > > + * OF THIS SOFTWARE.
> > > > + */
> > > > +
> > > > +#ifndef WESTON_SHARED_UTIL_H
> > > > +#define WESTON_SHARED_UTIL_H
> > > > +
> > > > +#include <math.h>
> > > > +
> > > > +static inline double
> > > > +wl_double_fixed_to_double(int32_t i, int32_t f)
> > >
> > > Should we have a struct wl_fixed_double_t { int32 i, int32 t } instead
> > > of having the two ints separated?
> >
> > That is something I considered, since the wire format would still be ii,
> > the the dispatcher would still spit out int32_t i, int32_t f, we'd still
> > need to first put the ints in the struct, then convert the struct to a
> > double, so didn't seem like a big win. We could probably make the
> > sending side abstract the data type behind a struct though, but it'd be
> > a bit wierd to have the sender send a struct, and the receiver receive
> > two ints.
> >
> > If we expect to keep a 64-bit fixed point around and not always convert
> > it to a double, then maybe it'd be worth it though. I'm probably easily
> > convinced to make that abstraction, if there are reasons enough to do
> > so.
> >
> > Jonas
> >
> > >
> > > > +{
> > > > + union {
> > > > + double d;
> > > > + int64_t i;
> > > > + } u;
> > > > +
> > > > + u.i = ((1023LL + (52LL - 31LL)) << 52) + (1LL << 51) + f;
> > > > +
> > > > + return i + (u.d - (3LL << (52 - 32)));
> > > > +}
> > > > +
> > > > +static inline void
> > > > +wl_double_fixed_from_double(double d, int32_t *i, int32_t *f)
> > > > +{
> > > > + double integral;
> > > > + union {
> > > > + double d;
> > > > + int64_t i;
> > > > + } u;
> > > > +
> > > > + u.d = modf(d, &integral) + (3LL << (51 - 31));
> > > > +
> > > > + *i = integral;
> > > > + *f = u.i;
> > > > +}
> > > > +
> > > > +#endif /* WESTON_SHARED_UTIL_H */
> > > > diff --git a/tests/double-fixed-benchmark.c
> > b/tests/double-fixed-benchmark.c
> > > > new file mode 100644
> > > > index 0000000..e4e5685
> > > > --- /dev/null
> > > > +++ b/tests/double-fixed-benchmark.c
> > > > @@ -0,0 +1,110 @@
> > > > +/*
> > > > + * Copyright © 2012 Intel Corporation
> > > > + * Copyright © 2015 Red Hat Inc.
> > > > + *
> > > > + * Permission to use, copy, modify, distribute, and sell this
> > software and its
> > > > + * documentation for any purpose is hereby granted without fee,
> > provided that
> > > > + * the above copyright notice appear in all copies and that both that
> > copyright
> > > > + * notice and this permission notice appear in supporting
> > documentation, and
> > > > + * that the name of the copyright holders not be used in advertising
> > or
> > > > + * publicity pertaining to distribution of the software without
> > specific,
> > > > + * written prior permission. The copyright holders make no
> > representations
> > > > + * about the suitability of this software for any purpose. It is
> > provided "as
> > > > + * is" without express or implied warranty.
> > > > + *
> > > > + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> > SOFTWARE,
> > > > + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS,
> > IN NO
> > > > + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL,
> > INDIRECT OR
> > > > + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
> > LOSS OF USE,
> > > > + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
> > OTHER
> > > > + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
> > PERFORMANCE
> > > > + * OF THIS SOFTWARE.
> > > > + */
> > > > +
> > > > +#include <stdlib.h>
> > > > +#include <stdio.h>
> > > > +#include <string.h>
> > > > +#include <time.h>
> > > > +#include <assert.h>
> > > > +#include <stdint.h>
> > > > +#include "shared/util.h"
> > > > +
> > > > +volatile double global_d;
> > > > +
> > > > +static void
> > > > +noop_conversion(void)
> > > > +{
> > > > + int32_t fp;
> > > > +
> > > > + union {
> > > > + int64_t i;
> > > > + double d;
> > > > + } u;
> > > > +
> > > > + for (fp = 0; fp < INT32_MAX; fp++) {
> > > > + u.i = fp;
> > > > + global_d = u.d;
> > > > + }
> > > > +}
> > > > +
> > > > +static void
> > > > +magic_conversion(void)
> > > > +{
> > > > + int32_t fp;
> > > > +
> > > > + for (fp = 0; fp < INT32_MAX; fp++) {
> > > > + global_d = wl_double_fixed_to_double(fp, fp);
> > > > + }
> > > > +}
> > > > +
> > > > +static void
> > > > +mul_conversion(void)
> > > > +{
> > > > + int32_t fp;
> > > > +
> > > > + for (fp = 0; fp < INT32_MAX; fp++) {
> > > > + global_d = fp + fp / 2147483648.0;
> > > > + }
> > > > +}
> > > > +
> > > > +double factor = 2147483648.0;
> > > > +
> > > > +static void
> > > > +div_conversion(void)
> > > > +{
> > > > + int32_t fp;
> > > > +
> > > > + for (fp = 0; fp < INT32_MAX; fp++) {
> > > > + global_d = fp + fp / factor;
> > > > + }
> > > > +}
> > > > +
> > > > +static void
> > > > +benchmark(const char *s, void (*f)(void))
> > > > +{
> > > > + struct timespec start, stop, elapsed;
> > > > +
> > > > + clock_gettime(CLOCK_MONOTONIC, &start);
> > > > + f();
> > > > + clock_gettime(CLOCK_MONOTONIC, &stop);
> > > > +
> > > > + elapsed.tv_sec = stop.tv_sec - start.tv_sec;
> > > > + elapsed.tv_nsec = stop.tv_nsec - start.tv_nsec;
> > > > + if (elapsed.tv_nsec < 0) {
> > > > + elapsed.tv_nsec += 1000000000;
> > > > + elapsed.tv_sec--;
> > > > + }
> > > > + printf("benchmarked %s:\t%ld.%09lds\n",
> > > > + s, elapsed.tv_sec, elapsed.tv_nsec);
> > > > +}
> > > > +
> > > > +int
> > > > +main(int argc, char *argv[])
> > > > +{
> > > > + benchmark("noop", noop_conversion);
> > > > + benchmark("magic", magic_conversion);
> > > > + benchmark("div", div_conversion);
> > > > + benchmark("mul", mul_conversion);
> > > > +
> > > > + return 0;
> > > > +}
> > > > --
> > > > 2.1.4
> > > >
> > > > _______________________________________________
> > > > wayland-devel mailing list
> > > > wayland-devel at lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >
> _______________________________________________
> 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