[Pixman] [RFC 1/2] Remove seemingly unneeded comparison(s)

2016-04-24 Thread Emil Velikov 
With commit ed39992564b "Use pixman_transform_point_31_16() from
pixman_transform_point()" we added some strange hunks.

Namely: we copy the data from the internal storage to the user vector
only to compare them immediately after.

Cc: Siarhei Siamashka 
---

Siarhei, what is the intent with the original commit ? Any ideas why
things crash ?

Seemingly this can be dropped/replaced with TRUE, yet it causes one of
the tests () to segfault in the optimised SSE2 codepath -
scaled_bilinear_scanline_sse2___SRC.

BILINEAR_INTERPOLATE_ONE_PIXEL -> BILINEAR_INTERPOLATE_ONE_PIXEL_HELPER

Regards,
Emil
---

 pixman/pixman-matrix.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/pixman/pixman-matrix.c b/pixman/pixman-matrix.c
index 65b3d32..117015b 100644
--- a/pixman/pixman-matrix.c
+++ b/pixman/pixman-matrix.c
@@ -393,9 +393,7 @@ pixman_transform_point_3d (const struct pixman_transform 
*transform,
 vector->vector[1] = tmp.v[1];
 vector->vector[2] = tmp.v[2];
 
-return vector->vector[0] == tmp.v[0] &&
-   vector->vector[1] == tmp.v[1] &&
-   vector->vector[2] == tmp.v[2];
+return TRUE;
 }
 
 PIXMAN_EXPORT pixman_bool_t
@@ -414,9 +412,7 @@ pixman_transform_point (const struct pixman_transform 
*transform,
 vector->vector[1] = tmp.v[1];
 vector->vector[2] = tmp.v[2];
 
-return vector->vector[0] == tmp.v[0] &&
-   vector->vector[1] == tmp.v[1] &&
-   vector->vector[2] == tmp.v[2];
+return TRUE;
 }
 
 PIXMAN_EXPORT pixman_bool_t
-- 
2.8.0

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [RFC 2/2] sse2: use uintptr_t for vx/unit_x

2016-04-24 Thread Emil Velikov 
Strictly speaking this is not correct, as the value itself can be signed
based on the definition of pixman_fixed_t. At the same time we check if
vx can be negative in some places, while in others we directly ">> 16"
and use the result as an index.

Obviously something isn't right here - should we add more checks,
convert to unsigned or there is something which implies that in some
codepaths the variable cannot be negative ?
---

Noticed while looking at the crashes due to previous commit.

---

 pixman/pixman-sse2.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/pixman/pixman-sse2.c b/pixman/pixman-sse2.c
index 8955103..67eed84 100644
--- a/pixman/pixman-sse2.c
+++ b/pixman/pixman-sse2.c
@@ -5698,8 +5698,8 @@ scaled_bilinear_scanline_sse2___SRC (uint32_t *   
dst,
 pixman_fixed_t   max_vx,
 pixman_bool_tzero_src)
 {
-intptr_t vx = vx_;
-intptr_t unit_x = unit_x_;
+uintptr_t vx = vx_;
+uintptr_t unit_x = unit_x_;
 BILINEAR_DECLARE_VARIABLES;
 uint32_t pix1, pix2;
 
@@ -5763,8 +5763,8 @@ scaled_bilinear_scanline_sse2_x888__SRC (uint32_t *   
dst,
 pixman_fixed_t   max_vx,
 pixman_bool_tzero_src)
 {
-intptr_t vx = vx_;
-intptr_t unit_x = unit_x_;
+uintptr_t vx = vx_;
+uintptr_t unit_x = unit_x_;
 BILINEAR_DECLARE_VARIABLES;
 uint32_t pix1, pix2;
 
@@ -5823,8 +5823,8 @@ scaled_bilinear_scanline_sse2___OVER (uint32_t *  
 dst,
  pixman_fixed_t   max_vx,
  pixman_bool_tzero_src)
 {
-intptr_t vx = vx_;
-intptr_t unit_x = unit_x_;
+uintptr_t vx = vx_;
+uintptr_t unit_x = unit_x_;
 BILINEAR_DECLARE_VARIABLES;
 uint32_t pix1, pix2;
 
@@ -5920,8 +5920,8 @@ scaled_bilinear_scanline_sse2__8__OVER (uint32_t 
*   dst,
pixman_fixed_t   max_vx,
pixman_bool_tzero_src)
 {
-intptr_t vx = vx_;
-intptr_t unit_x = unit_x_;
+uintptr_t vx = vx_;
+uintptr_t unit_x = unit_x_;
 BILINEAR_DECLARE_VARIABLES;
 uint32_t pix1, pix2;
 uint32_t m;
@@ -6078,8 +6078,8 @@ scaled_bilinear_scanline_sse2__n__OVER (uint32_t 
*   dst,
pixman_fixed_t   max_vx,
pixman_bool_tzero_src)
 {
-intptr_t vx = vx_;
-intptr_t unit_x = unit_x_;
+uintptr_t vx = vx_;
+uintptr_t unit_x = unit_x_;
 BILINEAR_DECLARE_VARIABLES;
 uint32_t pix1;
 __m128i xmm_mask;
-- 
2.8.0

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH 4/4] Hide the final internal function from global namespace

2016-04-24 Thread Emil Velikov 
Namely _pixman_internal_only_get_implementation. This workaround is no
longer needed as of last commit, so let's rename the function and make
it truly internal only.

Signed-off-by: Emil Velikov 
---
 pixman/pixman-private.h | 7 ++-
 pixman/pixman-utils.c   | 7 ++-
 test/combiner-test.c| 2 +-
 3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h
index 73a5414..70fa99b 100644
--- a/pixman/pixman-private.h
+++ b/pixman/pixman-private.h
@@ -778,11 +778,8 @@ get_implementation (void)
 return global_implementation;
 }
 
-/* This function is exported for the sake of the test suite and not part
- * of the ABI.
- */
-PIXMAN_EXPORT pixman_implementation_t *
-_pixman_internal_only_get_implementation (void);
+pixman_implementation_t *
+_pixman_get_implementation (void);
 
 /* Memory allocation helpers */
 void *
diff --git a/pixman/pixman-utils.c b/pixman/pixman-utils.c
index 4a3a835..893c5a8 100644
--- a/pixman/pixman-utils.c
+++ b/pixman/pixman-utils.c
@@ -303,11 +303,8 @@ pixman_region32_copy_from_region16 (pixman_region32_t *dst,
 return retval;
 }
 
-/* This function is exported for the sake of the test suite and not part
- * of the ABI.
- */
-PIXMAN_EXPORT pixman_implementation_t *
-_pixman_internal_only_get_implementation (void)
+pixman_implementation_t *
+_pixman_get_implementation (void)
 {
 return get_implementation ();
 }
diff --git a/test/combiner-test.c b/test/combiner-test.c
index 01f63a5..d30da1e 100644
--- a/test/combiner-test.c
+++ b/test/combiner-test.c
@@ -121,7 +121,7 @@ main ()
 
 enable_divbyzero_exceptions();
 
-impl = _pixman_internal_only_get_implementation();
+impl = _pixman_get_implementation();
 
 prng_srand (0);
 
-- 
2.8.0

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH 3/4] test: static link pixman in combiner-test

2016-04-24 Thread Emil Velikov 
Analogous to an earlier commit about matrix-test

Signed-off-by: Emil Velikov 
---
 test/Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/Makefile.am b/test/Makefile.am
index f70440e..d963071 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -6,6 +6,7 @@ LDADD = libutils.la $(top_builddir)/pixman/libpixman-1.la -lm  
$(PNG_LIBS) $(PTH
 AM_CPPFLAGS = -I$(top_srcdir)/pixman -I$(top_builddir)/pixman $(PNG_CFLAGS)
 
 matrix_test_LDFLAGS = -static
+combiner_test_LDFLAGS = -static
 
 libutils_la_SOURCES = $(libutils_sources) $(libutils_headers)
 
-- 
2.8.0

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH 1/4] test: static link pixman in matrix-test

2016-04-24 Thread Emil Velikov 
The test uses pixman internal symbols, which we currently export. To
resolve that static link the pixman library. This is an internal only
test ran at `make check' thus we are fine with the bloated binary and
alike.

Signed-off-by: Emil Velikov 
---
Afaics the Windows build already uses a static library thus things
should be fine there. I have NOT tested it though.

If people are concerned with this approach, we can build the core files
once and explicitly setup a pixman-static.la for the tests to use.
---
 test/Makefile.am | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/Makefile.am b/test/Makefile.am
index 88dc36d..f70440e 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -5,6 +5,8 @@ AM_LDFLAGS = $(OPENMP_CFLAGS) $(TESTPROGS_EXTRA_LDFLAGS) 
$(PTHREAD_LDFLAGS)
 LDADD = libutils.la $(top_builddir)/pixman/libpixman-1.la -lm  $(PNG_LIBS) 
$(PTHREAD_LIBS)
 AM_CPPFLAGS = -I$(top_srcdir)/pixman -I$(top_builddir)/pixman $(PNG_CFLAGS)
 
+matrix_test_LDFLAGS = -static
+
 libutils_la_SOURCES = $(libutils_sources) $(libutils_headers)
 
 noinst_LTLIBRARIES = libutils.la
-- 
2.8.0

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH 2/4] Remove pixman_transform_point_31_16* from global namespace

2016-04-24 Thread Emil Velikov 
These internal functions were needed by matrix-test. With the test
static linking pixman (as of last commit) we no longer need to export
these and we can move them back to being internal only.

Signed-off-by: Emil Velikov 
---
 pixman/pixman-matrix.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/pixman/pixman-matrix.c b/pixman/pixman-matrix.c
index 4032c13..65b3d32 100644
--- a/pixman/pixman-matrix.c
+++ b/pixman/pixman-matrix.c
@@ -194,7 +194,7 @@ fixed_112_16_to_fixed_48_16 (int64_t hi, int64_t lo, 
pixman_bool_t *clampflag)
  * and PAD repeats correctly) and the return value is FALSE to indicate that
  * such clamping has happened.
  */
-PIXMAN_EXPORT pixman_bool_t
+pixman_bool_t
 pixman_transform_point_31_16 (const pixman_transform_t*t,
   const pixman_vector_48_16_t *v,
   pixman_vector_48_16_t   *result)
@@ -303,7 +303,7 @@ pixman_transform_point_31_16 (const pixman_transform_t
*t,
 return !clampflag;
 }
 
-PIXMAN_EXPORT void
+void
 pixman_transform_point_31_16_affine (const pixman_transform_t*t,
  const pixman_vector_48_16_t *v,
  pixman_vector_48_16_t   *result)
@@ -334,7 +334,7 @@ pixman_transform_point_31_16_affine (const 
pixman_transform_t*t,
 result->v[2] = pixman_fixed_1;
 }
 
-PIXMAN_EXPORT void
+void
 pixman_transform_point_31_16_3d (const pixman_transform_t*t,
  const pixman_vector_48_16_t *v,
  pixman_vector_48_16_t   *result)
-- 
2.8.0

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman