[Mesa-dev] [PATCH] tgsi: Fix memory leak in out-of-memory path.

2011-10-25 Thread Vinson Lee
Fixes Coverity resource leak defect.
---
 src/gallium/auxiliary/tgsi/tgsi_exec.c |   17 ++---
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c 
b/src/gallium/auxiliary/tgsi/tgsi_exec.c
index b4eea54..1fb7f8f 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_exec.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c
@@ -674,16 +674,19 @@ tgsi_exec_machine_bind_shader(
 
if (mach->Processor == TGSI_PROCESSOR_GEOMETRY &&
!mach->UsedGeometryShader) {
-  struct tgsi_exec_vector *inputs =
- align_malloc(sizeof(struct tgsi_exec_vector) *
-  TGSI_MAX_PRIM_VERTICES * PIPE_MAX_ATTRIBS,
-  16);
-  struct tgsi_exec_vector *outputs =
- align_malloc(sizeof(struct tgsi_exec_vector) *
-  TGSI_MAX_TOTAL_VERTICES, 16);
+  struct tgsi_exec_vector *inputs;
+  struct tgsi_exec_vector *outputs;
+
+  inputs = align_malloc(sizeof(struct tgsi_exec_vector) *
+TGSI_MAX_PRIM_VERTICES * PIPE_MAX_ATTRIBS,
+16);
 
   if (!inputs)
  return;
+
+  outputs = align_malloc(sizeof(struct tgsi_exec_vector) *
+ TGSI_MAX_TOTAL_VERTICES, 16);
+
   if (!outputs) {
  align_free(inputs);
  return;
-- 
1.7.1

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


Re: [Mesa-dev] [PATCH] gallium: implement WGL_ARB_create_context

2011-10-25 Thread morgan armand
Here is the modified patch. I just checked if it works but doesn't
have a good test case for it. But I will write one for sure!

---
 src/gallium/state_trackers/wgl/SConscript  |1 +
 src/gallium/state_trackers/wgl/stw_context.c   |   47 +++-
 src/gallium/state_trackers/wgl/stw_context.h   |3 +
 src/gallium/state_trackers/wgl/stw_ext_context.c   |  119 
 .../state_trackers/wgl/stw_getprocaddress.c|3 +
 5 files changed, 166 insertions(+), 7 deletions(-)
 create mode 100644 src/gallium/state_trackers/wgl/stw_ext_context.c

diff --git a/src/gallium/state_trackers/wgl/SConscript
b/src/gallium/state_trackers/wgl/SConscript
index 7cb953b..1014b45 100644
--- a/src/gallium/state_trackers/wgl/SConscript
+++ b/src/gallium/state_trackers/wgl/SConscript
@@ -22,6 +22,7 @@ if not env['gles']:
 sources = [
 'stw_context.c',
 'stw_device.c',
+'stw_ext_context.c',
 'stw_ext_extensionsstring.c',
 'stw_ext_gallium.c',
 'stw_ext_pbuffer.c',
diff --git a/src/gallium/state_trackers/wgl/stw_context.c
b/src/gallium/state_trackers/wgl/stw_context.c
index c2839fe..6cc8a83 100644
--- a/src/gallium/state_trackers/wgl/stw_context.c
+++ b/src/gallium/state_trackers/wgl/stw_context.c
@@ -27,6 +27,11 @@

 #include 

+#define WGL_WGLEXT_PROTOTYPES
+
+#include 
+#include 
+
 #include "pipe/p_compiler.h"
 #include "pipe/p_context.h"
 #include "pipe/p_state.h"
@@ -121,23 +126,41 @@ DrvCreateLayerContext(
HDC hdc,
INT iLayerPlane )
 {
+   return stw_create_context_attribs(hdc, iLayerPlane, 0, 1, 0, 0,
WGL_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB);
+}
+
+DHGLRC
+stw_create_context_attribs(
+   HDC hdc,
+   INT iLayerPlane,
+   DHGLRC hShareContext,
+   int majorVersion, int minorVersion,
+   int contextFlags, int profileMask)
+{
int iPixelFormat;
const struct stw_pixelformat_info *pfi;
struct st_context_attribs attribs;
struct stw_context *ctx = NULL;
-
-   if(!stw_dev)
+   struct stw_context *shareCtx = NULL;
+
+   if (!stw_dev)
   return 0;
-
+
if (iLayerPlane != 0)
   return 0;

iPixelFormat = GetPixelFormat(hdc);
if(!iPixelFormat)
   return 0;
-
+
pfi = stw_pixelformat_get_info( iPixelFormat - 1 );
-
+
+   if (hShareContext != 0) {
+  pipe_mutex_lock( stw_dev->ctx_mutex );
+  shareCtx = stw_lookup_context_locked( hShareContext );
+  pipe_mutex_unlock( stw_dev->ctx_mutex );
+   }
+
ctx = CALLOC_STRUCT( stw_context );
if (ctx == NULL)
   goto no_ctx;
@@ -148,10 +171,20 @@ DrvCreateLayerContext(
memset(&attribs, 0, sizeof(attribs));
attribs.profile = ST_PROFILE_DEFAULT;
attribs.visual = pfi->stvis;
+   attribs.major = majorVersion;
+   attribs.minor = minorVersion;
+   if (contextFlags & WGL_CONTEXT_FORWARD_COMPATIBLE_BIT_ARB)
+  attribs.flags |= ST_CONTEXT_FLAG_FORWARD_COMPATIBLE;
+   if (contextFlags & WGL_CONTEXT_DEBUG_BIT_ARB)
+  attribs.flags |= ST_CONTEXT_FLAG_DEBUG;
+   if (profileMask & WGL_CONTEXT_CORE_PROFILE_BIT_ARB)
+  attribs.flags |= ST_CONTEXT_FLAG_CORE_PROFILE;
+   if (profileMask & WGL_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB)
+  attribs.flags |= ST_CONTEXT_FLAG_COMPATIBLE_PROFILE;

ctx->st = stw_dev->stapi->create_context(stw_dev->stapi,
- stw_dev->smapi, &attribs, NULL);
-   if (ctx->st == NULL)
+ stw_dev->smapi, &attribs, shareCtx ? shareCtx->st : NULL);
+   if (ctx->st == NULL)
   goto no_st_ctx;

ctx->st->st_manager_private = (void *) ctx;
diff --git a/src/gallium/state_trackers/wgl/stw_context.h
b/src/gallium/state_trackers/wgl/stw_context.h
index 0bbed84..07a5c7d 100644
--- a/src/gallium/state_trackers/wgl/stw_context.h
+++ b/src/gallium/state_trackers/wgl/stw_context.h
@@ -43,6 +43,9 @@ struct stw_context
struct stw_framebuffer *current_framebuffer;
 };

+DHGLRC stw_create_context_attribs( HDC hdc, INT iLayerPlane, DHGLRC
hShareContext,
+   int majorVersion, int
minorVersion, int contextFlags, int profileMask );
+
 DHGLRC stw_get_current_context( void );

 HDC stw_get_current_dc( void );
diff --git a/src/gallium/state_trackers/wgl/stw_ext_context.c
b/src/gallium/state_trackers/wgl/stw_ext_context.c
new file mode 100644
index 000..a3470ac
--- /dev/null
+++ b/src/gallium/state_trackers/wgl/stw_ext_context.c
@@ -0,0 +1,119 @@
+/*
+ * Mesa 3-D graphics library
+ * Version:  7.11
+ *
+ * Copyright (C) 2011 Morgan Armand 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included
+ * in all copies or substa

[Mesa-dev] [PATCH] swrast: Fix memory leak in out-of-memory path.

2011-10-25 Thread Vinson Lee
Fixes Coverity resource leak defect.
---
 src/mesa/swrast/s_texcombine.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/mesa/swrast/s_texcombine.c b/src/mesa/swrast/s_texcombine.c
index c67c356..a7cbb44 100644
--- a/src/mesa/swrast/s_texcombine.c
+++ b/src/mesa/swrast/s_texcombine.c
@@ -108,6 +108,7 @@ texture_combine( struct gl_context *ctx, GLuint unit, 
GLuint n,
 i--;
  }
  _mesa_error(ctx, GL_OUT_OF_MEMORY, "texture_combine");
+ free(rgba);
  return;
   }
}
-- 
1.7.1

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


Re: [Mesa-dev] [PATCH 1/3] mesa: Fold gallium's texture border stripping into a core Mesa option.

2011-10-25 Thread Kenneth Graunke
On 10/24/2011 04:15 PM, Eric Anholt wrote:
> We wanted to reuse this in the Intel driver.

As the Fedora Hot Dog theme chooser applet(*) says: "YES" or "HELL YES"!

For the series:
Reviewed-by: Kenneth Graunke 

(*) http://fedoraproject.org/wiki/Features/Hot_Dog

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


Re: [Mesa-dev] [PATCH 3/8] mesa: Add interpolation override bitfields.

2011-10-25 Thread Brian Paul

On 10/24/2011 05:37 PM, Paul Berry wrote:

On 24 October 2011 15:58, Brian Paul mailto:bri...@vmware.com>> wrote:

On 10/24/2011 03:38 PM, Paul Berry wrote:

This patch adds the bitfields InterpOverridesFlat,
InterpOverridesSmooth, and InterpOverridesNoperspective to
gl_fragment_program.  These bitfields keep track of which fragment
shader inputs are overridden with the GLSL "flat", "smooth", and
"noperspective" interpolation qualifiers.


The names of those fields seems a little confusing to me.  For
example, "InterpOverridesFlat" sounds like a field that overrides
flat shading with something else.

How about just "InterpFlat", "InterpSmooth", etc?


The meaning I was trying to convey with "overrides" is that bits are
only set in these bitfields if the interpolation mode is overridden in
GLSL.  For fragment shader inputs that don't have their interpolation
mode overridden, all of the bitfields have a zero.  (I had to do this
in order to avoid adding a bunch of code to the handling of fixed
function fragment shading and ARB fragment programs).


Or, how about an enum that indicates the interpolation mode for
each fragment shader input?  Something like this:

enum interp_mode
{
   INTERP_DEFAULT,  /* I _think_ we need a default mode, right? */
   INTERP_FLAT,
   INTERP_SMOOTH,
   INTERP_NO_PERSPECTIVE
};

struct gl_fragment_program
{
   ...
   enum interp_mode InterpMode[FRAG_ATTRIB_MAX];
   ...
};


This would also prevent a non-sensical state where a particular
bit is accidentally set in more than one of the masks.


Yeah, I prefer this too, and that's essentially what I did in patch 2
of the series.  To be honest I find all the bitfield stuff rather
ugly, and I would rather drop this patch entirely, but a lot of the
i965 code still relies on things being bitfields.  Is it the only
driver that does?


Which bitfields are you referring to specifically?  I think the ones 
in gl_program are used in the gallium state tracker, but I'd have to 
double-check.




If so, I suppose I might could be convinced to move
all the bitfield handling of interpolation into i965 where it won't
contaminate the other implementations.


I don't know what works best for i965 but using an enumerated type for 
the interpolation modes seems cleaner for core Mesa.




Finally, maybe use "CONSTANT", "LINEAR" and "PERSPECTIVE" instead
of "FLAT", "NO_PERSPECTIVE" and "SMOOTH".  That's what we have in
gallium.


Hmm, I'm ambivalent about this one.  The terms "linear" and
"perspective" are certainly clearer than "noperspective" and "smooth",
but "constant" seems downright misleading for flatshaded attributes,
since they are only constant across a single primitive.  I also think
there's a lot to be said for being consistent with GLSL terminology,
which is to call these things "flat", "noperspective", and "smooth".


It's not a huge deal to me either.



I also admit I'm a little surprised to hear that Gallium has already
dealt with this question.  In my greps, I couldn't find any Gallium
code that was referring to the ir_variable::interpolation field, so I
assumed i965 was the first driver to try to implement interpolation
qualifiers.  Did I miss something?  If I did, then I probably broke
gallium when I renamed things in patch 1 of the series.


The gallium state tracker doesn't yet handle interpolation qualifiers 
for Mesa's fragment programs, but we emit them when we create TGSI 
fragment program input register declarations (frag pos is linear, 
texcoords are perspective, etc).  For implementing glShadeModel(), 
however, we still rely on a rasterization state flag rather than emit 
different fragment programs with different input qualifiers.  That's 
something I'd like to fix someday.


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


Re: [Mesa-dev] [PATCH 3/8] mesa: Add interpolation override bitfields.

2011-10-25 Thread Brian Paul

On 10/24/2011 05:47 PM, Paul Berry wrote:

On 24 October 2011 16:37, Paul Berry mailto:stereotype...@gmail.com>> wrote:

On 24 October 2011 15:58, Brian Paul mailto:bri...@vmware.com>> wrote:

On 10/24/2011 03:38 PM, Paul Berry wrote:

This patch adds the bitfields InterpOverridesFlat,
InterpOverridesSmooth, and InterpOverridesNoperspective to
gl_fragment_program.  These bitfields keep track of which
fragment
shader inputs are overridden with the GLSL "flat",
"smooth", and
"noperspective" interpolation qualifiers.


The names of those fields seems a little confusing to me.  For
example, "InterpOverridesFlat" sounds like a field that
overrides flat shading with something else.

How about just "InterpFlat", "InterpSmooth", etc?


The meaning I was trying to convey with "overrides" is that bits
are only set in these bitfields if the interpolation mode is
overridden in GLSL.  For fragment shader inputs that don't have
their interpolation mode overridden, all of the bitfields have a
zero.  (I had to do this in order to avoid adding a bunch of code
to the handling of fixed function fragment shading and ARB
fragment programs).


Another thought: how would you feel about "InterpQualifiedFlat",
"InterpQualifiedSmooth", etc?  That would satisfy my desire to clarify
that these bits are only set when interpolation is overridden in GLSL,
but without the potential for the misinterpretation you were concerned
about.


If we stick with bitfields, that looks better to me.

-Brian

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


Re: [Mesa-dev] [PATCH] swrast: Fix memory leak in out-of-memory path.

2011-10-25 Thread Brian Paul

On 10/25/2011 11:12 AM, Vinson Lee wrote:

Fixes Coverity resource leak defect.
---
  src/mesa/swrast/s_texcombine.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/mesa/swrast/s_texcombine.c b/src/mesa/swrast/s_texcombine.c
index c67c356..a7cbb44 100644
--- a/src/mesa/swrast/s_texcombine.c
+++ b/src/mesa/swrast/s_texcombine.c
@@ -108,6 +108,7 @@ texture_combine( struct gl_context *ctx, GLuint unit, 
GLuint n,
  i--;
   }
   _mesa_error(ctx, GL_OUT_OF_MEMORY, "texture_combine");
+ free(rgba);
   return;
}
 }


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


Re: [Mesa-dev] [PATCH] tgsi: Fix memory leak in out-of-memory path.

2011-10-25 Thread Brian Paul

On 10/25/2011 09:26 AM, Vinson Lee wrote:

Fixes Coverity resource leak defect.
---
  src/gallium/auxiliary/tgsi/tgsi_exec.c |   17 ++---
  1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c 
b/src/gallium/auxiliary/tgsi/tgsi_exec.c
index b4eea54..1fb7f8f 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_exec.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c
@@ -674,16 +674,19 @@ tgsi_exec_machine_bind_shader(

 if (mach->Processor == TGSI_PROCESSOR_GEOMETRY&&
 !mach->UsedGeometryShader) {
-  struct tgsi_exec_vector *inputs =
- align_malloc(sizeof(struct tgsi_exec_vector) *
-  TGSI_MAX_PRIM_VERTICES * PIPE_MAX_ATTRIBS,
-  16);
-  struct tgsi_exec_vector *outputs =
- align_malloc(sizeof(struct tgsi_exec_vector) *
-  TGSI_MAX_TOTAL_VERTICES, 16);
+  struct tgsi_exec_vector *inputs;
+  struct tgsi_exec_vector *outputs;
+
+  inputs = align_malloc(sizeof(struct tgsi_exec_vector) *
+TGSI_MAX_PRIM_VERTICES * PIPE_MAX_ATTRIBS,
+16);

if (!inputs)
   return;
+
+  outputs = align_malloc(sizeof(struct tgsi_exec_vector) *
+ TGSI_MAX_TOTAL_VERTICES, 16);
+
if (!outputs) {
   align_free(inputs);
   return;


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


[Mesa-dev] [PATCH] state_trackers/vdpau: Implement VdpGenerateCSCMatrix partially

2011-10-25 Thread Maarten Lankhorst
Signed-off-by: Maarten Lankhorst struct_version > VDP_PROCAMP_VERSION)
+  return VDP_STATUS_INVALID_STRUCT_VERSION;
+
+   switch (standard) {
+  case VDP_COLOR_STANDARD_ITUR_BT_601: Kry = 0.299; Kby = .114; break;
+  case VDP_COLOR_STANDARD_ITUR_BT_709: Kry = 0.2126; Kby = .0722; break;
+  case VDP_COLOR_STANDARD_SMPTE_240M:  Kry = 0.212; Kby = .087; break;
+  default: return VDP_STATUS_INVALID_COLOR_STANDARD;
+   }
+   if (procamp->hue != 0.f || procamp->saturation != 1.f || procamp->contrast 
!= 1.f || procamp->brightness != 0.f)
+  VDPAU_MSG(VDPAU_WARN, "No support yet for color options\n");
+
+   mat[0][0] = mat[1][0] = mat[2][0] = 255. / Ry;
+   mat[0][3] = mat[1][3] = mat[2][3] = -ofsy / Ry;
+   Scr = (1. - Kry) / Rc;
+   mat[0][1] = 0.;
+   mat[0][2] = 255. * Scr;
+   mat[0][3] -= 128. * Scr;
+   Scb = -(1. - Kby) * Kby / ((1. - Kby) + (1. - Kry) - 1.) / Rc;
+   Scr = -(1. - Kry) * Kry / ((1. - Kby) + (1. - Kry) - 1.) / Rc;
+   mat[1][1] = 255. * Scb;
+   mat[1][2] = 255. * Scr;
+   mat[1][3] -= 128. * (Scb + Scr);
+   Scb = (1. - Kby) / Rc;
+   mat[2][1] = 255. * Scb;
+   mat[2][2] = 0.;
+   mat[2][3] -= 128. * Scb;
return VDP_STATUS_OK;
 }


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


Re: [Mesa-dev] [PATCH] state_trackers/vdpau: Implement VdpGenerateCSCMatrix partially

2011-10-25 Thread Younes Manton
On Tue, Oct 25, 2011 at 1:35 PM, Maarten Lankhorst
 wrote:
> Signed-off-by: Maarten Lankhorst 
> ---
>
> diff --git a/src/gallium/state_trackers/vdpau/mixer.c 
> b/src/gallium/state_trackers/vdpau/mixer.c
> index 8728157..83daddf 100644
> --- a/src/gallium/state_trackers/vdpau/mixer.c
> +++ b/src/gallium/state_trackers/vdpau/mixer.c
> @@ -257,9 +257,37 @@ vlVdpGenerateCSCMatrix(VdpProcamp *procamp,
>                        VdpColorStandard standard,
>                        VdpCSCMatrix *csc_matrix)
>  {
> -   VDPAU_MSG(VDPAU_TRACE, "[VDPAU] Generating CSCMatrix\n");
> +   double Kry, Kby, Ry = 219., Rc = 112., ofsy = 16., Scb, Scr;
> +   VdpProcamp mat;
>    if (!(csc_matrix && procamp))
>       return VDP_STATUS_INVALID_POINTER;
>
> +   if (procamp->struct_version > VDP_PROCAMP_VERSION)
> +      return VDP_STATUS_INVALID_STRUCT_VERSION;
> +
> +   switch (standard) {
> +      case VDP_COLOR_STANDARD_ITUR_BT_601: Kry = 0.299; Kby = .114; break;
> +      case VDP_COLOR_STANDARD_ITUR_BT_709: Kry = 0.2126; Kby = .0722; break;
> +      case VDP_COLOR_STANDARD_SMPTE_240M:  Kry = 0.212; Kby = .087; break;
> +      default: return VDP_STATUS_INVALID_COLOR_STANDARD;
> +   }
> +   if (procamp->hue != 0.f || procamp->saturation != 1.f || 
> procamp->contrast != 1.f || procamp->brightness != 0.f)
> +      VDPAU_MSG(VDPAU_WARN, "No support yet for color options\n");
> +
> +   mat[0][0] = mat[1][0] = mat[2][0] = 255. / Ry;
> +   mat[0][3] = mat[1][3] = mat[2][3] = -ofsy / Ry;
> +   Scr = (1. - Kry) / Rc;
> +   mat[0][1] = 0.;
> +   mat[0][2] = 255. * Scr;
> +   mat[0][3] -= 128. * Scr;
> +   Scb = -(1. - Kby) * Kby / ((1. - Kby) + (1. - Kry) - 1.) / Rc;
> +   Scr = -(1. - Kry) * Kry / ((1. - Kby) + (1. - Kry) - 1.) / Rc;
> +   mat[1][1] = 255. * Scb;
> +   mat[1][2] = 255. * Scr;
> +   mat[1][3] -= 128. * (Scb + Scr);
> +   Scb = (1. - Kby) / Rc;
> +   mat[2][1] = 255. * Scb;
> +   mat[2][2] = 0.;
> +   mat[2][3] -= 128. * Scb;
>    return VDP_STATUS_OK;
>  }
>

Is there a reason not to use/add to the code in vl_csc.c in this case?
It supports 601 and 709, wider gamut, and and it takes procamp into
account. It calculates a 4x4 matrix, but you can just drop the 4th row
for VDPAU.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] state_trackers/vdpau: Implement VdpGenerateCSCMatrix partially

2011-10-25 Thread Maarten Lankhorst
On 10/25/2011 08:00 PM, Younes Manton wrote:
> On Tue, Oct 25, 2011 at 1:35 PM, Maarten Lankhorst
>  wrote:
>> 
> Is there a reason not to use/add to the code in vl_csc.c in this case?
> It supports 601 and 709, wider gamut, and and it takes procamp into
> account. It calculates a 4x4 matrix, but you can just drop the 4th row
> for VDPAU.
Good suggestion, the main reason is that I was not aware of its existence. 
Oops. :)

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


[Mesa-dev] [PATCH 1/5] mesa: Skip texstore for 0-sized texture data.

2011-10-25 Thread Eric Anholt
The intel driver (and gallium, it looks like, though it doesn't use
these texstore functions at this point) doesn't bother making storage
for textures with 0 width, height, or depth.  This avoids them having
to deal with returning a mapping for that nonexistent data.

Fixes assertion failures with an upcoming intel driver change.
---
 src/mesa/main/texstore.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/src/mesa/main/texstore.c b/src/mesa/main/texstore.c
index cd92496..05c1964 100644
--- a/src/mesa/main/texstore.c
+++ b/src/mesa/main/texstore.c
@@ -4499,6 +4499,9 @@ _mesa_store_teximage1d(struct gl_context *ctx, GLenum 
target, GLint level,
 
(void) border;
 
+   if (width == 0)
+  return;
+
/* allocate storage for texture data */
if (!ctx->Driver.AllocTextureImageBuffer(ctx, texImage, texImage->TexFormat,
 width, 1, 1)) {
@@ -4560,6 +4563,9 @@ _mesa_store_teximage2d(struct gl_context *ctx, GLenum 
target, GLint level,
 
(void) border;
 
+   if (width == 0 || height == 0)
+  return;
+
/* allocate storage for texture data */
if (!ctx->Driver.AllocTextureImageBuffer(ctx, texImage, texImage->TexFormat,
 width, height, 1)) {
@@ -4651,6 +4657,9 @@ _mesa_store_teximage3d(struct gl_context *ctx, GLenum 
target, GLint level,
 
(void) border;
 
+   if (width == 0 || height == 0 || depth == 0)
+  return;
+
/* allocate storage for texture data */
if (!ctx->Driver.AllocTextureImageBuffer(ctx, texImage, texImage->TexFormat,
 width, height, depth)) {
-- 
1.7.7

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


Re: [Mesa-dev] [PATCH 1/3] mesa: Fold gallium's texture border

2011-10-25 Thread Eric Anholt
On Mon, 24 Oct 2011 19:23:53 -0600, Brian Paul  wrote:
> On Mon, Oct 24, 2011 at 5:15 PM, Eric Anholt  wrote:
> > @@ -2322,6 +2363,16 @@ teximage(struct gl_context *ctx, GLuint dims,
> >          return;   /* error was recorded */
> >       }
> >
> > +      /* Allow a hardware driver to just strip out the border, to provide
> > +       * reliable but slightly incorrect hardware rendering instead of
> > +       * rarely-tested software fallback rendering.
> > +       */
> > +      if (border && ctx->Texture.StripTextureBorder) {
> > +        strip_texture_border(&border, &width, &height, &depth, unpack,
> > +                             &unpack_no_border);
> > +        unpack = &unpack_no_border;
> > +      }
> > +
> 
> I think we also need this code in the glCopyTexImage() path.
> 
> -Brian

Good point.  I made the fix for that, then made a testcase, then fixed
the fix.

I also ended up deleting a bit more Intel code, which caught some
silliness where we do mappings to store data for 0-sized textures,
which made the driver now assert in an ES2 conformance case after
deleting more Border code.  I think this is a good fix for the
problem, by just bailing out of texstore instead of making the caller
avoid texstore in these cases.

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


[Mesa-dev] [PATCH 2/5] mesa: Fold gallium's texture border stripping into a core Mesa option.

2011-10-25 Thread Eric Anholt
We wanted to reuse this in the Intel driver.
---
 src/mesa/main/mtypes.h |   14 
 src/mesa/main/teximage.c   |   57 +--
 src/mesa/state_tracker/st_cb_texture.c |   58 ++--
 src/mesa/state_tracker/st_extensions.c |2 +
 4 files changed, 73 insertions(+), 58 deletions(-)

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 719dff3..4117686 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2732,6 +2732,20 @@ struct gl_constants
 
/* GL_ARB_robustness */
GLenum ResetStrategy;
+
+   /**
+* Whether the implementation strips out and ignores texture borders.
+*
+* Many GPU hardware implementations don't support rendering with texture
+* borders and mipmapped textures.  (Note: not static border color, but the
+* old 1-pixel border around each edge).  Implementations then have to do
+* slow fallbacks to be correct, or just ignore the border and be fast but
+* wrong.  Setting the flag stripts the border off of TexImage calls,
+* providing "fast but wrong" at significantly reduced driver complexity.
+*
+* Texture borders are deprecated in GL 3.0.
+**/
+   GLboolean StripTextureBorder;
 };
 
 
diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index 798201a..a93ae94 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -2246,6 +2246,45 @@ _mesa_choose_texture_format(struct gl_context *ctx,
return f;
 }
 
+/**
+ * Adjust pixel unpack params and image dimensions to strip off the
+ * texture border.
+ *
+ * Gallium and intel don't support texture borders.  They've seldem been used
+ * and seldom been implemented correctly anyway.
+ *
+ * \param unpackNew returns the new pixel unpack parameters
+ */
+static void
+strip_texture_border(GLint *border,
+ GLint *width, GLint *height, GLint *depth,
+ const struct gl_pixelstore_attrib *unpack,
+ struct gl_pixelstore_attrib *unpackNew)
+{
+   assert(*border > 0);  /* sanity check */
+
+   *unpackNew = *unpack;
+
+   if (unpackNew->RowLength == 0)
+  unpackNew->RowLength = *width;
+
+   if (depth && unpackNew->ImageHeight == 0)
+  unpackNew->ImageHeight = *height;
+
+   unpackNew->SkipPixels += *border;
+   if (height)
+  unpackNew->SkipRows += *border;
+   if (depth)
+  unpackNew->SkipImages += *border;
+
+   assert(*width >= 3);
+   *width = *width - 2 * *border;
+   if (height && *height >= 3)
+  *height = *height - 2 * *border;
+   if (depth && *depth >= 3)
+  *depth = *depth - 2 * *border;
+   *border = 0;
+}
 
 /**
  * Common code to implement all the glTexImage1D/2D/3D functions.
@@ -2258,6 +2297,8 @@ teximage(struct gl_context *ctx, GLuint dims,
  const GLvoid *pixels)
 {
GLboolean error;
+   struct gl_pixelstore_attrib unpack_no_border;
+   const struct gl_pixelstore_attrib *unpack = &ctx->Unpack;
 
ASSERT_OUTSIDE_BEGIN_END_AND_FLUSH(ctx);
 
@@ -2322,6 +2363,16 @@ teximage(struct gl_context *ctx, GLuint dims,
  return;   /* error was recorded */
   }
 
+  /* Allow a hardware driver to just strip out the border, to provide
+   * reliable but slightly incorrect hardware rendering instead of
+   * rarely-tested software fallback rendering.
+   */
+  if (border && ctx->Const.StripTextureBorder) {
+strip_texture_border(&border, &width, &height, &depth, unpack,
+ &unpack_no_border);
+unpack = &unpack_no_border;
+  }
+
   if (ctx->NewState & _NEW_PIXEL)
 _mesa_update_state(ctx);
 
@@ -2354,19 +2405,19 @@ teximage(struct gl_context *ctx, GLuint dims,
case 1:
   ctx->Driver.TexImage1D(ctx, target, level, internalFormat,
  width, border, format,
- type, pixels, &ctx->Unpack, texObj,
+ type, pixels, unpack, texObj,
  texImage);
   break;
case 2:
   ctx->Driver.TexImage2D(ctx, target, level, internalFormat,
  width, height, border, format,
- type, pixels, &ctx->Unpack, texObj,
+ type, pixels, unpack, texObj,
  texImage);
   break;
case 3:
   ctx->Driver.TexImage3D(ctx, target, level, internalFormat,
  width, height, depth, border, format,
- type, pixels, &ctx->Unpack, texObj,
+ type, pixels, unpack, texObj,
  texImage);
   break;
default:
diff --git a/src/

[Mesa-dev] [PATCH 4/5] intel: Enable stripping of texture borders.

2011-10-25 Thread Eric Anholt
This replaces software rendering of textures with the deprecated
1-pixel border (which is always bad, since mipmapping is rather broken
in swrast, and GLSL 1.30 is unsupported) with hardware rendering that
just pretends there was never a border (so you have potential seams on
apps that actually intentionally used the 1-pixel borders, but correct
rendering otherwise).

This doesn't regress any piglit tests on gen6 (since the texwrap
border/bordercolor cases already failed due to broken border color
handling), but regresses texwrap border cases on original gen4 since
those end up sampling the border color instead of the border pixels.
It's a small price to pay for not thinking about texture borders any
more.
---
 src/mesa/drivers/dri/intel/intel_context.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_context.c 
b/src/mesa/drivers/dri/intel/intel_context.c
index 433cd2e..48471fa 100644
--- a/src/mesa/drivers/dri/intel/intel_context.c
+++ b/src/mesa/drivers/dri/intel/intel_context.c
@@ -791,6 +791,8 @@ intelInitContext(struct intel_context *intel,
if (intel->gen >= 6)
   ctx->Const.MaxClipPlanes = 8;
 
+   ctx->Const.StripTextureBorder = GL_TRUE;
+
/* reinitialize the context point state.
 * It depend on constants in __struct gl_contextRec::Const
 */
-- 
1.7.7

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


[Mesa-dev] [PATCH 3/5] mesa: Apply StripTextureBorder to CopyTexImage as well.

2011-10-25 Thread Eric Anholt
---
 src/mesa/main/teximage.c |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index a93ae94..acf7187 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -2713,6 +2713,16 @@ copyteximage(struct gl_context *ctx, GLuint dims,
 
texObj = _mesa_get_current_tex_object(ctx, target);
 
+   if (border && ctx->Const.StripTextureBorder) {
+  x += border;
+  width -= border * 2;
+  if (dims == 2) {
+y += border;
+height -= border * 2;
+  }
+  border = 0;
+   }
+
_mesa_lock_texture(ctx, texObj);
{
   texImage = _mesa_get_tex_image(ctx, texObj, target, level);
-- 
1.7.7

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


[Mesa-dev] [PATCH] copyteximage-border: New test for glCopyTexImage2D() vs texture borders.

2011-10-25 Thread Eric Anholt
This test ignores the actual border pixels, because we don't care.
Just make sure the body of the texture works.
---
 tests/all.tests   |1 +
 tests/texturing/CMakeLists.gl.txt |1 +
 tests/texturing/copyteximage-border.c |  109 +
 3 files changed, 111 insertions(+), 0 deletions(-)
 create mode 100644 tests/texturing/copyteximage-border.c

diff --git a/tests/all.tests b/tests/all.tests
index 760487a..c076808 100644
--- a/tests/all.tests
+++ b/tests/all.tests
@@ -613,6 +613,7 @@ add_concurrent_test(texturing, '1-1-linear-texture')
 add_plain_test(texturing, 'array-texture')
 add_plain_test(texturing, 'copytexsubimage')
 add_plain_test(texturing, 'copyteximage')
+add_plain_test(texturing, 'copyteximage-border')
 add_plain_test(texturing, 'copyteximage-clipping')
 add_plain_test(texturing, 'cubemap')
 texturing['cubemap npot'] = PlainExecTest(['cubemap', '-auto', 'npot'])
diff --git a/tests/texturing/CMakeLists.gl.txt 
b/tests/texturing/CMakeLists.gl.txt
index a801dc5..ac9b91d 100644
--- a/tests/texturing/CMakeLists.gl.txt
+++ b/tests/texturing/CMakeLists.gl.txt
@@ -17,6 +17,7 @@ add_executable (1-1-linear-texture 1-1-linear-texture.c)
 add_executable (array-texture array-texture.c)
 add_executable (copytexsubimage copytexsubimage.c)
 add_executable (copyteximage copyteximage.c)
+add_executable (copyteximage-border copyteximage-border.c)
 add_executable (copyteximage-clipping copyteximage-clipping.c)
 IF(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
add_executable (crossbar crossbar.c)
diff --git a/tests/texturing/copyteximage-border.c 
b/tests/texturing/copyteximage-border.c
new file mode 100644
index 000..13132a4
--- /dev/null
+++ b/tests/texturing/copyteximage-border.c
@@ -0,0 +1,109 @@
+/*
+ * Copyright © 2011 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/**
+ * Test that glCopyTexImage() into a texture with a texture border
+ * gets correct non-border texels.
+ */
+
+
+#include "piglit-util.h"
+
+/* Size of the body of the texture, not counting border. */
+#define TEX_SIZE 64
+
+int piglit_width = TEX_SIZE * 2 + 30;
+int piglit_height = TEX_SIZE + 20;
+int piglit_window_mode = GLUT_DOUBLE | GLUT_RGB | GLUT_ALPHA;
+
+enum piglit_result
+piglit_display(void)
+{
+   bool pass = true;
+   int quad_w = TEX_SIZE / 2;
+   int quad_h = TEX_SIZE / 2;
+   float red[]   = {1.0, 0.0, 0.0, 0.0};
+   float green[] = {0.0, 1.0, 0.0, 0.0};
+   float blue[]  = {0.0, 0.0, 1.0, 0.0};
+   float white[] = {1.0, 1.0, 1.0, 0.0};
+   GLuint tex;
+   int x, y;
+
+   piglit_ortho_projection(piglit_width, piglit_height, false);
+
+   glClearColor(0.5, 0.5, 0.5, 0.5);
+   glClear(GL_COLOR_BUFFER_BIT);
+
+   x = 10;
+   y = 10;
+
+   glColor4fv(red);
+   piglit_draw_rect(x, y, quad_w, quad_h);
+   glColor4fv(green);
+   piglit_draw_rect(x + quad_w, y, quad_w, quad_h);
+   glColor4fv(blue);
+   piglit_draw_rect(x, y + quad_h, quad_w, quad_h);
+   glColor4fv(white);
+   piglit_draw_rect(x + quad_w, y + quad_h, quad_w, quad_h);
+
+   glGenTextures(1, &tex);
+   glBindTexture(GL_TEXTURE_2D, tex);
+   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+
+   /* Copy the rectangle drawn to our texture, with a 1-pixel
+* border around it.
+*/
+   glCopyTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA,
+x - 1, y - 1,
+TEX_SIZE + 2, TEX_SIZE + 2,
+1);
+
+   x = 20 + TEX_SIZE;
+   y = 10;
+
+   glEnable(GL_TEXTURE_2D);
+   glTexEnvi(GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_REPLACE);
+   piglit_draw_rect_tex(x, y, TEX_SIZE, TEX_SIZE,
+0, 0, 1, 1);
+   glDisable(GL_TEXTURE

[Mesa-dev] [PATCH 5/5] intel: Drop texture border support code.

2011-10-25 Thread Eric Anholt
Now that texture borders are gone, we never need to allocate our
textures through non-miptrees, which simplifies some irritating paths.

(There's additional code to be removed in copy_miptree_to_teximage,
which Chad has a patch for)
---
 src/mesa/drivers/dri/i965/brw_fallback.c|   17 +--
 src/mesa/drivers/dri/intel/intel_mipmap_tree.c  |4 --
 src/mesa/drivers/dri/intel/intel_tex.c  |   58 ---
 src/mesa/drivers/dri/intel/intel_tex_image.c|   36 +++---
 src/mesa/drivers/dri/intel/intel_tex_validate.c |7 ---
 5 files changed, 29 insertions(+), 93 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fallback.c 
b/src/mesa/drivers/dri/i965/brw_fallback.c
index ae08cf5..708bd0e 100644
--- a/src/mesa/drivers/dri/i965/brw_fallback.c
+++ b/src/mesa/drivers/dri/i965/brw_fallback.c
@@ -42,7 +42,6 @@
 static bool do_check_fallback(struct brw_context *brw)
 {
struct gl_context *ctx = &brw->intel.ctx;
-   GLuint i;
 
if (brw->intel.no_rast) {
   DBG("FALLBACK: rasterization disabled\n");
@@ -56,20 +55,6 @@ static bool do_check_fallback(struct brw_context *brw)
   return true;
}
 
-   /* _NEW_TEXTURE:
-*/
-   for (i = 0; i < BRW_MAX_TEX_UNIT; i++) {
-  struct gl_texture_unit *texUnit = &ctx->Texture.Unit[i];
-  if (texUnit->_ReallyEnabled) {
-struct gl_texture_object *tex_obj = texUnit->_Current;
-struct gl_texture_image *texImage = 
tex_obj->Image[0][tex_obj->BaseLevel];
-if (texImage->Border) {
-   DBG("FALLBACK: texture border\n");
-   return true;
-}
-  }
-   }
-
return false;
 }
 
@@ -80,7 +65,7 @@ static void check_fallback(struct brw_context *brw)
 
 const struct brw_tracked_state brw_check_fallback = {
.dirty = {
-  .mesa = _NEW_RENDERMODE | _NEW_TEXTURE | _NEW_STENCIL,
+  .mesa = _NEW_RENDERMODE | _NEW_STENCIL,
   .brw  = 0,
   .cache = 0
},
diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c 
b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
index 9eb81de..045de33 100644
--- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
@@ -246,10 +246,6 @@ intel_miptree_match_image(struct intel_mipmap_tree *mt,
GLuint level = intelImage->base.Base.Level;
int width, height, depth;
 
-   /* Images with borders are never pulled into mipmap trees. */
-   if (image->Border)
-  return false;
-
if (image->TexFormat != mt->format)
   return false;
 
diff --git a/src/mesa/drivers/dri/intel/intel_tex.c 
b/src/mesa/drivers/dri/intel/intel_tex.c
index 5806659..7af5138 100644
--- a/src/mesa/drivers/dri/intel/intel_tex.c
+++ b/src/mesa/drivers/dri/intel/intel_tex.c
@@ -61,6 +61,8 @@ intel_alloc_texture_image_buffer(struct gl_context *ctx,
struct intel_texture_object *intel_texobj = intel_texture_object(texobj);
GLuint slices;
 
+   assert(image->Border == 0);
+
/* Because the driver uses AllocTextureImageBuffer() internally, it may end
 * up mismatched with FreeTextureImageBuffer(), but that is safe to call
 * multiple times.
@@ -93,8 +95,7 @@ intel_alloc_texture_image_buffer(struct gl_context *ctx,
   DBG("%s: alloc obj %p level %d %dx%dx%d using object's miptree %p\n",
   __FUNCTION__, texobj, image->Level,
   width, height, depth, intel_texobj->mt);
-  return true;
-   } else if (image->Border == 0) {
+   } else {
   intel_image->mt = intel_miptree_create_for_teximage(intel, intel_texobj,
   intel_image,
   false);
@@ -114,14 +115,9 @@ intel_alloc_texture_image_buffer(struct gl_context *ctx,
   DBG("%s: alloc obj %p level %d %dx%dx%d using new miptree %p\n",
   __FUNCTION__, texobj, image->Level,
   width, height, depth, intel_image->mt);
-  return true;
}
 
-   DBG("%s: alloc obj %p level %d %dx%dx%d using swrast\n",
-   __FUNCTION__, texobj, image->Level, width, height, depth);
-
-   return _swrast_alloc_texture_image_buffer(ctx, image, format,
-width, height, depth);
+   return true;
 }
 
 static void
@@ -168,6 +164,11 @@ intel_map_texture_image(struct gl_context *ctx,
struct intel_texture_image *intel_image = intel_texture_image(tex_image);
struct intel_mipmap_tree *mt = intel_image->mt;
unsigned int bw, bh;
+   void *base;
+   unsigned int image_x, image_y;
+
+   /* Our texture data is always stored in a miptree. */
+   assert(mt);
 
/* Check that our caller wasn't confused about how to map a 1D texture. */
assert(tex_image->TexObject->Target != GL_TEXTURE_1D_ARRAY ||
@@ -190,36 +191,18 @@ intel_map_texture_image(struct gl_context *ctx,
assert(y % bh == 0);
y /= bh;
 
-   if (likely(mt)) {
-  void *base = intel_region_map(intel, mt->region, mode);
-  unsigned int image_x, image_y;
-
-  intel_miptree_get_imag

Re: [Mesa-dev] [PATCH 5/5] intel: Drop texture border support code.

2011-10-25 Thread Chad Versace
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 10/25/2011 02:58 PM, Eric Anholt wrote:
> Now that texture borders are gone, we never need to allocate our
> textures through non-miptrees, which simplifies some irritating paths.
> 
> (There's additional code to be removed in copy_miptree_to_teximage,
> which Chad has a patch for)

That patch is on master. So you can delete the parenthetical after rebasing.

- -- 
Chad Versace
c...@chad-versace.us
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJOpzSbAAoJEAIvNt057x8i7Z8QAKvUiLrgductiSfyjiZ1CChx
4Z8af4ci7h8Sj6co9JSt92QZyGrYouOmj3xvhK6Eqs/gZWNvsc58SslHV5i1UuI5
jEilWWuiFZo6URkmu4wIP0T4gzziNrBofvgK5A6dY8MREFpbHccYpW+P9HgYjLGp
qdUGhctekCXQlk/4a3pet4HtFii7tmOXCq9IeJpCOkj/gk95NhKshSu68e3Ohy4q
Q4aQZ4iYMYRaM++/2O6FlrBG/gSjnQ8M6fmrZHlqQed/ygjFj+dEeHBBCN+EKfF8
oYsTg+RzVbfd9Rw4MTj4N6a+m+LatFHn8z2NIHVL/hHdR/LmV+kW6aZ8c05CFHo6
hBXmRPw7KhoGlnG1aYEeLSU3zEYc2sfWKLEN40YSUO2pVDBnzv4s/KbyMyepKdYY
WEL4EHWhlOuALaSUVeMTTvshFXwtvQxNcuy/yiRlMxZod6TzIHQuDY30YUzParfO
ENfRynVtTzkc0mIvwVC0igK4JzvcuaZc3Y8kSykG/aCwkhB3x6J+gaibGxmlV6I9
DJqZXvu/jcgZYMa8JyoPlVNC5W6GsFKtWmANDM3r3NzONA2/6IL/XjO0IXcrY25X
OBmzobUgoMyuKc8TynRxjCS/FBpwHUKIh1OYyiuhhmoqWunKHklARxzt2GdxJVgf
FXg8w7+ZPwfA4EhkmlkD
=bldx
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/5] mesa: Skip texstore for 0-sized texture data.

2011-10-25 Thread Brian Paul

On 10/25/2011 03:58 PM, Eric Anholt wrote:

The intel driver (and gallium, it looks like, though it doesn't use
these texstore functions at this point) doesn't bother making storage
for textures with 0 width, height, or depth.  This avoids them having
to deal with returning a mapping for that nonexistent data.

Fixes assertion failures with an upcoming intel driver change.


For the series:  Reviewed-by: Brian Paul 


However, there are some additional issues with texture borders that 
have existed all along that we should probably fix up in follow-on 
patches.


Suppose we've got a GL_TEXTURE_2D_ARRAY texture, if there's a border, 
it only applies to the width and height, not the depth.  So we don't 
want to change the texture depth in this case.  Similarly for 1D 
texture arrays and the height.


We should fix this in the border stipping code and in 
_mesa_init_teximage_fields() and probably other places TBD.


A helper function like this might be useful to split up the 
user-provided border value into per-dimension values:


void
split_border(GLenum target, GLint border,
 GLint *w_border, GLint *h_border, GLint *d_border)
{
   switch (target) {
   case GL_TEXTURE_1D:
   case GL_TEXTURE_1D_ARRAY:
  *w_border = border;
  *h_border = *d_border = 0;
  break;
   case GL_TEXTURE_2D:
   case GL_TEXTURE_2D_ARRAY:
  *w_border = *h_border = border;
  *d_border = 0;
  break;
   case GL_TEXTURE_3D:
  *w_border = *h_border = *d_border = border;
  break;
   case ...
   }
}


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


[Mesa-dev] [PATCH 1/2] mesa: Add glsl_type::get_scalar_type() function.

2011-10-25 Thread Paul Berry
This function is similar to get_base_type(), but when called on
arrays, it returns the scalar type composing the array.  For example,
glsl_type(vec4[]) => float_type.
---
 src/glsl/glsl_types.cpp |   23 +++
 src/glsl/glsl_types.h   |   11 +++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index c94aec0..03e9987 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -258,6 +258,29 @@ const glsl_type *glsl_type::get_base_type() const
 }
 
 
+const glsl_type *glsl_type::get_scalar_type() const
+{
+   const glsl_type *type = this;
+
+   /* Handle arrays */
+   while (type->base_type == GLSL_TYPE_ARRAY)
+  type = type->fields.array;
+
+   /* Handle vectors and matrices */
+   switch (type->base_type) {
+   case GLSL_TYPE_UINT:
+  return uint_type;
+   case GLSL_TYPE_INT:
+  return int_type;
+   case GLSL_TYPE_FLOAT:
+  return float_type;
+   default:
+  /* Handle everything else */
+  return type;
+   }
+}
+
+
 void
 _mesa_glsl_release_types(void)
 {
diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
index 0486966..2f849af 100644
--- a/src/glsl/glsl_types.h
+++ b/src/glsl/glsl_types.h
@@ -178,6 +178,17 @@ struct glsl_type {
const glsl_type *get_base_type() const;
 
/**
+* Get the basic scalar type which this type aggregates.
+*
+* If the type is a numeric or boolean scalar, vector, or matrix, or an
+* array of any of those, this function gets the scalar type of the
+* individual components.  For structs and arrays of structs, this function
+* returns the struct type.  For samplers and arrays of samplers, this
+* function returns the sampler type.
+*/
+   const glsl_type *get_scalar_type() const;
+
+   /**
 * Query the type of elements in an array
 *
 * \return
-- 
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] i965: Fix flat integral varyings.

2011-10-25 Thread Paul Berry
Previously, the vertex and fragment shader back-ends assumed that all
varyings were floats.  In GLSL 1.30 this is no longer true--they can
also be of integral types provided that they have an interpolation
qualifier of "flat".

This required two changes in each back-end: assigning the correct type
to the register that holds the varying value during shader execution,
and assigning the correct type to the register that ties the varying
value to the rest of the graphics pipeline (the message register in
the case of VS, and the payload register in the case of FS).

Fixes piglit tests fs-int-interpolation and fs-uint-interpolation.
---
 src/mesa/drivers/dri/i965/brw_fs.cpp   |4 ++--
 src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 9aa8838..200db83 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -424,8 +424,7 @@ fs_reg *
 fs_visitor::emit_general_interpolation(ir_variable *ir)
 {
fs_reg *reg = new(this->mem_ctx) fs_reg(this, ir->type);
-   /* Interpolation is always in floating point regs. */
-   reg->type = BRW_REGISTER_TYPE_F;
+   reg->type = brw_type_for_base_type(ir->type->get_scalar_type());
fs_reg attr = *reg;
 
unsigned int array_elements;
@@ -465,6 +464,7 @@ fs_visitor::emit_general_interpolation(ir_variable *ir)
for (unsigned int k = 0; k < type->vector_elements; k++) {
   struct brw_reg interp = interp_reg(location, k);
   interp = suboffset(interp, 3);
+   interp.type = reg->type;
   emit(FS_OPCODE_CINTERP, attr, fs_reg(interp));
   attr.reg_offset++;
}
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index 02ecdaf..aac20a1 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -855,7 +855,8 @@ vec4_visitor::visit(ir_variable *ir)
   for (int i = 0; i < type_size(ir->type); i++) {
 output_reg[ir->location + i] = *reg;
 output_reg[ir->location + i].reg_offset = i;
-output_reg[ir->location + i].type = BRW_REGISTER_TYPE_F;
+output_reg[ir->location + i].type =
+brw_type_for_base_type(ir->type->get_scalar_type());
 output_reg_annotation[ir->location + i] = ir->name;
   }
   break;
@@ -1917,6 +1918,7 @@ void
 vec4_visitor::emit_generic_urb_slot(dst_reg reg, int vert_result)
 {
assert (vert_result < VERT_RESULT_MAX);
+   reg.type = output_reg[vert_result].type;
current_annotation = output_reg_annotation[vert_result];
/* Copy the register, saturating if necessary */
vec4_instruction *inst = emit(MOV(reg,
-- 
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 1/2] mesa: Add glsl_type::get_scalar_type() function.

2011-10-25 Thread Kenneth Graunke
On 10/25/2011 04:47 PM, Paul Berry wrote:
> This function is similar to get_base_type(), but when called on
> arrays, it returns the scalar type composing the array.  For example,
> glsl_type(vec4[]) => float_type.
> ---
>  src/glsl/glsl_types.cpp |   23 +++
>  src/glsl/glsl_types.h   |   11 +++
>  2 files changed, 34 insertions(+), 0 deletions(-)
> 
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index c94aec0..03e9987 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -258,6 +258,29 @@ const glsl_type *glsl_type::get_base_type() const
>  }
>  
>  
> +const glsl_type *glsl_type::get_scalar_type() const
> +{
> +   const glsl_type *type = this;
> +
> +   /* Handle arrays */
> +   while (type->base_type == GLSL_TYPE_ARRAY)
> +  type = type->fields.array;

This is going to be a very short loop; in fact, it's actually equivalent to:

   if (type->base_type == GLSL_TYPE_ARRAY)
  type = type->fields.array;

since GLSL explicitly disallows arrays of arrays.  That said, it doesn't
really hurt anything, so I'm more observing this than objecting to the code.

> +   /* Handle vectors and matrices */
> +   switch (type->base_type) {
> +   case GLSL_TYPE_UINT:
> +  return uint_type;
> +   case GLSL_TYPE_INT:
> +  return int_type;
> +   case GLSL_TYPE_FLOAT:
> +  return float_type;
> +   default:
> +  /* Handle everything else */
> +  return type;
> +   }
> +}
> +
> +
>  void
>  _mesa_glsl_release_types(void)
>  {
> diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
> index 0486966..2f849af 100644
> --- a/src/glsl/glsl_types.h
> +++ b/src/glsl/glsl_types.h
> @@ -178,6 +178,17 @@ struct glsl_type {
> const glsl_type *get_base_type() const;
>  
> /**
> +* Get the basic scalar type which this type aggregates.
> +*
> +* If the type is a numeric or boolean scalar, vector, or matrix, or an
> +* array of any of those, this function gets the scalar type of the
> +* individual components.  For structs and arrays of structs, this 
> function
> +* returns the struct type.  For samplers and arrays of samplers, this
> +* function returns the sampler type.
> +*/
> +   const glsl_type *get_scalar_type() const;
> +
> +   /**
>  * Query the type of elements in an array
>  *
>  * \return

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


Re: [Mesa-dev] [PATCH 1/2] mesa: Add glsl_type::get_scalar_type() function.

2011-10-25 Thread Paul Berry
On 25 October 2011 16:55, Kenneth Graunke  wrote:

> On 10/25/2011 04:47 PM, Paul Berry wrote:
> > This function is similar to get_base_type(), but when called on
> > arrays, it returns the scalar type composing the array.  For example,
> > glsl_type(vec4[]) => float_type.
> > ---
> >  src/glsl/glsl_types.cpp |   23 +++
> >  src/glsl/glsl_types.h   |   11 +++
> >  2 files changed, 34 insertions(+), 0 deletions(-)
> >
> > diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> > index c94aec0..03e9987 100644
> > --- a/src/glsl/glsl_types.cpp
> > +++ b/src/glsl/glsl_types.cpp
> > @@ -258,6 +258,29 @@ const glsl_type *glsl_type::get_base_type() const
> >  }
> >
> >
> > +const glsl_type *glsl_type::get_scalar_type() const
> > +{
> > +   const glsl_type *type = this;
> > +
> > +   /* Handle arrays */
> > +   while (type->base_type == GLSL_TYPE_ARRAY)
> > +  type = type->fields.array;
>
> This is going to be a very short loop; in fact, it's actually equivalent
> to:
>
>   if (type->base_type == GLSL_TYPE_ARRAY)
>  type = type->fields.array;
>
> since GLSL explicitly disallows arrays of arrays.  That said, it doesn't
> really hurt anything, so I'm more observing this than objecting to the
> code.
>
>
That's a fair point.  I think I prefer to leave it as originally written, on
the theory that if arrays of arrays are ever allowed, then the loop will be
required.  But it's not a strong preference.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] i965: Fix flat integral varyings.

2011-10-25 Thread Kenneth Graunke
On 10/25/2011 04:47 PM, Paul Berry wrote:
> Previously, the vertex and fragment shader back-ends assumed that all
> varyings were floats.  In GLSL 1.30 this is no longer true--they can
> also be of integral types provided that they have an interpolation
> qualifier of "flat".
> 
> This required two changes in each back-end: assigning the correct type
> to the register that holds the varying value during shader execution,
> and assigning the correct type to the register that ties the varying
> value to the rest of the graphics pipeline (the message register in
> the case of VS, and the payload register in the case of FS).
> 
> Fixes piglit tests fs-int-interpolation and fs-uint-interpolation.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp   |4 ++--
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |4 +++-
>  2 files changed, 5 insertions(+), 3 deletions(-)

Now that I see the code, I'd honestly just update brw_type_for_base_type
to do:

if (type->is_array())
   type = type->element_type();

and then change the GLSL_TYPE_ARRAY case to be an assert.

brw_type_for_base_type was returning a bogus value before, and I'm
pretty sure nothing depends on the prior behavior.  This patch
demonstrates a use where you really want it to return something
sensible.  I don't think the old bogus value (UD) is ever preferable to
this new behavior.

Then we don't need another odd "get related type" function, either.

Other than the bike-shedding, your changes here look good.


> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 9aa8838..200db83 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -424,8 +424,7 @@ fs_reg *
>  fs_visitor::emit_general_interpolation(ir_variable *ir)
>  {
> fs_reg *reg = new(this->mem_ctx) fs_reg(this, ir->type);
> -   /* Interpolation is always in floating point regs. */
> -   reg->type = BRW_REGISTER_TYPE_F;
> +   reg->type = brw_type_for_base_type(ir->type->get_scalar_type());
> fs_reg attr = *reg;
>  
> unsigned int array_elements;
> @@ -465,6 +464,7 @@ fs_visitor::emit_general_interpolation(ir_variable *ir)
>   for (unsigned int k = 0; k < type->vector_elements; k++) {
>  struct brw_reg interp = interp_reg(location, k);
>  interp = suboffset(interp, 3);
> +   interp.type = reg->type;
>  emit(FS_OPCODE_CINTERP, attr, fs_reg(interp));
>  attr.reg_offset++;
>   }
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 02ecdaf..aac20a1 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -855,7 +855,8 @@ vec4_visitor::visit(ir_variable *ir)
>for (int i = 0; i < type_size(ir->type); i++) {
>output_reg[ir->location + i] = *reg;
>output_reg[ir->location + i].reg_offset = i;
> -  output_reg[ir->location + i].type = BRW_REGISTER_TYPE_F;
> +  output_reg[ir->location + i].type =
> +brw_type_for_base_type(ir->type->get_scalar_type());
>output_reg_annotation[ir->location + i] = ir->name;
>}
>break;
> @@ -1917,6 +1918,7 @@ void
>  vec4_visitor::emit_generic_urb_slot(dst_reg reg, int vert_result)
>  {
> assert (vert_result < VERT_RESULT_MAX);
> +   reg.type = output_reg[vert_result].type;
> current_annotation = output_reg_annotation[vert_result];
> /* Copy the register, saturating if necessary */
> vec4_instruction *inst = emit(MOV(reg,
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] i965: Fix flat integral varyings.

2011-10-25 Thread Paul Berry
On 25 October 2011 17:08, Kenneth Graunke  wrote:

> On 10/25/2011 04:47 PM, Paul Berry wrote:
> > Previously, the vertex and fragment shader back-ends assumed that all
> > varyings were floats.  In GLSL 1.30 this is no longer true--they can
> > also be of integral types provided that they have an interpolation
> > qualifier of "flat".
> >
> > This required two changes in each back-end: assigning the correct type
> > to the register that holds the varying value during shader execution,
> > and assigning the correct type to the register that ties the varying
> > value to the rest of the graphics pipeline (the message register in
> > the case of VS, and the payload register in the case of FS).
> >
> > Fixes piglit tests fs-int-interpolation and fs-uint-interpolation.
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs.cpp   |4 ++--
> >  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |4 +++-
> >  2 files changed, 5 insertions(+), 3 deletions(-)
>
> Now that I see the code, I'd honestly just update brw_type_for_base_type
> to do:
>
> if (type->is_array())
>   type = type->element_type();
>
> and then change the GLSL_TYPE_ARRAY case to be an assert.
>
> brw_type_for_base_type was returning a bogus value before, and I'm
> pretty sure nothing depends on the prior behavior.  This patch
> demonstrates a use where you really want it to return something
> sensible.  I don't think the old bogus value (UD) is ever preferable to
> this new behavior.
>
> Then we don't need another odd "get related type" function, either.
>

That's what I was going to do at first too, but before I tried it I ran an
experiment to convince myself that nothing depended on the prior behavior: I
changed brw_type_for_base_type() to assert whenever it is called on an array
type, and then did a piglit run.  Result: lots of assertion failures.  Does
that experimental result change your opinion?  It makes me worried that your
proposal will break some part of the shader back-ends that I don't
understand.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/8] mesa: Add interpolation override bitfields.

2011-10-25 Thread Paul Berry
On 25 October 2011 10:21, Brian Paul  wrote:

> On 10/24/2011 05:37 PM, Paul Berry wrote:
>
>> On 24 October 2011 15:58, Brian Paul > > wrote:
>>
>>On 10/24/2011 03:38 PM, Paul Berry wrote:
>>
>>This patch adds the bitfields InterpOverridesFlat,
>>InterpOverridesSmooth, and InterpOverridesNoperspective to
>>gl_fragment_program.  These bitfields keep track of which fragment
>>shader inputs are overridden with the GLSL "flat", "smooth", and
>>"noperspective" interpolation qualifiers.
>>
>>
>>The names of those fields seems a little confusing to me.  For
>>example, "InterpOverridesFlat" sounds like a field that overrides
>>flat shading with something else.
>>
>>How about just "InterpFlat", "InterpSmooth", etc?
>>
>>
>> The meaning I was trying to convey with "overrides" is that bits are
>> only set in these bitfields if the interpolation mode is overridden in
>> GLSL.  For fragment shader inputs that don't have their interpolation
>> mode overridden, all of the bitfields have a zero.  (I had to do this
>> in order to avoid adding a bunch of code to the handling of fixed
>> function fragment shading and ARB fragment programs).
>>
>>
>>Or, how about an enum that indicates the interpolation mode for
>>each fragment shader input?  Something like this:
>>
>>enum interp_mode
>>{
>>   INTERP_DEFAULT,  /* I _think_ we need a default mode, right? */
>>   INTERP_FLAT,
>>   INTERP_SMOOTH,
>>   INTERP_NO_PERSPECTIVE
>>};
>>
>>struct gl_fragment_program
>>{
>>   ...
>>   enum interp_mode InterpMode[FRAG_ATTRIB_MAX];
>>   ...
>>};
>>
>>
>>This would also prevent a non-sensical state where a particular
>>bit is accidentally set in more than one of the masks.
>>
>>
>> Yeah, I prefer this too, and that's essentially what I did in patch 2
>> of the series.  To be honest I find all the bitfield stuff rather
>> ugly, and I would rather drop this patch entirely, but a lot of the
>> i965 code still relies on things being bitfields.  Is it the only
>> driver that does?
>>
>
> Which bitfields are you referring to specifically?  I think the ones in
> gl_program are used in the gallium state tracker, but I'd have to
> double-check.


>
>  If so, I suppose I might could be convinced to move
>> all the bitfield handling of interpolation into i965 where it won't
>> contaminate the other implementations.
>>
>
> I don't know what works best for i965 but using an enumerated type for the
> interpolation modes seems cleaner for core Mesa.


Ok, after rereading your comments and discussing this with Eric, Ian, and
Ken, I reconsidered and actually tried implementing this as an array of
enums, as you suggested.  It pushes a little bit of complication into i965,
but it's not bad, and core mesa gets a lot cleaner.  On balance, I think
it's an improvement.  So now that I've seen your perspective, you can color
me convinced.

I'll rebase and send out a revised patch series.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH v2 0/8] Interpolation qualifier support for i965

2011-10-25 Thread Paul Berry
Here is try #2 at support for GLSL 1.30 interpolation qualifiers for
i965 Gen6+.

Based on comments from Brian Paul, I got rid of the bitfields
InterpOverridesFlat, InterpOverridesSmooth, and
InterpOverridesNoperspective, and replaced them with an array,
InterpQualifier, that specifies the interpolation qualifier for each
fragment shader input.  This simplifies the core mesa code a lot (at
the expense of complicating i965 code only slightly), and it carries
the added bonuses that (a) there's no danger of the bitfields falling
out of sync and resulting in nonsensical combinations, and (b) the
code won't break if we ever increase the number of fragment shader
inputs beyond 64.

The most substantial differences from v1 are in patch 1/8 (which
creates the InterpQualifier array) and patch 8/8 (which took on a bit
of added complexity in brw_compute_barycentric_interp_modes() due to
not having access to interpolation mode bitfields).

[PATCH v2 1/8] mesa: Expose GLSL interpolation qualifiers in 
gl_fragment_program.
[PATCH v2 2/8] glsl: Distinguish between no interpolation qualifier and 'smooth'
[PATCH v2 3/8] glsl: add ir_variable::determine_interpolation_mode() function.
[PATCH v2 4/8] i965/fs: Fix split_virtual_grfs() when delta_xy not in a virtual 
register.
[PATCH v2 5/8] i965/gen6+: Parameterize barycentric interpolation modes.
[PATCH v2 6/8] i965/fs: use determine_interpolation_mode().
[PATCH v2 7/8] i965/gen6+: Rename GEN6_CLIP_BARYCENTRIC_ENABLE.
[PATCH v2 8/8] i965/gen6+: Add support for noperspective interpolation.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH v2 1/8] mesa: Expose GLSL interpolation qualifiers in gl_fragment_program.

2011-10-25 Thread Paul Berry
This patch makes GLSL interpolation qualifiers visible to drivers via
the array InterpQualifier[] in gl_fragment_program, so that they can
easily be used by driver back-ends to select the correct interpolation
mode.

Previous to this patch, the GLSL compiler was using the enum
ir_variable_interpolation to represent interpolation types.  Rather
than make a duplicate enum in core mesa to represent the same thing, I
moved the enum into mtypes.h and renamed it to be more consistent with
the other enums defined there.
---
 src/glsl/ast_to_hir.cpp|8 ++--
 src/glsl/ir.cpp|8 ++--
 src/glsl/ir.h  |   10 ++
 src/glsl/ir_reader.cpp |6 ++--
 src/glsl/ir_set_program_inouts.cpp |   47 
 src/mesa/main/mtypes.h |   19 +++
 src/mesa/program/ir_to_mesa.cpp|2 +-
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp |2 +-
 8 files changed, 69 insertions(+), 33 deletions(-)

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 70afb67..d090d31 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -1962,11 +1962,11 @@ apply_type_qualifier_to_variable(const struct 
ast_type_qualifier *qual,
}
 
if (qual->flags.q.flat)
-  var->interpolation = ir_var_flat;
+  var->interpolation = INTERP_QUALIFIER_FLAT;
else if (qual->flags.q.noperspective)
-  var->interpolation = ir_var_noperspective;
+  var->interpolation = INTERP_QUALIFIER_NOPERSPECTIVE;
else
-  var->interpolation = ir_var_smooth;
+  var->interpolation = INTERP_QUALIFIER_SMOOTH;
 
var->pixel_center_integer = qual->flags.q.pixel_center_integer;
var->origin_upper_left = qual->flags.q.origin_upper_left;
@@ -2630,7 +2630,7 @@ ast_declarator_list::hir(exec_list *instructions,
   && state->current_function == NULL
   && var->type->is_integer()
   && var->mode == ir_var_out
-  && var->interpolation != ir_var_flat) {
+  && var->interpolation != INTERP_QUALIFIER_FLAT) {
 
  _mesa_glsl_error(&loc, state, "If a vertex output is an integer, "
   "then it must be qualified with 'flat'");
diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
index d968890..046ce25 100644
--- a/src/glsl/ir.cpp
+++ b/src/glsl/ir.cpp
@@ -1320,7 +1320,7 @@ ir_swizzle::variable_referenced() const
 ir_variable::ir_variable(const struct glsl_type *type, const char *name,
 ir_variable_mode mode)
: max_array_access(0), read_only(false), centroid(false), invariant(false),
- mode(mode), interpolation(ir_var_smooth)
+ mode(mode), interpolation(INTERP_QUALIFIER_SMOOTH)
 {
this->ir_type = ir_type_variable;
this->type = type;
@@ -1343,9 +1343,9 @@ const char *
 ir_variable::interpolation_string() const
 {
switch (this->interpolation) {
-   case ir_var_smooth:return "smooth";
-   case ir_var_flat:  return "flat";
-   case ir_var_noperspective: return "noperspective";
+   case INTERP_QUALIFIER_SMOOTH:return "smooth";
+   case INTERP_QUALIFIER_FLAT:  return "flat";
+   case INTERP_QUALIFIER_NOPERSPECTIVE: return "noperspective";
}
 
assert(!"Should not get here.");
diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index b707634..4ea8764 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -34,6 +34,7 @@
 #include "list.h"
 #include "ir_visitor.h"
 #include "ir_hierarchical_visitor.h"
+#include "main/mtypes.h"
 
 /**
  * \defgroup IR Intermediate representation nodes
@@ -227,12 +228,6 @@ enum ir_variable_mode {
ir_var_temporary/**< Temporary variable generated during compilation. */
 };
 
-enum ir_variable_interpolation {
-   ir_var_smooth = 0,
-   ir_var_flat,
-   ir_var_noperspective
-};
-
 /**
  * \brief Layout qualifiers for gl_FragDepth.
  *
@@ -1679,7 +1674,8 @@ extern bool
 ir_has_call(ir_instruction *ir);
 
 extern void
-do_set_program_inouts(exec_list *instructions, struct gl_program *prog);
+do_set_program_inouts(exec_list *instructions, struct gl_program *prog,
+  bool is_fragment_shader);
 
 extern char *
 prototype_string(const glsl_type *return_type, const char *name,
diff --git a/src/glsl/ir_reader.cpp b/src/glsl/ir_reader.cpp
index afb06b3..e3a3ed9 100644
--- a/src/glsl/ir_reader.cpp
+++ b/src/glsl/ir_reader.cpp
@@ -405,11 +405,11 @@ ir_reader::read_declaration(s_expression *expr)
   } else if (strcmp(qualifier->value(), "inout") == 0) {
 var->mode = ir_var_inout;
   } else if (strcmp(qualifier->value(), "smooth") == 0) {
-var->interpolation = ir_var_smooth;
+var->interpolation = INTERP_QUALIFIER_SMOOTH;
   } else if (strcmp(qualifier->value(), "flat") == 0) {
-var->interpolation = ir_var_flat;
+var->interpolation = INTERP_QUALIFIER_FLAT;
   } else if (strcmp(qualifier->value(), "noperspective") == 0) {
-var->interpolati

[Mesa-dev] [PATCH v2 2/8] glsl: Distinguish between no interpolation qualifier and 'smooth'

2011-10-25 Thread Paul Berry
Previously, we treated the 'smooth' qualifier as equivalent to no
qualifier at all.  However, this is incorrect for the built-in color
variables (gl_FrontColor, gl_BackColor, gl_FrontSecondaryColor, and
gl_BackSecondaryColor).  For those variables, if there is no qualifier
at all, interpolation should be flat if the shade model is GL_FLAT,
and smooth if the shade model is GL_SMOOTH.

To make this possible, I added a new value to the
glsl_interp_qualifier enum, INTERP_QUALIFIER_NONE.
---
 src/glsl/ast_to_hir.cpp |4 +++-
 src/glsl/ir.cpp |3 ++-
 src/glsl/ir.h   |4 
 src/mesa/main/mtypes.h  |6 +-
 4 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index d090d31..fa6206e 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -1965,8 +1965,10 @@ apply_type_qualifier_to_variable(const struct 
ast_type_qualifier *qual,
   var->interpolation = INTERP_QUALIFIER_FLAT;
else if (qual->flags.q.noperspective)
   var->interpolation = INTERP_QUALIFIER_NOPERSPECTIVE;
-   else
+   else if (qual->flags.q.smooth)
   var->interpolation = INTERP_QUALIFIER_SMOOTH;
+   else
+  var->interpolation = INTERP_QUALIFIER_NONE;
 
var->pixel_center_integer = qual->flags.q.pixel_center_integer;
var->origin_upper_left = qual->flags.q.origin_upper_left;
diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
index 046ce25..9aad0fc 100644
--- a/src/glsl/ir.cpp
+++ b/src/glsl/ir.cpp
@@ -1320,7 +1320,7 @@ ir_swizzle::variable_referenced() const
 ir_variable::ir_variable(const struct glsl_type *type, const char *name,
 ir_variable_mode mode)
: max_array_access(0), read_only(false), centroid(false), invariant(false),
- mode(mode), interpolation(INTERP_QUALIFIER_SMOOTH)
+ mode(mode), interpolation(INTERP_QUALIFIER_NONE)
 {
this->ir_type = ir_type_variable;
this->type = type;
@@ -1343,6 +1343,7 @@ const char *
 ir_variable::interpolation_string() const
 {
switch (this->interpolation) {
+   case INTERP_QUALIFIER_NONE:  return "no";
case INTERP_QUALIFIER_SMOOTH:return "smooth";
case INTERP_QUALIFIER_FLAT:  return "flat";
case INTERP_QUALIFIER_NOPERSPECTIVE: return "noperspective";
diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index 4ea8764..0c0cccb 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -283,6 +283,10 @@ public:
 * \return The string that would be used in a shader to specify \c
 * mode will be returned.
 *
+* This function is used to generate error messages of the form "shader
+* uses %s interpolation qualifier", so in the case where there is no
+* interpolation qualifier, it returns "no".
+*
 * This function should only be used on a shader input or output variable.
 */
const char *interpolation_string() const;
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index bd5e020..47d8a96 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -1793,9 +1793,13 @@ typedef enum
 /**
  * The possible interpolation qualifiers that can be applied to a fragment
  * shader input in GLSL.
+ *
+ * Note: INTERP_QUALIFIER_NONE must be 0 so that memsetting the
+ * gl_fragment_program data structure to 0 causes the default behavior.
  */
 enum glsl_interp_qualifier
 {
+   INTERP_QUALIFIER_NONE = 0,
INTERP_QUALIFIER_SMOOTH,
INTERP_QUALIFIER_FLAT,
INTERP_QUALIFIER_NOPERSPECTIVE
@@ -1906,7 +1910,7 @@ struct gl_fragment_program
/**
 * GLSL interpolation qualifier associated with each fragment shader input.
 * For inputs that do not have an interpolation qualifier specified in
-* GLSL, the value is INTERP_QUALIFIER_SMOOTH.
+* GLSL, the value is INTERP_QUALIFIER_NONE.
 */
enum glsl_interp_qualifier InterpQualifier[FRAG_ATTRIB_MAX];
 };
-- 
1.7.6.4

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


[Mesa-dev] [PATCH v2 3/8] glsl: add ir_variable::determine_interpolation_mode() function.

2011-10-25 Thread Paul Berry
This function determines how a variable should be interpolated based
both on interpolation qualifiers and the current shade model.
---
 src/glsl/ir.cpp |   15 +++
 src/glsl/ir.h   |   11 +++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
index 9aad0fc..ef7300e 100644
--- a/src/glsl/ir.cpp
+++ b/src/glsl/ir.cpp
@@ -1354,6 +1354,21 @@ ir_variable::interpolation_string() const
 }
 
 
+glsl_interp_qualifier
+ir_variable::determine_interpolation_mode(bool flat_shade)
+{
+   if (this->interpolation != INTERP_QUALIFIER_NONE)
+  return (glsl_interp_qualifier) this->interpolation;
+   int location = this->location;
+   bool is_gl_Color =
+  location == FRAG_ATTRIB_COL0 || location == FRAG_ATTRIB_COL1;
+   if (flat_shade && is_gl_Color)
+  return INTERP_QUALIFIER_FLAT;
+   else
+  return INTERP_QUALIFIER_SMOOTH;
+}
+
+
 ir_function_signature::ir_function_signature(const glsl_type *return_type)
: return_type(return_type), is_defined(false), _function(NULL)
 {
diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index 0c0cccb..404d4cf 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -292,6 +292,17 @@ public:
const char *interpolation_string() const;
 
/**
+* Determine how this variable should be interpolated based on its
+* interpolation qualifier (if present), whether it is gl_Color or
+* gl_SecondaryColor, and whether flatshading is enabled in the current GL
+* state.
+*
+* The return value will always be either INTERP_QUALIFIER_SMOOTH,
+* INTERP_QUALIFIER_NOPERSPECTIVE, or INTERP_QUALIFIER_FLAT.
+*/
+   glsl_interp_qualifier determine_interpolation_mode(bool flat_shade);
+
+   /**
 * Delcared name of the variable
 */
const char *name;
-- 
1.7.6.4

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


[Mesa-dev] [PATCH v2 4/8] i965/fs: Fix split_virtual_grfs() when delta_xy not in a virtual register.

2011-10-25 Thread Paul Berry
This patch modifies the special case in
fs_visitor::split_virtual_grfs() that prevents splitting from being
applied to the delta_x/delta_y register pair (this register pair needs
to remain contiguous so that it can be used by the PLN instruction).

When gen>=6, this register pair is in a fixed location, not a virtual
register, so it was in no danger of being split.  And
split_virtual_grfs' attempt not to split it was preventing some other
unrelated register from being split.
---
 src/mesa/drivers/dri/i965/brw_fs.cpp |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index c0d93c0..3848915 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -768,7 +768,7 @@ fs_visitor::split_virtual_grfs()
 split_grf[i] = false;
}
 
-   if (brw->has_pln) {
+   if (brw->has_pln && this->delta_x.file == GRF) {
   /* PLN opcodes rely on the delta_xy being contiguous. */
   split_grf[this->delta_x.reg] = false;
}
-- 
1.7.6.4

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


[Mesa-dev] [PATCH v2 5/8] i965/gen6+: Parameterize barycentric interpolation modes.

2011-10-25 Thread Paul Berry
This patch modifies the fragment shader back-end so that instead of
using a single delta_x/delta_y register pair to store barycentric
coordinates, it uses an array of such register pairs, one for each
possible intepolation mode.

When setting up the WM, we intstruct it to only provide the
barycentric coordinates that are actually needed by the fragment
shader--that is computed by brw_compute_barycentric_interp_modes().
Currently this function returns just
BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC, because this is the only
interpolation mode we support.  However, that will change in a later
patch.
---
 src/mesa/drivers/dri/i965/brw_context.h   |5 ++
 src/mesa/drivers/dri/i965/brw_defines.h   |   18 ++---
 src/mesa/drivers/dri/i965/brw_fs.cpp  |   25 +---
 src/mesa/drivers/dri/i965/brw_fs.h|4 +-
 src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp |   11 +-
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp  |   29 +-
 src/mesa/drivers/dri/i965/brw_wm.c|   42 -
 src/mesa/drivers/dri/i965/brw_wm.h|1 +
 src/mesa/drivers/dri/i965/gen6_wm_state.c |3 +-
 src/mesa/drivers/dri/i965/gen7_wm_state.c |3 +-
 10 files changed, 103 insertions(+), 38 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 7e2675a..f83d5fc 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -970,6 +970,11 @@ void brw_compute_vue_map(struct brw_vue_map *vue_map,
  GLbitfield64 outputs_written);
 gl_clip_plane *brw_select_clip_planes(struct gl_context *ctx);
 
+/* brw_wm.c */
+unsigned
+brw_compute_barycentric_interp_modes(void);
+
+
 
 /*==
  * Inline conversion functions.  These are better-typed than the
diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
b/src/mesa/drivers/dri/i965/brw_defines.h
index 5314ac6..42a8dad 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -1207,6 +1207,16 @@ enum brw_message_target {
 /* DW12: attr 0-7 wrap shortest enables */
 /* DW13: attr 8-16 wrap shortest enables */
 
+enum brw_wm_barycentric_interp_mode {
+   BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC= 0,
+   BRW_WM_PERSPECTIVE_CENTROID_BARYCENTRIC = 1,
+   BRW_WM_PERSPECTIVE_SAMPLE_BARYCENTRIC   = 2,
+   BRW_WM_NONPERSPECTIVE_PIXEL_BARYCENTRIC = 3,
+   BRW_WM_NONPERSPECTIVE_CENTROID_BARYCENTRIC  = 4,
+   BRW_WM_NONPERSPECTIVE_SAMPLE_BARYCENTRIC= 5,
+   BRW_WM_BARYCENTRIC_INTERP_MODE_COUNT  = 6
+};
+
 #define _3DSTATE_WM0x7814 /* GEN6+ */
 /* DW1: kernel pointer */
 /* DW2 */
@@ -1261,6 +1271,7 @@ enum brw_message_target {
 # define GEN6_WM_PERSPECTIVE_SAMPLE_BARYCENTRIC(1 << 12)
 # define GEN6_WM_PERSPECTIVE_CENTROID_BARYCENTRIC  (1 << 11)
 # define GEN6_WM_PERSPECTIVE_PIXEL_BARYCENTRIC (1 << 10)
+# define GEN6_WM_BARYCENTRIC_INTERPOLATION_MODE_SHIFT   10
 # define GEN6_WM_POINT_RASTRULE_UPPER_RIGHT(1 << 9)
 # define GEN6_WM_MSRAST_OFF_PIXEL  (0 << 1)
 # define GEN6_WM_MSRAST_OFF_PATTERN(1 << 1)
@@ -1298,12 +1309,7 @@ enum brw_message_target {
 # define GEN7_WM_POSITION_ZW_PIXEL (0 << 17)
 # define GEN7_WM_POSITION_ZW_CENTROID  (2 << 17)
 # define GEN7_WM_POSITION_ZW_SAMPLE(3 << 17)
-# define GEN7_WM_NONPERSPECTIVE_SAMPLE_BARYCENTRIC (1 << 16)
-# define GEN7_WM_NONPERSPECTIVE_CENTROID_BARYCENTRIC   (1 << 15)
-# define GEN7_WM_NONPERSPECTIVE_PIXEL_BARYCENTRIC  (1 << 14)
-# define GEN7_WM_PERSPECTIVE_SAMPLE_BARYCENTRIC(1 << 13)
-# define GEN7_WM_PERSPECTIVE_CENTROID_BARYCENTRIC  (1 << 12)
-# define GEN7_WM_PERSPECTIVE_PIXEL_BARYCENTRIC (1 << 11)
+# define GEN7_WM_BARYCENTRIC_INTERPOLATION_MODE_SHIFT   11
 # define GEN7_WM_USES_INPUT_COVERAGE_MASK  (1 << 10)
 # define GEN7_WM_LINE_END_CAP_AA_WIDTH_0_5 (0 << 8)
 # define GEN7_WM_LINE_END_CAP_AA_WIDTH_1_0 (1 << 8)
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 3848915..b3ad505 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -407,8 +407,10 @@ fs_visitor::emit_fragcoord_interpolation(ir_variable *ir)
   emit(BRW_OPCODE_MOV, wpos,
   fs_reg(brw_vec8_grf(c->source_depth_reg, 0)));
} else {
-  emit(FS_OPCODE_LINTERP, wpos, this->delta_x, this->delta_y,
-  interp_reg(FRAG_ATTRIB_WPOS, 2));
+  emit(FS_OPCODE_LINTERP, wpos,
+   this->delta_x[BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC],
+   this->delta_y[BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC],
+   interp_reg(FRAG_ATTRIB_WPOS, 2));
}
wpos.reg_offset++;
 
@@ -4

[Mesa-dev] [PATCH v2 6/8] i965/fs: use determine_interpolation_mode().

2011-10-25 Thread Paul Berry
This patch changes how fs_visitor::emit_general_interpolation()
decides what kind of interpolation to do.  Previously, it used the
shade model to determine how to interpolate colors, and used smooth
interpolation on everything else.  Now it uses
ir_variable::determine_interpolation_mode(), so that it respects GLSL
1.30 interpolation qualifiers.

Fixes piglit tests interpolation-flat-*-smooth-{distance,fixed,vertex}
and interpolation-flat-other-flat-{distance,fixed,vertex}.
---
 src/mesa/drivers/dri/i965/brw_fs.cpp |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index b3ad505..185e00d 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -442,6 +442,9 @@ fs_visitor::emit_general_interpolation(ir_variable *ir)
   type = ir->type;
}
 
+   glsl_interp_qualifier interpolation_mode =
+  ir->determine_interpolation_mode(c->key.flat_shade);
+
int location = ir->location;
for (unsigned int i = 0; i < array_elements; i++) {
   for (unsigned int j = 0; j < type->matrix_columns; j++) {
@@ -454,10 +457,7 @@ fs_visitor::emit_general_interpolation(ir_variable *ir)
continue;
 }
 
-bool is_gl_Color =
-   location == FRAG_ATTRIB_COL0 || location == FRAG_ATTRIB_COL1;
-
-if (c->key.flat_shade && is_gl_Color) {
+if (interpolation_mode == INTERP_QUALIFIER_FLAT) {
/* Constant interpolation (flat shading) case. The SF has
 * handed us defined values in only the constant offset
 * field of the setup reg.
-- 
1.7.6.4

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


[Mesa-dev] [PATCH v2 7/8] i965/gen6+: Rename GEN6_CLIP_BARYCENTRIC_ENABLE.

2011-10-25 Thread Paul Berry
The name was misleading.  The actual effect of the bit is to cause
the clipper to emit *non-perspective* barycentric coordinate
information (which is only needed when doing noperspective
interpolation).
---
 src/mesa/drivers/dri/i965/brw_defines.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
b/src/mesa/drivers/dri/i965/brw_defines.h
index 42a8dad..fcf55d5 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -1100,7 +1100,7 @@ enum brw_message_target {
 # define GEN6_CLIP_MODE_REJECT_ALL (3 << 13)
 # define GEN6_CLIP_MODE_ACCEPT_ALL (4 << 13)
 # define GEN6_CLIP_PERSPECTIVE_DIVIDE_DISABLE  (1 << 9)
-# define GEN6_CLIP_BARYCENTRIC_ENABLE  (1 << 8)
+# define GEN6_CLIP_NON_PERSPECTIVE_BARYCENTRIC_ENABLE  (1 << 8)
 # define GEN6_CLIP_TRI_PROVOKE_SHIFT   4
 # define GEN6_CLIP_LINE_PROVOKE_SHIFT  2
 # define GEN6_CLIP_TRIFAN_PROVOKE_SHIFT0
-- 
1.7.6.4

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


[Mesa-dev] [PATCH v2 8/8] i965/gen6+: Add support for noperspective interpolation.

2011-10-25 Thread Paul Berry
This required the following changes:

- WM setup now makes the appropriate set of barycentric coordinates
  (perspective vs. noperspective) available to the fragment shader,
  based on whether the shader requires perspective interpolation,
  noperspective interpolation, both, or neither.

- The fragment shader backend now uses the appropriate set of
  barycentric coordiantes when interpolating, based on the
  interpolation mode returned by
  ir_variable::determine_interpolation_mode().

- SF setup now uses _mesa_compute_flag_attributes_flat() to determine
  which attributes are to be flat shaded (as opposed to the old logic,
  which only flat shaded colors).

- CLIP setup now ensures that the clipper outputs non-perspective
  barycentric coordinates when they are needed by the fragment shader.

Fixes the remaining piglit tests of interpolation qualifiers that were
failing:
- interpolation-flat-*-smooth-none
- interpolation-flat-other-flat-none
- interpolation-noperspective-*
- interpolation-smooth-gl_*Color-flat-*
---
 src/mesa/drivers/dri/i965/brw_context.h |7 -
 src/mesa/drivers/dri/i965/brw_fs.cpp|9 --
 src/mesa/drivers/dri/i965/brw_wm.c  |   40 ---
 src/mesa/drivers/dri/i965/gen6_clip_state.c |   32 +-
 src/mesa/drivers/dri/i965/gen6_sf_state.c   |   21 +++---
 src/mesa/drivers/dri/i965/gen6_wm_state.c   |9 +-
 src/mesa/drivers/dri/i965/gen7_clip_state.c |   12 +++-
 src/mesa/drivers/dri/i965/gen7_sf_state.c   |   21 +++---
 src/mesa/drivers/dri/i965/gen7_wm_state.c   |   10 +-
 9 files changed, 126 insertions(+), 35 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index f83d5fc..4bb089a 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -972,7 +972,12 @@ gl_clip_plane *brw_select_clip_planes(struct gl_context 
*ctx);
 
 /* brw_wm.c */
 unsigned
-brw_compute_barycentric_interp_modes(void);
+brw_compute_barycentric_interp_modes(bool shade_model_flat,
+ const struct gl_fragment_program *fprog);
+
+/* gen6_clip_state.c */
+bool
+brw_fprog_uses_noperspective(const struct gl_fragment_program *fprog);
 
 
 
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 185e00d..31c3116 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -469,7 +469,7 @@ fs_visitor::emit_general_interpolation(ir_variable *ir)
   attr.reg_offset++;
}
 } else {
-   /* Perspective interpolation case. */
+   /* Smooth/noperspective interpolation case. */
for (unsigned int k = 0; k < type->vector_elements; k++) {
   /* FINISHME: At some point we probably want to push
* this farther by giving similar treatment to the
@@ -483,8 +483,11 @@ fs_visitor::emit_general_interpolation(ir_variable *ir)
  emit(BRW_OPCODE_MOV, attr, fs_reg(1.0f));
   } else {
  struct brw_reg interp = interp_reg(location, k);
-  brw_wm_barycentric_interp_mode barycoord_mode =
- BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC;
+  brw_wm_barycentric_interp_mode barycoord_mode;
+  if (interpolation_mode == INTERP_QUALIFIER_SMOOTH)
+ barycoord_mode = BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC;
+  else
+ barycoord_mode = BRW_WM_NONPERSPECTIVE_PIXEL_BARYCENTRIC;
   emit(FS_OPCODE_LINTERP, attr,
this->delta_x[barycoord_mode],
this->delta_y[barycoord_mode], fs_reg(interp));
diff --git a/src/mesa/drivers/dri/i965/brw_wm.c 
b/src/mesa/drivers/dri/i965/brw_wm.c
index 546d522..923beaf 100644
--- a/src/mesa/drivers/dri/i965/brw_wm.c
+++ b/src/mesa/drivers/dri/i965/brw_wm.c
@@ -129,10 +129,41 @@ brw_wm_non_glsl_emit(struct brw_context *brw, struct 
brw_wm_compile *c)
  * (see enum brw_wm_barycentric_interp_mode) is needed by the fragment shader.
  */
 unsigned
-brw_compute_barycentric_interp_modes(void)
+brw_compute_barycentric_interp_modes(bool shade_model_flat,
+ const struct gl_fragment_program *fprog)
 {
-   /* At the moment the only interpolation mode we support is perspective. */
-   return (1 << BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC);
+   unsigned barycentric_interp_modes = 0;
+   int attr;
+
+   /* Loop through all fragment shader inputs to figure out what interpolation
+* modes are in use, and set the appropriate bits in
+* barycentric_interp_modes.
+*/
+   for (attr = 0; attr < FRAG_ATTRIB_MAX; ++attr) {
+  enum glsl_interp_qualifier interp_qualifier =
+ fprog->InterpQualifier[attr];
+  bool is_gl_Color = attr == FRAG_ATTRIB_COL0 || attr == FRAG_ATTRIB_COL1;
+
+  /* Igno