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

2010-03-25 Thread Jakob Bornecrantz
On Thu, Mar 25, 2010 at 11:52 PM, Brian Paul  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.

--
Download IntelĀ® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev


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

2010-03-25 Thread Brian Paul
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

--
Download IntelĀ® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev


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

2010-03-25 Thread Jakob Bornecrantz
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.
From 25fd65438993cf0b1e1f0846a9f94f970280cb32 Mon Sep 17 00:00:00 2001
From: Jakob Bornecrantz 
Date: Thu, 25 Mar 2010 00:18:30 +0100
Subject: [PATCH] draw: Use translate function instead of switch cases

---
 src/gallium/auxiliary/draw/draw_pipe_vbuf.c|   37 +++--
 src/gallium/auxiliary/draw/draw_pt_emit.c  |   37 +++--
 src/gallium/auxiliary/draw/draw_pt_fetch_emit.c|   42 +---
 .../auxiliary/draw/draw_pt_fetch_shade_emit.c  |   29 ++
 src/gallium/auxiliary/draw/draw_vertex.c   |   32 ---
 src/gallium/auxiliary/draw/draw_vertex.h   |   18 
 6 files changed, 53 insertions(+), 142 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_pipe_vbuf.c b/src/gallium/auxiliary/draw/draw_pipe_vbuf.c
index 2709957..69e3304 100644
--- a/src/gallium/auxiliary/draw/draw_pipe_vbuf.c
+++ b/src/gallium/auxiliary/draw/draw_pipe_vbuf.c
@@ -238,38 +238,15 @@ vbuf_start_prim( struct vbuf_stage *vbuf, uint prim )
   unsigned output_format;
   unsigned src_offset = (vbuf->vinfo->attrib[i].src_index * 4 * sizeof(float) );
 
-  switch (vbuf->vinfo->attrib[i].emit) {
-  case EMIT_4F:
-	 output_format = PIPE_FORMAT_R32G32B32A32_FLOAT;
-	 emit_sz = 4 * sizeof(float);
-	 break;
-  case EMIT_3F:
-	 output_format = PIPE_FORMAT_R32G32B32_FLOAT;
-	 emit_sz = 3 * sizeof(float);
-	 break;
-  case EMIT_2F:
-	 output_format = PIPE_FORMAT_R32G32_FLOAT;
-	 emit_sz = 2 * sizeof(float);
-	 break;
-  case EMIT_1F:
-	 output_format = PIPE_FORMAT_R32_FLOAT;
-	 emit_sz = 1 * sizeof(float);
-	 break;
-  case EMIT_1F_PSIZE:
-	 output_format = PIPE_FORMAT_R32_FLOAT;
-	 emit_sz = 1 * sizeof(float);
+  output_format = draw_translate_vinfo_format(vbuf->vinfo->attrib[i].emit);
+  emit_sz = draw_translate_vinfo_format_size(vbuf->vinfo->attrib[i].emit);
+
+  /* doesn't handle EMIT_OMIT or unknown */
+  assert(emit_sz != 0);
+
+  if (vbuf->vinfo->attrib[i].emit == EMIT_1F_PSIZE) {
 	 src_buffer = 1;
 	 src_offset = 0;
-	 break;
-  case EMIT_4UB:
-	 output_format = PIPE_FORMAT_A8R8G8B8_UNORM;
-	 emit_sz = 4 * sizeof(ubyte);
- break;
-  default:
-	 assert(0);
-	 output_format = PIPE_FORMAT_NONE;
-	 emit_sz = 0;
-	 break;
   }
 
   hw_key.element[i].type = TRANSLATE_ELEMENT_NORMAL;
diff --git a/src/gallium/auxiliary/draw/draw_pt_emit.c b/src/gallium/auxiliary/draw/draw_pt_emit.c
index ae357b5..56fab1c 100644
--- a/src/gallium/auxiliary/draw/draw_pt_emit.c
+++ b/src/gallium/auxiliary/draw/draw_pt_emit.c
@@ -86,40 +86,15 @@ void draw_pt_emit_prepare( struct pt_emit *emit,
   unsigned output_format;
   unsigned src_offset = (vinfo->attrib[i].src_index * 4 * sizeof(float) );
 
+  output_format = draw_translate_vinfo_format(vinfo->attrib[i].emit);
+  emit_sz = draw_translate_vinfo_format_size(vinfo->attrib[i].emit);
 
- 
-  switch (vinfo->attrib[i].emit) {
-  case EMIT_4F:
-	 output_format = PIPE_FORMAT_R32G32B32A32_FLOAT;
-	 emit_sz = 4 * sizeof(float);
-	 break;
-  case EMIT_3F:
-	 output_format = PIPE_FORMAT_R32G32B32_FLOAT;
-	 emit_sz = 3 * sizeof(float);
-	 break;
-  case EMIT_2F:
-	 output_format = PIPE_FORMAT_R32G32_FLOAT;
-	 emit_sz = 2 * sizeof(float);
-	 break;
-  case EMIT_1F:
-	 output_format = PIPE_FORMAT_R32_FLOAT;
-	 emit_sz = 1 * sizeof(float);
-	 break;
-  case EMIT_1F_PSIZE:
-	 output_format = PIPE_FORMAT_R32_FLOAT;
-	 emit_sz = 1 * sizeof(float);
+  /* doesn't handle EMIT_OMIT or unknown */
+  assert(emit_sz != 0);
+
+  if (vinfo->attrib[i].emit == EMIT_1F_PSIZE) {
 	 src_buffer = 1;
 	 src_offset = 0;
-	 break;
-  case EMIT_4UB:
-	 output_format = PIPE_FORMAT_A8R8G8B8_UNORM;
-	 emit_sz = 4 * sizeof(ubyte);
- break;
-  default:
-	 assert(0);
-	 output_format = PIPE_FORMAT_NONE;
-	 emit_sz = 0;
-	 break;
   }
 
   hw_key.element[i].type = TRANSLATE_ELEMENT_NORMAL;
diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_emit.c b/src/gallium/auxiliary/draw/draw_pt_fetch_emit.c
index 2a60447..4c8086f 100644
--- a/src/gallium/auxiliary/draw/draw_pt_fetch_emit.c
+++ b/src/gallium/auxiliary/draw/draw_pt_fetch_emit.c
@@ -129,41 +129,19 @@ static vo