<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 7, 2016 at 1:15 AM, Pekka Paalanen <span dir="ltr"><<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@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 class="HOEnZb"><div class="h5">On Fri, 4 Mar 2016 19:12:36 -0800<br> Bill Spitzak <<a href="mailto:spitzak@gmail.com">spitzak@gmail.com</a>> wrote:<br> <br> > On Fri, Mar 4, 2016 at 4:17 PM, Søren Sandmann <<a href="mailto:soren.sandmann@gmail.com">soren.sandmann@gmail.com</a>><br> > wrote:<br> ><br> > > On Wed, Feb 10, 2016 at 1:25 AM, <<a href="mailto:spitzak@gmail.com">spitzak@gmail.com</a>> wrote:<br> > ><br> > >> From: Bill Spitzak <<a href="mailto:spitzak@gmail.com">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<br> > >> 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">spitzak@gmail.com</a>><br> > >><br> > ><br> > > The short answer to this patch is NACK - it doesn't make sense to change<br> > > the meaning of GOOD and BEST.<br> > ><br> ><br> > Okay that is going to need a big WTF?<br> <br> </div></div>No, it's perfectly understandable.<br> <span class=""><br> > > There are at least three reasons:<br> > ><br> > > 1. Pixman is a low-level API that is in the camp of "do what I say" not<br> > > "do what I mean". When users such as cairo specify GOOD and BEST, they are<br> > > essentially asking Pixman to make a tradeoff that it doesn't have the<br> > > knowledge to do. As such, I think it was a mistake to have these values in<br> > > the first place (and indeed also a mistake to have them in the Render<br> > > extension since X11 is also supposed to be a do-what-I-say API). As such, I<br> > > think they should remain aliases to BILINEAR as they are now.<br> > ><br> ><br> > I have no idea why having three ways to say BILINEAR is necessary to "do<br> > what I say". One seems sufficient.<br> <br> </span>You must forget any resemblance the strings "GOOD" and "BEST" have<br> with English, and see them as abstract names for particular<br> mathematical functions.<br> <br> That is what do-what-I-say means.<br> <br> Therefore you cannot change what mathematical function they correspond<br> to.<br> <br> There are three ways of saying the same only because of a mistake in<br> the past which became evident only much later. It happens. Get over it.<br> <span class=""><br> > > The way to go is to add a new SeparateConvolution filter to Render and make<br> > > fb call into Pixman to implement it. Once that is in place, the hardware<br> > > drivers can then implement it.<br> > ><br> ><br> > You seem to think the hardware drivers will implement this version of<br> > SeparateConvolution? Really???<br> <br> </span>That is implied by do-what-I-say API policy.<br> <span class=""><br> > The purpose of this was so that the hardware drivers could implement<br> > something they call "good" and something they call "best" using whatever<br> > hardware resources are available. This is not going to happen if they are<br> > expected to read the filter array.<br> <br> </span>That will violate the do-what-I-say policy.<br> <span class=""><br> > 3. The patch is completely broken as written. I could go into details, but<br> > > there is no point since I don't think the patch should be pushed even if<br> > > fixed. I'll just point out that if the environment variable<br> > > PIXMAN_DISABLE=fast is set, the patch does absolutely nothing (you can<br> > > verify this with the scale program). And setting this environment variable<br> > > is not supposed to change Pixman's behavior, only potentially its<br> > > performance.<br> > ><br> ><br> > Will look into that. I would like the rest of the detail please.<br> <br> </span>That won't help this patch get through.<br> <span class=""><br> > Currently Linux 2D rendering is a joke. Nobody was using the<br> > SeparateConvolution until I patched up Cairo to do it, but I sure figured<br> > that was a very temporary patch until it could be done in a proper<br> > location. Users want "good", not "low level api". So now Linux is 1.5 years<br> > further behind. Really sad.<br> <br> </span>Users do not use Pixman. Pixman is an implementation detail of Render<br> and Cairo. Pixman *is* a low level API, lower than Render or Cairo.</blockquote><div><br></div><div>Please look at the new patch, where I split this from other changes so at least you can push the other changes that fix the filter generator.</div><div><br></div><div>I agree that users should use Render/Cairo but because the api to them just has "good" and "best" it makes no difference to the user what the communication between it and Pixman looks like. And I believe the current design is making it impossible to do optimizations expected on modern systems as Pixman is lacking significant information that it needs.</div><div><br></div><div>Currently Pixman cannot optimize the separable convolution because it does not know enough information about the filters. The code I put in Cairo does about the best that I can manage, but this duplicates matrix analysis that Pixman is doing anyway. And it lacks optimizations that Pixman could do if it knew the filter was normalized and symmetric (I now plan to add a patch so Pixman looks at the filter to figure these out).</div><div><br></div><div>X11 might have bugs sending GOOD/BEST through, but it completely lacks any attempt to send the separable convolution filters through. I know which of these problems would be easier to fix!</div><div><br></div><div>Hardware/shader implementations do not use a data structure resembling the convolution array at all. The most common approach is to bilinear interpret a 1-d image that represents the filter (yes I know there are trickier ones). Take a look at the gnuplot output and you will see that constructing such an image by reinterlacing the subsamples fails badly on the box and triangle filters. Knowledge of the filter would allow a perfect lookup image to be created trivially, or for shaders/hardware to directly calculate them (bilinear is by far the most common hardware supported filter, it is a triangle, but I doubt any implementation uses a lookup table for it!).</div><div><br></div><div>The current api is not usable for non-affine transforms, as the filter has to change size depending on the location. All the optimization attempts in Cairo are useless as they have to be performed per-pixel, not per-transform.</div><div><br></div><div>I suppose an api that looks more like the api to the filter *generator*, with an enumeration or other description of the functions, would work and maybe be more in keeping with the Pixman design. I'm just not very certain that any users want this. If Cairo just translates Good->set1 and Best->set2, then all effort in supporting other sets is wasted code that will never be used.</div><div><br></div></div></div></div>