Re: [Pixman] [PATCH 08/15] pixman-filter: Don't recurse unnecessarily.

2015-12-23 Thread Oded Gabbay 
On Tue, Dec 22, 2015 at 9:38 PM, Bill Spitzak  wrote:
> On Tue, Dec 22, 2015 at 1:38 AM, Oded Gabbay  wrote:
>>
>> On Sat, Dec 12, 2015 at 8:06 PM,   wrote:
>> > From: Bill Spitzak 
>> >
>> > Only LINEAR is not differentiable at zero,
>>
>> I'm sorry, I don't understand this sentence. Could you please give me
>> a more detailed explanation + reference ?
>
>
> All the other filter functions have a continuous first derivative over their
> entire range.
>
> The LINEAR function is a triangle and the first derivative changes from +1
> to -1 at 0.0.

ok, so add it to the commit msg and this patch is:

Acked-by: Oded Gabbay 


>
> I believe Søren was concerned that the Simpson's integration would not work
> at this point. He solved this by splitting the integral into 2 or 3 at the
> zeros, so the integration is not done across these points.
>
> I figured I should keep this, though I suspect the error is really invisibly
> tiny. Apparently Simpsons integration will have some error for any function
> where the third derivative is not constant, which is true for all the
> filters here except box. But the error is really really tiny and was being
> ignored for all the other filters, and it may be ok to ignore it here too.
>
>> Thanks,
>>
>> Oded
>>
>> > so only do the recursive split of the integral for it.
>> > ---
>> >  pixman/pixman-filter.c | 34 +-
>> >  1 file changed, 17 insertions(+), 17 deletions(-)
>> >
>> > diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
>> > index 782f73d..0cd4a68 100644
>> > --- a/pixman/pixman-filter.c
>> > +++ b/pixman/pixman-filter.c
>> > @@ -160,38 +160,38 @@ integral (pixman_kernel_t reconstruct, double x1,
>> >   pixman_kernel_t sample, double scale, double x2,
>> >   double width)
>> >  {
>> > +if (reconstruct == PIXMAN_KERNEL_IMPULSE)
>> > +{
>> > +   assert (width == 0.0);
>> > +   return filters[sample].func (x2 / scale);
>> > +}
>> > +else if (reconstruct == PIXMAN_KERNEL_BOX && sample ==
>> > PIXMAN_KERNEL_BOX)
>> > +{
>> > +   assert (width <= 1.0);
>> > +   return width;
>> > +}
>> > +else if (sample == PIXMAN_KERNEL_IMPULSE)
>> > +{
>> > +   assert (width == 0.0);
>> > +   return filters[reconstruct].func (x1);
>> > +}
>> >  /* If the integration interval crosses zero, break it into
>> >   * two separate integrals. This ensures that filters such
>> >   * as LINEAR that are not differentiable at 0 will still
>> >   * integrate properly.
>> >   */
>> > -if (x1 < 0 && x1 + width > 0)
>> > +else if (reconstruct == PIXMAN_KERNEL_LINEAR && x1 < 0 && x1 +
>> > width > 0)
>> >  {
>> > return
>> > integral (reconstruct, x1, sample, scale, x2, - x1) +
>> > integral (reconstruct, 0, sample, scale, x2 - x1, width +
>> > x1);
>> >  }
>> > -else if (x2 < 0 && x2 + width > 0)
>> > +else if (sample == PIXMAN_KERNEL_LINEAR && x2 < 0 && x2 + width >
>> > 0)
>> >  {
>> > return
>> > integral (reconstruct, x1, sample, scale, x2, - x2) +
>> > integral (reconstruct, x1 - x2, sample, scale, 0, width +
>> > x2);
>> >  }
>> > -else if (reconstruct == PIXMAN_KERNEL_IMPULSE)
>> > -{
>> > -   assert (width == 0.0);
>> > -   return filters[sample].func (x2 / scale);
>> > -}
>> > -else if (reconstruct == PIXMAN_KERNEL_BOX && sample ==
>> > PIXMAN_KERNEL_BOX)
>> > -{
>> > -   assert (width <= 1.0);
>> > -   return width;
>> > -}
>> > -else if (sample == PIXMAN_KERNEL_IMPULSE)
>> > -{
>> > -   assert (width == 0.0);
>> > -   return filters[reconstruct].func (x1);
>> > -}
>> >  else
>> >  {
>> > /* Integration via Simpson's rule */
>> > --
>> > 1.9.1
>> >
>> > ___
>> > Pixman mailing list
>> > Pixman@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/pixman
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 09/15] pixman-filter: put filter error on center pixel

2015-12-23 Thread Oded Gabbay 
On Tue, Dec 22, 2015 at 9:23 PM, Bill Spitzak  wrote:
> This is forcing the filter to be normalized (sum to 1.0) despite any math
> errors.
>
> p is a post-increment pointer used to store the filter values, so it now
> points after the last filter value. The filter is summed and the difference
> from 1.0 is then added to the middle pixel with this code.
>
> If the width is an odd number such as 5, the filter is stored in f[0]..f[4],
> and p points at f[5]. The old code would add the extra error value to f[3],
> which is not the center sample. The new code adds it to f[2]. (If the width
> is even such as 4, then both the old and new code add the error to the f[2]
> which is the pixel to the right of the center.)
>
> The mistake was only visible in the width==1 case. Some combinations of
> filters produced a sample of 0, and then calculated the error as 1.0. The
> old code would put the error on f[1] (past the end). The new code puts it on
> f[0], thus producing a value of 1.0.
>

ok, so please add this to the commit msg and this patch is:

Acked-by: Oded Gabbay 

> On Tue, Dec 22, 2015 at 2:41 AM, Oded Gabbay  wrote:
>>
>> On Sat, Dec 12, 2015 at 8:06 PM,   wrote:
>> > From: Bill Spitzak 
>> >
>> > Any error in filter normalization is placed on the center of odd-sized
>> > filters,
>> > rather than 1 pixel to the right.
>> >
>> > In particular this fixes the 1-wide filters produced by impulse sampling
>> > so they are 1.0 rather than 0.0.
>> > ---
>> >  pixman/pixman-filter.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
>> > index 0cd4a68..fbc657d 100644
>> > --- a/pixman/pixman-filter.c
>> > +++ b/pixman/pixman-filter.c
>> > @@ -299,7 +299,7 @@ create_1d_filter (int  width,
>> > }
>> >
>> > if (new_total != pixman_fixed_1)
>> > -   *(p - width / 2) += (pixman_fixed_1 - new_total);
>> > +   *(p - (width + 1) / 2) += (pixman_fixed_1 - new_total);
>> >  }
>> >  }
>> >
>> > --
>> > 1.9.1
>> >
>> > ___
>> > Pixman mailing list
>> > Pixman@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/pixman
>>
>>
>> I see that the result is indeed better (in the scale demo), but do you
>> mind explaining a bit more about the underlying of this patch ?
>> I indeed see that the width is 1, so without your patch, the new value
>> is written to *p and with your patch, it is written to *(p-1). But I
>> have no idea what that means :(
>>
>> Oded
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH 4/4] build: Use `del` instead of `rm` on `cmd.exe` shells

2015-12-23 Thread Andrea Canciani 
From: Simon Richter 

The `rm` command is not usually available when running on Win32 in a
`cmd.exe` shell. Instead the shell provides the `del` builtin, which
has somewhat more limited wildcars expansion and error handling.

This makes all of the Makefile targets work on Win32 both using
`cmd.exe` and using the MSYS environment.

Signed-off-by: Simon Richter 
Signed-off-by: Andrea Canciani 
---
 Makefile.win32.common | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Makefile.win32.common b/Makefile.win32.common
index a759ddc..756fc94 100644
--- a/Makefile.win32.common
+++ b/Makefile.win32.common
@@ -5,6 +5,10 @@ LD = link
 AR = lib
 PERL = perl
 
+ifneq ($(shell echo ""),)
+RM = del
+endif
+
 ifeq ($(top_builddir),)
 top_builddir = $(top_srcdir)
 endif
@@ -51,7 +55,7 @@ $(CFG_VAR):
 $(CFG_VAR)/%.obj: %.c $(libpixman_headers) | $(CFG_VAR)
@$(CC) -c $(PIXMAN_CFLAGS) -Fo"$@" $<
 
-clean: inform
-   @$(RM) $(CFG_VAR)/*.{exe,ilk,lib,obj,pdb} || exit 0
+clean: inform $(CFG_VAR)
+   @cd $(CFG_VAR) && echo > silence_error.exe && $(RM) *.exe *.ilk *.lib 
*.obj *.pdb
 
 .PHONY: inform clean
-- 
2.5.4 (Apple Git-61)

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

[Pixman] [PATCH 3/4] build: Do not use `mkdir -p` on Windows

2015-12-23 Thread Andrea Canciani 
When the build is performed using `cmd.exe` as shell, the `mkdir`
command does not support the `-p` flag. The ability to create multiple
netsted folder is not used, hence it can be easily replaced by only
creating the directory if it does not exist.

This makes the build work on the `cmd.exe` shell, except for the
`clean` targets.

Signed-off-by: Andrea Canciani 
---
 Makefile.win32.common | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile.win32.common b/Makefile.win32.common
index b498c2f..a759ddc 100644
--- a/Makefile.win32.common
+++ b/Makefile.win32.common
@@ -45,9 +45,10 @@ endif
 endif
 endif
 
+$(CFG_VAR):
+   @mkdir $@
 
-$(CFG_VAR)/%.obj: %.c $(libpixman_headers)
-   @mkdir -p $(CFG_VAR)
+$(CFG_VAR)/%.obj: %.c $(libpixman_headers) | $(CFG_VAR)
@$(CC) -c $(PIXMAN_CFLAGS) -Fo"$@" $<
 
 clean: inform
-- 
2.5.4 (Apple Git-61)

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

[Pixman] [PATCH 1/4] build: Remove use of BUILT_SOURCES from Makefile.win32

2015-12-23 Thread Andrea Canciani 
Since 3d81d89c292058522cce91338028d9b4c4a23c24 BUILT_SOURCES is not
used anymore, but it was unintentionally left in Win32 Makefiles.

Signed-off-by: Andrea Canciani 
---
 Makefile.win32.common | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.win32.common b/Makefile.win32.common
index 777f94c..b498c2f 100644
--- a/Makefile.win32.common
+++ b/Makefile.win32.common
@@ -51,6 +51,6 @@ $(CFG_VAR)/%.obj: %.c $(libpixman_headers)
@$(CC) -c $(PIXMAN_CFLAGS) -Fo"$@" $<
 
 clean: inform
-   @$(RM) $(CFG_VAR)/*.{exe,ilk,lib,obj,pdb} $(BUILT_SOURCES) || exit 0
+   @$(RM) $(CFG_VAR)/*.{exe,ilk,lib,obj,pdb} || exit 0
 
 .PHONY: inform clean
-- 
2.5.4 (Apple Git-61)

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

[Pixman] [PATCH 2/4] build: Avoid phony `pixman` target in test/Makefile.win32

2015-12-23 Thread Andrea Canciani 
Instead of explicitly depending on "pixman" for the "all" and "check"
targets, rely on the dependency to the .lib file

Signed-off-by: Andrea Canciani 
---
 test/Makefile.win32 | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/test/Makefile.win32 b/test/Makefile.win32
index 6cfb4a7..bdd9b7f 100644
--- a/test/Makefile.win32
+++ b/test/Makefile.win32
@@ -16,9 +16,9 @@ OBJECTS = $(patsubst %.c, $(CFG_VAR)/%.obj, $(SOURCES))
 TESTS   = $(patsubst %,   $(CFG_VAR)/%.exe, $(TESTPROGRAMS))
 OTHERS  = $(patsubst %,   $(CFG_VAR)/%.exe, $(OTHERPROGRAMS))
 
-all: pixman inform $(TESTS) $(OTHERS)
+all: inform $(TESTS) $(OTHERS)
 
-check: pixman inform $(TESTS)
+check: inform $(TESTS)
@failures=0 ; \
total=0 ; \
for test in $(TESTS) ; \
@@ -46,9 +46,7 @@ $(CFG_VAR)/libutils.lib: $(libutils_OBJECTS)
 $(CFG_VAR)/%.exe: $(CFG_VAR)/%.obj $(TEST_LDADD)
@$(LD) $(PIXMAN_LDFLAGS) -OUT:$@ $^
 
-$(top_builddir)/pixman/$(CFG_VAR)/$(LIBRARY).lib: pixman
-
-pixman:
+$(top_builddir)/pixman/$(CFG_VAR)/$(LIBRARY).lib:
@$(MAKE) -C $(top_builddir)/pixman -f Makefile.win32
 
-.PHONY: all check pixman
+.PHONY: all check
-- 
2.5.4 (Apple Git-61)

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

Re: [Pixman] [PATCH 2/4] build: Avoid phony `pixman` target in test/Makefile.win32

2015-12-23 Thread Oded Gabbay 
On Wed, Dec 23, 2015 at 1:24 PM, Andrea Canciani  wrote:
> Instead of explicitly depending on "pixman" for the "all" and "check"
> targets, rely on the dependency to the .lib file
>
> Signed-off-by: Andrea Canciani 
> ---
>  test/Makefile.win32 | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/test/Makefile.win32 b/test/Makefile.win32
> index 6cfb4a7..bdd9b7f 100644
> --- a/test/Makefile.win32
> +++ b/test/Makefile.win32
> @@ -16,9 +16,9 @@ OBJECTS = $(patsubst %.c, $(CFG_VAR)/%.obj, $(SOURCES))
>  TESTS   = $(patsubst %,   $(CFG_VAR)/%.exe, $(TESTPROGRAMS))
>  OTHERS  = $(patsubst %,   $(CFG_VAR)/%.exe, $(OTHERPROGRAMS))
>
> -all: pixman inform $(TESTS) $(OTHERS)
> +all: inform $(TESTS) $(OTHERS)
>
> -check: pixman inform $(TESTS)
> +check: inform $(TESTS)
> @failures=0 ; \
> total=0 ; \
> for test in $(TESTS) ; \
> @@ -46,9 +46,7 @@ $(CFG_VAR)/libutils.lib: $(libutils_OBJECTS)
>  $(CFG_VAR)/%.exe: $(CFG_VAR)/%.obj $(TEST_LDADD)
> @$(LD) $(PIXMAN_LDFLAGS) -OUT:$@ $^
>
> -$(top_builddir)/pixman/$(CFG_VAR)/$(LIBRARY).lib: pixman
> -
> -pixman:
> +$(top_builddir)/pixman/$(CFG_VAR)/$(LIBRARY).lib:
> @$(MAKE) -C $(top_builddir)/pixman -f Makefile.win32
>
> -.PHONY: all check pixman
> +.PHONY: all check
> --
> 2.5.4 (Apple Git-61)
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman

This patch is

Reviewed-by: Oded Gabbay 
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 3/4] build: Do not use `mkdir -p` on Windows

2015-12-23 Thread Oded Gabbay 
On Wed, Dec 23, 2015 at 1:24 PM, Andrea Canciani  wrote:
> When the build is performed using `cmd.exe` as shell, the `mkdir`
> command does not support the `-p` flag. The ability to create multiple
> netsted folder is not used, hence it can be easily replaced by only
> creating the directory if it does not exist.
>
> This makes the build work on the `cmd.exe` shell, except for the
> `clean` targets.
>
> Signed-off-by: Andrea Canciani 
> ---
>  Makefile.win32.common | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile.win32.common b/Makefile.win32.common
> index b498c2f..a759ddc 100644
> --- a/Makefile.win32.common
> +++ b/Makefile.win32.common
> @@ -45,9 +45,10 @@ endif
>  endif
>  endif
>
> +$(CFG_VAR):
> +   @mkdir $@
>
> -$(CFG_VAR)/%.obj: %.c $(libpixman_headers)
> -   @mkdir -p $(CFG_VAR)
> +$(CFG_VAR)/%.obj: %.c $(libpixman_headers) | $(CFG_VAR)
> @$(CC) -c $(PIXMAN_CFLAGS) -Fo"$@" $<
>
>  clean: inform
> --
> 2.5.4 (Apple Git-61)
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman

Acked-by: Oded Gabbay 
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 4/4] build: Use `del` instead of `rm` on `cmd.exe` shells

2015-12-23 Thread Oded Gabbay 
On Wed, Dec 23, 2015 at 1:24 PM, Andrea Canciani  wrote:
> From: Simon Richter 
>
> The `rm` command is not usually available when running on Win32 in a
> `cmd.exe` shell. Instead the shell provides the `del` builtin, which
> has somewhat more limited wildcars expansion and error handling.
>
> This makes all of the Makefile targets work on Win32 both using
> `cmd.exe` and using the MSYS environment.
>
> Signed-off-by: Simon Richter 
> Signed-off-by: Andrea Canciani 
> ---
>  Makefile.win32.common | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile.win32.common b/Makefile.win32.common
> index a759ddc..756fc94 100644
> --- a/Makefile.win32.common
> +++ b/Makefile.win32.common
> @@ -5,6 +5,10 @@ LD = link
>  AR = lib
>  PERL = perl
>
> +ifneq ($(shell echo ""),)
> +RM = del
> +endif
> +
>  ifeq ($(top_builddir),)
>  top_builddir = $(top_srcdir)
>  endif
> @@ -51,7 +55,7 @@ $(CFG_VAR):
>  $(CFG_VAR)/%.obj: %.c $(libpixman_headers) | $(CFG_VAR)
> @$(CC) -c $(PIXMAN_CFLAGS) -Fo"$@" $<
>
> -clean: inform
> -   @$(RM) $(CFG_VAR)/*.{exe,ilk,lib,obj,pdb} || exit 0
> +clean: inform $(CFG_VAR)
> +   @cd $(CFG_VAR) && echo > silence_error.exe && $(RM) *.exe *.ilk *.lib 
> *.obj *.pdb
>
>  .PHONY: inform clean
> --
> 2.5.4 (Apple Git-61)
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman

Acked-by: Oded Gabbay 
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH] Allow building on Windows with cmd.exe

2015-12-23 Thread Andrea Canciani 
On Tue, Dec 22, 2015 at 2:19 PM, Oded Gabbay  wrote:

> On Thu, Jun 4, 2015 at 12:02 PM, Andrea Canciani 
> wrote:
> > The patch does not regress the mingw-based build (basically that used in
> > http://cairographics.org/end_to_end_build_for_win32/ )
> > I did not manage to get the build working on the cmd.exe shell. Do you
> have
> > any instructions (or possibly indications about what is the correct
> > environment) for that?
> >
> > AFAICT the change still relies on a Unix-like make command.
> > A bigger (but possibly even more interesting) change would be an
> NMAKE-based
> > build, so that the command-line tools available in Visual Studio are
> > sufficient for building the whole project.
> >
> > Andrea
> >
>
> Hi Andrea,
>
> I know it has been almost 6 months since you last looked at this
> patch, but I would like to ask if you can give it an r-b and then I
> will push it to master.
>

Actually, this is a good time for me to work on pixman, as I have some more
free time :)


> If not, what needs to be done to give it an r-b ?
>

The main thing blocking me from an r-b tag originally was that I had been
unable to get it working.
I set up an environment to test it and found that it mostly behaves as
intended, with the main glitch being that the "clean" target can spit out
undesired error logs.

I pushed a branch which is based on Simon's patch here:
http://cgit.freedesktop.org/~ranma42/pixman/log/?h=win32-build

The commits include:
 - cleanup of the win32 build system (mostly unrelated to the original
patch)
   ea8a070 build: Remove use of BUILT_SOURCES from Makefile.win32
   8989759 build: Avoid phony `pixman` target in test/Makefile.win32
 - shell-generic compatibility
   db72f26 build: Do not use `mkdir -p` on Windows
 - cmd.exe compatibility
   6223be5 build: Use `del` instead of `rm` on `cmd.exe` shells

The last commit is based on the original patch, with some minor differences
to avoid path separator handling (instead I changed the current directory)
and to avoid error messages from `del` (when it finds no file to delete, it
logs a warning that apparently cannot be silenced by any of the flags).

While doing this I discovered that on Win32 fence-image-self-test and
cover-test are currently failing (regardless of these patches and
apparently also with any combination of MMX, SSE2, SSSE3).
I will try to investigate ASAP.

I believe that the first two commits should be "obviously" ok, while the
other two are not as trivial.
The whole patchset has been tested on Win32, both in an MSYS environment
and on cmd.exe (in both cases, using GNU make).

Andrea

PS: it looks like pixman has adopted Reviewed-by, Signed-off-by, etc.
I assume that the first three patches should be marked signed off by me,
while I am unsure about the tagging for the last patch (I guess it should
be credited to Simon).


Unfortunately, I don't have a Windows machine to check this.
>
> Thanks,
>
> Oded
>
> >
> > On Mon, Jun 1, 2015 at 8:44 AM, Andrea Canciani 
> wrote:
> >>
> >> The ping was good and the changes make sense. Currently I am replying
> from
> >> the phone and I will not have access to my computer until the 3rd, but
> I am
> >> flagging this mail so that I will test and review the patch asap.
> >>
> >> Sorry for the delay
> >> Andrea
> >>
> >> On Jun 1, 2015 8:09 AM, "Siarhei Siamashka" <
> siarhei.siamas...@gmail.com>
> >> wrote:
> >>>
> >>> On Wed, 22 Apr 2015 04:01:31 +0200
> >>> Simon Richter  wrote:
> >>>
> >>> > This finds out whether the standard shell for make is cmd.exe, and
> >>> > adjusts
> >>> > the build process accordingly.
> >>> > ---
> >>> >  Makefile.win32.common | 14 --
> >>> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >>>
> >>> Thanks for the patch.
> >>>
> >>> But it is a bit difficult for non-Windows folks to see if
> >>> it makes any sense or not. So it would be best if some other
> >>> Windows user could review and confirm that it works.
> >>>
> >>> I have added Andrea Canciani (the author of the win32 makefiles)
> >>> to CC.
> >>>
> >>> > diff --git a/Makefile.win32.common b/Makefile.win32.common
> >>> > index 777f94c..335886f 100644
> >>> > --- a/Makefile.win32.common
> >>> > +++ b/Makefile.win32.common
> >>> > @@ -1,5 +1,15 @@
> >>> >  LIBRARY = pixman-1
> >>> >
> >>> > +ifeq ($(shell echo ""),)
> >>> > +# POSIX style shell
> >>> > +mkdir_p = mkdir -p $1
> >>> > +rm = $(RM) $1
> >>> > +else
> >>> > +# DOS/Windows style shell
> >>> > +mkdir_p = if not exist $(subst /,\,$1) md $(subst /,\,$1)
> >>> > +rm = del $(subst /,\,$1)
> >>> > +endif
> >>> > +
> >>> >  CC = cl
> >>> >  LD = link
> >>> >  AR = lib
> >>> > @@ -47,10 +57,10 @@ endif
> >>> >
> >>> >
> >>> >  $(CFG_VAR)/%.obj: %.c $(libpixman_headers)
> >>> > - @mkdir -p $(CFG_VAR)
> >>> > + @$(call mkdir_p,$(@D))
> >>> >   @$(CC) -c $(PIXMAN_CFLAGS) -Fo"$@" $<
> >>> >
> >>> >  clean: inform
> >>> > - @$(RM)

Re: [Pixman] [PATCH] Allow building on Windows with cmd.exe

2015-12-23 Thread Oded Gabbay 
On Wed, Dec 23, 2015 at 1:05 PM, Andrea Canciani  wrote:
> On Tue, Dec 22, 2015 at 2:19 PM, Oded Gabbay  wrote:
>>
>> On Thu, Jun 4, 2015 at 12:02 PM, Andrea Canciani 
>> wrote:
>> > The patch does not regress the mingw-based build (basically that used in
>> > http://cairographics.org/end_to_end_build_for_win32/ )
>> > I did not manage to get the build working on the cmd.exe shell. Do you
>> > have
>> > any instructions (or possibly indications about what is the correct
>> > environment) for that?
>> >
>> > AFAICT the change still relies on a Unix-like make command.
>> > A bigger (but possibly even more interesting) change would be an
>> > NMAKE-based
>> > build, so that the command-line tools available in Visual Studio are
>> > sufficient for building the whole project.
>> >
>> > Andrea
>> >
>>
>> Hi Andrea,
>>
>> I know it has been almost 6 months since you last looked at this
>> patch, but I would like to ask if you can give it an r-b and then I
>> will push it to master.
>
>
> Actually, this is a good time for me to work on pixman, as I have some more
> free time :)

Thanks Andrea, much appreciated.

>
>>
>> If not, what needs to be done to give it an r-b ?
>
>
> The main thing blocking me from an r-b tag originally was that I had been
> unable to get it working.
> I set up an environment to test it and found that it mostly behaves as
> intended, with the main glitch being that the "clean" target can spit out
> undesired error logs.
>
> I pushed a branch which is based on Simon's patch here:
> http://cgit.freedesktop.org/~ranma42/pixman/log/?h=win32-build
>
> The commits include:
>  - cleanup of the win32 build system (mostly unrelated to the original
> patch)
>ea8a070 build: Remove use of BUILT_SOURCES from Makefile.win32
>8989759 build: Avoid phony `pixman` target in test/Makefile.win32
>  - shell-generic compatibility
>db72f26 build: Do not use `mkdir -p` on Windows
>  - cmd.exe compatibility
>6223be5 build: Use `del` instead of `rm` on `cmd.exe` shells
>
> The last commit is based on the original patch, with some minor differences
> to avoid path separator handling (instead I changed the current directory)
> and to avoid error messages from `del` (when it finds no file to delete, it
> logs a warning that apparently cannot be silenced by any of the flags).
>
> While doing this I discovered that on Win32 fence-image-self-test and
> cover-test are currently failing (regardless of these patches and apparently
> also with any combination of MMX, SSE2, SSSE3).
> I will try to investigate ASAP.

Thanks.

>
> I believe that the first two commits should be "obviously" ok, while the
> other two are not as trivial.
> The whole patchset has been tested on Win32, both in an MSYS environment and
> on cmd.exe (in both cases, using GNU make).
>
> Andrea
>
> PS: it looks like pixman has adopted Reviewed-by, Signed-off-by, etc.
> I assume that the first three patches should be marked signed off by me,
> while I am unsure about the tagging for the last patch (I guess it should be
> credited to Simon).

Yes, you are correct. Could you please send the patches as a patch
series to the pixman ML, so we could do a quick review process and
merge it ?

If you want to credit the last patch to Simon, just make sure he is
the author of the patch (you can change the author git commit --amend
--author). Then, send it as part of your series.

Thanks,

Oded
>
>
>> Unfortunately, I don't have a Windows machine to check this.
>>
>> Thanks,
>>
>> Oded
>>
>> >
>> > On Mon, Jun 1, 2015 at 8:44 AM, Andrea Canciani 
>> > wrote:
>> >>
>> >> The ping was good and the changes make sense. Currently I am replying
>> >> from
>> >> the phone and I will not have access to my computer until the 3rd, but
>> >> I am
>> >> flagging this mail so that I will test and review the patch asap.
>> >>
>> >> Sorry for the delay
>> >> Andrea
>> >>
>> >> On Jun 1, 2015 8:09 AM, "Siarhei Siamashka"
>> >> 
>> >> wrote:
>> >>>
>> >>> On Wed, 22 Apr 2015 04:01:31 +0200
>> >>> Simon Richter  wrote:
>> >>>
>> >>> > This finds out whether the standard shell for make is cmd.exe, and
>> >>> > adjusts
>> >>> > the build process accordingly.
>> >>> > ---
>> >>> >  Makefile.win32.common | 14 --
>> >>> >  1 file changed, 12 insertions(+), 2 deletions(-)
>> >>>
>> >>> Thanks for the patch.
>> >>>
>> >>> But it is a bit difficult for non-Windows folks to see if
>> >>> it makes any sense or not. So it would be best if some other
>> >>> Windows user could review and confirm that it works.
>> >>>
>> >>> I have added Andrea Canciani (the author of the win32 makefiles)
>> >>> to CC.
>> >>>
>> >>> > diff --git a/Makefile.win32.common b/Makefile.win32.common
>> >>> > index 777f94c..335886f 100644
>> >>> > --- a/Makefile.win32.common
>> >>> > +++ b/Makefile.win32.common
>> >>> > @@ -1,5 +1,15 @@
>> >>> >

Re: [Pixman] [PATCH 10/15] pixman-filter: don't range-check impulse filter

2015-12-23 Thread Oded Gabbay 
On Tue, Dec 22, 2015 at 10:51 PM, Bill Spitzak  wrote:
> The problem is that *Cairo* does not call this function. This is because
> Cairo already has my patches which work around the broken filtering by
> generating it's own filtering. The whole point of this series of patches is
> so that this work-around can be removed from Cairo.
>

So my response here is at the same I wrote about patch 7/15 (defer
until cairo will actually use this function), only here we are talking
about showing results of cairo test suite and not benchmarking.

Thanks,

   Oded

>
> On Tue, Dec 22, 2015 at 12:12 PM, Oded Gabbay  wrote:
>>
>> On Tue, Dec 22, 2015 at 9:25 PM, Bill Spitzak  wrote:
>> >
>> >
>> > On Tue, Dec 22, 2015 at 2:32 AM, Oded Gabbay 
>> > wrote:
>> >>
>> >> On Sat, Dec 12, 2015 at 8:06 PM,   wrote:
>> >> > From: Bill Spitzak 
>> >> >
>> >> > The other filters don't range-check, so there is no need for this
>> >> > one to either. It is only called with x==0.
>> >>
>> >> Actually, I tried to stop at this function in gdb and didn't manage to
>> >> do it (using the scale demo). I then looked at the code and it seems
>> >> to me that the only way to reach this function is when both
>> >> reconstruction and sample kernels are IMPLUSE. That's because:
>> >>
>> >> 1. If both reconstruction and sample are *not* IMPLUSE, then of course
>> >> we won't reach it.
>> >> 2. If only one of them is IMPLUSE, than the code will immediately
>> >> return the value of the function of the other kernel, which is *not*
>> >> IMPLUSE.
>> >>
>> >> However, when I put both of them to IMPLUSE in the scale demo, the
>> >> picture simply disappears *and* the impluse_kernel is still not
>> >> reached. Actually, in that case, the integral() func is never reached
>> >> as well.
>> >>
>> >> What am I missing ?
>> >
>> >
>> > I believe at this point the calling code calculated a width of zero for
>> > the
>> > filter, and this caused all kinds of problems.
>> >
>> > I think you are correct that in most or all versions of this code, that
>> > impulse function is never called, and it could be a null pointer
>> > instead.
>>
>> Well, I wouldn't go that far, but what I'm implying is maybe we can
>> defer this patch until a time when pixman's code will actually call
>> this function. Then, we can re-evaluate the patch based on the inputs
>> we will see.
>>
>>Oded
>>
>> >>
>> >>
>> >>Oded
>> >>
>> >> > ---
>> >> >  pixman/pixman-filter.c | 2 +-
>> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
>> >> > index fbc657d..00126cd 100644
>> >> > --- a/pixman/pixman-filter.c
>> >> > +++ b/pixman/pixman-filter.c
>> >> > @@ -45,7 +45,7 @@ typedef struct
>> >> >  static double
>> >> >  impulse_kernel (double x)
>> >> >  {
>> >> > -return (x == 0.0)? 1.0 : 0.0;
>> >> > +return 1;
>> >> >  }
>> >> >
>> >> >  static double
>> >> > --
>> >> > 1.9.1
>> >> >
>> >> > ___
>> >> > Pixman mailing list
>> >> > Pixman@lists.freedesktop.org
>> >> > http://lists.freedesktop.org/mailman/listinfo/pixman
>> >
>> >
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman