Re: [Pixman] Dithering patches, v2

[Bryce Harrington]

On Tue, May 28, 2019 at 01:36:40PM -0400, Adam Jackson wrote:
> On Sat, 2019-05-25 at 07:50 -0700, Bryce Harrington wrote:
> 
> > Looks like since the move to gitlab.freedesktop.org I no longer have
> > commit access on pixman; you'll need to land them for us.
> 
> Apologies for the oversight, I've added you to the pixman group.
> 
> - ajax

Thanks ajax!

Dithering patchset pushed:

remote: To ssh://kemper.freedesktop.org/git/pixman
remote:85bfa8b..cb2ec42  cb2ec4268fbde0df3b588ce5cbe2e43e04654520 -> master
To ssh://gitlab.freedesktop.org/pixman/pixman
   85bfa8b..cb2ec42  master -> master

Thanks Basile and Marc, please keep an eye out for any issues that may
arise, I'll make myself available to review followup work on this
feature.

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

Re: [Pixman] Dithering patches, v2

[Adam Jackson]

On Sat, 2019-05-25 at 07:50 -0700, Bryce Harrington wrote:

> Looks like since the move to gitlab.freedesktop.org I no longer have
> commit access on pixman; you'll need to land them for us.

Apologies for the oversight, I've added you to the pixman group.

- ajax

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

Re: [Pixman] Dithering patches, v2

[Bryce Harrington]

On Mon, May 13, 2019 at 10:26:00AM -0700, Matt Turner wrote:
> On Sat, May 11, 2019 at 7:42 AM Bryce Harrington
>  wrote:
> >
> > On Tue, May 07, 2019 at 09:52:39AM -0700, Matt Turner wrote:
> > > On Sun, May 5, 2019 at 11:50 AM Bryce Harrington
> > >  wrote:
> > > >
> > > > On Mon, Apr 22, 2019 at 09:26:48AM -0700, Matt Turner wrote:
> > > > > On Fri, Apr 19, 2019 at 4:52 PM Bryce Harrington
> > > > >  wrote:
> > > > > > Inkscape would love to see Basile's dithering patches included.  Our
> > > > > > testing shows that they make a huge quality difference for our 
> > > > > > users;
> > > > > > this solves a critical need.
> > > > > >
> > > > > > Mc and I have done some preliminary investigation into how to plumb 
> > > > > > this
> > > > > > into Cairo, and would love to hear your review of Basile's approach 
> > > > > > to
> > > > > > the problem.
> > > > >
> > > > > I don't feel like I'm experienced enough with that side of pixman to
> > > > > offer meaningful comments. I've Cc'd Søren in the hopes that he
> > > > > remains interested enough in the project to review the patches that
> > > > > Basile says implement the approach Søren described.
> > > >
> > > > I totally understand, I'd feel the same.  But I think this is an
> > > > important patch, so how can we move forward with it?
> > >
> > > If you're happy with the patches, I'd say let's commit them.
> >
> > Works for me, would you prefer me to commit them, or will you be
> > committing them yourself?
> 
> I'd prefer you commit them since they're for Inkscape.

Looks like since the move to gitlab.freedesktop.org I no longer have
commit access on pixman; you'll need to land them for us.

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

Re: [Pixman] Dithering patches, v2

[Matt Turner]

On Sat, May 11, 2019 at 7:42 AM Bryce Harrington
 wrote:
>
> On Tue, May 07, 2019 at 09:52:39AM -0700, Matt Turner wrote:
> > On Sun, May 5, 2019 at 11:50 AM Bryce Harrington
> >  wrote:
> > >
> > > On Mon, Apr 22, 2019 at 09:26:48AM -0700, Matt Turner wrote:
> > > > On Fri, Apr 19, 2019 at 4:52 PM Bryce Harrington
> > > >  wrote:
> > > > > Inkscape would love to see Basile's dithering patches included.  Our
> > > > > testing shows that they make a huge quality difference for our users;
> > > > > this solves a critical need.
> > > > >
> > > > > Mc and I have done some preliminary investigation into how to plumb 
> > > > > this
> > > > > into Cairo, and would love to hear your review of Basile's approach to
> > > > > the problem.
> > > >
> > > > I don't feel like I'm experienced enough with that side of pixman to
> > > > offer meaningful comments. I've Cc'd Søren in the hopes that he
> > > > remains interested enough in the project to review the patches that
> > > > Basile says implement the approach Søren described.
> > >
> > > I totally understand, I'd feel the same.  But I think this is an
> > > important patch, so how can we move forward with it?
> >
> > If you're happy with the patches, I'd say let's commit them.
>
> Works for me, would you prefer me to commit them, or will you be
> committing them yourself?

I'd prefer you commit them since they're for Inkscape.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] Dithering patches, v2

[Bryce Harrington]

On Tue, May 07, 2019 at 09:52:39AM -0700, Matt Turner wrote:
> On Sun, May 5, 2019 at 11:50 AM Bryce Harrington
>  wrote:
> >
> > On Mon, Apr 22, 2019 at 09:26:48AM -0700, Matt Turner wrote:
> > > On Fri, Apr 19, 2019 at 4:52 PM Bryce Harrington
> > >  wrote:
> > > > Inkscape would love to see Basile's dithering patches included.  Our
> > > > testing shows that they make a huge quality difference for our users;
> > > > this solves a critical need.
> > > >
> > > > Mc and I have done some preliminary investigation into how to plumb this
> > > > into Cairo, and would love to hear your review of Basile's approach to
> > > > the problem.
> > >
> > > I don't feel like I'm experienced enough with that side of pixman to
> > > offer meaningful comments. I've Cc'd Søren in the hopes that he
> > > remains interested enough in the project to review the patches that
> > > Basile says implement the approach Søren described.
> >
> > I totally understand, I'd feel the same.  But I think this is an
> > important patch, so how can we move forward with it?
> 
> If you're happy with the patches, I'd say let's commit them.

Works for me, would you prefer me to commit them, or will you be
committing them yourself?

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

Re: [Pixman] Dithering patches, v2

[Matt Turner]

On Sun, May 5, 2019 at 11:50 AM Bryce Harrington
 wrote:
>
> On Mon, Apr 22, 2019 at 09:26:48AM -0700, Matt Turner wrote:
> > On Fri, Apr 19, 2019 at 4:52 PM Bryce Harrington
> >  wrote:
> > > Inkscape would love to see Basile's dithering patches included.  Our
> > > testing shows that they make a huge quality difference for our users;
> > > this solves a critical need.
> > >
> > > Mc and I have done some preliminary investigation into how to plumb this
> > > into Cairo, and would love to hear your review of Basile's approach to
> > > the problem.
> >
> > I don't feel like I'm experienced enough with that side of pixman to
> > offer meaningful comments. I've Cc'd Søren in the hopes that he
> > remains interested enough in the project to review the patches that
> > Basile says implement the approach Søren described.
>
> I totally understand, I'd feel the same.  But I think this is an
> important patch, so how can we move forward with it?

If you're happy with the patches, I'd say let's commit them.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] Dithering patches, v2

[Bryce Harrington]

On Mon, Apr 22, 2019 at 09:26:48AM -0700, Matt Turner wrote:
> On Fri, Apr 19, 2019 at 4:52 PM Bryce Harrington
>  wrote:
> > Inkscape would love to see Basile's dithering patches included.  Our
> > testing shows that they make a huge quality difference for our users;
> > this solves a critical need.
> >
> > Mc and I have done some preliminary investigation into how to plumb this
> > into Cairo, and would love to hear your review of Basile's approach to
> > the problem.
> 
> I don't feel like I'm experienced enough with that side of pixman to
> offer meaningful comments. I've Cc'd Søren in the hopes that he
> remains interested enough in the project to review the patches that
> Basile says implement the approach Søren described.

I totally understand, I'd feel the same.  But I think this is an
important patch, so how can we move forward with it?

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

Re: [Pixman] Dithering patches, v2

[Basile Clement]

There is less calculation, but the noise matrix takes up cache space
(about half a modern CPU's L1) and hence increases memory pressure.  The
actual performance relationship is probably hardware dependent, but on
my laptop doing a quick check with patched lowlevel-blt-bench:

 - Bayer is ~8% faster than blue noise
 - Bayer is only ~5% faster if I use `dither_blue_noise_64x64[((y &
0x3f) << 6) | (x & 0x3f)]` in the index computation, which I would have
expected GCC to do automatically but oh well.
 - Both are about the same speed with uint16_t array for the blue noise
(halves memory consumption).

I will make those two changes (they are clearly desirable) and re-post
the blue noise patch.  Since this is in a write-back loop which should
be memory-bound in general, I think it makes sense to keep Bayer as FAST
dithering -- but I'm not opposed to removing it.

- Basile

On 4/22/19 8:12 PM, Bill Spitzak wrote:
> Is the "blue noise" version actually slower than the Bayer? Seems to
> be doing a bit less calculation but it reading the array of weights.
> The removal of the need for the pattern offset seems like a win.
>
> On Mon, Apr 22, 2019 at 9:27 AM Matt Turner  > wrote:
>
> On Fri, Apr 19, 2019 at 4:52 PM Bryce Harrington
> mailto:br...@bryceharrington.org>> wrote:
> > Inkscape would love to see Basile's dithering patches included.  Our
> > testing shows that they make a huge quality difference for our
> users;
> > this solves a critical need.
> >
> > Mc and I have done some preliminary investigation into how to
> plumb this
> > into Cairo, and would love to hear your review of Basile's
> approach to
> > the problem.
>
> I don't feel like I'm experienced enough with that side of pixman to
> offer meaningful comments. I've Cc'd Søren in the hopes that he
> remains interested enough in the project to review the patches that
> Basile says implement the approach Søren described.
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org 
> https://lists.freedesktop.org/mailman/listinfo/pixman
>
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] Dithering patches, v2

[Bill Spitzak]

Is the "blue noise" version actually slower than the Bayer? Seems to be
doing a bit less calculation but it reading the array of weights. The
removal of the need for the pattern offset seems like a win.

On Mon, Apr 22, 2019 at 9:27 AM Matt Turner  wrote:

> On Fri, Apr 19, 2019 at 4:52 PM Bryce Harrington
>  wrote:
> > Inkscape would love to see Basile's dithering patches included.  Our
> > testing shows that they make a huge quality difference for our users;
> > this solves a critical need.
> >
> > Mc and I have done some preliminary investigation into how to plumb this
> > into Cairo, and would love to hear your review of Basile's approach to
> > the problem.
>
> I don't feel like I'm experienced enough with that side of pixman to
> offer meaningful comments. I've Cc'd Søren in the hopes that he
> remains interested enough in the project to review the patches that
> Basile says implement the approach Søren described.
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] Dithering patches, v2

[Matt Turner]

On Fri, Apr 19, 2019 at 4:52 PM Bryce Harrington
 wrote:
> Inkscape would love to see Basile's dithering patches included.  Our
> testing shows that they make a huge quality difference for our users;
> this solves a critical need.
>
> Mc and I have done some preliminary investigation into how to plumb this
> into Cairo, and would love to hear your review of Basile's approach to
> the problem.

I don't feel like I'm experienced enough with that side of pixman to
offer meaningful comments. I've Cc'd Søren in the hopes that he
remains interested enough in the project to review the patches that
Basile says implement the approach Søren described.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] Dithering patches, v2

[Bryce Harrington]

Hey Matt,

Inkscape would love to see Basile's dithering patches included.  Our
testing shows that they make a huge quality difference for our users;
this solves a critical need.

Mc and I have done some preliminary investigation into how to plumb this
into Cairo, and would love to hear your review of Basile's approach to
the problem.

Thanks,
Bryce

On Wed, Apr 10, 2019 at 12:01:07AM +0200, Basile Clement wrote:
> I forgot to mention two things:
> 
>  - Enabling dithering makes processing much slower, since it forces the
> wide pipeline to be used (and some optimisations are disabled).  This
> could be solved through the use of fast paths where needed (e.g. when
> blitting images from an equal or lower bits-per-pixels format, or with
> fused gradient+dithering paths).  In any case, the design of the
> implementation allows setting the `dither` property of a destination
> image in between calls to `pixman_image_composite` where dithering is
> required, allowing users to pay the cost of the slower wide pipeline
> only for those calls where dithering is actually required.
> 
>  - The dithering is not gamma corrected, which doesn't matter much for
> 8bpp formats -- but makes a noticeable difference for lower bpp formats,
> producing images which are way too luminous (this is particularily
> visible in the dithering demos when using 2bpp and 1bpp formats).  Since
> the main use case for now is a8r8g8b8 gradient export in Inkscape (and
> since the series is starting to be a bit large), I did not include gamma
> correction in this series.  If needed, it can be added in subsequent
> patches, and should probably be configurable since the proper curve to
> use depend on the characteristic of the target device, which pixman may
> not know about.
> 
> - Basile
> 
> On 4/9/19 11:29 PM, basile-pix...@clement.pm wrote:
> > I am resubmitting for review and inclusion my series of patches on
> > dithering from October, which I ended up not having the time to finish
> > then.  There are only a couple changes to make the patches compatible
> > with the recently added floating point formats, and I have also added a
> > dithering matrix based on blue noise which provides more pleasant
> > results than the Bayer matrix from the first series.
> >
> > Maarten, I would appreciate if you can take a second look (although as
> > noted above not much has changed since your first review -- I don't
> > think you had comments on the dithering part?).
> >
> > - Basile
> >
> >
> > ___
> > Pixman mailing list
> > Pixman@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/pixman
> 
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] Dithering patches, v2

[Basile Clement]

I'm not sure how, but I badly broke the formatting on that email -- my
apologies.  Below is the previous email formatted as intended.

On 4/10/19 1:04 AM, Bill Spitzak wrote:

> How difficult is it to just make the gradients use dithering, so
> they produce 8-bit (or whatever) results directly but with the dithering
> pattern in them?

We don't have the destination bpp when we compute the gradients.  Søren
Sandmann explained the various issues with this approach earlier
https://lists.freedesktop.org/archives/pixman/2018-April/004711.html

Note that it is always possible to add fast paths for gradient+dithering
if performance is an issue.

> What other operations would need dithering (a large blur comes to
> mind, also zooming way in on an image in bilinear)?

Any type of filter (eg blur) or zoom could benefit from dithering, as
well as any high bpp to low bpp compositing (compositing to r5g6b5 was
mentioned in one of the previous discussions).  Although for that one
proper gamma correction may be required.

> I think the blue noise large pattern is a great idea.

All credits go to Wikipedia :)

By the way re-reading the email I linked reminded me --

Søren, I don't know if you are still reading this mailing list, but you
said in that email one year ago:

> I could elaborate on that subject and have done so several times in
> the past, but in my experience those long emails about pixman
> internals were mostly just a waste of time. I don't recall a single 
> one of them ever resulting in a useful patch.

Your emails (especially the two I linked above, but also a couple
others) were instrumental in getting a good enough grasp on the pixman
internals for writing the floating point gradient computation and
dithering patches, so thank you for taking the time to share your
knowledge and insights.  Hopefully mine will be (are?) those
long-awaited useful patches.

- Basile


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

Re: [Pixman] Dithering patches, v2

[Basile Clement]

On 4/10/19 1:04 AM, Bill Spitzak wrote:
> How difficult is it to just make the gradients use dithering, so > they 
> produce 8-bit (or whatever) results directly but with the >
dithering pattern in them? >

We don't have the destination bpp when we compute the gradients.  Søren
Sandmann explained the various issues with this approach earlier
https://lists.freedesktop.org/archives/pixman/2018-April/004711.html

Note that it is always possible to add fast paths for gradient+dithering
if performance is an issue.

> What other operations would need dithering (a large blur comes to > mind, 
> also zooming way in on an image in bilinear)?

Any type of filter (eg blur) or zoom could benefit from dithering, as
well as any high bpp to low bpp compositing (compositing to r5g6b5 was
mentioned in one of the previous discussions).  Although for that one
proper gamma correction may be required.

> I think the blue noise large pattern is a great idea.

All credits go to Wikipedia :)

By the way re-reading the email I linked reminded me --

Søren, I don't know if you are still reading this mailing list, but you
said in that email one year ago:

> I could elaborate on that subject and have done so several times in > the 
> past, but in my experience those long emails about pixman >
internals were mostly just a waste of time. I don't recall a single >
one of them ever resulting in a useful patch.
Your emails (especially the two I linked above, but also a couple
others) were instrumental in getting a good enough grasp on the pixman
internals for writing the floating point gradient computation and
dithering patches, so thank you for taking the time to share your
knowledge and insights.  Hopefully mine will be (are?) those
long-awaited useful patches.

- Basile


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

Re: [Pixman] Dithering patches, v2

[Bill Spitzak]

How difficult is it to just make the gradients use dithering, so they
produce 8-bit (or whatever) results directly but with the dithering pattern
in them?

What other operations would need dithering (a large blur comes to mind,
also zooming way in on an image in bilinear)?

I think the blue noise large pattern is a great idea. Could have used that
years ago in Nuke (it uses error diffusion but people always complained
that the pattern did not reproduce when tiny changes were made to the
image).


On Tue, Apr 9, 2019 at 3:01 PM Basile Clement 
wrote:

> I forgot to mention two things:
>
>  - Enabling dithering makes processing much slower, since it forces the
> wide pipeline to be used (and some optimisations are disabled).  This
> could be solved through the use of fast paths where needed (e.g. when
> blitting images from an equal or lower bits-per-pixels format, or with
> fused gradient+dithering paths).  In any case, the design of the
> implementation allows setting the `dither` property of a destination
> image in between calls to `pixman_image_composite` where dithering is
> required, allowing users to pay the cost of the slower wide pipeline
> only for those calls where dithering is actually required.
>
>  - The dithering is not gamma corrected, which doesn't matter much for
> 8bpp formats -- but makes a noticeable difference for lower bpp formats,
> producing images which are way too luminous (this is particularily
> visible in the dithering demos when using 2bpp and 1bpp formats).  Since
> the main use case for now is a8r8g8b8 gradient export in Inkscape (and
> since the series is starting to be a bit large), I did not include gamma
> correction in this series.  If needed, it can be added in subsequent
> patches, and should probably be configurable since the proper curve to
> use depend on the characteristic of the target device, which pixman may
> not know about.
>
> - Basile
>
> On 4/9/19 11:29 PM, basile-pix...@clement.pm wrote:
> > I am resubmitting for review and inclusion my series of patches on
> > dithering from October, which I ended up not having the time to finish
> > then.  There are only a couple changes to make the patches compatible
> > with the recently added floating point formats, and I have also added a
> > dithering matrix based on blue noise which provides more pleasant
> > results than the Bayer matrix from the first series.
> >
> > Maarten, I would appreciate if you can take a second look (although as
> > noted above not much has changed since your first review -- I don't
> > think you had comments on the dithering part?).
> >
> > - Basile
> >
> >
> > ___
> > Pixman mailing list
> > Pixman@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/pixman
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] Dithering patches, v2

[Basile Clement]

I forgot to mention two things:

 - Enabling dithering makes processing much slower, since it forces the
wide pipeline to be used (and some optimisations are disabled).  This
could be solved through the use of fast paths where needed (e.g. when
blitting images from an equal or lower bits-per-pixels format, or with
fused gradient+dithering paths).  In any case, the design of the
implementation allows setting the `dither` property of a destination
image in between calls to `pixman_image_composite` where dithering is
required, allowing users to pay the cost of the slower wide pipeline
only for those calls where dithering is actually required.

 - The dithering is not gamma corrected, which doesn't matter much for
8bpp formats -- but makes a noticeable difference for lower bpp formats,
producing images which are way too luminous (this is particularily
visible in the dithering demos when using 2bpp and 1bpp formats).  Since
the main use case for now is a8r8g8b8 gradient export in Inkscape (and
since the series is starting to be a bit large), I did not include gamma
correction in this series.  If needed, it can be added in subsequent
patches, and should probably be configurable since the proper curve to
use depend on the characteristic of the target device, which pixman may
not know about.

- Basile

On 4/9/19 11:29 PM, basile-pix...@clement.pm wrote:
> I am resubmitting for review and inclusion my series of patches on
> dithering from October, which I ended up not having the time to finish
> then.  There are only a couple changes to make the patches compatible
> with the recently added floating point formats, and I have also added a
> dithering matrix based on blue noise which provides more pleasant
> results than the Bayer matrix from the first series.
>
> Maarten, I would appreciate if you can take a second look (although as
> noted above not much has changed since your first review -- I don't
> think you had comments on the dithering part?).
>
> - Basile
>
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman

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

[Pixman] Dithering patches, v2

[basile-pixman]

I am resubmitting for review and inclusion my series of patches on
dithering from October, which I ended up not having the time to finish
then.  There are only a couple changes to make the patches compatible
with the recently added floating point formats, and I have also added a
dithering matrix based on blue noise which provides more pleasant
results than the Bayer matrix from the first series.

Maarten, I would appreciate if you can take a second look (although as
noted above not much has changed since your first review -- I don't
think you had comments on the dithering part?).

- Basile


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