Re: [Pixman] [PATCH v13 13/14] pixman-image: Implement PIXMAN_FILTER_GOOD/BEST as separable convolutions

[Bill Spitzak]

On Mon, Mar 7, 2016 at 1:15 AM, Pekka Paalanen  wrote:

> On Fri, 4 Mar 2016 19:12:36 -0800
> Bill Spitzak  wrote:
>
> > On Fri, Mar 4, 2016 at 4:17 PM, Søren Sandmann  >
> > wrote:
> >
> > > On Wed, Feb 10, 2016 at 1:25 AM,  wrote:
> > >
> > >> From: Bill Spitzak 
> > >>
> > >> Detects and uses PIXMAN_FILTER_NEAREST for all 8 90-degree rotations
> and
> > >> reflections when the scale is 1.0 and integer translation.
> > >>
> > >> GOOD uses:
> > >>
> > >>  scale < 1/16 : BOX.BOX at size 16
> > >>  scale < 3/4 : BOX.BOX at size 1/scale
> > >>  larger : BOX.BOX at size 1
> > >>
> > >>  If both directions have a scale >= 3/4 or a scale of 1/2 and an
> integer
> > >>  translation, the faster PIXMAN_FILTER_BILINEAR code is used. This is
> > >>  compatable at these scales with older versions of pixman where
> bilinear
> > >>  was always used for GOOD.
> > >>
> > >> BEST uses:
> > >>
> > >>  scale < 1/24 : BOX.BOX at size 24
> > >>  scale < 1/16 : BOX.BOX at size 1/scale
> > >>  scale < 1 : IMPULSE.LANCZOS2 at size 1/scale
> > >>  scale < 2.333 : IMPULSE.LANCZOS2 at size 1
> > >>  scale < 128 : BOX.LANCZOS2 at size 1/(scale-1) (antialiased square
> > >> pixels)
> > >>  larger : BOX.LANCZOS2 at size 1/127 (antialias blur gets thicker)
> > >>
> > >> v8: Cutoff in BEST between IMPULSE.LANCZOS2 and BOX.LANCZOS2 adjusted
> for
> > >> a better match between the filters.
> > >>
> > >> v9: Use the new negative subsample controls to scale the subsamples.
> These
> > >> were chosen by finding the lowest number that did not add visible
> > >> artifacts to the zone plate image.
> > >>
> > >> Scale demo altered to default to GOOD and locked-together x+y
> scale
> > >>
> > >> Fixed divide-by-zero from all-zero matrix found by stress-test
> > >>
> > >> v11: Whitespace and formatting fixes
> > >>  Moved demo changes to a later patch
> > >>
> > >> v12: Whitespace and formatting fixes
> > >>
> > >> Signed-off-by: Bill Spitzak 
> > >>
> > >
> > > The short answer to this patch is NACK - it doesn't make sense to
> change
> > > the meaning of GOOD and BEST.
> > >
> >
> > Okay that is going to need a big WTF?
>
> No, it's perfectly understandable.
>
> > > There are at least three reasons:
> > >
> > > 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.
> > >
> >
> > I have no idea why having three ways to say BILINEAR is necessary to "do
> > what I say". One seems sufficient.
>
> You must forget any resemblance the strings "GOOD" and "BEST" have
> with English, and see them as abstract names for particular
> mathematical functions.
>
> That is what do-what-I-say means.
>
> Therefore you cannot change what mathematical function they correspond
> to.
>
> There are three ways of saying the same only because of a mistake in
> the past which became evident only much later. It happens. Get over it.
>
> > > 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.
> > >
> >
> > You seem to think the hardware drivers will implement this version of
> > SeparateConvolution? Really???
>
> That is implied by do-what-I-say API policy.
>
> > 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.
>
> That will violate the do-what-I-say policy.
>
> > 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.
> > >
> >
> > Will look into that. I would like the rest of the detail please.
>
> That won't help this patch get through.
>
> > 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 

Re: [Pixman] [PATCH v13 13/14] pixman-image: Implement PIXMAN_FILTER_GOOD/BEST as separable convolutions

[Pekka Paalanen]

On Fri, 4 Mar 2016 19:12:36 -0800
Bill Spitzak  wrote:

> On Fri, Mar 4, 2016 at 4:17 PM, Søren Sandmann 
> wrote:
> 
> > On Wed, Feb 10, 2016 at 1:25 AM,  wrote:
> >  
> >> From: Bill Spitzak 
> >>
> >> Detects and uses PIXMAN_FILTER_NEAREST for all 8 90-degree rotations and
> >> reflections when the scale is 1.0 and integer translation.
> >>
> >> GOOD uses:
> >>
> >>  scale < 1/16 : BOX.BOX at size 16
> >>  scale < 3/4 : BOX.BOX at size 1/scale
> >>  larger : BOX.BOX at size 1
> >>
> >>  If both directions have a scale >= 3/4 or a scale of 1/2 and an integer
> >>  translation, the faster PIXMAN_FILTER_BILINEAR code is used. This is
> >>  compatable at these scales with older versions of pixman where bilinear
> >>  was always used for GOOD.
> >>
> >> BEST uses:
> >>
> >>  scale < 1/24 : BOX.BOX at size 24
> >>  scale < 1/16 : BOX.BOX at size 1/scale
> >>  scale < 1 : IMPULSE.LANCZOS2 at size 1/scale
> >>  scale < 2.333 : IMPULSE.LANCZOS2 at size 1
> >>  scale < 128 : BOX.LANCZOS2 at size 1/(scale-1) (antialiased square
> >> pixels)
> >>  larger : BOX.LANCZOS2 at size 1/127 (antialias blur gets thicker)
> >>
> >> v8: Cutoff in BEST between IMPULSE.LANCZOS2 and BOX.LANCZOS2 adjusted for
> >> a better match between the filters.
> >>
> >> v9: Use the new negative subsample controls to scale the subsamples. These
> >> were chosen by finding the lowest number that did not add visible
> >> artifacts to the zone plate image.
> >>
> >> Scale demo altered to default to GOOD and locked-together x+y scale
> >>
> >> Fixed divide-by-zero from all-zero matrix found by stress-test
> >>
> >> v11: Whitespace and formatting fixes
> >>  Moved demo changes to a later patch
> >>
> >> v12: Whitespace and formatting fixes
> >>
> >> Signed-off-by: Bill Spitzak 
> >>  
> >
> > The short answer to this patch is NACK - it doesn't make sense to change
> > the meaning of GOOD and BEST.
> >  
> 
> Okay that is going to need a big WTF?

No, it's perfectly understandable.

> > There are at least three reasons:
> >
> > 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.
> >  
> 
> I have no idea why having three ways to say BILINEAR is necessary to "do
> what I say". One seems sufficient.

You must forget any resemblance the strings "GOOD" and "BEST" have
with English, and see them as abstract names for particular
mathematical functions.

That is what do-what-I-say means.

Therefore you cannot change what mathematical function they correspond
to.

There are three ways of saying the same only because of a mistake in
the past which became evident only much later. It happens. Get over it.

> > 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.
> >  
> 
> You seem to think the hardware drivers will implement this version of
> SeparateConvolution? Really???

That is implied by do-what-I-say API policy.

> 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.

That will violate the do-what-I-say policy.

> 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.
> >  
> 
> Will look into that. I would like the rest of the detail please.

That won't help this patch get through.

> 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.

Users do not use Pixman. Pixman is an implementation detail of Render
and Cairo. Pixman *is* a low level API, lower than Render or Cairo.


Thanks,
pq


pgpJcCKiryK_N.pgp
Description: OpenPGP digital signature