<div dir="ltr"><div><div><div><div>Here are some images:<br><br></div>- with no dithering (current pixman):<br><br>        <a href="http://ssp.impulsetrain.com/upload/gradient-none.png">http://ssp.impulsetrain.com/upload/gradient-none.png</a><br><br></div>- with the proposed patch applied:<br><br>        <a href="http://ssp.impulsetrain.com/upload/gradient-patch.png">http://ssp.impulsetrain.com/upload/gradient-patch.png</a><br><br></div>- with proper dithering:<br><br>        <a href="http://ssp.impulsetrain.com/upload/gradient-proper.png">http://ssp.impulsetrain.com/upload/gradient-proper.png</a><br><br></div><div>All images were rendered to 565.<br><br><br></div>Søren<br><div><div><div><div><br></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 2, 2018 at 8:40 PM, Søren Sandmann <span dir="ltr"><<a href="mailto:soren.sandmann@gmail.com" target="_blank">soren.sandmann@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div>Hi,<br><br></div>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.<br><br>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.<br><br></div>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.<br><div><div><br></div><div>The shortest path to getting dithering into pixman properly is probably these two steps:<br><br></div><div>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.<br><br>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.<br></div><div><br>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.<span class="HOEnZb"><font color="#888888"><br><br><br></font></span></div><span class="HOEnZb"><font color="#888888"><div>Søren<br></div><div><br></div></font></span></div><div><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 2, 2018 at 6:26 PM, Bryce Harrington <span dir="ltr"><<a href="mailto:bryce@osg.samsung.com" target="_blank">bryce@osg.samsung.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Søren,<br>
<br>
I'd really like to see this change available for Cairo.  Like you<br>
remarked in your referenced post, dithering has a huge improvement for<br>
gradients on 16 bit destinations.  I'm glad to hear you agree the<br>
general idea is in-scope for pixman.  The improvements to banding are<br>
almost embarrasingly good.  A couple users posted example images to show<br>
with and without Mc's patch:<br>
<br>
  <a href="http://www.bryceharrington.org/Files/device_texture_template_s8_x_metal_depth_comparison.png" rel="noreferrer" target="_blank">http://www.bryceharrington.org<wbr>/Files/device_texture_template<wbr>_s8_x_metal_depth_comparison.<wbr>png</a><br>
  (Work by Chris Rogers)<br>
<br>
and<br>
<br>
  <a href="https://my.mixtape.moe/sqjmba.png" rel="noreferrer" target="_blank">https://my.mixtape.moe/sqjmba.<wbr>png</a><br>
  <a href="https://my.mixtape.moe/jqyvni.png" rel="noreferrer" target="_blank">https://my.mixtape.moe/jqyvni.<wbr>png</a><br>
  (Work by Liam White, I believe)<br>
<br>
The significance of the banding change is discernable even to my old man<br>
non-artist eyes.<br>
<br>
The approach you outlined in your 2012 post - to do dithering late in<br>
the pipeline - sounds interesting to me, too.  However, from your<br>
description it sounds like it would require some rather invasive changes<br>
to pixman's internal infrastructure.<br>
<br>
Has progress been made on implementation of this approach?  If not,<br>
would you mind considering adding a pre-compositing solution as an<br>
interim solution?  6 years is a lot of time to have waited, would be<br>
nice to not have to wait more years.  Or, if you're firm in only seeing<br>
the post-compositing approach, could you elaborate more on how this<br>
would be implemented?<br>
<br>
I will be rolling a 1.15.12 release of Cairo within a week or so, and<br>
while I'd love to be able to show off this improvement, I understand if<br>
it will take a longer time frame.  I will also be doing a 1.16.0 release<br>
in the coming months, so let me know if this feels like something that<br>
may be achievable in pixman in that time frame; if not we can figure<br>
something else out.  I am also at your service if you want help<br>
finalizing/landing this feature in pixman.<br>
<br>
Thanks,<br>
Bryce<br>
<span><br>
On Mon, Mar 26, 2018 at 08:04:48PM -0400, Søren Sandmann wrote:<br>
> Hi,<br>
><br>
> A long time ago I wrote this:<br>
><br>
>     <a href="https://lists.freedesktop.org/archives/pixman/2012-July/002175.html" rel="noreferrer" target="_blank">https://lists.freedesktop.<wbr>org/archives/pixman/2012-July/<wbr>002175.html</a><br>
><br>
> about how dithering could be added to pixman. The basic idea is that<br>
> "dithering" is a property of the destination image, not of the gradient.  I<br>
> still think this is the right way to do it.<br>
><br>
><br>
> Søren<br>
><br>
</span><div><div class="m_1206459903851476739h5">> On Mon, Mar 26, 2018 at 6:37 PM, Marc Jeanmougin <marc at <a href="http://jeanmougin.fr" rel="noreferrer" target="_blank">jeanmougin.fr</a>> wrote:<br>
><br>
> > Hi,<br>
> ><br>
> > I'm Marc, I'm an Inkscape contributor, and I would like to improve<br>
> > pixman by providing a patch to allow gradient dithering, to eliminate<br>
> > all gradient banding. This "banding" problem is hugely visible in many<br>
> > Inkscape files, and I hope you could help me putting it into pixman,<br>
> > then in exposing the functionality into cairo.<br>
> ><br>
> > I tried to very closely follow the existing code style, and I came up<br>
> > with the proof of concept (attached Proof_of_concept.patch ), which<br>
> > applies to pixman/master to enable the feature directly.<br>
> ><br>
> > Since Inkscape also uses pixman to render files when exporting to png,<br>
> > and that kind of dithering affects very significantly the file size (PNG<br>
> > compression algorithm and randomness don't mix well) we also need a<br>
> > switch to enable/disable it "at will".<br>
> ><br>
> > I tried to understand how it could be done by looking at<br>
> > pixman_image_set_* functions and finally came up with the other patch<br>
> > attached 0001-Adds-a-gradient-dithering<wbr>-function-to-pixman.patch (which<br>
> > cannot directly be tested, since it would require changes in cairo to<br>
> > call pixman_image_set_dithering, but changing "gradient->dithering =<br>
> > PIXMAN_DITHERING_NONE;" to _BEST in pixman-image.c works)<br>
> ><br>
> > Thanks for any help in merging this, or any pointer to how to improve it,<br>
> ><br>
> > --<br>
> > Marc<br>
> ><br>
> > ______________________________<wbr>_________________<br>
> > Pixman mailing list<br>
</div></div>> > Pixman at <a href="http://lists.freedesktop.org" rel="noreferrer" target="_blank">lists.freedesktop.org</a><br>
> > <a href="https://lists.freedesktop.org/mailman/listinfo/pixman" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/pixman</a><br>
> ><br>
> ><br>
> -------------- next part --------------<br>
> An HTML attachment was scrubbed...<br>
> URL: <<a href="https://lists.freedesktop.org/archives/pixman/attachments/20180326/dd64cca0/attachment.html" rel="noreferrer" target="_blank">https://lists.freedesktop.org<wbr>/archives/pixman/attachments/<wbr>20180326/dd64cca0/attachment.<wbr>html</a>><br>
<div class="m_1206459903851476739HOEnZb"><div class="m_1206459903851476739h5">______________________________<wbr>_________________<br>
Pixman mailing list<br>
<a href="mailto:Pixman@lists.freedesktop.org" target="_blank">Pixman@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/pixman" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/pixman</a><br>
</div></div></blockquote></div><br></div></div></div></div>
</blockquote></div><br></div></div></div>