[Pixman] Gradient dithering into pixman

Tue Apr 3 02:08:17 UTC 2018

Here are some images:

- with no dithering (current pixman):

        http://ssp.impulsetrain.com/upload/gradient-none.png

- with the proposed patch applied:

        http://ssp.impulsetrain.com/upload/gradient-patch.png

- with proper dithering:

        http://ssp.impulsetrain.com/upload/gradient-proper.png

All images were rendered to 565.

Søren

On Mon, Apr 2, 2018 at 8:40 PM, Søren Sandmann <soren.sandmann at gmail.com>
wrote:

> 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/1ef2dbcc/attachment-0001.html>