mesa3d-dev

[Mesa3d-dev] [RFC] Draw module patch and BGRA fix for i915g

[Jakob B. <wal...@gm...>]

Hi Brian, Keith, Zack et al.

So I tried to get i915g running again and it looks like there is a
RGBA vs BGRA bug in the draw module. I have attached two patches that
I would like to have some comments on before commit.

Patch 1 removes a bunch of switch cases that was strew all over the
draw module that all pretty much did exactly the same thing. Which
made it hard to fix the RGBA issue for i915g. In draw_pipe_vbuf and
draw_pt_emit the format for EMIT_4UB was one thing and in
draw_pt_fetch_emit it was another. In light of the format clean up I
standardized on following the same as the float formats for EMIT_4UB.

Patch 2 then introduces EMIT_4UB_BGRA and is used by i915g to make the
colors look correct again.

Comments please.

Cheers Jakob.

Re: [Mesa3d-dev] [RFC] Draw module patch and BGRA fix for i915g

[Brian P. <br...@vm...>]

Jakob Bornecrantz wrote:
> Hi Brian, Keith, Zack et al.
> 
> So I tried to get i915g running again and it looks like there is a
> RGBA vs BGRA bug in the draw module. I have attached two patches that
> I would like to have some comments on before commit.
> 
> Patch 1 removes a bunch of switch cases that was strew all over the
> draw module that all pretty much did exactly the same thing. Which
> made it hard to fix the RGBA issue for i915g. In draw_pipe_vbuf and
> draw_pt_emit the format for EMIT_4UB was one thing and in
> draw_pt_fetch_emit it was another. In light of the format clean up I
> standardized on following the same as the float formats for EMIT_4UB.
> 
> Patch 2 then introduces EMIT_4UB_BGRA and is used by i915g to make the
> colors look correct again.
> 
> Comments please.
> 
> Cheers Jakob.
> 


The new translsation function:

static INLINE unsigned draw_translate_vinfo_format_size(unsigned format)

and the existing:

static INLINE unsigned draw_translate_vinfo_format(unsigned format )

should probably both take a 'enum attrib_emit' instead of unsigned int.

Also, the default switch cases should probably have an assert(0 && 
"unexpected format") in case someone were to add a new format value 
but forget to update the helper functions.

Looks good otherwise.

-Brian

Re: [Mesa3d-dev] [RFC] Draw module patch and BGRA fix for i915g

[Jakob B. <wal...@gm...>]

On Thu, Mar 25, 2010 at 11:52 PM, Brian Paul <br...@vm...> wrote:
> Jakob Bornecrantz wrote:
>>
>> Hi Brian, Keith, Zack et al.
>>
>> So I tried to get i915g running again and it looks like there is a
>> RGBA vs BGRA bug in the draw module. I have attached two patches that
>> I would like to have some comments on before commit.
>>
>> Patch 1 removes a bunch of switch cases that was strew all over the
>> draw module that all pretty much did exactly the same thing. Which
>> made it hard to fix the RGBA issue for i915g. In draw_pipe_vbuf and
>> draw_pt_emit the format for EMIT_4UB was one thing and in
>> draw_pt_fetch_emit it was another. In light of the format clean up I
>> standardized on following the same as the float formats for EMIT_4UB.
>>
>> Patch 2 then introduces EMIT_4UB_BGRA and is used by i915g to make the
>> colors look correct again.
>>
>> Comments please.
>>
>> Cheers Jakob.
>>
>
>
> The new translsation function:
>
> static INLINE unsigned draw_translate_vinfo_format_size(unsigned format)
>
> and the existing:
>
> static INLINE unsigned draw_translate_vinfo_format(unsigned format )
>
> should probably both take a 'enum attrib_emit' instead of unsigned int.

Done.

>
> Also, the default switch cases should probably have an assert(0 &&
> "unexpected format") in case someone were to add a new format value but
> forget to update the helper functions.

Done.

>
> Looks good otherwise.

Thanks for the review, pushed to master.

Cheers Jakob.