Re: ddx/nv50: use drawable.bitsPerPixel to decide format

[Marcin Slusarz <marcin.slusarz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On Wed, Dec 30, 2009 at 06:52:40PM +0100, Christoph Bumiller wrote:
> On 12/30/2009 06:37 PM, Christoph Bumiller wrote:
> > On 12/29/2009 06:06 PM, Marcin Slusarz wrote:
> >> On Mon, Dec 28, 2009 at 06:55:23PM +0100, Maarten Maathuis wrote:
> >>> On Mon, Dec 28, 2009 at 6:37 PM, Marcin Slusarz
> >>> <marcin.slusarz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>>>
> >>>> ---
> >>>>  src/nv50_exa.c |  155 ++++++++++++++++++++++++++++++++++++--------------------
> >>>>  1 files changed, 100 insertions(+), 55 deletions(-)
> >>>>
> >>>> diff --git a/src/nv50_exa.c b/src/nv50_exa.c
> >>>> index 1f5a4ac..54968e8 100644
> >>>> --- a/src/nv50_exa.c
> >>>> +++ b/src/nv50_exa.c
> >>>> @@ -79,15 +79,16 @@ NV50EXA2DSurfaceFormat(PixmapPtr ppix, uint32_t *fmt)
> >>>>
> >>>>        switch (ppix->drawable.depth) {
> >>>>        case 8 : *fmt = NV50_2D_SRC_FORMAT_R8_UNORM; break;
> >>>> -       case 15: *fmt = NV50_2D_SRC_FORMAT_X1R5G5B5_UNORM; break;
> >>>> -       case 16: *fmt = NV50_2D_SRC_FORMAT_R5G6B5_UNORM; break;
> >>>> +       case 15:
> >>>> +       case 16:
> >>>> +               *fmt = NV50_2D_SRC_FORMAT_R5G6B5_UNORM;
> >>>> +               break;
> >>>
> >>> depth 15 is R5G5B5, why this change?
> >>
> >> it's the only value which passes rendercheck tests
> >> (rendercheck -t blend -o Src -f x1r5g5b5,r5g5b5)
> >>
> > This is as far as I can tell because x1r5g5b5 (XRender direct format as
> > queries by rendercheck) has depth set to 16, and therefore uses case 16
> > here.
> *queried
> > If you use the same for depth 15, the rendercheck test check succeeds,
> > but doing this is wrong.
> > If you check bitsPerPixel instead, this will be 15 for both x1r5g5b5
> > and r5g5b5, and all looks fine then.

I agree.

> If we e.g. copy from 15bpp to 16bpp though, can the 16th bit be alpha
> instead of green ? That would be problematic.

xserver advertises a1r5g5b5 as supported too so it's possible,
but rendercheck currently omits it, because it can't handle
(it uses colors which can't be represented in low-bits color formats,
rounds them and the "error" looks huge)
i don't know how to test it...

> > See the attached patch. I'm not sure if I should have changed the xv
> > stuff also.

rendercheck is happy with your patch :)

ps: i'll remove the piece you fixed differently and resend my patch soon