[Pixman] Gradient dithering into pixman

Tue Apr 3 00:40:42 UTC 2018

Hi,

If I were still maintainer of pixman, I would definitely stand firm that
the proposed patch is not the right approach. For one, it doesn't actually
have the desired effect on 16 bit destinations. The attached patch (to be
applied on top of the dithering patch) modifies demos/linear-gradient.c to
show this. Rather than remove banding, it just blurs the transitions
between bands.

And this is not just due to some bug in the patch; it is a consequence of
it taking the fundamentally wrong approach. The amount of noise that should
be added to the gradient depends on how many bits it will be quantized to,
and this information is not available at the point in the pipeline where
this patch applies.

The patch will indeed improve Inkscape's usecase, which is rendering
gradients onto 24/32-bit destinations. This can also be seen with the
modified demos/linear-gradient.c, if the format of dest_img is changed back
to x8r8g8b8. The result is indeed much better with dithering turned on. But
that is really an accident of pixman's internal pipeline being 32 bit.

The shortest path to getting dithering into pixman properly is probably
these two steps:

1. Gradients in current pixman are internally rendered in 8 bits per
channel, which is insufficient for wider formats such as a2r10g10b10.
Instead, when gradients are asked render in floating point (through
"linear_get_scanline_wide()" for example), they should actually do so
instead of first rendering to 32 bits and then expanding to floating point.
This is simply a bug and the fix for it is useful regardless of dithering,
and should be sent as a separate patch.

2. With that in place, a "dither" property can then be added to BITS
images, which if turned image causes two things to happen: (a) rendering to
that image to will go through the floating point pipeline, and (b)
dithering noise with an amplitude determined by the image's format is added
somewhere in the general implementation before writing the temporary buffer
back to the destination.

These changes are not particularly invasive, but I don't think making them
is possible without some understanding of how pixman's rendering pipeline
actually works. 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.

Søren

On Mon, Apr 2, 2018 at 6:26 PM, Bryce Harrington <bryce at osg.samsung.com>
wrote:

> Hi Søren,
>
> I'd really like to see this change available for Cairo.  Like you
> remarked in your referenced post, dithering has a huge improvement for
> gradients on 16 bit destinations.  I'm glad to hear you agree the
> general idea is in-scope for pixman.  The improvements to banding are
> almost embarrasingly good.  A couple users posted example images to show
> with and without Mc's patch:
>
>   http://www.bryceharrington.org/Files/device_texture_
> template_s8_x_metal_depth_comparison.png
>   (Work by Chris Rogers)
>
> and
>
>   https://my.mixtape.moe/sqjmba.png
>   https://my.mixtape.moe/jqyvni.png
>   (Work by Liam White, I believe)
>
> The significance of the banding change is discernable even to my old man
> non-artist eyes.
>
> The approach you outlined in your 2012 post - to do dithering late in
> the pipeline - sounds interesting to me, too.  However, from your
> description it sounds like it would require some rather invasive changes
> to pixman's internal infrastructure.
>
> Has progress been made on implementation of this approach?  If not,
> would you mind considering adding a pre-compositing solution as an
> interim solution?  6 years is a lot of time to have waited, would be
> nice to not have to wait more years.  Or, if you're firm in only seeing
> the post-compositing approach, could you elaborate more on how this
> would be implemented?
>
> I will be rolling a 1.15.12 release of Cairo within a week or so, and
> while I'd love to be able to show off this improvement, I understand if
> it will take a longer time frame.  I will also be doing a 1.16.0 release
> in the coming months, so let me know if this feels like something that
> may be achievable in pixman in that time frame; if not we can figure
> something else out.  I am also at your service if you want help
> finalizing/landing this feature in pixman.
>
> Thanks,
> Bryce
>
> On Mon, Mar 26, 2018 at 08:04:48PM -0400, Søren Sandmann wrote:
> > Hi,
> >
> > A long time ago I wrote this:
> >
> >     https://lists.freedesktop.org/archives/pixman/2012-July/002175.html
> >
> > about how dithering could be added to pixman. The basic idea is that
> > "dithering" is a property of the destination image, not of the
> gradient.  I
> > still think this is the right way to do it.
> >
> >
> > Søren
> >
> > On Mon, Mar 26, 2018 at 6:37 PM, Marc Jeanmougin <marc at jeanmougin.fr>
> wrote:
> >
> > > Hi,
> > >
> > > I'm Marc, I'm an Inkscape contributor, and I would like to improve
> > > pixman by providing a patch to allow gradient dithering, to eliminate
> > > all gradient banding. This "banding" problem is hugely visible in many
> > > Inkscape files, and I hope you could help me putting it into pixman,
> > > then in exposing the functionality into cairo.
> > >
> > > I tried to very closely follow the existing code style, and I came up
> > > with the proof of concept (attached Proof_of_concept.patch ), which
> > > applies to pixman/master to enable the feature directly.
> > >
> > > Since Inkscape also uses pixman to render files when exporting to png,
> > > and that kind of dithering affects very significantly the file size
> (PNG
> > > compression algorithm and randomness don't mix well) we also need a
> > > switch to enable/disable it "at will".
> > >
> > > I tried to understand how it could be done by looking at
> > > pixman_image_set_* functions and finally came up with the other patch
> > > attached 0001-Adds-a-gradient-dithering-function-to-pixman.patch
> (which
> > > cannot directly be tested, since it would require changes in cairo to
> > > call pixman_image_set_dithering, but changing "gradient->dithering =
> > > PIXMAN_DITHERING_NONE;" to _BEST in pixman-image.c works)
> > >
> > > Thanks for any help in merging this, or any pointer to how to improve
> it,
> > >
> > > --
> > > Marc
> > >
> > > _______________________________________________
> > > Pixman mailing list
> > > Pixman at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/pixman
> > >
> > >
> > -------------- next part --------------
> > An HTML attachment was scrubbed...
> > URL: <https://lists.freedesktop.org/archives/pixman/
> attachments/20180326/dd64cca0/attachment.html>
> _______________________________________________
> Pixman mailing list
> Pixman at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/pixman/attachments/20180402/22aaba26/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gradient.patch
Type: text/x-patch
Size: 1642 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/pixman/attachments/20180402/22aaba26/attachment.bin>