<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 4, 2016 at 4:17 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 class="h5">On Wed, Feb 10, 2016 at 1:25 AM, <span dir="ltr"><<a href="mailto:spitzak@gmail.com" target="_blank">spitzak@gmail.com</a>></span> wrote:<br></div></div><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Bill Spitzak <<a href="mailto:spitzak@gmail.com" target="_blank">spitzak@gmail.com</a>><br> <br> Detects and uses PIXMAN_FILTER_NEAREST for all 8 90-degree rotations and<br> reflections when the scale is 1.0 and integer translation.<br> <br> GOOD uses:<br> <br> scale < 1/16 : BOX.BOX at size 16<br> scale < 3/4 : BOX.BOX at size 1/scale<br> larger : BOX.BOX at size 1<br> <br> If both directions have a scale >= 3/4 or a scale of 1/2 and an integer<br> translation, the faster PIXMAN_FILTER_BILINEAR code is used. This is<br> compatable at these scales with older versions of pixman where bilinear<br> was always used for GOOD.<br> <br> BEST uses:<br> <br> scale < 1/24 : BOX.BOX at size 24<br> scale < 1/16 : BOX.BOX at size 1/scale<br> scale < 1 : IMPULSE.LANCZOS2 at size 1/scale<br> scale < 2.333 : IMPULSE.LANCZOS2 at size 1<br> scale < 128 : BOX.LANCZOS2 at size 1/(scale-1) (antialiased square pixels)<br> larger : BOX.LANCZOS2 at size 1/127 (antialias blur gets thicker)<br> <br> v8: Cutoff in BEST between IMPULSE.LANCZOS2 and BOX.LANCZOS2 adjusted for<br> a better match between the filters.<br> <br> v9: Use the new negative subsample controls to scale the subsamples. These<br> were chosen by finding the lowest number that did not add visible<br> artifacts to the zone plate image.<br> <br> Scale demo altered to default to GOOD and locked-together x+y scale<br> <br> Fixed divide-by-zero from all-zero matrix found by stress-test<br> <br> v11: Whitespace and formatting fixes<br> Moved demo changes to a later patch<br> <br> v12: Whitespace and formatting fixes<br> <br> Signed-off-by: Bill Spitzak <<a href="mailto:spitzak@gmail.com" target="_blank">spitzak@gmail.com</a>><br></blockquote><div><br></div></div></div><div>The short answer to this patch is NACK - it doesn't make sense to change the meaning of GOOD and BEST.<br></div></div></div></div></blockquote><div><br></div><div>Okay that is going to need a big WTF?</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>There are at least three reasons:<br><br></div><div>1. Pixman is a low-level API that is in the camp of "do what I say" not "do what I mean". When users such as cairo specify GOOD and BEST, they are essentially asking Pixman to make a tradeoff that it doesn't have the knowledge to do. As such, I think it was a mistake to have these values in the first place (and indeed also a mistake to have them in the Render extension since X11 is also supposed to be a do-what-I-say API). As such, I think they should remain aliases to BILINEAR as they are now.<br></div></div></div></div></blockquote><div><br></div><div>I have no idea why having three ways to say BILINEAR is necessary to "do what I say". One seems sufficient.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>2. The motivation for doing this in Pixman (and not in cairo) is that supposedly the X server will then pick it up automatically and use the high-quality filters when PictFilterBest and PictFilterGood is set. But that doesn't actually work because the X server doesn't map those Render filters to the corresponding Pixman filters. See:<br><br> <a href="https://cgit.freedesktop.org/xorg/xserver/tree/fb/fbpict.c#n420" target="_blank">https://cgit.freedesktop.org/xorg/xserver/tree/fb/fbpict.c#n420</a><br></div><div><br></div><div>So it looks to me like the whole scheme just won't work. But even if it did, you would still need to make all the hardware drivers do the same thing, so it's not just a matter of adding some constants in that function.<br></div></div></div></div></blockquote><div><br></div><div>If X is not currently sending GOOD/BEST through, then this is even a better reason why it is ok to change them, as it cannot possibly change the behavior of any X programs. All other pixman-using programs are directly linked with the library. I did figure X would remain broken for long after this is fixed, and Cairo would use the image backend for such transforms until X is updated, or it is replaced with Wayland (where the image backend is used all the time).</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>The way to go is to add a new SeparateConvolution filter to Render and make fb call into Pixman to implement it. Once that is in place, the hardware drivers can then implement it.<br></div></div></div></div></blockquote><div><br></div><div>You seem to think the hardware drivers will implement this version of SeparateConvolution? Really???</div><div><br></div><div>The purpose of this was so that the hardware drivers could implement something they call "good" and something they call "best" using whatever hardware resources are available. This is not going to happen if they are expected to read the filter array.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>3. The patch is completely broken as written. I could go into details, but there is no point since I don't think the patch should be pushed even if fixed. I'll just point out that if the environment variable PIXMAN_DISABLE=fast is set, the patch does absolutely nothing (you can verify this with the scale program). And setting this environment variable is not supposed to change Pixman's behavior, only potentially its performance.</div></div></div></div></blockquote><div><br></div><div>Will look into that. I would like the rest of the detail please.</div><div><br></div><div>Currently Linux 2D rendering is a joke. Nobody was using the SeparateConvolution until I patched up Cairo to do it, but I sure figured that was a very temporary patch until it could be done in a proper location. Users want "good", not "low level api". So now Linux is 1.5 years further behind. Really sad.</div><div><br></div></div></div></div>