[Mesa-dev] [PATCH] r600g: fixup AR handling (v4)

[Dave Airlie]

From: Dave Airlie 

So it appears R600s (except rv670) do AR handling different using a different
opcode. This patch fixes up r600g to work properly on r600.

This fixes ~100 piglit tests here (in GLSL1.30 mode) on rv610.

v3: add index_mode as per the docs.

This still fails any dst relative tests for some reason I can't quite see yet,
but it passes a lot more tests than without.

v4: add a nop after dst.rel this could be improved using a second pass,
where we only insert nops if two instructions are sure to collide.

Signed-off-by: Dave Airlie 
---
 src/gallium/drivers/r600/r600_asm.c|   82 ++--
 src/gallium/drivers/r600/r600_asm.h|9 +++-
 src/gallium/drivers/r600/r600_shader.c |2 +-
 src/gallium/drivers/r600/r600_sq.h |7 +++
 4 files changed, 93 insertions(+), 7 deletions(-)

diff --git a/src/gallium/drivers/r600/r600_asm.c 
b/src/gallium/drivers/r600/r600_asm.c
index 8234744..16c1143 100644
--- a/src/gallium/drivers/r600/r600_asm.c
+++ b/src/gallium/drivers/r600/r600_asm.c
@@ -94,6 +94,7 @@ static inline unsigned int 
r600_bytecode_get_num_operands(struct r600_bytecode *
case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOV:
case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA:
case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_FLOOR:
+   case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT:
case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_INT:
case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FRACT:
case V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FLOOR:
@@ -249,8 +250,18 @@ static struct r600_bytecode_tex *r600_bytecode_tex(void)
return tex;
 }
 
-void r600_bytecode_init(struct r600_bytecode *bc, enum chip_class chip_class)
+void r600_bytecode_init(struct r600_bytecode *bc, enum chip_class chip_class, 
enum radeon_family family)
 {
+   if ((chip_class == R600) && (family != CHIP_RV670))
+   bc->ar_handling = AR_HANDLE_RV6XX;
+   else
+   bc->ar_handling = AR_HANDLE_NORMAL;
+
+   if ((chip_class == R600) && (family != CHIP_RV670 && family != 
CHIP_RS780 &&
+  family != CHIP_RS880))
+   bc->r6xx_nop_after_rel_dst = 1;
+   else
+   bc->r6xx_nop_after_rel_dst = 0;
LIST_INITHEAD(&bc->cf);
bc->chip_class = chip_class;
 }
@@ -441,7 +452,8 @@ static int is_alu_mova_inst(struct r600_bytecode *bc, 
struct r600_bytecode_alu *
return !alu->is_op3 && (
alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA ||
alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_FLOOR 
||
-   alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_INT);
+   alu->inst == V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_INT ||
+   alu->inst == 
V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT);
case EVERGREEN:
case CAYMAN:
default:
@@ -457,7 +469,8 @@ static int is_alu_vec_unit_inst(struct r600_bytecode *bc, 
struct r600_bytecode_a
case R600:
case R700:
return is_alu_reduction_inst(bc, alu) ||
-   is_alu_mova_inst(bc, alu);
+   (is_alu_mova_inst(bc, alu) && 
+(alu->inst != 
V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT));
case EVERGREEN:
case CAYMAN:
default:
@@ -478,6 +491,7 @@ static int is_alu_trans_unit_inst(struct r600_bytecode *bc, 
struct r600_bytecode
case R700:
if (!alu->is_op3)
return alu->inst == 
V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_ASHR_INT ||
+   alu->inst == 
V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_MOVA_GPR_INT ||
alu->inst == 
V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_INT_TO_FLT ||
alu->inst == 
V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FLT_TO_UINT ||
alu->inst == 
V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_FLT_TO_INT ||
@@ -1048,6 +1062,10 @@ static int merge_inst_groups(struct r600_bytecode *bc, 
struct r600_bytecode_alu
alu = slots[i];
num_once_inst += is_alu_once_inst(bc, alu);
 
+   /* don't reschedule NOPs */
+   if (alu->inst == BC_INST(bc, 
V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_NOP))
+   return 0;
+
/* Let's check dst gpr. */
if (alu->dst.rel) {
if (have_mova)
@@ -1236,12 +1254,60 @@ static int r600_bytecode_alloc_kcache_lines(struct 
r600_bytecode *bc, struct r60
return 0;
 }
 
+static int insert_nop_r6xx(struct r600_bytecode *bc)
+{
+   struct r600_bytecode_alu alu;
+   int r, i;
+
+   for (i = 0; i < 4; i++) {
+   memset(&alu, 0, sizeof(alu));
+   alu.inst = V_SQ_ALU_WORD1_OP2_SQ_OP2_INST_NOP;
+   alu.src[0].chan = i;
+   alu.dst.chan = i;
+   alu.l

[Mesa-dev] [PATCH] st/mesa: add fallback pipe formats for (compressed) R, RG formats

[Brian Paul]

If we don't find an exact PIPE_FORMAT_x for a GL_(COMPRESSED)_RED/RG format,
try uncompressed formats.  We were already doing this for the RGB(A) formats.

Fixes piglit arb_texture_compression-internal-format-query test.

NOTE: This is a candidate for the stable branches.
---
 configs/default|4 +++-
 configs/linux-dri  |3 ++-
 src/mesa/main/context.c|   13 +
 src/mesa/main/texparam.c   |4 
 src/mesa/state_tracker/st_format.c |   34 ++
 5 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/configs/default b/configs/default
index 2ca6fe4..5e7bc64 100644
--- a/configs/default
+++ b/configs/default
@@ -119,7 +119,9 @@ GBM_BACKEND_DIRS = dri
 # Gallium directories and
 GALLIUM_DIRS = auxiliary drivers state_trackers
 GALLIUM_AUXILIARIES = $(TOP)/src/gallium/auxiliary/libgallium.a
-GALLIUM_DRIVERS_DIRS = softpipe trace rbug noop identity galahad i915 svga 
r300 nvfx nv50
+#GALLIUM_DRIVERS_DIRS = softpipe trace rbug noop identity galahad i915 svga 
r300 nvfx nv50
+GALLIUM_DRIVERS_DIRS = softpipe trace rbug noop identity galahad i915 svga r300
+
 GALLIUM_DRIVERS = $(foreach 
DIR,$(GALLIUM_DRIVERS_DIRS),$(TOP)/src/gallium/drivers/$(DIR)/lib$(DIR).a)
 GALLIUM_WINSYS_DIRS = sw sw/xlib
 GALLIUM_TARGET_DIRS = libgl-xlib
diff --git a/configs/linux-dri b/configs/linux-dri
index dde6408..6db0aca 100644
--- a/configs/linux-dri
+++ b/configs/linux-dri
@@ -62,7 +62,8 @@ GALLIUM_WINSYS_DIRS = sw sw/xlib drm/vmware drm/intel svga/drm
 GALLIUM_TARGET_DIRS = dri-vmwgfx
 GALLIUM_STATE_TRACKERS_DIRS = egl dri
 
-DRI_DIRS = i915 nouveau r200 radeon swrast
+#DRI_DIRS = i915 i965 nouveau r200 radeon swrast
+DRI_DIRS =
 
 INTEL_LIBS = $(shell $(PKG_CONFIG) --libs libdrm_intel)
 INTEL_CFLAGS = $(shell $(PKG_CONFIG) --cflags libdrm_intel)
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index f39cab5..5409062 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -906,6 +906,19 @@ _mesa_initialize_context(struct gl_context *ctx,
struct gl_shared_state *shared;
int i;
 
+   if (1) {
+  FILE * f = fopen("/tmp/mesa.log", "a");
+  fprintf(f, "init %d\n", getpid());
+  fprintf(f, "Sleeping\n");
+  fflush(f);
+  sleep(1);
+  fprintf(f, "Done Sleeping\n");
+  fclose(f);
+   }
+
+#if 0
+#endif
+
/*ASSERT(driverContext);*/
assert(driverFunctions->NewTextureObject);
assert(driverFunctions->FreeTextureImageBuffer);
diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
index 0f92a5b..e179e49 100644
--- a/src/mesa/main/texparam.c
+++ b/src/mesa/main/texparam.c
@@ -314,6 +314,7 @@ set_tex_parameteri(struct gl_context *ctx,
  return GL_FALSE;
   }
   incomplete(ctx, texObj);
+  printf("Set BASE LEVEL %d\n", params[0]);
   texObj->BaseLevel = params[0];
   return GL_TRUE;
 
@@ -326,6 +327,7 @@ set_tex_parameteri(struct gl_context *ctx,
  return GL_FALSE;
   }
   incomplete(ctx, texObj);
+  printf("Set MAX LEVEL %d\n", params[0]);
   texObj->MaxLevel = params[0];
   return GL_TRUE;
 
@@ -504,6 +506,7 @@ set_tex_parameterf(struct gl_context *ctx,
   if (texObj->Sampler.MinLod == params[0])
  return GL_FALSE;
   flush(ctx);
+  printf("Set MIN_LOD %f\n", params[0]);
   texObj->Sampler.MinLod = params[0];
   return GL_TRUE;
 
@@ -511,6 +514,7 @@ set_tex_parameterf(struct gl_context *ctx,
   if (texObj->Sampler.MaxLod == params[0])
  return GL_FALSE;
   flush(ctx);
+  printf("Set MAX_LOD %f\n", params[0]);
   texObj->Sampler.MaxLod = params[0];
   return GL_TRUE;
 
diff --git a/src/mesa/state_tracker/st_format.c 
b/src/mesa/state_tracker/st_format.c
index 5f9ae91..620910d 100644
--- a/src/mesa/state_tracker/st_format.c
+++ b/src/mesa/state_tracker/st_format.c
@@ -839,6 +839,15 @@ struct format_mapping
   PIPE_FORMAT_S8_UINT_Z24_UNORM, \
   0
 
+#define DEFAULT_SNORM8_RGBA_FORMATS \
+  PIPE_FORMAT_R8G8B8A8_SNORM, \
+  0
+
+#define DEFAULT_UNORM16_RGBA_FORMATS \
+  PIPE_FORMAT_R16G16B16A16_UNORM, \
+  DEFAULT_RGBA_FORMATS
+
+
 /**
  * This table maps OpenGL texture format enums to Gallium pipe_format enums.
  * Multiple GL enums might map to multiple pipe_formats.
@@ -1145,54 +1154,55 @@ static const struct format_mapping format_map[] = {
/* R, RG formats */
{
   { GL_RED, GL_R8, 0 },
-  { PIPE_FORMAT_R8_UNORM, 0 }
+  { PIPE_FORMAT_R8_UNORM, PIPE_FORMAT_R8G8_UNORM, DEFAULT_RGBA_FORMATS }
},
{
   { GL_RG, GL_RG8, 0 },
-  { PIPE_FORMAT_R8G8_UNORM, 0 }
+  { PIPE_FORMAT_R8G8_UNORM, DEFAULT_RGBA_FORMATS }
},
{
   { GL_R16, 0 },
-  { PIPE_FORMAT_R16_UNORM, 0 }
+  { PIPE_FORMAT_R16_UNORM, PIPE_FORMAT_R16G16_UNORM,
+DEFAULT_UNORM16_RGBA_FORMATS }
},
{
   { GL_RG16, 0 },
-  { PIPE_FORMAT_R16G16_UNORM, 0 }
+  { PIPE_FORMAT_R16G16_UNORM, DE

Re: [Mesa-dev] [PATCH] st/mesa: add fallback pipe formats for (compressed) R, RG formats

[Marek Olšák]

On Thu, Jan 19, 2012 at 4:54 PM, Brian Paul  wrote:
> If we don't find an exact PIPE_FORMAT_x for a GL_(COMPRESSED)_RED/RG format,
> try uncompressed formats.  We were already doing this for the RGB(A) formats.
>
> Fixes piglit arb_texture_compression-internal-format-query test.
>
> NOTE: This is a candidate for the stable branches.
> ---
>  configs/default                    |    4 +++-
>  configs/linux-dri                  |    3 ++-
>  src/mesa/main/context.c            |   13 +
>  src/mesa/main/texparam.c           |    4 

Hi Brian,

I guess you didn't want to commit changes for the files above, did you?

Anyway, I agree that GL_COMPRESSED_RED and GL_COMPRESSED_RG should be
able to fallback to a non-compressed format, but specifically adding
fallbacks for RGTC1/2, LATC1/2, and 3DC is not any useful. If a driver
doesn't expose the extensions for them, we shouldn't get them at all.
I don't have a strong opinion about this, I just don't think it's a
good idea to add fallbacks where they are not needed.

Marek

>  src/mesa/state_tracker/st_format.c |   34 ++
>  5 files changed, 44 insertions(+), 14 deletions(-)
>
> diff --git a/configs/default b/configs/default
> index 2ca6fe4..5e7bc64 100644
> --- a/configs/default
> +++ b/configs/default
> @@ -119,7 +119,9 @@ GBM_BACKEND_DIRS = dri
>  # Gallium directories and
>  GALLIUM_DIRS = auxiliary drivers state_trackers
>  GALLIUM_AUXILIARIES = $(TOP)/src/gallium/auxiliary/libgallium.a
> -GALLIUM_DRIVERS_DIRS = softpipe trace rbug noop identity galahad i915 svga 
> r300 nvfx nv50
> +#GALLIUM_DRIVERS_DIRS = softpipe trace rbug noop identity galahad i915 svga 
> r300 nvfx nv50
> +GALLIUM_DRIVERS_DIRS = softpipe trace rbug noop identity galahad i915 svga 
> r300
> +
>  GALLIUM_DRIVERS = $(foreach 
> DIR,$(GALLIUM_DRIVERS_DIRS),$(TOP)/src/gallium/drivers/$(DIR)/lib$(DIR).a)
>  GALLIUM_WINSYS_DIRS = sw sw/xlib
>  GALLIUM_TARGET_DIRS = libgl-xlib
> diff --git a/configs/linux-dri b/configs/linux-dri
> index dde6408..6db0aca 100644
> --- a/configs/linux-dri
> +++ b/configs/linux-dri
> @@ -62,7 +62,8 @@ GALLIUM_WINSYS_DIRS = sw sw/xlib drm/vmware drm/intel 
> svga/drm
>  GALLIUM_TARGET_DIRS = dri-vmwgfx
>  GALLIUM_STATE_TRACKERS_DIRS = egl dri
>
> -DRI_DIRS = i915 nouveau r200 radeon swrast
> +#DRI_DIRS = i915 i965 nouveau r200 radeon swrast
> +DRI_DIRS =
>
>  INTEL_LIBS = $(shell $(PKG_CONFIG) --libs libdrm_intel)
>  INTEL_CFLAGS = $(shell $(PKG_CONFIG) --cflags libdrm_intel)
> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
> index f39cab5..5409062 100644
> --- a/src/mesa/main/context.c
> +++ b/src/mesa/main/context.c
> @@ -906,6 +906,19 @@ _mesa_initialize_context(struct gl_context *ctx,
>    struct gl_shared_state *shared;
>    int i;
>
> +   if (1) {
> +      FILE * f = fopen("/tmp/mesa.log", "a");
> +      fprintf(f, "init %d\n", getpid());
> +      fprintf(f, "Sleeping\n");
> +      fflush(f);
> +      sleep(1);
> +      fprintf(f, "Done Sleeping\n");
> +      fclose(f);
> +   }
> +
> +#if 0
> +#endif
> +
>    /*ASSERT(driverContext);*/
>    assert(driverFunctions->NewTextureObject);
>    assert(driverFunctions->FreeTextureImageBuffer);
> diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
> index 0f92a5b..e179e49 100644
> --- a/src/mesa/main/texparam.c
> +++ b/src/mesa/main/texparam.c
> @@ -314,6 +314,7 @@ set_tex_parameteri(struct gl_context *ctx,
>          return GL_FALSE;
>       }
>       incomplete(ctx, texObj);
> +      printf("Set BASE LEVEL %d\n", params[0]);
>       texObj->BaseLevel = params[0];
>       return GL_TRUE;
>
> @@ -326,6 +327,7 @@ set_tex_parameteri(struct gl_context *ctx,
>          return GL_FALSE;
>       }
>       incomplete(ctx, texObj);
> +      printf("Set MAX LEVEL %d\n", params[0]);
>       texObj->MaxLevel = params[0];
>       return GL_TRUE;
>
> @@ -504,6 +506,7 @@ set_tex_parameterf(struct gl_context *ctx,
>       if (texObj->Sampler.MinLod == params[0])
>          return GL_FALSE;
>       flush(ctx);
> +      printf("Set MIN_LOD %f\n", params[0]);
>       texObj->Sampler.MinLod = params[0];
>       return GL_TRUE;
>
> @@ -511,6 +514,7 @@ set_tex_parameterf(struct gl_context *ctx,
>       if (texObj->Sampler.MaxLod == params[0])
>          return GL_FALSE;
>       flush(ctx);
> +      printf("Set MAX_LOD %f\n", params[0]);
>       texObj->Sampler.MaxLod = params[0];
>       return GL_TRUE;
>
> diff --git a/src/mesa/state_tracker/st_format.c 
> b/src/mesa/state_tracker/st_format.c
> index 5f9ae91..620910d 100644
> --- a/src/mesa/state_tracker/st_format.c
> +++ b/src/mesa/state_tracker/st_format.c
> @@ -839,6 +839,15 @@ struct format_mapping
>       PIPE_FORMAT_S8_UINT_Z24_UNORM, \
>       0
>
> +#define DEFAULT_SNORM8_RGBA_FORMATS \
> +      PIPE_FORMAT_R8G8B8A8_SNORM, \
> +      0
> +
> +#define DEFAULT_UNORM16_RGBA_FORMATS \
> +      PIPE_FORMAT_R16G16B16A16_UNORM, \
> +      DEFAULT_RGBA_FORMATS
> +
> +
>  /**
>  * This table maps OpenGL texture format 

Re: [Mesa-dev] dd_function_table::Clear()

[Dee Sharpe]

On 1/18/2012 11:19 AM, Brian Paul wrote:

On 01/18/2012 09:01 AM, Dee Sharpe wrote:
While implementing Clear() for a platform, which buffers in GLContext 
need to be cleared out of the list of:


DrawBuffer
ReadBuffer
WinSysDrawBuffer
WinSysReadBuffer

Or do they all need to be cleared?


Clearing effects "draw" buffers, not "read" buffers.

Unless a user-created FBO is bound, ctx->WinSysDrawBuffer == 
ctx->DrawBuffer.


You need to clear the color buffers attached to ctx->DrawBuffer.  The 
number of buffers to clear is ctx->DrawBuffer->_NumColorDrawBuffers 
and the indexes/pointers to the renderbuffers is in 
_ColorDrawBufferIndexes[] and _ColorDrawBuffers[].


I may actually get rid of the later (redundant) array and just write a 
helper function that returns a pointer to a renderbuffer given a 
buffer index.


Thanks for the quick reply. So, if I want to directly access the 
renderbuffer that I designated as the front left buffer, then I need to 
use _ColorDrawBuffers[BUFFER_FRONT_LEFT], correct?


--

Dee Sharpe

The difference between what IS done
&  what COULD be done is relational to
what you ARE doing&  what you COULD be doing!

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/mesa: add fallback pipe formats for (compressed) R, RG formats

[Brian Paul]

On 01/19/2012 09:55 AM, Marek Olšák wrote:

On Thu, Jan 19, 2012 at 4:54 PM, Brian Paul  wrote:

If we don't find an exact PIPE_FORMAT_x for a GL_(COMPRESSED)_RED/RG format,
try uncompressed formats.  We were already doing this for the RGB(A) formats.

Fixes piglit arb_texture_compression-internal-format-query test.

NOTE: This is a candidate for the stable branches.
---
  configs/default|4 +++-
  configs/linux-dri  |3 ++-
  src/mesa/main/context.c|   13 +
  src/mesa/main/texparam.c   |4 


Hi Brian,

I guess you didn't want to commit changes for the files above, did you?


No, Jose caught that too.



Anyway, I agree that GL_COMPRESSED_RED and GL_COMPRESSED_RG should be
able to fallback to a non-compressed format, but specifically adding
fallbacks for RGTC1/2, LATC1/2, and 3DC is not any useful. If a driver
doesn't expose the extensions for them, we shouldn't get them at all.
I don't have a strong opinion about this, I just don't think it's a
good idea to add fallbacks where they are not needed.


Well, I already pushed the patch.  When I get time I'd like to review 
the whole format mapping table.  I have a feeling there's other 
omissions and extraneous case.


Thanks.

-Brian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] dd_function_table::Clear()

[Brian Paul]

On 01/19/2012 09:58 AM, Dee Sharpe wrote:

On 1/18/2012 11:19 AM, Brian Paul wrote:

On 01/18/2012 09:01 AM, Dee Sharpe wrote:

While implementing Clear() for a platform, which buffers in
GLContext need to be cleared out of the list of:

DrawBuffer
ReadBuffer
WinSysDrawBuffer
WinSysReadBuffer

Or do they all need to be cleared?


Clearing effects "draw" buffers, not "read" buffers.

Unless a user-created FBO is bound, ctx->WinSysDrawBuffer ==
ctx->DrawBuffer.

You need to clear the color buffers attached to ctx->DrawBuffer. The
number of buffers to clear is ctx->DrawBuffer->_NumColorDrawBuffers
and the indexes/pointers to the renderbuffers is in
_ColorDrawBufferIndexes[] and _ColorDrawBuffers[].

I may actually get rid of the later (redundant) array and just write
a helper function that returns a pointer to a renderbuffer given a
buffer index.


Thanks for the quick reply. So, if I want to directly access the
renderbuffer that I designated as the front left buffer, then I need
to use _ColorDrawBuffers[BUFFER_FRONT_LEFT], correct?


No.  The _ColorDrawBuffers[] array is indexed by an integer between 0 
and _NumColorDrawBuffers-1.  The contents of the array is updated by 
calls to glDrawBuffer() or glDrawBuffersARB().


If you specifically want the front-left buffer you'd use:

ctx->DrawBuffer->Attachment[BUFFER_FRONT_LEFT]->Renderbuffer.

-Brian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO

[Ian Romanick]

On 01/18/2012 06:30 PM, Yuanhan Liu wrote:

When rendering to FBO, rendering is inverted. At the same time, we would
also make sure the point sprite origin is inverted. Or, we will get an
inverted result correspoinding to rendering to the default winsys FBO.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613

NOTE: This is a candidate for stable release branches.

v2: add the simliar logic to ivb, too (comments from Ian)
 simplify the logic operation (comments from Brian)

Signed-off-by: Yuanhan Liu
---
  src/mesa/drivers/dri/i965/brw_defines.h   |1 +
  src/mesa/drivers/dri/i965/gen6_sf_state.c |   15 +--
  src/mesa/drivers/dri/i965/gen7_sf_state.c |   20 +---
  3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
b/src/mesa/drivers/dri/i965/brw_defines.h
index 4d90a99..029be87 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -1128,6 +1128,7 @@ enum brw_message_target {
  /* DW1 (for gen6) */
  # define GEN6_SF_NUM_OUTPUTS_SHIFT22
  # define GEN6_SF_SWIZZLE_ENABLE   (1<<  21)
+# define GEN6_SF_POINT_SPRITE_UPPERLEFT(0<<  20)
  # define GEN6_SF_POINT_SPRITE_LOWERLEFT   (1<<  20)
  # define GEN6_SF_URB_ENTRY_READ_LENGTH_SHIFT  11
  # define GEN6_SF_URB_ENTRY_READ_OFFSET_SHIFT  4
diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c 
b/src/mesa/drivers/dri/i965/gen6_sf_state.c
index 548c5a3..67c208b 100644
--- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
@@ -129,6 +129,7 @@ upload_sf_state(struct brw_context *brw)
 float point_size;
 uint16_t attr_overrides[FRAG_ATTRIB_MAX];
 bool userclip_active;
+   uint32_t point_sprite_origin;

 /* _NEW_TRANSFORM */
 userclip_active = (ctx->Transform.ClipPlanesEnabled != 0);
@@ -258,8 +259,18 @@ upload_sf_state(struct brw_context *brw)
 /* Clamp to the hardware limits and convert to fixed point */
 dw4 |= U_FIXED(CLAMP(point_size, 0.125, 255.875), 3);

-   if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT)
-  dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT;
+   /*
+* When rendering to FBO, rendering is inverted. At the same time,
+* we would also make sure the point sprite origin is inverted.
+* Or, we will get an inverted result corresponding to rendering
+* to the default/window FBO.
+*/
+   if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) ^ render_to_fbo) {


I (mostly) like that.  I was trying to think of a way to simplify the 
if-statements in the original patch, but I couldn't think of a good way.


However, using the bit-wise xor is not correct here.  The compiler 
accepts it because everything is an integer in C.  Some tools, like 
Coverty, will probably complain about this.  You really want a ^^ 
(logical xor), but C doesn't have that.  What C does have, that does 
exactly the same thing, is ==.  I suggest changing this to


  if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) == render_to_fbo) {

It looks a bit weird, but it is correct.


+  point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT;
+   } else {
+  point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT;
+   }
+   dw1 |= point_sprite_origin;

 /* _NEW_LIGHT */
 if (ctx->Light.ProvokingVertex != GL_FIRST_VERTEX_CONVENTION) {
diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c 
b/src/mesa/drivers/dri/i965/gen7_sf_state.c
index 7691cb2..75a1cb0 100644
--- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
@@ -48,6 +48,9 @@ upload_sbe_state(struct brw_context *brw)
 int urb_entry_read_offset = 1;
 bool userclip_active = (ctx->Transform.ClipPlanesEnabled != 0);
 uint16_t attr_overrides[FRAG_ATTRIB_MAX];
+   /* _NEW_BUFFERS */
+   bool render_to_fbo = brw->intel.ctx.DrawBuffer->Name != 0;
+   uint32_t point_sprite_origin;

 brw_compute_vue_map(&vue_map, intel, userclip_active, vs_outputs_written);
 urb_entry_read_length = (vue_map.num_slots + 1)/2 - urb_entry_read_offset;
@@ -65,9 +68,20 @@ upload_sbe_state(struct brw_context *brw)
urb_entry_read_length<<  GEN7_SBE_URB_ENTRY_READ_LENGTH_SHIFT |
urb_entry_read_offset<<  GEN7_SBE_URB_ENTRY_READ_OFFSET_SHIFT;

-   /* _NEW_POINT */
-   if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT)
-  dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT;
+   /* _NEW_POINT
+*
+* When rendering to FBO, rendering is inverted. At the same time,
+* we would also make sure the point sprite origin is inverted.
+* Or, we will get an inverted result corresponding to rendering
+* to the default/window FBO.
+*/
+   if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) ^ render_to_fbo) {
+  point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT;
+   } else {
+  point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT;
+   }
+   dw1 |= point_sprite_origin;
+

Re: [Mesa-dev] [PATCH] glsl: Fix isinf() for non-C99-compliant compilers.

[Ian Romanick]

On 01/18/2012 01:09 PM, Paul Berry wrote:

On 18 January 2012 12:43, Jose Fonseca mailto:jfons...@vmware.com>> wrote:

Looks good to me. Thanks

Should there be a case for -INF while we are at it?


I think you can make arguments both for and against.  On the one hand,
C99 requires strtod to recognize "INF", "+INF", "-INF", "INFINITY",
"+INFINITY", "-INFINITY", and capitalization variations of those, in
addition to a bunch of representations of NaN, so for maximum
portability you might argue we should make our s-expression parser
handle all of those even on non-C99 systems.  On the other hand,
s-expression parsing is only used for builtin functions, and at the
moment builtin functions only use "+INF", so any code we write to
support other forms won't actually be exercised.  Personally, I favor


If this support were added before having built-ins that exercise it, we 
should add a unit test to verify the functionality.



just doing "+INF" on the grounds that it is the smallest fix that meets
our present needs, and that adding anything else would require extra
testing effort to validate.  Keeping the fix as small as possible seems
especially prudent considering that this fix is a candidate for the 8.0
branch.


I agree.


Paul

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] st/mesa: copy num_immediates field when copying the immediates array

[Brian Paul]

Two assignments to num_immediates were missing in
get_pixel_transfer_visitor() and get_bitmap_visitor().
The uninitialized value led to valgrind errors and crashes in some
cases.

Added new assertions to catch future problems in this area.  Also
changed num_immediates to unsigned to avoid signed/unsigned
comparison warnings.

NOTE: This is a candiate for the 8.0 branch.
---
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index dc841ff..92dffe2 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -357,7 +357,7 @@ public:
 
/** List of immediate_storage */
exec_list immediates;
-   int num_immediates;
+   unsigned num_immediates;
 
/** List of function_entry */
exec_list function_signatures;
@@ -3645,6 +3645,7 @@ get_pixel_transfer_visitor(struct st_fragment_program *fp,
v->indirect_addr_temps = original->indirect_addr_temps;
v->indirect_addr_consts = original->indirect_addr_consts;
memcpy(&v->immediates, &original->immediates, sizeof(v->immediates));
+   v->num_immediates = original->num_immediates;
 
/*
 * Get initial pixel color from the texture.
@@ -3775,6 +3776,7 @@ get_bitmap_visitor(struct st_fragment_program *fp,
v->indirect_addr_temps = original->indirect_addr_temps;
v->indirect_addr_consts = original->indirect_addr_consts;
memcpy(&v->immediates, &original->immediates, sizeof(v->immediates));
+   v->num_immediates = original->num_immediates;
 
/* TEX tmp0, fragment.texcoord[0], texture[0], 2D; */
coord = st_src_reg(PROGRAM_INPUT, FRAG_ATTRIB_TEX0, glsl_type::vec2_type);
@@ -4679,8 +4681,10 @@ st_translate_program(
i = 0;
foreach_iter(exec_list_iterator, iter, program->immediates) {
   immediate_storage *imm = (immediate_storage *)iter.get();
+  assert(i < program->num_immediates);
   t->immediates[i++] = emit_immediate(t, imm->values, imm->type, 
imm->size);
}
+   assert(i == program->num_immediates);
 
/* texture samplers */
for (i = 0; i < ctx->Const.MaxTextureImageUnits; i++) {
-- 
1.7.3.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO

[Brian Paul]

On 01/19/2012 10:17 AM, Ian Romanick wrote:

On 01/18/2012 06:30 PM, Yuanhan Liu wrote:

When rendering to FBO, rendering is inverted. At the same time, we
would
also make sure the point sprite origin is inverted. Or, we will get an
inverted result correspoinding to rendering to the default winsys FBO.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613

NOTE: This is a candidate for stable release branches.

v2: add the simliar logic to ivb, too (comments from Ian)
simplify the logic operation (comments from Brian)

Signed-off-by: Yuanhan Liu
---
src/mesa/drivers/dri/i965/brw_defines.h | 1 +
src/mesa/drivers/dri/i965/gen6_sf_state.c | 15 +--
src/mesa/drivers/dri/i965/gen7_sf_state.c | 20 +---
3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
b/src/mesa/drivers/dri/i965/brw_defines.h
index 4d90a99..029be87 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -1128,6 +1128,7 @@ enum brw_message_target {
/* DW1 (for gen6) */
# define GEN6_SF_NUM_OUTPUTS_SHIFT 22
# define GEN6_SF_SWIZZLE_ENABLE (1<< 21)
+# define GEN6_SF_POINT_SPRITE_UPPERLEFT (0<< 20)
# define GEN6_SF_POINT_SPRITE_LOWERLEFT (1<< 20)
# define GEN6_SF_URB_ENTRY_READ_LENGTH_SHIFT 11
# define GEN6_SF_URB_ENTRY_READ_OFFSET_SHIFT 4
diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c
b/src/mesa/drivers/dri/i965/gen6_sf_state.c
index 548c5a3..67c208b 100644
--- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
@@ -129,6 +129,7 @@ upload_sf_state(struct brw_context *brw)
float point_size;
uint16_t attr_overrides[FRAG_ATTRIB_MAX];
bool userclip_active;
+ uint32_t point_sprite_origin;

/* _NEW_TRANSFORM */
userclip_active = (ctx->Transform.ClipPlanesEnabled != 0);
@@ -258,8 +259,18 @@ upload_sf_state(struct brw_context *brw)
/* Clamp to the hardware limits and convert to fixed point */
dw4 |= U_FIXED(CLAMP(point_size, 0.125, 255.875), 3);

- if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT)
- dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT;
+ /*
+ * When rendering to FBO, rendering is inverted. At the same time,
+ * we would also make sure the point sprite origin is inverted.
+ * Or, we will get an inverted result corresponding to rendering
+ * to the default/window FBO.
+ */
+ if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) ^ render_to_fbo) {


I (mostly) like that. I was trying to think of a way to simplify the
if-statements in the original patch, but I couldn't think of a good way.

However, using the bit-wise xor is not correct here. The compiler
accepts it because everything is an integer in C. Some tools, like
Coverty, will probably complain about this. You really want a ^^
(logical xor), but C doesn't have that. What C does have, that does
exactly the same thing, is ==. I suggest changing this to

if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) == render_to_fbo) {

It looks a bit weird, but it is correct.


I suggested ^ to Yuanhan.  Yes you have to be careful with it.  Note 
that "X ^ render_to_fbo" is already used earlier in the function.


But I think you want != in this case, not ==.

-Brian

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/mesa: copy num_immediates field when copying the immediates array

[Jose Fonseca]

I'm not familiar with this code, but looks sensisble to me.

Jose

- Original Message -
> Two assignments to num_immediates were missing in
> get_pixel_transfer_visitor() and get_bitmap_visitor().
> The uninitialized value led to valgrind errors and crashes in some
> cases.
> 
> Added new assertions to catch future problems in this area.  Also
> changed num_immediates to unsigned to avoid signed/unsigned
> comparison warnings.
> 
> NOTE: This is a candiate for the 8.0 branch.
> ---
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp |6 +-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index dc841ff..92dffe2 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -357,7 +357,7 @@ public:
>  
> /** List of immediate_storage */
> exec_list immediates;
> -   int num_immediates;
> +   unsigned num_immediates;
>  
> /** List of function_entry */
> exec_list function_signatures;
> @@ -3645,6 +3645,7 @@ get_pixel_transfer_visitor(struct
> st_fragment_program *fp,
> v->indirect_addr_temps = original->indirect_addr_temps;
> v->indirect_addr_consts = original->indirect_addr_consts;
> memcpy(&v->immediates, &original->immediates,
> sizeof(v->immediates));
> +   v->num_immediates = original->num_immediates;
>  
> /*
>  * Get initial pixel color from the texture.
> @@ -3775,6 +3776,7 @@ get_bitmap_visitor(struct st_fragment_program
> *fp,
> v->indirect_addr_temps = original->indirect_addr_temps;
> v->indirect_addr_consts = original->indirect_addr_consts;
> memcpy(&v->immediates, &original->immediates,
> sizeof(v->immediates));
> +   v->num_immediates = original->num_immediates;
>  
> /* TEX tmp0, fragment.texcoord[0], texture[0], 2D; */
> coord = st_src_reg(PROGRAM_INPUT, FRAG_ATTRIB_TEX0,
> glsl_type::vec2_type);
> @@ -4679,8 +4681,10 @@ st_translate_program(
> i = 0;
> foreach_iter(exec_list_iterator, iter, program->immediates) {
>immediate_storage *imm = (immediate_storage *)iter.get();
> +  assert(i < program->num_immediates);
>t->immediates[i++] = emit_immediate(t, imm->values, imm->type,
>imm->size);
> }
> +   assert(i == program->num_immediates);
>  
> /* texture samplers */
> for (i = 0; i < ctx->Const.MaxTextureImageUnits; i++) {
> --
> 1.7.3.4
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/4] glapi: remove non-existent parameter references

[Ian Romanick]

On 01/18/2012 09:54 AM, nobled wrote:

On Tue, Jan 17, 2012 at 6:10 PM, Brian Paul  wrote:

On 01/16/2012 04:45 PM, nobled wrote:


glGetTexImage, for example, has no width/height/depth parameters.

Also, copy some missing parameter info from the original versions
of certain functions over to their ARB_robustness counterparts.



These look OK to me.  There's no real change in the generated .[ch] files,
right?

I can't actually tell; whenever I try to run `make` in
src/mapi/glapi/gen/, it errors out with "Set XORG_BASE env var"
instead.


This is because the scripts in Mesa also generate GLX protocol handling 
code for the xserver.  Either set the environment variable to point at 
an xserver source tree, or run make with -B (ignore errors).

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/7] i965: use AM_CPPFLAGS instead of AM_CFLAGS/CXXFLAGS

[Eric Anholt]

On Wed, 18 Jan 2012 13:38:38 -0500, Gaetan Nadon  wrote:
> On 12-01-18 01:11 PM, Eric Anholt wrote:
> > On Tue, 17 Jan 2012 15:58:11 -0500, Matt Turner  wrote:
> >
> > As far as I can tell, the point of AM_CPPFLAGS existing is if you want
> > to make a target that separately calls the preprocessor, so you a list
> > of includes and a list of non-preprocessor compiler flags.  Given that
> > we don't have any preprocessor invocations, we're sure to leak actual
> > cflags into the cppflags, so I don't see why we'd make this change..
> That's the only scenario I am aware of that will actually "break". You
> never know when this target might be added in the makefile. It's
> preferable to keep the flags separate where possible. Automake keeps
> them separate from top to bottom. A builder can append flags at make
> invocation using CFLAGS and CPPFLAGS, knowing they will used with the
> correct target.
> 
> Unfortunately too many developers don't bother and when mixed flags are
> passed in through a variable from another module, they cannot be
> separated again.
> 
> In another post, I expressed my reservations due to INTEL_CFLAGS that
> may contain mixed flags. Using AM_CFLAGS would be safer, unfortunately.

We have no preprocessor-using those targets, so the person adding one
would have to deal with it.  If you try to split today with no target
present, they'll still have to deal with it when they add the first
target, because it will be broken.

Part of the reason I'm so confident that it will be broken when the
first target is added (if ever -- I don't expect one, making this all a
waste of time) is that llvm puts compiler flags in their config, and I
expect to see llvm used in more pieces of Mesa over time.


pgp4fOKb51Qei.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO

[Eric Anholt]

On Thu, 19 Jan 2012 10:30:53 +0800, Yuanhan Liu  
wrote:
> When rendering to FBO, rendering is inverted. At the same time, we would
> also make sure the point sprite origin is inverted. Or, we will get an
> inverted result correspoinding to rendering to the default winsys FBO.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613
> 
> NOTE: This is a candidate for stable release branches.
> 
> v2: add the simliar logic to ivb, too (comments from Ian)
> simplify the logic operation (comments from Brian)
> 
> Signed-off-by: Yuanhan Liu 
> ---
>  src/mesa/drivers/dri/i965/brw_defines.h   |1 +
>  src/mesa/drivers/dri/i965/gen6_sf_state.c |   15 +--
>  src/mesa/drivers/dri/i965/gen7_sf_state.c |   20 +---
>  3 files changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
> b/src/mesa/drivers/dri/i965/brw_defines.h
> index 4d90a99..029be87 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -1128,6 +1128,7 @@ enum brw_message_target {
>  /* DW1 (for gen6) */
>  # define GEN6_SF_NUM_OUTPUTS_SHIFT   22
>  # define GEN6_SF_SWIZZLE_ENABLE  (1 << 21)
> +# define GEN6_SF_POINT_SPRITE_UPPERLEFT  (0 << 20)
>  # define GEN6_SF_POINT_SPRITE_LOWERLEFT  (1 << 20)
>  # define GEN6_SF_URB_ENTRY_READ_LENGTH_SHIFT 11
>  # define GEN6_SF_URB_ENTRY_READ_OFFSET_SHIFT 4
> diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c 
> b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> index 548c5a3..67c208b 100644
> --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> @@ -129,6 +129,7 @@ upload_sf_state(struct brw_context *brw)
> float point_size;
> uint16_t attr_overrides[FRAG_ATTRIB_MAX];
> bool userclip_active;
> +   uint32_t point_sprite_origin;
>  
> /* _NEW_TRANSFORM */
> userclip_active = (ctx->Transform.ClipPlanesEnabled != 0);
> @@ -258,8 +259,18 @@ upload_sf_state(struct brw_context *brw)
> /* Clamp to the hardware limits and convert to fixed point */
> dw4 |= U_FIXED(CLAMP(point_size, 0.125, 255.875), 3);
>  
> -   if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT)
> -  dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT;
> +   /*
> +* When rendering to FBO, rendering is inverted. At the same time,
> +* we would also make sure the point sprite origin is inverted.
> +* Or, we will get an inverted result corresponding to rendering
> +* to the default/window FBO.
> +*/

I think this comment could be simplified to "Window coordinates in an
FBO are inverted, which means point sprite origin must be inverted."

> +   if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) ^ render_to_fbo) {
> +  point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT;
> +   } else {
> +  point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT;
> +   }

Much better.  That logic was hideous before.


pgp0PVx1V3VSe.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO

[Ian Romanick]

On 01/19/2012 09:32 AM, Brian Paul wrote:

On 01/19/2012 10:17 AM, Ian Romanick wrote:

On 01/18/2012 06:30 PM, Yuanhan Liu wrote:

When rendering to FBO, rendering is inverted. At the same time, we
would
also make sure the point sprite origin is inverted. Or, we will get an
inverted result correspoinding to rendering to the default winsys FBO.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613

NOTE: This is a candidate for stable release branches.

v2: add the simliar logic to ivb, too (comments from Ian)
simplify the logic operation (comments from Brian)

Signed-off-by: Yuanhan Liu
---
src/mesa/drivers/dri/i965/brw_defines.h | 1 +
src/mesa/drivers/dri/i965/gen6_sf_state.c | 15 +--
src/mesa/drivers/dri/i965/gen7_sf_state.c | 20 +---
3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
b/src/mesa/drivers/dri/i965/brw_defines.h
index 4d90a99..029be87 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -1128,6 +1128,7 @@ enum brw_message_target {
/* DW1 (for gen6) */
# define GEN6_SF_NUM_OUTPUTS_SHIFT 22
# define GEN6_SF_SWIZZLE_ENABLE (1<< 21)
+# define GEN6_SF_POINT_SPRITE_UPPERLEFT (0<< 20)
# define GEN6_SF_POINT_SPRITE_LOWERLEFT (1<< 20)
# define GEN6_SF_URB_ENTRY_READ_LENGTH_SHIFT 11
# define GEN6_SF_URB_ENTRY_READ_OFFSET_SHIFT 4
diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c
b/src/mesa/drivers/dri/i965/gen6_sf_state.c
index 548c5a3..67c208b 100644
--- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
@@ -129,6 +129,7 @@ upload_sf_state(struct brw_context *brw)
float point_size;
uint16_t attr_overrides[FRAG_ATTRIB_MAX];
bool userclip_active;
+ uint32_t point_sprite_origin;

/* _NEW_TRANSFORM */
userclip_active = (ctx->Transform.ClipPlanesEnabled != 0);
@@ -258,8 +259,18 @@ upload_sf_state(struct brw_context *brw)
/* Clamp to the hardware limits and convert to fixed point */
dw4 |= U_FIXED(CLAMP(point_size, 0.125, 255.875), 3);

- if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT)
- dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT;
+ /*
+ * When rendering to FBO, rendering is inverted. At the same time,
+ * we would also make sure the point sprite origin is inverted.
+ * Or, we will get an inverted result corresponding to rendering
+ * to the default/window FBO.
+ */
+ if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) ^ render_to_fbo) {


I (mostly) like that. I was trying to think of a way to simplify the
if-statements in the original patch, but I couldn't think of a good way.

However, using the bit-wise xor is not correct here. The compiler
accepts it because everything is an integer in C. Some tools, like
Coverty, will probably complain about this. You really want a ^^
(logical xor), but C doesn't have that. What C does have, that does
exactly the same thing, is ==. I suggest changing this to

if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) == render_to_fbo) {

It looks a bit weird, but it is correct.


I suggested ^ to Yuanhan. Yes you have to be careful with it. Note that
"X ^ render_to_fbo" is already used earlier in the function.


I hadn't noticed that it was used earlier.  I'll submit a patch to fix 
those. :)



But I think you want != in this case, not ==.


Of course.  *blush*


-Brian

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] svga: move svga_texture() casts/calls in svga_surface_copy()

[Brian Paul]

To fix failed assertions when calling glCopyBufferSubData().

svga_texture() asserts that the resource is a texture.  Simply move the
calls to svga_texture() after the code that handles non-texture copies
so that we don't call it with non-texture resources.

Fixes glean bufferObject failure.

NOTE: This is a candidate for the 8.0 branch.
---
 src/gallium/drivers/svga/svga_pipe_blit.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/svga/svga_pipe_blit.c 
b/src/gallium/drivers/svga/svga_pipe_blit.c
index c4f122f..8e114d3 100644
--- a/src/gallium/drivers/svga/svga_pipe_blit.c
+++ b/src/gallium/drivers/svga/svga_pipe_blit.c
@@ -44,8 +44,7 @@ static void svga_surface_copy(struct pipe_context *pipe,
   const struct pipe_box *src_box)
  {
struct svga_context *svga = svga_context(pipe);
-   struct svga_texture *stex = svga_texture(src_tex);
-   struct svga_texture *dtex = svga_texture(dst_tex);
+   struct svga_texture *stex, *dtex;
 /*   struct pipe_screen *screen = pipe->screen;
SVGA3dCopyBox *box;
enum pipe_error ret;
@@ -63,6 +62,9 @@ static void svga_surface_copy(struct pipe_context *pipe,
   return;
}
 
+   stex = svga_texture(src_tex);
+   dtex = svga_texture(dst_tex);
+
 #if 0
srcsurf = screen->get_tex_surface(screen, src_tex,
  src_level, src_box->z, src_box->z,
-- 
1.7.3.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/4] glapi: add GL_ARB_texture_compression_rgtc

[Eric Anholt]

On Mon, 16 Jan 2012 18:50:44 -0500, nobled  wrote:
> Noticed this was missing when writing the "glapi: sort ARB extensions
> by number" commit, which at least shows it was effective.
> ---
>  .../glapi/gen/ARB_texture_compression_rgtc.xml |   15 +++
>  src/mapi/glapi/gen/Makefile|1 +
>  src/mapi/glapi/gen/gl_API.xml  |2 +-
>  3 files changed, 17 insertions(+), 1 deletions(-)
>  create mode 100644 src/mapi/glapi/gen/ARB_texture_compression_rgtc.xml
> 
> diff --git a/src/mapi/glapi/gen/ARB_texture_compression_rgtc.xml
> b/src/mapi/glapi/gen/ARB_texture_compression_rgtc.xml
> new file mode 100644
> index 000..f0ecabd
> --- /dev/null
> +++ b/src/mapi/glapi/gen/ARB_texture_compression_rgtc.xml
> @@ -0,0 +1,15 @@
> +
> +
> +
> +
> +
> +

missing "compression"?

> +
> +
> +
> +
> +
> +
> +
> +
> +

I don't understand how the XML is used well enough to review this
series, really.


pgpsu4GpgzkVs.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa: Loosen glBlitFramebuffer restrictions on depthstencil buffers (v2)

[Ian Romanick]

On 01/18/2012 12:31 PM, Chad Versace wrote:

--- snip

Supporting Z32 ->  Z32_FLOAT is a bad idea. So I've amended the patch
to avoid that.

--- end snip


This loosens the format validation in glBlitFramebuffer. When blitting
depth bits, don't require an exact match between the depth formats; only
require that the two formats have the same number of depth bits and the
same depth datatype (float vs uint). Ditto for stencil.

Between S8_Z24 buffers, the EXT_framebuffer_blit spec allows
glBlitFramebuffer to blit the depth and stencil bits separately. So I see
no reason to prevent blitting the depth bits between X8_Z24 and S8_Z24 or
the stencil bits between S8 and S8_Z24. However, we of course don't want
to allow blitting from Z32 to Z32_FLOAT.



Fixes Piglit fbo/fbo-blit-d24s8 on Intel drivers with separate stencil
enabled.

The problem was that, on Intel drivers with separate stencil, the default
framebuffer has separate depth and stencil buffers with formats X8_Z24 and
S8. The test attempts to blit the depth bits from a S8_Z24 buffer into the
default framebuffer.

v2: Check that depth datatypes match in order prevent Z32 ->  Z32_FLOAT.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44665
Note: This is a candidate for the 8.0 branch.
CC: Brian Paul
CC: Kenneth Graunke
CC: Iam Romanick


Reviewed-by: Ian Romanick 


Reported-by: Xunx Fang
Signed-off-by: Chad Versace
---
  src/mesa/main/fbobject.c |   13 ++---
  1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 0524959..2b3ac2e 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -2709,9 +2709,13 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint srcY0, GLint 
srcX1, GLint srcY1,
if ((readRb == NULL) || (drawRb == NULL)) {
 mask&= ~GL_STENCIL_BUFFER_BIT;
}
-  else if (readRb->Format != drawRb->Format) {
+  else if (_mesa_get_format_bits(readRb->Format, GL_STENCIL_BITS) !=
+  _mesa_get_format_bits(drawRb->Format, GL_STENCIL_BITS)) {
+/* There is no need to check the stencil datatype here, because
+ * there is only one: GL_UNSIGNED_INT.
+ */
   _mesa_error(ctx, GL_INVALID_OPERATION,
- "glBlitFramebufferEXT(stencil buffer format mismatch)");
+ "glBlitFramebufferEXT(stencil buffer size mismatch)");
   return;
}
 }
@@ -2731,7 +2735,10 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint srcY0, GLint 
srcX1, GLint srcY1,
if ((readRb == NULL) || (drawRb == NULL)) {
 mask&= ~GL_DEPTH_BUFFER_BIT;
}
-  else if (readRb->Format != drawRb->Format) {
+  else if ((_mesa_get_format_bits(readRb->Format, GL_DEPTH_BITS) !=
+   _mesa_get_format_bits(drawRb->Format, GL_DEPTH_BITS)) ||
+  (_mesa_get_format_datatype(readRb->Format) !=
+   _mesa_get_format_datatype(drawRb->Format))) {
   _mesa_error(ctx, GL_INVALID_OPERATION,
   "glBlitFramebufferEXT(depth buffer format mismatch)");
   return;


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] dd_function_table::Clear()

[Dee Sharpe]

On 1/19/2012 11:07 AM, Brian Paul wrote:
No.  The _ColorDrawBuffers[] array is indexed by an integer between 0 
and _NumColorDrawBuffers-1.  The contents of the array is updated by 
calls to glDrawBuffer() or glDrawBuffersARB().


If you specifically want the front-left buffer you'd use:

ctx->DrawBuffer->Attachment[BUFFER_FRONT_LEFT]->Renderbuffer.


As it turns out, that's exactly what I was looking for, thanks!

--

Dee Sharpe

The difference between what IS done
&  what COULD be done is relational to
what you ARE doing&  what you COULD be doing!

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa: Loosen glBlitFramebuffer restrictions on depthstencil buffers (v2)

[Christoph Bumiller]

On 19.01.2012 19:53, Ian Romanick wrote:
> On 01/18/2012 12:31 PM, Chad Versace wrote:
>> --- snip
>>
>> Supporting Z32 ->  Z32_FLOAT is a bad idea. So I've amended the patch
>> to avoid that.
>>
>> --- end snip
>>
>>
>> This loosens the format validation in glBlitFramebuffer. When blitting
>> depth bits, don't require an exact match between the depth formats; only
>> require that the two formats have the same number of depth bits and the
>> same depth datatype (float vs uint). Ditto for stencil.
>>
>> Between S8_Z24 buffers, the EXT_framebuffer_blit spec allows
>> glBlitFramebuffer to blit the depth and stencil bits separately. So I
>> see
>> no reason to prevent blitting the depth bits between X8_Z24 and
>> S8_Z24 or
>> the stencil bits between S8 and S8_Z24. However, we of course don't want
>> to allow blitting from Z32 to Z32_FLOAT.
>>

So is it also allowed to blit from S8Z24 to Z24S8 ? Could we also allow
to blit from RGBA8 to BGRA8 then, please ?

That would fix my multisampling fail - all applications that use
framebuffer_multisample try to blit resolve directly from an MS
renderbuffer to the window system back buffer and that means I have to
know somehow which ordering the window system chooses, which is rather
inconvenient and made me hack-disable RGBA8 in the gallium driver, i.e.
lie about what formats it supports.

If I recall correctly, the NV blob does allow resolving between BGRA and
RGBA.

>> 
>>
>> Fixes Piglit fbo/fbo-blit-d24s8 on Intel drivers with separate stencil
>> enabled.
>>
>> The problem was that, on Intel drivers with separate stencil, the
>> default
>> framebuffer has separate depth and stencil buffers with formats
>> X8_Z24 and
>> S8. The test attempts to blit the depth bits from a S8_Z24 buffer
>> into the
>> default framebuffer.
>>
>> v2: Check that depth datatypes match in order prevent Z32 ->  Z32_FLOAT.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44665
>> Note: This is a candidate for the 8.0 branch.
>> CC: Brian Paul
>> CC: Kenneth Graunke
>> CC: Iam Romanick
>
> Reviewed-by: Ian Romanick 
>
>> Reported-by: Xunx Fang
>> Signed-off-by: Chad Versace
>> ---
>>   src/mesa/main/fbobject.c |   13 ++---
>>   1 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>> index 0524959..2b3ac2e 100644
>> --- a/src/mesa/main/fbobject.c
>> +++ b/src/mesa/main/fbobject.c
>> @@ -2709,9 +2709,13 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint
>> srcY0, GLint srcX1, GLint srcY1,
>> if ((readRb == NULL) || (drawRb == NULL)) {
>>mask&= ~GL_STENCIL_BUFFER_BIT;
>> }
>> -  else if (readRb->Format != drawRb->Format) {
>> +  else if (_mesa_get_format_bits(readRb->Format,
>> GL_STENCIL_BITS) !=
>> +   _mesa_get_format_bits(drawRb->Format, GL_STENCIL_BITS)) {
>> + /* There is no need to check the stencil datatype here, because
>> +  * there is only one: GL_UNSIGNED_INT.
>> +  */
>>_mesa_error(ctx, GL_INVALID_OPERATION,
>> - "glBlitFramebufferEXT(stencil buffer format
>> mismatch)");
>> + "glBlitFramebufferEXT(stencil buffer size
>> mismatch)");
>>return;
>> }
>>  }
>> @@ -2731,7 +2735,10 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint
>> srcY0, GLint srcX1, GLint srcY1,
>> if ((readRb == NULL) || (drawRb == NULL)) {
>>mask&= ~GL_DEPTH_BUFFER_BIT;
>> }
>> -  else if (readRb->Format != drawRb->Format) {
>> +  else if ((_mesa_get_format_bits(readRb->Format, GL_DEPTH_BITS) !=
>> +_mesa_get_format_bits(drawRb->Format, GL_DEPTH_BITS)) ||
>> +   (_mesa_get_format_datatype(readRb->Format) !=
>> +_mesa_get_format_datatype(drawRb->Format))) {
>>_mesa_error(ctx, GL_INVALID_OPERATION,
>>"glBlitFramebufferEXT(depth buffer format
>> mismatch)");
>>return;
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] svga: move svga_texture() casts/calls in svga_surface_copy()

[Jose Fonseca]

Looks good Brian.

Jose

- Original Message -
> To fix failed assertions when calling glCopyBufferSubData().
> 
> svga_texture() asserts that the resource is a texture.  Simply move
> the
> calls to svga_texture() after the code that handles non-texture
> copies
> so that we don't call it with non-texture resources.
> 
> Fixes glean bufferObject failure.
> 
> NOTE: This is a candidate for the 8.0 branch.
> ---
>  src/gallium/drivers/svga/svga_pipe_blit.c |6 --
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/drivers/svga/svga_pipe_blit.c
> b/src/gallium/drivers/svga/svga_pipe_blit.c
> index c4f122f..8e114d3 100644
> --- a/src/gallium/drivers/svga/svga_pipe_blit.c
> +++ b/src/gallium/drivers/svga/svga_pipe_blit.c
> @@ -44,8 +44,7 @@ static void svga_surface_copy(struct pipe_context
> *pipe,
>const struct pipe_box *src_box)
>   {
> struct svga_context *svga = svga_context(pipe);
> -   struct svga_texture *stex = svga_texture(src_tex);
> -   struct svga_texture *dtex = svga_texture(dst_tex);
> +   struct svga_texture *stex, *dtex;
>  /*   struct pipe_screen *screen = pipe->screen;
> SVGA3dCopyBox *box;
> enum pipe_error ret;
> @@ -63,6 +62,9 @@ static void svga_surface_copy(struct pipe_context
> *pipe,
>return;
> }
>  
> +   stex = svga_texture(src_tex);
> +   dtex = svga_texture(dst_tex);
> +
>  #if 0
> srcsurf = screen->get_tex_surface(screen, src_tex,
>   src_level, src_box->z,
>   src_box->z,
> --
> 1.7.3.4
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] matypes.h

[Dee Sharpe]

Hello all,

I'm having a problem with trying to create matypes.h. Is there a global 
enabling variable that needs to be set to ensure that this header is 
generated when porting to another OS on the X86 platform?


--

Dee Sharpe

The difference between what IS done
&  what COULD be done is relational to
what you ARE doing&  what you COULD be doing!

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] intel/gen6: Some framebuffers having separate depthstencil should be unsupported

[Chad Versace]

When the framebuffer has separate depth and stencil buffers, and HiZ is
not enabled on the depth buffer, mark the framebuffer as unsupported. This
happens when trying to create a framebuffer with Z16/S8 because we haven't
enabled HiZ on Z16 yet.

Fixes gles2conform test stencil8.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44948
Reported-by: Ian Romanick 
Note: This is a candiate for the 8.0 branch.
Signed-off-by: Chad Versace 
---
 src/mesa/drivers/dri/intel/intel_fbo.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c 
b/src/mesa/drivers/dri/intel/intel_fbo.c
index c37075c..45a6827 100644
--- a/src/mesa/drivers/dri/intel/intel_fbo.c
+++ b/src/mesa/drivers/dri/intel/intel_fbo.c
@@ -804,6 +804,15 @@ intel_validate_framebuffer(struct gl_context *ctx, struct 
gl_framebuffer *fb)
fb->_Status = GL_FRAMEBUFFER_UNSUPPORTED_EXT;
 if (stencil_mt->format != MESA_FORMAT_S8)
fb->_Status = GL_FRAMEBUFFER_UNSUPPORTED_EXT;
+if (intel->gen < 7 && depth_mt->hiz_mt == NULL) {
+   /* Before Gen7, separate depth and stencil buffers can be used
+* only if HiZ is enabled. From the Sandybridge PRM, Volume 2,
+* Part 1, Bit 3DSTATE_DEPTH_BUFFER.SeparateStencilBufferEnable:
+* [DevSNB]: This field must be set to the same value (enabled
+* or disabled) as Hierarchical Depth Buffer Enable.
+*/
+   fb->_Status = GL_FRAMEBUFFER_UNSUPPORTED;
+}
   }
}
 
-- 
1.7.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO

[Yuanhan Liu]

On Thu, Jan 19, 2012 at 09:51:32AM -0800, Eric Anholt wrote:
> On Thu, 19 Jan 2012 10:30:53 +0800, Yuanhan Liu  
> wrote:
> > When rendering to FBO, rendering is inverted. At the same time, we would
> > also make sure the point sprite origin is inverted. Or, we will get an
> > inverted result correspoinding to rendering to the default winsys FBO.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613
> > 
> > NOTE: This is a candidate for stable release branches.
> > 
> > v2: add the simliar logic to ivb, too (comments from Ian)
> > simplify the logic operation (comments from Brian)
> > 
> > Signed-off-by: Yuanhan Liu 
> > ---
> >  src/mesa/drivers/dri/i965/brw_defines.h   |1 +
> >  src/mesa/drivers/dri/i965/gen6_sf_state.c |   15 +--
> >  src/mesa/drivers/dri/i965/gen7_sf_state.c |   20 +---
> >  3 files changed, 31 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
> > b/src/mesa/drivers/dri/i965/brw_defines.h
> > index 4d90a99..029be87 100644
> > --- a/src/mesa/drivers/dri/i965/brw_defines.h
> > +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> > @@ -1128,6 +1128,7 @@ enum brw_message_target {
> >  /* DW1 (for gen6) */
> >  # define GEN6_SF_NUM_OUTPUTS_SHIFT 22
> >  # define GEN6_SF_SWIZZLE_ENABLE(1 << 21)
> > +# define GEN6_SF_POINT_SPRITE_UPPERLEFT(0 << 20)
> >  # define GEN6_SF_POINT_SPRITE_LOWERLEFT(1 << 20)
> >  # define GEN6_SF_URB_ENTRY_READ_LENGTH_SHIFT   11
> >  # define GEN6_SF_URB_ENTRY_READ_OFFSET_SHIFT   4
> > diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c 
> > b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> > index 548c5a3..67c208b 100644
> > --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> > @@ -129,6 +129,7 @@ upload_sf_state(struct brw_context *brw)
> > float point_size;
> > uint16_t attr_overrides[FRAG_ATTRIB_MAX];
> > bool userclip_active;
> > +   uint32_t point_sprite_origin;
> >  
> > /* _NEW_TRANSFORM */
> > userclip_active = (ctx->Transform.ClipPlanesEnabled != 0);
> > @@ -258,8 +259,18 @@ upload_sf_state(struct brw_context *brw)
> > /* Clamp to the hardware limits and convert to fixed point */
> > dw4 |= U_FIXED(CLAMP(point_size, 0.125, 255.875), 3);
> >  
> > -   if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT)
> > -  dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT;
> > +   /*
> > +* When rendering to FBO, rendering is inverted. At the same time,
> > +* we would also make sure the point sprite origin is inverted.
> > +* Or, we will get an inverted result corresponding to rendering
> > +* to the default/window FBO.
> > +*/
> 
> I think this comment could be simplified to "Window coordinates in an
> FBO are inverted, which means point sprite origin must be inverted."

looks better, will change that. Thanks!

> 
> > +   if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) ^ render_to_fbo) {
> > +  point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT;
> > +   } else {
> > +  point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT;
> > +   }
> 
> Much better.  That logic was hideous before.

Yeah, agreed. Well, the logic before was somehow clear ;)

Thanks,
Yuanhan Liu


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO

[Yuanhan Liu]

On Thu, Jan 19, 2012 at 10:32:30AM -0700, Brian Paul wrote:
> On 01/19/2012 10:17 AM, Ian Romanick wrote:
> >On 01/18/2012 06:30 PM, Yuanhan Liu wrote:
> >>When rendering to FBO, rendering is inverted. At the same time, we
> >>would
> >>also make sure the point sprite origin is inverted. Or, we will get an
> >>inverted result correspoinding to rendering to the default winsys FBO.
> >>
> >>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613
> >>
> >>NOTE: This is a candidate for stable release branches.
> >>
> >>v2: add the simliar logic to ivb, too (comments from Ian)
> >>simplify the logic operation (comments from Brian)
> >>
> >>Signed-off-by: Yuanhan Liu
> >>---
> >>src/mesa/drivers/dri/i965/brw_defines.h | 1 +
> >>src/mesa/drivers/dri/i965/gen6_sf_state.c | 15 +--
> >>src/mesa/drivers/dri/i965/gen7_sf_state.c | 20 +---
> >>3 files changed, 31 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
> >>b/src/mesa/drivers/dri/i965/brw_defines.h
> >>index 4d90a99..029be87 100644
> >>--- a/src/mesa/drivers/dri/i965/brw_defines.h
> >>+++ b/src/mesa/drivers/dri/i965/brw_defines.h
> >>@@ -1128,6 +1128,7 @@ enum brw_message_target {
> >>/* DW1 (for gen6) */
> >># define GEN6_SF_NUM_OUTPUTS_SHIFT 22
> >># define GEN6_SF_SWIZZLE_ENABLE (1<< 21)
> >>+# define GEN6_SF_POINT_SPRITE_UPPERLEFT (0<< 20)
> >># define GEN6_SF_POINT_SPRITE_LOWERLEFT (1<< 20)
> >># define GEN6_SF_URB_ENTRY_READ_LENGTH_SHIFT 11
> >># define GEN6_SF_URB_ENTRY_READ_OFFSET_SHIFT 4
> >>diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c
> >>b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> >>index 548c5a3..67c208b 100644
> >>--- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
> >>+++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> >>@@ -129,6 +129,7 @@ upload_sf_state(struct brw_context *brw)
> >>float point_size;
> >>uint16_t attr_overrides[FRAG_ATTRIB_MAX];
> >>bool userclip_active;
> >>+ uint32_t point_sprite_origin;
> >>
> >>/* _NEW_TRANSFORM */
> >>userclip_active = (ctx->Transform.ClipPlanesEnabled != 0);
> >>@@ -258,8 +259,18 @@ upload_sf_state(struct brw_context *brw)
> >>/* Clamp to the hardware limits and convert to fixed point */
> >>dw4 |= U_FIXED(CLAMP(point_size, 0.125, 255.875), 3);
> >>
> >>- if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT)
> >>- dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT;
> >>+ /*
> >>+ * When rendering to FBO, rendering is inverted. At the same time,
> >>+ * we would also make sure the point sprite origin is inverted.
> >>+ * Or, we will get an inverted result corresponding to rendering
> >>+ * to the default/window FBO.
> >>+ */
> >>+ if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) ^ render_to_fbo) {
> >
> >I (mostly) like that. I was trying to think of a way to simplify the
> >if-statements in the original patch, but I couldn't think of a good way.
> >
> >However, using the bit-wise xor is not correct here. The compiler
> >accepts it because everything is an integer in C. Some tools, like
> >Coverty, will probably complain about this. You really want a ^^
> >(logical xor), but C doesn't have that. What C does have, that does
> >exactly the same thing, is ==. I suggest changing this to

Thanks for the information.

> >
> >if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) == render_to_fbo) {
> >
> >It looks a bit weird, but it is correct.
> 
> I suggested ^ to Yuanhan.  Yes you have to be careful with it.  Note
> that "X ^ render_to_fbo" is already used earlier in the function.
> 
> But I think you want != in this case, not ==.

Using '!=' is ok to me. Will change that.

Thanks,
Yuanhan Liu
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO

[Yuanhan Liu]

When rendering to FBO, rendering is inverted. At the same time, we would
also make sure the point sprite origin is inverted. Or, we will get an
inverted result correspoinding to rendering to the default winsys FBO.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613

NOTE: This is a candidate for stable release branches.

v2: add the simliar logic to ivb, too (comments from Ian)
simplify the logic operation (comments from Brian)

v3: pick a better comment from Eric
use != for the logic instead of ^ (comments from Ian)

Signed-off-by: Yuanhan Liu 
---
 src/mesa/drivers/dri/i965/brw_defines.h   |1 +
 src/mesa/drivers/dri/i965/gen6_sf_state.c |   13 +++--
 src/mesa/drivers/dri/i965/gen7_sf_state.c |   18 +++---
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
b/src/mesa/drivers/dri/i965/brw_defines.h
index 4d90a99..029be87 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -1128,6 +1128,7 @@ enum brw_message_target {
 /* DW1 (for gen6) */
 # define GEN6_SF_NUM_OUTPUTS_SHIFT 22
 # define GEN6_SF_SWIZZLE_ENABLE(1 << 21)
+# define GEN6_SF_POINT_SPRITE_UPPERLEFT(0 << 20)
 # define GEN6_SF_POINT_SPRITE_LOWERLEFT(1 << 20)
 # define GEN6_SF_URB_ENTRY_READ_LENGTH_SHIFT   11
 # define GEN6_SF_URB_ENTRY_READ_OFFSET_SHIFT   4
diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c 
b/src/mesa/drivers/dri/i965/gen6_sf_state.c
index 548c5a3..163b54c 100644
--- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
@@ -129,6 +129,7 @@ upload_sf_state(struct brw_context *brw)
float point_size;
uint16_t attr_overrides[FRAG_ATTRIB_MAX];
bool userclip_active;
+   uint32_t point_sprite_origin;
 
/* _NEW_TRANSFORM */
userclip_active = (ctx->Transform.ClipPlanesEnabled != 0);
@@ -258,8 +259,16 @@ upload_sf_state(struct brw_context *brw)
/* Clamp to the hardware limits and convert to fixed point */
dw4 |= U_FIXED(CLAMP(point_size, 0.125, 255.875), 3);
 
-   if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT)
-  dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT;
+   /*
+* Window coordinates in an FBO are inverted, which means point
+* sprite origin must be inverted, too.
+*/
+   if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) != render_to_fbo) {
+  point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT;
+   } else {
+  point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT;
+   }
+   dw1 |= point_sprite_origin;
 
/* _NEW_LIGHT */
if (ctx->Light.ProvokingVertex != GL_FIRST_VERTEX_CONVENTION) {
diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c 
b/src/mesa/drivers/dri/i965/gen7_sf_state.c
index 7691cb2..da7ef81 100644
--- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
@@ -48,6 +48,9 @@ upload_sbe_state(struct brw_context *brw)
int urb_entry_read_offset = 1;
bool userclip_active = (ctx->Transform.ClipPlanesEnabled != 0);
uint16_t attr_overrides[FRAG_ATTRIB_MAX];
+   /* _NEW_BUFFERS */
+   bool render_to_fbo = ctx->DrawBuffer->Name != 0;
+   uint32_t point_sprite_origin;
 
brw_compute_vue_map(&vue_map, intel, userclip_active, vs_outputs_written);
urb_entry_read_length = (vue_map.num_slots + 1)/2 - urb_entry_read_offset;
@@ -65,9 +68,18 @@ upload_sbe_state(struct brw_context *brw)
   urb_entry_read_length << GEN7_SBE_URB_ENTRY_READ_LENGTH_SHIFT |
   urb_entry_read_offset << GEN7_SBE_URB_ENTRY_READ_OFFSET_SHIFT;
 
-   /* _NEW_POINT */
-   if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT)
-  dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT;
+   /* _NEW_POINT
+*
+* Window coordinates in an FBO are inverted, which means point
+* sprite origin must be inverted.
+*/
+   if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) != render_to_fbo) {
+  point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT;
+   } else {
+  point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT;
+   }
+   dw1 |= point_sprite_origin;
+
 
dw10 = 0;
dw11 = 0;
-- 
1.7.4.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/2] mesa: Make sure _TexEnvProgram points at the current ff fragment program

[Ian Romanick]

From: Ian Romanick 

At least one place, the _mesa_need_secondary_color function in
state.h, uses this to make decisions.  The next patch in this series
will add another dependency.  Ideally, this field would go away and be
replace by a flag or something.

NOTE: This is a candidate for the 8.0 branch.

Signed-off-by: Ian Romanick 
---
 src/mesa/main/state.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/src/mesa/main/state.c b/src/mesa/main/state.c
index b910543..c59eff7 100644
--- a/src/mesa/main/state.c
+++ b/src/mesa/main/state.c
@@ -259,6 +259,8 @@ update_program(struct gl_context *ctx)
   _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._Current,
   (struct gl_fragment_program *)
   
fsProg->_LinkedShaders[MESA_SHADER_FRAGMENT]->Program);
+  _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._TexEnvProgram,
+  NULL);
}
else if (ctx->FragmentProgram._Enabled) {
   /* Use user-defined fragment program */
@@ -267,6 +269,8 @@ update_program(struct gl_context *ctx)
 NULL);
   _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._Current,
ctx->FragmentProgram.Current);
+  _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._TexEnvProgram,
+  NULL);
}
else if (ctx->FragmentProgram._MaintainTexEnvProgram) {
   /* Use fragment program generated from fixed-function state */
@@ -278,10 +282,15 @@ update_program(struct gl_context *ctx)
   _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._Current,
   (struct gl_fragment_program *)

f->_LinkedShaders[MESA_SHADER_FRAGMENT]->Program);
+  _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._TexEnvProgram,
+  (struct gl_fragment_program *)
+   
f->_LinkedShaders[MESA_SHADER_FRAGMENT]->Program);
}
else {
   /* No fragment program */
   _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._Current, NULL);
+  _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._TexEnvProgram,
+  NULL);
}
 
if (gsProg && gsProg->LinkStatus
-- 
1.7.6.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] swrast: Use fixed-function processing if the _TexEnvProgram is empty

[Ian Romanick]

From: Ian Romanick 

This is a hack to work around drivers such as i965 that:

- Set _MaintainTexEnvProgram to generate GLSL IR for
  fixed-function fragment processing.
- Don't call _mesa_ir_link_shader to generate Mesa IR from the
  GLSL IR.
- May use swrast to handle glDrawPixels.

Since _mesa_ir_link_shader is never called, there is no Mesa IR to
execute.  Instead do regular fixed-function processing.

NOTE: This is a candidate for the 8.0 branch.

Signed-off-by: Ian Romanick 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44749
---
 src/mesa/swrast/s_span.c |   21 +++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/mesa/swrast/s_span.c b/src/mesa/swrast/s_span.c
index e899303..439b0e9 100644
--- a/src/mesa/swrast/s_span.c
+++ b/src/mesa/swrast/s_span.c
@@ -50,6 +50,7 @@
 #include "s_stencil.h"
 #include "s_texcombine.h"
 
+#include 
 
 /**
  * Set default fragment attributes for the span using the
@@ -968,7 +969,23 @@ convert_color_type(SWspan *span, GLenum newType, GLuint 
output)
 static inline void
 shade_texture_span(struct gl_context *ctx, SWspan *span)
 {
-   if (ctx->FragmentProgram._Current ||
+   /* This is a hack to work around drivers such as i965 that:
+*
+* - Set _MaintainTexEnvProgram to generate GLSL IR for
+*   fixed-function fragment processing.
+* - Don't call _mesa_ir_link_shader to generate Mesa IR from
+*   the GLSL IR.
+* - May use swrast to handle glDrawPixels.
+*
+* Since _mesa_ir_link_shader is never called, there is no Mesa IR
+* to execute.  Instead do regular fixed-function processing.
+*/
+   const bool use_fragment_program =
+  ctx->FragmentProgram._Current
+  && (ctx->FragmentProgram._Current != ctx->FragmentProgram._TexEnvProgram
+ || ctx->FragmentProgram._Current->Base.NumInstructions != 0);
+
+   if (use_fragment_program ||
ctx->ATIFragmentShader._Enabled) {
   /* programmable shading */
   if (span->primitive == GL_BITMAP && span->array->ChanType != GL_FLOAT) {
@@ -997,7 +1014,7 @@ shade_texture_span(struct gl_context *ctx, SWspan *span)
  interpolate_wpos(ctx, span);
 
   /* Run fragment program/shader now */
-  if (ctx->FragmentProgram._Current) {
+  if (use_fragment_program) {
  _swrast_exec_fragment_program(ctx, span);
   }
   else {
-- 
1.7.6.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa: Loosen glBlitFramebuffer restrictions on depthstencil buffers (v2)

[Ian Romanick]

On 01/19/2012 12:12 PM, Christoph Bumiller wrote:

On 19.01.2012 19:53, Ian Romanick wrote:

On 01/18/2012 12:31 PM, Chad Versace wrote:

--- snip

Supporting Z32 ->   Z32_FLOAT is a bad idea. So I've amended the patch
to avoid that.

--- end snip


This loosens the format validation in glBlitFramebuffer. When blitting
depth bits, don't require an exact match between the depth formats; only
require that the two formats have the same number of depth bits and the
same depth datatype (float vs uint). Ditto for stencil.

Between S8_Z24 buffers, the EXT_framebuffer_blit spec allows
glBlitFramebuffer to blit the depth and stencil bits separately. So I
see
no reason to prevent blitting the depth bits between X8_Z24 and
S8_Z24 or
the stencil bits between S8 and S8_Z24. However, we of course don't want
to allow blitting from Z32 to Z32_FLOAT.



So is it also allowed to blit from S8Z24 to Z24S8 ? Could we also allow
to blit from RGBA8 to BGRA8 then, please ?


Based on the spec language that Brian quoted, that should already be 
allowed.  I'd swear that there was a piglit test that tries to blit from 
a RGBA8 to RGB10A2 FBO...



That would fix my multisampling fail - all applications that use
framebuffer_multisample try to blit resolve directly from an MS
renderbuffer to the window system back buffer and that means I have to
know somehow which ordering the window system chooses, which is rather
inconvenient and made me hack-disable RGBA8 in the gallium driver, i.e.
lie about what formats it supports.

If I recall correctly, the NV blob does allow resolving between BGRA and
RGBA.




Fixes Piglit fbo/fbo-blit-d24s8 on Intel drivers with separate stencil
enabled.

The problem was that, on Intel drivers with separate stencil, the
default
framebuffer has separate depth and stencil buffers with formats
X8_Z24 and
S8. The test attempts to blit the depth bits from a S8_Z24 buffer
into the
default framebuffer.

v2: Check that depth datatypes match in order prevent Z32 ->   Z32_FLOAT.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44665
Note: This is a candidate for the 8.0 branch.
CC: Brian Paul
CC: Kenneth Graunke
CC: Iam Romanick


Reviewed-by: Ian Romanick


Reported-by: Xunx Fang
Signed-off-by: Chad Versace
---
   src/mesa/main/fbobject.c |   13 ++---
   1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 0524959..2b3ac2e 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -2709,9 +2709,13 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint
srcY0, GLint srcX1, GLint srcY1,
 if ((readRb == NULL) || (drawRb == NULL)) {
mask&= ~GL_STENCIL_BUFFER_BIT;
 }
-  else if (readRb->Format != drawRb->Format) {
+  else if (_mesa_get_format_bits(readRb->Format,
GL_STENCIL_BITS) !=
+   _mesa_get_format_bits(drawRb->Format, GL_STENCIL_BITS)) {
+ /* There is no need to check the stencil datatype here, because
+  * there is only one: GL_UNSIGNED_INT.
+  */
_mesa_error(ctx, GL_INVALID_OPERATION,
- "glBlitFramebufferEXT(stencil buffer format
mismatch)");
+ "glBlitFramebufferEXT(stencil buffer size
mismatch)");
return;
 }
  }
@@ -2731,7 +2735,10 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint
srcY0, GLint srcX1, GLint srcY1,
 if ((readRb == NULL) || (drawRb == NULL)) {
mask&= ~GL_DEPTH_BUFFER_BIT;
 }
-  else if (readRb->Format != drawRb->Format) {
+  else if ((_mesa_get_format_bits(readRb->Format, GL_DEPTH_BITS) !=
+_mesa_get_format_bits(drawRb->Format, GL_DEPTH_BITS)) ||
+   (_mesa_get_format_datatype(readRb->Format) !=
+_mesa_get_format_datatype(drawRb->Format))) {
_mesa_error(ctx, GL_INVALID_OPERATION,
"glBlitFramebufferEXT(depth buffer format
mismatch)");
return;


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa: Loosen glBlitFramebuffer restrictions on depthstencil buffers (v2)

[Eric Anholt]

On Thu, 19 Jan 2012 21:12:31 +0100, Christoph Bumiller 
 wrote:
> On 19.01.2012 19:53, Ian Romanick wrote:
> > On 01/18/2012 12:31 PM, Chad Versace wrote:
> >> --- snip
> >>
> >> Supporting Z32 ->  Z32_FLOAT is a bad idea. So I've amended the patch
> >> to avoid that.
> >>
> >> --- end snip
> >>
> >>
> >> This loosens the format validation in glBlitFramebuffer. When blitting
> >> depth bits, don't require an exact match between the depth formats; only
> >> require that the two formats have the same number of depth bits and the
> >> same depth datatype (float vs uint). Ditto for stencil.
> >>
> >> Between S8_Z24 buffers, the EXT_framebuffer_blit spec allows
> >> glBlitFramebuffer to blit the depth and stencil bits separately. So I
> >> see
> >> no reason to prevent blitting the depth bits between X8_Z24 and
> >> S8_Z24 or
> >> the stencil bits between S8 and S8_Z24. However, we of course don't want
> >> to allow blitting from Z32 to Z32_FLOAT.
> >>
> 
> So is it also allowed to blit from S8Z24 to Z24S8 ? Could we also allow
> to blit from RGBA8 to BGRA8 then, please ?

That's already allowed.

If the color formats of the read and draw framebuffers do not
match, and  includes COLOR_BUFFER_BIT, the pixel groups are
converted to match the destination format as in CopyPixels, except
that no pixel transfer operations apply and clamping behaves as if
CLAMP_FRAGMENT_COLOR_ARB is set to FIXED_ONLY_ARB.


pgp0EawSfGYPp.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO

[Ian Romanick]

On 01/19/2012 03:48 PM, Yuanhan Liu wrote:

When rendering to FBO, rendering is inverted. At the same time, we would
also make sure the point sprite origin is inverted. Or, we will get an
inverted result correspoinding to rendering to the default winsys FBO.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613

NOTE: This is a candidate for stable release branches.

v2: add the simliar logic to ivb, too (comments from Ian)
 simplify the logic operation (comments from Brian)

v3: pick a better comment from Eric
 use != for the logic instead of ^ (comments from Ian)

Signed-off-by: Yuanhan Liu


Reviewed-by: Ian Romanick 


---
  src/mesa/drivers/dri/i965/brw_defines.h   |1 +
  src/mesa/drivers/dri/i965/gen6_sf_state.c |   13 +++--
  src/mesa/drivers/dri/i965/gen7_sf_state.c |   18 +++---
  3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
b/src/mesa/drivers/dri/i965/brw_defines.h
index 4d90a99..029be87 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -1128,6 +1128,7 @@ enum brw_message_target {
  /* DW1 (for gen6) */
  # define GEN6_SF_NUM_OUTPUTS_SHIFT22
  # define GEN6_SF_SWIZZLE_ENABLE   (1<<  21)
+# define GEN6_SF_POINT_SPRITE_UPPERLEFT(0<<  20)
  # define GEN6_SF_POINT_SPRITE_LOWERLEFT   (1<<  20)
  # define GEN6_SF_URB_ENTRY_READ_LENGTH_SHIFT  11
  # define GEN6_SF_URB_ENTRY_READ_OFFSET_SHIFT  4
diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c 
b/src/mesa/drivers/dri/i965/gen6_sf_state.c
index 548c5a3..163b54c 100644
--- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
@@ -129,6 +129,7 @@ upload_sf_state(struct brw_context *brw)
 float point_size;
 uint16_t attr_overrides[FRAG_ATTRIB_MAX];
 bool userclip_active;
+   uint32_t point_sprite_origin;

 /* _NEW_TRANSFORM */
 userclip_active = (ctx->Transform.ClipPlanesEnabled != 0);
@@ -258,8 +259,16 @@ upload_sf_state(struct brw_context *brw)
 /* Clamp to the hardware limits and convert to fixed point */
 dw4 |= U_FIXED(CLAMP(point_size, 0.125, 255.875), 3);

-   if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT)
-  dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT;
+   /*
+* Window coordinates in an FBO are inverted, which means point
+* sprite origin must be inverted, too.
+*/
+   if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) != render_to_fbo) {
+  point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT;
+   } else {
+  point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT;
+   }
+   dw1 |= point_sprite_origin;

 /* _NEW_LIGHT */
 if (ctx->Light.ProvokingVertex != GL_FIRST_VERTEX_CONVENTION) {
diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c 
b/src/mesa/drivers/dri/i965/gen7_sf_state.c
index 7691cb2..da7ef81 100644
--- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
@@ -48,6 +48,9 @@ upload_sbe_state(struct brw_context *brw)
 int urb_entry_read_offset = 1;
 bool userclip_active = (ctx->Transform.ClipPlanesEnabled != 0);
 uint16_t attr_overrides[FRAG_ATTRIB_MAX];
+   /* _NEW_BUFFERS */
+   bool render_to_fbo = ctx->DrawBuffer->Name != 0;
+   uint32_t point_sprite_origin;

 brw_compute_vue_map(&vue_map, intel, userclip_active, vs_outputs_written);
 urb_entry_read_length = (vue_map.num_slots + 1)/2 - urb_entry_read_offset;
@@ -65,9 +68,18 @@ upload_sbe_state(struct brw_context *brw)
urb_entry_read_length<<  GEN7_SBE_URB_ENTRY_READ_LENGTH_SHIFT |
urb_entry_read_offset<<  GEN7_SBE_URB_ENTRY_READ_OFFSET_SHIFT;

-   /* _NEW_POINT */
-   if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT)
-  dw1 |= GEN6_SF_POINT_SPRITE_LOWERLEFT;
+   /* _NEW_POINT
+*
+* Window coordinates in an FBO are inverted, which means point
+* sprite origin must be inverted.
+*/
+   if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) != render_to_fbo) {
+  point_sprite_origin = GEN6_SF_POINT_SPRITE_LOWERLEFT;
+   } else {
+  point_sprite_origin = GEN6_SF_POINT_SPRITE_UPPERLEFT;
+   }
+   dw1 |= point_sprite_origin;
+

 dw10 = 0;
 dw11 = 0;


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] intel/gen6: Some framebuffers having separate depthstencil should be unsupported

[Eric Anholt]

On Thu, 19 Jan 2012 15:09:05 -0800, Chad Versace  
wrote:
> When the framebuffer has separate depth and stencil buffers, and HiZ is
> not enabled on the depth buffer, mark the framebuffer as unsupported. This
> happens when trying to create a framebuffer with Z16/S8 because we haven't
> enabled HiZ on Z16 yet.
> 
> Fixes gles2conform test stencil8.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44948
> Reported-by: Ian Romanick 
> Note: This is a candiate for the 8.0 branch.
> Signed-off-by: Chad Versace 

Reviewed-by: Eric Anholt 


pgpPPaHaGyV0E.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] i965/vs: Fix bogus assertion in emit_block_move()

[Paul Berry]

i965 processes assignments of whole structures using
vec4_visitor::emit_block_move, a recursive function which visits each
element of a structure or array (to arbitrary nesting depth) and
copies it from the source to the destination.  Then it increments the
source and destination register numbers so that further recursive
invocations will copy the rest of the structure.  In addition, it sets
the swizzle field for the source register to an appropriate value of
swizzle_for_size(...) for the size of each element being copied, so
that later optimization passes won't be fooled into thinking that
unused vector elements are live.

This all works fine.  However, emit_block_move also contains an
assertion to verify, before setting the swizzle field for the source
register, that the source register doesn't already contain a
nontrivial swizzle.  The intention is to make sure that the caller of
emit_block_move hasn't already done some swizzling of the data before
the call, which emit_block_move would then counteract when it
overwrites the swizzle field.  But the assertion is at the lowest
level of nesting of emit_block_move, which means that after the first
element is copied, instead of checking the swizzle field set by the
caller, it checks the swizzle field used when moving the previous
element.  That means that if the structure contains elements of
different vector sizes (which therefore require different swizzles),
the assertion will erroneously fire.

This patch moves the assertion from emit_block_move to the calling
function, vec4_visitor::visit(ir_assignment *).  Since the caller is
non-recursive, the assertion will only happen once, and won't be
fooled by emit_block_move's modification of the swizzle field.

This patch also reverts commit fe006a7 (i965/vs: Fix swizzle related
assertion), which attempted to fix the bug by making the assertion
more lenient, but only worked properly for structures, arrays, and
matrices in which each constituent vector is the same size.

This fixes the problem described in comment 9 of
https://bugs.freedesktop.org/show_bug.cgi?id=40865.  Unfortunately, it
doesn't fix the whole bug, since the test in question is also failing
due to lack of register spilling support in the VS.

Fixes piglit test fdo40865_c9.  No piglit regressions on Sandy Bridge.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=40865#c9
---
 src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |   12 +---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index 5df2470..10b3c20 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -1532,9 +1532,6 @@ vec4_visitor::emit_block_move(dst_reg *dst, src_reg *src,
 
dst->writemask = (1 << type->vector_elements) - 1;
 
-   /* Do we need to worry about swizzling a swizzle? */
-   assert(src->swizzle == BRW_SWIZZLE_NOOP
- || src->swizzle == swizzle_for_size(type->vector_elements));
src->swizzle = swizzle_for_size(type->vector_elements);
 
vec4_instruction *inst = emit(MOV(*dst, *src));
@@ -1617,6 +1614,15 @@ vec4_visitor::visit(ir_assignment *ir)
 emit_bool_to_cond_code(ir->condition, &predicate);
   }
 
+  /* emit_block_move doesn't account for swizzles in the source register.
+   * This should be ok, since the source register is a structure or an
+   * array, and those can't be swizzled.  But double-check to be sure.
+   */
+  assert(src.swizzle ==
+ (ir->rhs->type->is_matrix()
+  ? swizzle_for_size(ir->rhs->type->vector_elements)
+  : BRW_SWIZZLE_NOOP));
+
   emit_block_move(&dst, &src, ir->rhs->type, predicate);
   return;
}
-- 
1.7.6.5

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] swrast: Use fixed-function processing if the _TexEnvProgram is empty

[Eric Anholt]

On Thu, 19 Jan 2012 17:53:12 -0800, "Ian Romanick"  wrote:
> From: Ian Romanick 
> 
> This is a hack to work around drivers such as i965 that:
> 
> - Set _MaintainTexEnvProgram to generate GLSL IR for
>   fixed-function fragment processing.
> - Don't call _mesa_ir_link_shader to generate Mesa IR from the
>   GLSL IR.
> - May use swrast to handle glDrawPixels.
> 
> Since _mesa_ir_link_shader is never called, there is no Mesa IR to
> execute.  Instead do regular fixed-function processing.
> 
> NOTE: This is a candidate for the 8.0 branch.
> 
> Signed-off-by: Ian Romanick 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44749
> ---
>  src/mesa/swrast/s_span.c |   21 +++--
>  1 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/swrast/s_span.c b/src/mesa/swrast/s_span.c
> index e899303..439b0e9 100644
> --- a/src/mesa/swrast/s_span.c
> +++ b/src/mesa/swrast/s_span.c
> @@ -50,6 +50,7 @@
>  #include "s_stencil.h"
>  #include "s_texcombine.h"
>  
> +#include 
>  
>  /**
>   * Set default fragment attributes for the span using the
> @@ -968,7 +969,23 @@ convert_color_type(SWspan *span, GLenum newType, GLuint 
> output)
>  static inline void
>  shade_texture_span(struct gl_context *ctx, SWspan *span)
>  {
> -   if (ctx->FragmentProgram._Current ||
> +   /* This is a hack to work around drivers such as i965 that:
> +*
> +* - Set _MaintainTexEnvProgram to generate GLSL IR for
> +*   fixed-function fragment processing.
> +* - Don't call _mesa_ir_link_shader to generate Mesa IR from
> +*   the GLSL IR.
> +* - May use swrast to handle glDrawPixels.
> +*
> +* Since _mesa_ir_link_shader is never called, there is no Mesa IR
> +* to execute.  Instead do regular fixed-function processing.
> +*/
> +   const bool use_fragment_program =
> +  ctx->FragmentProgram._Current
> +  && (ctx->FragmentProgram._Current != 
> ctx->FragmentProgram._TexEnvProgram
> +   || ctx->FragmentProgram._Current->Base.NumInstructions != 0);

We used to have a hack like this in the driver just to reduce the
performance penalty.  I think you could just drop the NumInstructions
check to duplicate that.


pgpvQXGqOADHn.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] intel/gen6: Some framebuffers having separate depthstencil should be unsupported

[Ian Romanick]

On 01/19/2012 03:09 PM, Chad Versace wrote:

When the framebuffer has separate depth and stencil buffers, and HiZ is
not enabled on the depth buffer, mark the framebuffer as unsupported. This
happens when trying to create a framebuffer with Z16/S8 because we haven't
enabled HiZ on Z16 yet.

Fixes gles2conform test stencil8.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44948
Reported-by: Ian Romanick
Note: This is a candiate for the 8.0 branch.
Signed-off-by: Chad Versace


Reviewed-and-tested-by: Ian Romanick 


---
  src/mesa/drivers/dri/intel/intel_fbo.c |9 +
  1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c 
b/src/mesa/drivers/dri/intel/intel_fbo.c
index c37075c..45a6827 100644
--- a/src/mesa/drivers/dri/intel/intel_fbo.c
+++ b/src/mesa/drivers/dri/intel/intel_fbo.c
@@ -804,6 +804,15 @@ intel_validate_framebuffer(struct gl_context *ctx, struct 
gl_framebuffer *fb)
fb->_Status = GL_FRAMEBUFFER_UNSUPPORTED_EXT;
 if (stencil_mt->format != MESA_FORMAT_S8)
fb->_Status = GL_FRAMEBUFFER_UNSUPPORTED_EXT;
+if (intel->gen<  7&&  depth_mt->hiz_mt == NULL) {
+   /* Before Gen7, separate depth and stencil buffers can be used
+* only if HiZ is enabled. From the Sandybridge PRM, Volume 2,
+* Part 1, Bit 3DSTATE_DEPTH_BUFFER.SeparateStencilBufferEnable:
+* [DevSNB]: This field must be set to the same value (enabled
+* or disabled) as Hierarchical Depth Buffer Enable.
+*/
+   fb->_Status = GL_FRAMEBUFFER_UNSUPPORTED;
+}
}
 }



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965/vs: Fix bogus assertion in emit_block_move()

[Paul Berry]

On 19 January 2012 18:28, Paul Berry  wrote:

> i965 processes assignments of whole structures using
> vec4_visitor::emit_block_move, a recursive function which visits each
> element of a structure or array (to arbitrary nesting depth) and
> copies it from the source to the destination.  Then it increments the
> source and destination register numbers so that further recursive
> invocations will copy the rest of the structure.  In addition, it sets
> the swizzle field for the source register to an appropriate value of
> swizzle_for_size(...) for the size of each element being copied, so
> that later optimization passes won't be fooled into thinking that
> unused vector elements are live.
>
> This all works fine.  However, emit_block_move also contains an
> assertion to verify, before setting the swizzle field for the source
> register, that the source register doesn't already contain a
> nontrivial swizzle.  The intention is to make sure that the caller of
> emit_block_move hasn't already done some swizzling of the data before
> the call, which emit_block_move would then counteract when it
> overwrites the swizzle field.  But the assertion is at the lowest
> level of nesting of emit_block_move, which means that after the first
> element is copied, instead of checking the swizzle field set by the
> caller, it checks the swizzle field used when moving the previous
> element.  That means that if the structure contains elements of
> different vector sizes (which therefore require different swizzles),
> the assertion will erroneously fire.
>
> This patch moves the assertion from emit_block_move to the calling
> function, vec4_visitor::visit(ir_assignment *).  Since the caller is
> non-recursive, the assertion will only happen once, and won't be
> fooled by emit_block_move's modification of the swizzle field.
>
> This patch also reverts commit fe006a7 (i965/vs: Fix swizzle related
> assertion), which attempted to fix the bug by making the assertion
> more lenient, but only worked properly for structures, arrays, and
> matrices in which each constituent vector is the same size.
>
> This fixes the problem described in comment 9 of
> https://bugs.freedesktop.org/show_bug.cgi?id=40865.  Unfortunately, it
> doesn't fix the whole bug, since the test in question is also failing
> due to lack of register spilling support in the VS.
>
> Fixes piglit test fdo40865_c9.  No piglit regressions on Sandy Bridge.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=40865#c9
>

I forgot to mention: This is a candidate for the 8.0 release branch.


> ---
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |   12 +---
>  1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 5df2470..10b3c20 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -1532,9 +1532,6 @@ vec4_visitor::emit_block_move(dst_reg *dst, src_reg
> *src,
>
>dst->writemask = (1 << type->vector_elements) - 1;
>
> -   /* Do we need to worry about swizzling a swizzle? */
> -   assert(src->swizzle == BRW_SWIZZLE_NOOP
> - || src->swizzle == swizzle_for_size(type->vector_elements));
>src->swizzle = swizzle_for_size(type->vector_elements);
>
>vec4_instruction *inst = emit(MOV(*dst, *src));
> @@ -1617,6 +1614,15 @@ vec4_visitor::visit(ir_assignment *ir)
> emit_bool_to_cond_code(ir->condition, &predicate);
>   }
>
> +  /* emit_block_move doesn't account for swizzles in the source
> register.
> +   * This should be ok, since the source register is a structure or an
> +   * array, and those can't be swizzled.  But double-check to be sure.
> +   */
> +  assert(src.swizzle ==
> + (ir->rhs->type->is_matrix()
> +  ? swizzle_for_size(ir->rhs->type->vector_elements)
> +  : BRW_SWIZZLE_NOOP));
> +
>   emit_block_move(&dst, &src, ir->rhs->type, predicate);
>   return;
>}
> --
> 1.7.6.5
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa: allocate transform_feedback_info::Outputs array dynamically

[Paul Berry]

On 17 January 2012 11:29, Christoph Bumiller
wrote:

> The nvc0 gallium driver is advertising 128 MAX_INTERLEAVED_COMPS
> which made it always assert in the linker when TFB was used since
> the Outputs array was smaller than that maximum.
>
> v2: added assertions
>
> NOTE: This is a candidate for the 8.0 branch.
> ---
>  src/glsl/linker.cpp|   64
> ++--
>  src/mesa/main/mtypes.h |   32 ---
>  2 files changed, 57 insertions(+), 39 deletions(-)
>
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 0d85aee..b6c5754 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1388,9 +1388,10 @@ public:
>static bool is_same(const tfeedback_decl &x, const tfeedback_decl &y);
>bool assign_location(struct gl_context *ctx, struct gl_shader_program
> *prog,
> ir_variable *output_var);
> +   bool accumulate_num_outputs(struct gl_shader_program *prog, unsigned
> *count);
>bool store(struct gl_context *ctx, struct gl_shader_program *prog,
>   struct gl_transform_feedback_info *info, unsigned buffer,
> - unsigned varying) const;
> +  unsigned varying, const unsigned max_outputs) const;
>
>
>/**
> @@ -1624,16 +1625,9 @@ tfeedback_decl::assign_location(struct gl_context
> *ctx,
>  }
>
>
> -/**
> - * Update gl_transform_feedback_info to reflect this tfeedback_decl.
> - *
> - * If an error occurs, the error is reported through linker_error() and
> false
> - * is returned.
> - */
>  bool
> -tfeedback_decl::store(struct gl_context *ctx, struct gl_shader_program
> *prog,
> -  struct gl_transform_feedback_info *info,
> -  unsigned buffer, unsigned varying) const
> +tfeedback_decl::accumulate_num_outputs(struct gl_shader_program *prog,
> +   unsigned *count)
>  {
>if (!this->is_assigned()) {
>   /* From GL_EXT_transform_feedback:
> @@ -1648,6 +1642,28 @@ tfeedback_decl::store(struct gl_context *ctx,
> struct gl_shader_program *prog,
>   return false;
>}
>
> +   unsigned translated_size = this->size;
> +   if (this->is_clip_distance_mesa)
> +  translated_size = (translated_size + 3) / 4;
> +
> +   *count += translated_size * this->matrix_columns;
> +
> +   return true;
> +}
> +
> +
> +/**
> + * Update gl_transform_feedback_info to reflect this tfeedback_decl.
> + *
> + * If an error occurs, the error is reported through linker_error() and
> false
> + * is returned.
> + */
> +bool
> +tfeedback_decl::store(struct gl_context *ctx, struct gl_shader_program
> *prog,
> +  struct gl_transform_feedback_info *info,
> +  unsigned buffer,
> +  unsigned varying, const unsigned max_outputs) const
> +{
>/* From GL_EXT_transform_feedback:
> *   A program will fail to link if:
> *
> @@ -1663,19 +1679,6 @@ tfeedback_decl::store(struct gl_context *ctx,
> struct gl_shader_program *prog,
>   return false;
>}
>
> -   /* Verify that the checks on
> MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS
> -* and MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS are sufficient to
> prevent
> -* overflow of info->Outputs[].  In worst case we generate one entry in
> -* Outputs[] per component so a conservative check is to verify that
> the
> -* size of the array is greater than or equal to both
> -* MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS and
> -* MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS.
> -*/
> -   assert(Elements(info->Outputs) >=
> -  ctx->Const.MaxTransformFeedbackInterleavedComponents);
> -   assert(Elements(info->Outputs) >=
> -  ctx->Const.MaxTransformFeedbackSeparateComponents);
> -
>unsigned translated_size = this->size;
>if (this->is_clip_distance_mesa)
>   translated_size = (translated_size + 3) / 4;
> @@ -1683,6 +1686,7 @@ tfeedback_decl::store(struct gl_context *ctx, struct
> gl_shader_program *prog,
>for (unsigned index = 0; index < translated_size; ++index) {
>   for (unsigned v = 0; v < this->matrix_columns; ++v) {
>  unsigned num_components = this->vector_elements;
> + assert(info->NumOutputs < max_outputs);
>  info->Outputs[info->NumOutputs].ComponentOffset = 0;
>  if (this->is_clip_distance_mesa) {
> if (this->is_subscripted) {
> @@ -1976,6 +1980,7 @@ store_tfeedback_info(struct gl_context *ctx, struct
> gl_shader_program *prog,
>   prog->TransformFeedback.BufferMode == GL_SEPARATE_ATTRIBS;
>
>ralloc_free(prog->LinkedTransformFeedback.Varyings);
> +   ralloc_free(prog->LinkedTransformFeedback.Outputs);
>
>memset(&prog->LinkedTransformFeedback, 0,
>   sizeof(prog->LinkedTransformFeedback));
> @@ -1988,12 +1993,23 @@ store_tfeedback_info(struct gl_context *ctx,
> struct gl_shader_program *prog,
>struct gl_transform_feedback_varying_info,
>num_tfe

Re: [Mesa-dev] [PATCH] i965: fix inverted point sprite origin when rendering to FBO

[Liu Aleaxander]

Thanks for the reviewed-by.

Ian, would you please help me to push this patch? I'm on the way to home
for chines new year, and I can't access the net(both at home).

Thanks advance!

--
Yuanhan Liu
(Sent by my phone, please forgive the poor format)
On Jan 20, 2012 10:25 AM, "Ian Romanick"  wrote:

> On 01/19/2012 03:48 PM, Yuanhan Liu wrote:
>
>> When rendering to FBO, rendering is inverted. At the same time, we would
>> also make sure the point sprite origin is inverted. Or, we will get an
>> inverted result correspoinding to rendering to the default winsys FBO.
>>
>> Bugzilla: 
>> https://bugs.freedesktop.org/**show_bug.cgi?id=44613
>>
>> NOTE: This is a candidate for stable release branches.
>>
>> v2: add the simliar logic to ivb, too (comments from Ian)
>> simplify the logic operation (comments from Brian)
>>
>> v3: pick a better comment from Eric
>> use != for the logic instead of ^ (comments from Ian)
>>
>> Signed-off-by: Yuanhan 
>> Liu
>> >
>>
>
> Reviewed-by: Ian Romanick 
>
>  ---
>>  src/mesa/drivers/dri/i965/brw_**defines.h   |1 +
>>  src/mesa/drivers/dri/i965/**gen6_sf_state.c |   13 +++--
>>  src/mesa/drivers/dri/i965/**gen7_sf_state.c |   18 +++---
>>  3 files changed, 27 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/**brw_defines.h
>> b/src/mesa/drivers/dri/i965/**brw_defines.h
>> index 4d90a99..029be87 100644
>> --- a/src/mesa/drivers/dri/i965/**brw_defines.h
>> +++ b/src/mesa/drivers/dri/i965/**brw_defines.h
>> @@ -1128,6 +1128,7 @@ enum brw_message_target {
>>  /* DW1 (for gen6) */
>>  # define GEN6_SF_NUM_OUTPUTS_SHIFT22
>>  # define GEN6_SF_SWIZZLE_ENABLE   (1<<  21)
>> +# define GEN6_SF_POINT_SPRITE_UPPERLEFT(0<<  20)
>>  # define GEN6_SF_POINT_SPRITE_LOWERLEFT   (1<<  20)
>>  # define GEN6_SF_URB_ENTRY_READ_LENGTH_**SHIFT  11
>>  # define GEN6_SF_URB_ENTRY_READ_OFFSET_**SHIFT  4
>> diff --git a/src/mesa/drivers/dri/i965/**gen6_sf_state.c
>> b/src/mesa/drivers/dri/i965/**gen6_sf_state.c
>> index 548c5a3..163b54c 100644
>> --- a/src/mesa/drivers/dri/i965/**gen6_sf_state.c
>> +++ b/src/mesa/drivers/dri/i965/**gen6_sf_state.c
>> @@ -129,6 +129,7 @@ upload_sf_state(struct brw_context *brw)
>> float point_size;
>> uint16_t attr_overrides[FRAG_ATTRIB_**MAX];
>> bool userclip_active;
>> +   uint32_t point_sprite_origin;
>>
>> /* _NEW_TRANSFORM */
>> userclip_active = (ctx->Transform.**ClipPlanesEnabled != 0);
>> @@ -258,8 +259,16 @@ upload_sf_state(struct brw_context *brw)
>> /* Clamp to the hardware limits and convert to fixed point */
>> dw4 |= U_FIXED(CLAMP(point_size, 0.125, 255.875), 3);
>>
>> -   if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT)
>> -  dw1 |= GEN6_SF_POINT_SPRITE_**LOWERLEFT;
>> +   /*
>> +* Window coordinates in an FBO are inverted, which means point
>> +* sprite origin must be inverted, too.
>> +*/
>> +   if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) != render_to_fbo) {
>> +  point_sprite_origin = GEN6_SF_POINT_SPRITE_**LOWERLEFT;
>> +   } else {
>> +  point_sprite_origin = GEN6_SF_POINT_SPRITE_**UPPERLEFT;
>> +   }
>> +   dw1 |= point_sprite_origin;
>>
>> /* _NEW_LIGHT */
>> if (ctx->Light.ProvokingVertex != GL_FIRST_VERTEX_CONVENTION) {
>> diff --git a/src/mesa/drivers/dri/i965/**gen7_sf_state.c
>> b/src/mesa/drivers/dri/i965/**gen7_sf_state.c
>> index 7691cb2..da7ef81 100644
>> --- a/src/mesa/drivers/dri/i965/**gen7_sf_state.c
>> +++ b/src/mesa/drivers/dri/i965/**gen7_sf_state.c
>> @@ -48,6 +48,9 @@ upload_sbe_state(struct brw_context *brw)
>> int urb_entry_read_offset = 1;
>> bool userclip_active = (ctx->Transform.**ClipPlanesEnabled != 0);
>> uint16_t attr_overrides[FRAG_ATTRIB_**MAX];
>> +   /* _NEW_BUFFERS */
>> +   bool render_to_fbo = ctx->DrawBuffer->Name != 0;
>> +   uint32_t point_sprite_origin;
>>
>> brw_compute_vue_map(&vue_map, intel, userclip_active,
>> vs_outputs_written);
>> urb_entry_read_length = (vue_map.num_slots + 1)/2 -
>> urb_entry_read_offset;
>> @@ -65,9 +68,18 @@ upload_sbe_state(struct brw_context *brw)
>>urb_entry_read_length<<  GEN7_SBE_URB_ENTRY_READ_**LENGTH_SHIFT |
>>urb_entry_read_offset<<  GEN7_SBE_URB_ENTRY_READ_**OFFSET_SHIFT;
>>
>> -   /* _NEW_POINT */
>> -   if (ctx->Point.SpriteOrigin == GL_LOWER_LEFT)
>> -  dw1 |= GEN6_SF_POINT_SPRITE_**LOWERLEFT;
>> +   /* _NEW_POINT
>> +*
>> +* Window coordinates in an FBO are inverted, which means point
>> +* sprite origin must be inverted.
>> +*/
>> +   if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) != render_to_fbo) {
>> +  point_sprite_origin = GEN6_SF_POINT_SPRITE_**LOWERLEFT;
>> +   } else {
>> +  point_sprite_origin = GEN6_SF_POINT_SPRITE_**UPPERLEFT;
>> +   }
>> +   dw1 |= point_sprite_origin;
>> +
>>
>> dw10 = 0;
>> dw11 = 0;
>>
>
> __

[Mesa-dev] [Bug 38970] [bisected]piglit glx/glx-pixmap-multi failed

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=38970

fangxun  changed:

   What|Removed |Added

 Blocks||42993

--- Comment #15 from fangxun  2012-01-19 22:12:51 PST ---
It also happens on mesa 8.0 branch.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev