Re: [PATCH] wined3d: fix color in range comparison

[Nikolay Sivov]

On 27.05.2015 20:12, Sergey Isakov wrote:
> I think the patch must be accepted because original codes are more wrong.
>
> Tests will be created when WINED3D_CKEY_COLORSPACE will be implemented.

Tests are needed to prove that your patch is correct.
> The game affected is in bug 38581.
>
> Meanwhile, the policy to reject any patches is not constructive.
> If one developer propose not perfect codes then next developer can
> correct them and obtain a result.
> If you rejected the first developer then you will not have a result.

That's not how it works.
> Best wishes!
>
> On 27 мая 2015 г., at 10:39, Stefan Dösinger <[e-mail]
> > wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> >
> > Am 2015-05-26 um 14:41 schrieb Sergey Isakov:
> > > +#define RED_KEY(x)   (x & 0xff0000)
> > > +#define GREEN_KEY(x) (x & 0x00ff00)
> > > +#define BLUE_KEY(x)  (x & 0x0000ff)
> > > - return ((RED_KEY(color) >=
> > >
> > > RED_KEY(color_key->color_space_low_value)) &&
> > > -
> > >        (RED_KEY(color) <=
> > >
> > > RED_KEY(color_key->color_space_high_value)) &&
> > > -
> > >        (GREEN_KEY(color) >=
> > >
> > > GREEN_KEY(color_key->color_space_low_value)) &&
> > > -
> > >        (GREEN_KEY(color) <=
> > >
> > > GREEN_KEY(color_key->color_space_high_value)) &&
> > > -
> > >        (BLUE_KEY(color) >=
> > >
> > > BLUE_KEY(color_key->color_space_low_value)) &&
> > > -
> > >        (BLUE_KEY(color) <=
> > >
> > > BLUE_KEY(color_key->color_space_high_value)));
> > > +#undef RED_KEY
> > > +#undef GREEN_KEY
> > > +#undef BLUE_KEY
> >
> > The basic idea is most likely right, but it needs tests. Do you have a
> > game that needs this behavior?
> >
> > The way you're extracting individual colors from the key is wrong. The
> > bitmasks depend on the texture format.
> >
> > The code you're modifying is unused on any modern GPU. On GPUs that
> > support ARB_fragment_program or GLSL we do the color key comparison in
> > the fragment shader nowadays.
> >
> > -----BEGIN PGP SIGNATURE-----
> > Version: GnuPG v2
> >
> > iQIcBAEBAgAGBQJVZXSWAAoJEN0/YqbEcdMwbEUP/RM4QUsbdSzhSLyIz5j00Epb
> > AW1ZcNOg61yrfymjHdhJkqfdNmEiKQFsRshD+ZQAxVHqMsZLTxXtRFfBB14WtmNI
> > Ng5jwwiAzDqlmpjftlZ4CmkR1w7DMxH6Nt4e9OQBlDKShSIoGngL6RremV+2Wq+f
> > nNC6HfdmUiAIReVvI2aMm5B9C5L624SKLvoAZNI8QW+HJeZeESSGmT9bx4IRVQwl
> > QX+aKojT43CJwq6UpZRu7ZkniotrmO9aDC+6S3S0od0ZJWSP5Q9upCNWNcN+iPG7
> > 9kCPwZMsEAxNUjvh/wQx8xE1+wiR7Ugjl/X6yTN8HFesFHChrqC2UeHUAkUKU++L
> > WC4kwijfbktKSZnIiSvrTAzWvLOeCZMifFQqm8aqmlOqyvbNwyOJjh6yM/nqZi+c
> > 2aEdEIMY5aaoswDIlyGRRN57zeWtqjZVmg7UzVxmZqfiocmUVxgdmdRv8BvFi0RF
> > R4A6YTDP5EYb33SvH3qQE5kIrtDdtL8qtanmBsuHnhhwd1rdgeeq/9N0tqSjH1GQ
> > s8vsXVmd3qU+mQMzyaxN9Cc9RGxGNgd+g1LkkoPqe+aVUw/JDzvAdMDGpy63zjhM
> > QUlfS771f+Zt43wku/20XP1engXXti4m+KfnSOS3nCdKO1xRKOGHAMlUZzd8+jhU
> > 2rWKuh42D3mSMDk1OhLR
> > =IdO/
> > -----END PGP SIGNATURE-----