[Mesa-dev] [Bug 91673] Segfault when calling glTexSubImage2D on storage texture to bound FBO

2015-08-20 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=91673

Tapani Pälli  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|mesa-dev@lists.freedesktop. |lem...@gmail.com
   |org |

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] mesa: update fbo state in glTexStorage

2015-08-20 Thread Tapani Pälli
We have to re-validate FBOs rendering to the texture like is done
with TexImage and CopyTexImage.

Signed-off-by: Tapani Pälli 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91673
Cc: "10.6" 
---
 src/mesa/main/texstorage.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/src/mesa/main/texstorage.c b/src/mesa/main/texstorage.c
index db2b63d..51f49b3 100644
--- a/src/mesa/main/texstorage.c
+++ b/src/mesa/main/texstorage.c
@@ -189,6 +189,20 @@ clear_texture_fields(struct gl_context *ctx,
 }
 
 
+/**
+ * Update/re-validate framebuffer object.
+ */
+static void
+update_fbo_texture(struct gl_context *ctx, struct gl_texture_object *texObj)
+{
+   const unsigned numFaces = _mesa_num_tex_faces(texObj->Target);
+   for (int level = 0; level < ARRAY_SIZE(texObj->Image[0]); level++) {
+  for (unsigned face = 0; face < numFaces; face++)
+ _mesa_update_fbo_texture(ctx, texObj, face, level);
+   }
+}
+
+
 GLboolean
 _mesa_is_legal_tex_storage_format(struct gl_context *ctx, GLenum 
internalformat)
 {
@@ -446,6 +460,7 @@ _mesa_texture_storage(struct gl_context *ctx, GLuint dims,
 
   _mesa_set_texture_view_state(ctx, texObj, target, levels);
 
+  update_fbo_texture(ctx, texObj);
}
 }
 
-- 
2.1.0

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


Re: [Mesa-dev] [PATCH 07/10] i965: Move texture swizzle resolving into dispatcher

2015-08-20 Thread Pohjolainen, Topi
On Wed, Aug 19, 2015 at 05:54:18PM +0300, Francisco Jerez wrote:
> Topi Pohjolainen  writes:
> 
> > v2: Non-trivial rebase
> >
> > Reviewed-by: Matt Turner  (v1)
> > Reviewed-by: Kenneth Graunke  (v1)
> > Signed-off-by: Topi Pohjolainen 
> >
> > Signed-off-by: Topi Pohjolainen 
> 
> Uhm, and here you dropped my "[ Francisco Jerez: Non-trivial rebase. ]"
> and R-b tags and replaced them with a duplicate S-o-b line?  Fine.

Oops, sorry, I have no idea how that happened. Anyway, I actually used my own
original series dating back to spring as the basis.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 03/10] i965: Move tex buffer dispatch into hw independent setup

2015-08-20 Thread Pohjolainen, Topi
On Wed, Aug 19, 2015 at 05:34:14PM +0300, Francisco Jerez wrote:
> Topi Pohjolainen  writes:
> 
> > All generations do the same exact dispatch and it could be
> > therefore done in the hardware independent stage.
> >
> > v2: Rebased where there are still duplicate calls in gen7 and gen8
> > handlers. These will be dropped in subsequent patches.
> >
> > Reviewed-by: Matt Turner  (v1)
> > Reviewed-by: Kenneth Graunke  (v1)
> > Signed-off-by: Topi Pohjolainen 
> > ---
> >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 28 
> > ++--
> >  1 file changed, 16 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
> > b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > index 73aa719..dca67e8 100644
> > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > @@ -319,12 +319,6 @@ brw_update_texture_surface(struct gl_context *ctx,
> > struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
> > uint32_t *surf;
> >  
> > -   /* BRW_NEW_TEXTURE_BUFFER */
> > -   if (tObj->Target == GL_TEXTURE_BUFFER) {
> > -  brw_update_buffer_texture_surface(brw, tObj, surf_offset);
> > -  return;
> > -   }
> > -
> > surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
> >   6 * 4, 32, surf_offset);
> >  
> > @@ -819,14 +813,24 @@ update_stage_texture_surfaces(struct brw_context *brw,
> > for (unsigned s = 0; s < num_samplers; s++) {
> >surf_offset[s] = 0;
> >  
> > -  if (prog->SamplersUsed & (1 << s)) {
> > - const unsigned unit = prog->SamplerUnits[s];
> > +  if (!(prog->SamplersUsed & (1 << s)))
> > + continue;
> >  
> > - /* _NEW_TEXTURE */
> > - if (ctx->Texture.Unit[unit]._Current) {
> > -brw->vtbl.update_texture_surface(ctx, unit, surf_offset + s, 
> > for_gather);
> > - }
> > +  const unsigned unit = prog->SamplerUnits[s];
> > +  struct gl_texture_object *tex = ctx->Texture.Unit[unit]._Current;
> > +
> > +  if (!tex)
> > + continue;
> > +
> > +  /* BRW_NEW_TEXTURE_BUFFER */
> > +  if (tex->Target == GL_TEXTURE_BUFFER) {
> > + brw_update_buffer_texture_surface(brw, tex, surf_offset);
> 
> You probably didn't mean to always pass the first surface state entry
> (missing "+ s"?).

Oh, that's really bad. Many thanks for the careful review. I wonder how I
got clean piglit run, I need to recheck this.

> 
> > + continue;
> >}
> > +
> > +  /* _NEW_TEXTURE */
> > +  brw->vtbl.update_texture_surface(ctx, unit,
> > +   surf_offset + s, for_gather);
> 
> I'd keep the control flow structured here instead of adding a jump,
> like:
> 
> | if (tex->Target == GL_TEXTURE_BUFFER) {
> |  // Handle buffer textures.
> | } else {
> |  // Handle non-buffer textures.
> | }

I fully agree, thanks.

> 
> Anyway, just nitpicking.  With these fixed:
> 
> Reviewed-by: Francisco Jerez 
> 
> > }
> >  }
> >  
> > -- 
> > 1.9.3
> >
> > ___
> > 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 03/10] i965: Move tex buffer dispatch into hw independent setup

2015-08-20 Thread Pohjolainen, Topi
On Thu, Aug 20, 2015 at 11:20:31AM +0300, Pohjolainen, Topi wrote:
> On Wed, Aug 19, 2015 at 05:34:14PM +0300, Francisco Jerez wrote:
> > Topi Pohjolainen  writes:
> > 
> > > All generations do the same exact dispatch and it could be
> > > therefore done in the hardware independent stage.
> > >
> > > v2: Rebased where there are still duplicate calls in gen7 and gen8
> > > handlers. These will be dropped in subsequent patches.
> > >
> > > Reviewed-by: Matt Turner  (v1)
> > > Reviewed-by: Kenneth Graunke  (v1)
> > > Signed-off-by: Topi Pohjolainen 
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 28 
> > > ++--
> > >  1 file changed, 16 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
> > > b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > index 73aa719..dca67e8 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > @@ -319,12 +319,6 @@ brw_update_texture_surface(struct gl_context *ctx,
> > > struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
> > > uint32_t *surf;
> > >  
> > > -   /* BRW_NEW_TEXTURE_BUFFER */
> > > -   if (tObj->Target == GL_TEXTURE_BUFFER) {
> > > -  brw_update_buffer_texture_surface(brw, tObj, surf_offset);
> > > -  return;
> > > -   }
> > > -
> > > surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
> > > 6 * 4, 32, surf_offset);
> > >  
> > > @@ -819,14 +813,24 @@ update_stage_texture_surfaces(struct brw_context 
> > > *brw,
> > > for (unsigned s = 0; s < num_samplers; s++) {
> > >surf_offset[s] = 0;
> > >  
> > > -  if (prog->SamplersUsed & (1 << s)) {
> > > - const unsigned unit = prog->SamplerUnits[s];
> > > +  if (!(prog->SamplersUsed & (1 << s)))
> > > + continue;
> > >  
> > > - /* _NEW_TEXTURE */
> > > - if (ctx->Texture.Unit[unit]._Current) {
> > > -brw->vtbl.update_texture_surface(ctx, unit, surf_offset + s, 
> > > for_gather);
> > > - }
> > > +  const unsigned unit = prog->SamplerUnits[s];
> > > +  struct gl_texture_object *tex = ctx->Texture.Unit[unit]._Current;
> > > +
> > > +  if (!tex)
> > > + continue;
> > > +
> > > +  /* BRW_NEW_TEXTURE_BUFFER */
> > > +  if (tex->Target == GL_TEXTURE_BUFFER) {
> > > + brw_update_buffer_texture_surface(brw, tex, surf_offset);
> > 
> > You probably didn't mean to always pass the first surface state entry
> > (missing "+ s"?).
> 
> Oh, that's really bad. Many thanks for the careful review. I wonder how I
> got clean piglit run, I need to recheck this.

I tried with jenkins and piglit doesn't get upset about it. I guess this is
a path that just doesn't get tickled.
Therefore I'm even more grateful for the careful review :)

> 
> > 
> > > + continue;
> > >}
> > > +
> > > +  /* _NEW_TEXTURE */
> > > +  brw->vtbl.update_texture_surface(ctx, unit,
> > > +   surf_offset + s, for_gather);
> > 
> > I'd keep the control flow structured here instead of adding a jump,
> > like:
> > 
> > | if (tex->Target == GL_TEXTURE_BUFFER) {
> > |  // Handle buffer textures.
> > | } else {
> > |  // Handle non-buffer textures.
> > | }
> 
> I fully agree, thanks.

Well, actually I had a reason for the jump. The latter case will get augmented
quite a bit by the rest of the series and I wanted to keep the indentation
level (instead of growing the else-branch). I'd rather have it this way if
you don't mind.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH V2 2/8] i965: Use intel_get_tile_dims() to get tile masks

2015-08-20 Thread Pohjolainen, Topi
On Wed, Aug 19, 2015 at 05:24:49PM -0700, Anuj Phogat wrote:
> This will require change in the parameters passed to
> intel_miptree_get_tile_masks().
> 
> V2: Rearrange the order of parameters. (Ben)
> Change the name to intel_get_tile_masks(). (Topi)
> 
> Cc: Ben Widawsky 
> Cc: Topi Pohjolainen 
> Signed-off-by: Anuj Phogat 

Patches 1 and 2:

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


Re: [Mesa-dev] [PATCH 21/25] mesa: Add extension enable and table entry for OES_shader_image_atomic.

2015-08-20 Thread Tapani Pälli



On 08/19/2015 02:36 PM, Francisco Jerez wrote:

Ian Romanick  writes:


On 08/17/2015 09:56 AM, Ilia Mirkin wrote:

On Mon, Aug 17, 2015 at 12:46 PM, Francisco Jerez  wrote:

---
  src/mesa/main/extensions.c | 1 +
  src/mesa/main/mtypes.h | 1 +
  2 files changed, 2 insertions(+)

diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
index d934d19..38c7bc8 100644
--- a/src/mesa/main/extensions.c
+++ b/src/mesa/main/extensions.c
@@ -319,6 +319,7 @@ static const struct extension extension_table[] = {
 { "GL_OES_query_matrix",o(dummy_true), 
  ES1,   2003 },
 { "GL_OES_read_format", o(dummy_true), 
 GL | ES1,   2003 },
 { "GL_OES_rgb8_rgba8",  o(dummy_true), 
  ES1 | ES2, 2005 },
+   { "GL_OES_shader_image_atomic", o(OES_shader_image_atomic), 
 ES3,   2015 },
 { "GL_OES_single_precision",o(dummy_true), 
  ES1,   2003 },
 { "GL_OES_standard_derivatives",
o(OES_standard_derivatives),   ES2, 2005 },
 { "GL_OES_stencil1",o(dummy_false),
 DISABLE,2005 },
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 9964fb4..b489f8c 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3970,6 +3970,7 @@ struct gl_extensions
 GLboolean OES_draw_texture;
 GLboolean OES_depth_texture_cube_map;
 GLboolean OES_EGL_image_external;
+   GLboolean OES_shader_image_atomic;


Does this add any functionality over ARB_shader_image_load_store? I
guess extension bools are cheap, but... seems unnecessary.


Yes and no.  In terms of functionality, all desktop hardware that can do
GL_ARB_shader_image_load_store can do this extension.


Yeah, in principle they should, although it might need non-trivial
driver changes if the hardware doesn't support atomic operations at all
on floating-point surfaces.  You may need to lie to the hardware and
pretend that the surface contains integer data.

If we don't expect to have to deal with hardware that can do image loads
and stores at the GLES 3.1 level but without native support for atomics,
Ilia may be right and the OES_shader_image_atomic extension enable may
be redundant.  I'd be fine with removing it, in which case the
ARB_shader_image_load_store enable would really mean
MESA_shader_image_load_store, largely equivalent to
ARB_shader_image_load_store but with the additional atomic built-in
required for OES_shader_image_atomic support.



I vote for keeping the boolean, this is part of ES32 and extension on 
3.1, I think we might have some more of similar cases when enabling 32.


Reviewed-by: Tapani Palli 



However, this extension has one annoying bit of text:

 "OpenGL ES 3.1 and GLSL ES 3.10 are required."

We may need an ES31 tag.  Right now several drivers support OpenGL ES
3.0, and they could incorrectly advertise this extension.  This may be a
transient state, so it may not matter much in the long run.


Sounds reasonable to me, I'll add an ES31 tag and squash this into this
commit:
| --- a/src/mesa/main/extensions.c
| +++ b/src/mesa/main/extensions.c
| @@ -319,7 +320,7 @@ static const struct extension extension_table[] = {
| { "GL_OES_query_matrix",o(dummy_true),
   ES1,   2003 },
| { "GL_OES_read_format", o(dummy_true),
  GL | ES1,   2003 },
| { "GL_OES_rgb8_rgba8",  o(dummy_true),
   ES1 | ES2, 2005 },
| -   { "GL_OES_shader_image_atomic", 
o(OES_shader_image_atomic),  ES3,   2015 },
| +   { "GL_OES_shader_image_atomic", 
o(OES_shader_image_atomic),ES31,2015 },
| { "GL_OES_single_precision",o(dummy_true),
   ES1,   2003 },
| { "GL_OES_standard_derivatives",
o(OES_standard_derivatives),   ES2, 2005 },
| { "GL_OES_stencil1",o(dummy_false),   
  DISABLE,2005 },


 GLboolean OES_texture_float;
 GLboolean OES_texture_float_linear;
 GLboolean OES_texture_half_float;
--
2.4.6

___
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 mailing list
mesa-dev@lists.freedesktop.org
http://lists.freed

Re: [Mesa-dev] [PATCH 21/25] mesa: Add extension enable and table entry for OES_shader_image_atomic.

2015-08-20 Thread Francisco Jerez
Tapani Pälli  writes:

> On 08/19/2015 02:36 PM, Francisco Jerez wrote:
>> Ian Romanick  writes:
>>
>>> On 08/17/2015 09:56 AM, Ilia Mirkin wrote:
 On Mon, Aug 17, 2015 at 12:46 PM, Francisco Jerez  
 wrote:
> ---
>   src/mesa/main/extensions.c | 1 +
>   src/mesa/main/mtypes.h | 1 +
>   2 files changed, 2 insertions(+)
>
> diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
> index d934d19..38c7bc8 100644
> --- a/src/mesa/main/extensions.c
> +++ b/src/mesa/main/extensions.c
> @@ -319,6 +319,7 @@ static const struct extension extension_table[] = {
>  { "GL_OES_query_matrix",o(dummy_true),   
> ES1,   2003 },
>  { "GL_OES_read_format", o(dummy_true),   
>GL | ES1,   2003 },
>  { "GL_OES_rgb8_rgba8",  o(dummy_true),   
> ES1 | ES2, 2005 },
> +   { "GL_OES_shader_image_atomic", 
> o(OES_shader_image_atomic),  ES3,   2015 },
>  { "GL_OES_single_precision",o(dummy_true),   
> ES1,   2003 },
>  { "GL_OES_standard_derivatives",
> o(OES_standard_derivatives),   ES2, 2005 },
>  { "GL_OES_stencil1",o(dummy_false),  
>DISABLE,2005 },
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 9964fb4..b489f8c 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -3970,6 +3970,7 @@ struct gl_extensions
>  GLboolean OES_draw_texture;
>  GLboolean OES_depth_texture_cube_map;
>  GLboolean OES_EGL_image_external;
> +   GLboolean OES_shader_image_atomic;

 Does this add any functionality over ARB_shader_image_load_store? I
 guess extension bools are cheap, but... seems unnecessary.
>>>
>>> Yes and no.  In terms of functionality, all desktop hardware that can do
>>> GL_ARB_shader_image_load_store can do this extension.
>>
>> Yeah, in principle they should, although it might need non-trivial
>> driver changes if the hardware doesn't support atomic operations at all
>> on floating-point surfaces.  You may need to lie to the hardware and
>> pretend that the surface contains integer data.
>>
>> If we don't expect to have to deal with hardware that can do image loads
>> and stores at the GLES 3.1 level but without native support for atomics,
>> Ilia may be right and the OES_shader_image_atomic extension enable may
>> be redundant.  I'd be fine with removing it, in which case the
>> ARB_shader_image_load_store enable would really mean
>> MESA_shader_image_load_store, largely equivalent to
>> ARB_shader_image_load_store but with the additional atomic built-in
>> required for OES_shader_image_atomic support.
>
>
> I vote for keeping the boolean, this is part of ES32 and extension on 
> 3.1, I think we might have some more of similar cases when enabling 32.
>
> Reviewed-by: Tapani Palli 
>
Thanks!

>
>>> However, this extension has one annoying bit of text:
>>>
>>>  "OpenGL ES 3.1 and GLSL ES 3.10 are required."
>>>
>>> We may need an ES31 tag.  Right now several drivers support OpenGL ES
>>> 3.0, and they could incorrectly advertise this extension.  This may be a
>>> transient state, so it may not matter much in the long run.
>>>
>> Sounds reasonable to me, I'll add an ES31 tag and squash this into this
>> commit:
>> | --- a/src/mesa/main/extensions.c
>> | +++ b/src/mesa/main/extensions.c
>> | @@ -319,7 +320,7 @@ static const struct extension extension_table[] = {
>> | { "GL_OES_query_matrix",o(dummy_true), 
>>   ES1,   2003 },
>> | { "GL_OES_read_format", o(dummy_true), 
>>  GL | ES1,   2003 },
>> | { "GL_OES_rgb8_rgba8",  o(dummy_true), 
>>   ES1 | ES2, 2005 },
>> | -   { "GL_OES_shader_image_atomic", 
>> o(OES_shader_image_atomic),  ES3,   2015 },
>> | +   { "GL_OES_shader_image_atomic", 
>> o(OES_shader_image_atomic),ES31,2015 },
>> | { "GL_OES_single_precision",o(dummy_true), 
>>   ES1,   2003 },
>> | { "GL_OES_standard_derivatives",
>> o(OES_standard_derivatives),   ES2, 2005 },
>> | { "GL_OES_stencil1",o(dummy_false),
>>  DISABLE,2005 },
>>
>  GLboolean OES_texture_float;
>  GLboolean OES_texture_float_linear;
>  GLboolean OES_texture_half_float;
> --
> 2.4.6

Re: [Mesa-dev] [PATCH 03/25] glsl: clean-up link uniform code

2015-08-20 Thread Thomas Helland
2015-08-20 6:37 GMT+02:00 Timothy Arceri :
> These changes are also needed to allow linking of
> struct and interface arrays of arrays.
> ---
>  src/glsl/link_uniforms.cpp | 16 ++--
>  1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> index 254086d..42f75e9 100644
> --- a/src/glsl/link_uniforms.cpp
> +++ b/src/glsl/link_uniforms.cpp
> @@ -72,6 +72,7 @@ void
>  program_resource_visitor::process(ir_variable *var)
>  {
> const glsl_type *t = var->type;
> +   const glsl_type *t_without_array = var->type->without_array();
> const bool row_major =
>var->data.matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR;
>
> @@ -141,12 +142,8 @@ program_resource_visitor::process(ir_variable *var)
>char *name = ralloc_strdup(NULL, var->name);
>recursion(var->type, &name, strlen(name), row_major, NULL, false);
>ralloc_free(name);
> -   } else if (t->is_interface()) {
> -  char *name = ralloc_strdup(NULL, var->type->name);
> -  recursion(var->type, &name, strlen(name), row_major, NULL, false);
> -  ralloc_free(name);
> -   } else if (t->is_array() && t->fields.array->is_interface()) {
> -  char *name = ralloc_strdup(NULL, var->type->fields.array->name);
> +   } else if (t_without_array->is_interface()) {
> +  char *name = ralloc_strdup(NULL, t_without_array->name);
>recursion(var->type, &name, strlen(name), row_major, NULL, false);
>ralloc_free(name);
> } else {
> @@ -217,8 +214,8 @@ program_resource_visitor::recursion(const glsl_type *t, 
> char **name,
>   (*name)[name_length] = '\0';
>   this->leave_record(t, *name, row_major);
>}
> -   } else if (t->is_array() && (t->fields.array->is_record()
> -|| t->fields.array->is_interface())) {
> +   } else if (t->without_array()->is_record()
> +  || t->without_array()->is_interface()) {

I think we normally put the "||" on the previous line.
Apart from that this looks correct and is

Reviewed-by: Thomas Helland 

>if (record_type == NULL && t->fields.array->is_record())
>   record_type = t->fields.array;
>
> @@ -810,8 +807,7 @@ link_update_uniform_buffer_variables(struct gl_shader 
> *shader)
>
>if (var->type->is_record()) {
>   sentinel = '.';
> -  } else if (var->type->is_array()
> - && var->type->fields.array->is_record()) {
> +  } else if (var->type->without_array()->is_record()) {
>   sentinel = '[';
>}
>
> --
> 2.4.3
>
> ___
> 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] [PATCH v2] docs: Update todo regarding StencilOp and StencilOpSeparate.

2015-08-20 Thread Rhys Kidd
OpenGL 2.0 function StencilOp() is in part internally implemented via
StencilOpSeparate(). This change happened some time ago, however the
accompanying doxygen todo comment was not accordingly updated.

Replace the outdated portion of this doxygen todo comment, leaving the
remainder unchanged.

Also better respect the 80 character suggested line length in this file.

v2: Fully remove comment, following code review by t_arc...@yahoo.com.au
Signed-off-by: Rhys Kidd 
---
 src/mesa/main/stencil.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/src/mesa/main/stencil.c b/src/mesa/main/stencil.c
index 2a19a17..409b2f0 100644
--- a/src/mesa/main/stencil.c
+++ b/src/mesa/main/stencil.c
@@ -124,8 +124,8 @@ _mesa_ClearStencil( GLint s )
  * \sa glStencilFunc().
  *
  * Verifies the parameters and updates the respective values in
- * __struct gl_contextRec::Stencil. On change flushes the vertices and 
notifies the
- * driver via the dd_function_table::StencilFunc callback.
+ * __struct gl_contextRec::Stencil. On change flushes the vertices and notifies
+ * the driver via the dd_function_table::StencilFunc callback.
  */
 void GLAPIENTRY
 _mesa_StencilFuncSeparateATI( GLenum frontfunc, GLenum backfunc, GLint ref, 
GLuint mask )
@@ -178,8 +178,8 @@ _mesa_StencilFuncSeparateATI( GLenum frontfunc, GLenum 
backfunc, GLint ref, GLui
  * \sa glStencilFunc().
  *
  * Verifies the parameters and updates the respective values in
- * __struct gl_contextRec::Stencil. On change flushes the vertices and 
notifies the
- * driver via the dd_function_table::StencilFunc callback.
+ * __struct gl_contextRec::Stencil. On change flushes the vertices and notifies
+ * the driver via the dd_function_table::StencilFunc callback.
  */
 void GLAPIENTRY
 _mesa_StencilFunc( GLenum func, GLint ref, GLuint mask )
@@ -298,8 +298,8 @@ _mesa_StencilMask( GLuint mask )
  * \sa glStencilOp().
  * 
  * Verifies the parameters and updates the respective fields in
- * __struct gl_contextRec::Stencil. On change flushes the vertices and 
notifies the
- * driver via the dd_function_table::StencilOp callback.
+ * __struct gl_contextRec::Stencil. On change flushes the vertices and notifies
+ * the driver via the dd_function_table::StencilOp callback.
  */
 void GLAPIENTRY
 _mesa_StencilOp(GLenum fail, GLenum zfail, GLenum zpass)
@@ -389,12 +389,6 @@ _mesa_ActiveStencilFaceEXT(GLenum face)
 
 
 
-/**
- * OpenGL 2.0 function.
- * \todo Make StencilOp() call this function.  And eventually remove the
- * ctx->Driver.StencilOp function and use ctx->Driver.StencilOpSeparate
- * instead.
- */
 void GLAPIENTRY
 _mesa_StencilOpSeparate(GLenum face, GLenum sfail, GLenum zfail, GLenum zpass)
 {
-- 
2.1.4

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


Re: [Mesa-dev] [PATCH v2] docs: Update todo regarding StencilOp and StencilOpSeparate.

2015-08-20 Thread Thomas Helland
2015-08-20 15:03 GMT+02:00 Rhys Kidd :
> OpenGL 2.0 function StencilOp() is in part internally implemented via
> StencilOpSeparate(). This change happened some time ago, however the
> accompanying doxygen todo comment was not accordingly updated.
>
> Replace the outdated portion of this doxygen todo comment, leaving the
> remainder unchanged.
>
> Also better respect the 80 character suggested line length in this file.
>
> v2: Fully remove comment, following code review by t_arc...@yahoo.com.au
> Signed-off-by: Rhys Kidd 
> ---
>  src/mesa/main/stencil.c | 18 ++
>  1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/src/mesa/main/stencil.c b/src/mesa/main/stencil.c
> index 2a19a17..409b2f0 100644
> --- a/src/mesa/main/stencil.c
> +++ b/src/mesa/main/stencil.c
> @@ -124,8 +124,8 @@ _mesa_ClearStencil( GLint s )
>   * \sa glStencilFunc().
>   *
>   * Verifies the parameters and updates the respective values in
> - * __struct gl_contextRec::Stencil. On change flushes the vertices and 
> notifies the
> - * driver via the dd_function_table::StencilFunc callback.
> + * __struct gl_contextRec::Stencil. On change flushes the vertices and 
> notifies
> + * the driver via the dd_function_table::StencilFunc callback.
>   */
>  void GLAPIENTRY
>  _mesa_StencilFuncSeparateATI( GLenum frontfunc, GLenum backfunc, GLint ref, 
> GLuint mask )
> @@ -178,8 +178,8 @@ _mesa_StencilFuncSeparateATI( GLenum frontfunc, GLenum 
> backfunc, GLint ref, GLui
>   * \sa glStencilFunc().
>   *
>   * Verifies the parameters and updates the respective values in
> - * __struct gl_contextRec::Stencil. On change flushes the vertices and 
> notifies the
> - * driver via the dd_function_table::StencilFunc callback.
> + * __struct gl_contextRec::Stencil. On change flushes the vertices and 
> notifies
> + * the driver via the dd_function_table::StencilFunc callback.
>   */
>  void GLAPIENTRY
>  _mesa_StencilFunc( GLenum func, GLint ref, GLuint mask )
> @@ -298,8 +298,8 @@ _mesa_StencilMask( GLuint mask )
>   * \sa glStencilOp().
>   *
>   * Verifies the parameters and updates the respective fields in
> - * __struct gl_contextRec::Stencil. On change flushes the vertices and 
> notifies the
> - * driver via the dd_function_table::StencilOp callback.
> + * __struct gl_contextRec::Stencil. On change flushes the vertices and 
> notifies
> + * the driver via the dd_function_table::StencilOp callback.
>   */
>  void GLAPIENTRY
>  _mesa_StencilOp(GLenum fail, GLenum zfail, GLenum zpass)
> @@ -389,12 +389,6 @@ _mesa_ActiveStencilFaceEXT(GLenum face)
>
>
>
> -/**
> - * OpenGL 2.0 function.
> - * \todo Make StencilOp() call this function.  And eventually remove the
> - * ctx->Driver.StencilOp function and use ctx->Driver.StencilOpSeparate
> - * instead.
> - */
>  void GLAPIENTRY
>  _mesa_StencilOpSeparate(GLenum face, GLenum sfail, GLenum zfail, GLenum 
> zpass)
>  {
> --
> 2.1.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

This patch looks good and is:

Reviewed-by: Thomas Helland 

I'll pull it down, add the R-b tag, and temporarily store it 'til I get
a small bunch of reviewed patches. Then I'll send a request to one
of the guys with commit-access, and ask them to commit the bunch.

Thanks a lot for your contribution!

PS: Your last patch series landed a couple days ago.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 02/21] i965: Make setup_vec4_uniform_value and _image_uniform_values take an offset

2015-08-20 Thread Francisco Jerez
Jason Ekstrand  writes:

> This way they don't implicitly increment the uniforms variable and don't
> have to be called in-sequence during uniform setup.
>
> Cc: Francisco Jerez 
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp   |  7 ---
>  src/mesa/drivers/dri/i965/brw_fs.h |  3 ++-
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp   |  7 +--
>  src/mesa/drivers/dri/i965/brw_shader.cpp   | 16 +---
>  src/mesa/drivers/dri/i965/brw_shader.h |  6 --
>  src/mesa/drivers/dri/i965/brw_vec4.h   |  3 ++-
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 18 --
>  7 files changed, 38 insertions(+), 22 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 47cc167..6ee9f3a 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -943,16 +943,17 @@ fs_visitor::import_uniforms(fs_visitor *v)
>  }
>  
>  void
> -fs_visitor::setup_vec4_uniform_value(const gl_constant_value *values,
> +fs_visitor::setup_vec4_uniform_value(unsigned param_offset,
> + const gl_constant_value *values,
>   unsigned n)
>  {
> static const gl_constant_value zero = { 0 };
>  
> for (unsigned i = 0; i < n; ++i)
> -  stage_prog_data->param[uniforms++] = &values[i];
> +  stage_prog_data->param[param_offset + i] = &values[i];
>  
> for (unsigned i = n; i < 4; ++i)
> -  stage_prog_data->param[uniforms++] = &zero;
> +  stage_prog_data->param[param_offset + i] = &zero;
>  }
>  
>  fs_reg *
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index 1a56c2a..9484e63 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -291,7 +291,8 @@ public:
>  
> struct brw_reg interp_reg(int location, int channel);
>  
> -   virtual void setup_vec4_uniform_value(const gl_constant_value *values,
> +   virtual void setup_vec4_uniform_value(unsigned param_offset,
> + const gl_constant_value *values,
>   unsigned n);
>  
> int implied_mrf_writes(fs_inst *inst);
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index ce4153d..7873d3f 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -244,10 +244,13 @@ fs_visitor::nir_setup_uniform(nir_variable *var)
>* space for them here at the end of the parameter array.
>*/
>   var->data.driver_location = uniforms;
> - param_size[uniforms] =
> + unsigned size =

const?

>  BRW_IMAGE_PARAM_SIZE * MAX2(storage->array_elements, 1);
>  
> - setup_image_uniform_values(storage);
> + setup_image_uniform_values(uniforms, storage);
> +
> + param_size[uniforms] = size;
> + uniforms += size;
>} else {
>   unsigned slots = storage->type->component_slots();
>   if (storage->array_elements)
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp 
> b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index 48fbe8f..a7453fa 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -1420,7 +1420,8 @@ 
> backend_shader::assign_common_binding_table_offsets(uint32_t 
> next_binding_table_
>  }
>  
>  void
> -backend_shader::setup_image_uniform_values(const gl_uniform_storage *storage)
> +backend_shader::setup_image_uniform_values(unsigned param_offset,
> +   const gl_uniform_storage *storage)
>  {
> const unsigned stage = _mesa_program_enum_to_shader_stage(prog->Target);
>  
> @@ -1431,18 +1432,19 @@ backend_shader::setup_image_uniform_values(const 
> gl_uniform_storage *storage)
>/* Upload the brw_image_param structure.  The order is expected to 
> match
> * the BRW_IMAGE_PARAM_*_OFFSET defines.
> */
> -  setup_vec4_uniform_value(
> +  setup_vec4_uniform_value(param_offset,
>   (const gl_constant_value *)¶m->surface_idx, 1);
> -  setup_vec4_uniform_value(
> +  setup_vec4_uniform_value(param_offset + 4,
>   (const gl_constant_value *)param->offset, 2);
> -  setup_vec4_uniform_value(
> +  setup_vec4_uniform_value(param_offset + 8,
>   (const gl_constant_value *)param->size, 3);
> -  setup_vec4_uniform_value(
> +  setup_vec4_uniform_value(param_offset + 12,
>   (const gl_constant_value *)param->stride, 4);
> -  setup_vec4_uniform_value(
> +  setup_vec4_uniform_value(param_offset + 16,
>   (const gl_constant_value *)param->tiling, 3);
> -  setup_vec4_uniform_value(
> +  setup_vec4_uniform_value(param_offset + 20,
>   (const gl_constant_value *)param->swizzling, 2);
> +  param_

Re: [Mesa-dev] [PATCH 02/21] i965: Make setup_vec4_uniform_value and _image_uniform_values take an offset

2015-08-20 Thread Jason Ekstrand
On Thu, Aug 20, 2015 at 7:20 AM, Francisco Jerez  wrote:
> Jason Ekstrand  writes:
>
>> This way they don't implicitly increment the uniforms variable and don't
>> have to be called in-sequence during uniform setup.
>>
>> Cc: Francisco Jerez 
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.cpp   |  7 ---
>>  src/mesa/drivers/dri/i965/brw_fs.h |  3 ++-
>>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp   |  7 +--
>>  src/mesa/drivers/dri/i965/brw_shader.cpp   | 16 +---
>>  src/mesa/drivers/dri/i965/brw_shader.h |  6 --
>>  src/mesa/drivers/dri/i965/brw_vec4.h   |  3 ++-
>>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 18 --
>>  7 files changed, 38 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 47cc167..6ee9f3a 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -943,16 +943,17 @@ fs_visitor::import_uniforms(fs_visitor *v)
>>  }
>>
>>  void
>> -fs_visitor::setup_vec4_uniform_value(const gl_constant_value *values,
>> +fs_visitor::setup_vec4_uniform_value(unsigned param_offset,
>> + const gl_constant_value *values,
>>   unsigned n)
>>  {
>> static const gl_constant_value zero = { 0 };
>>
>> for (unsigned i = 0; i < n; ++i)
>> -  stage_prog_data->param[uniforms++] = &values[i];
>> +  stage_prog_data->param[param_offset + i] = &values[i];
>>
>> for (unsigned i = n; i < 4; ++i)
>> -  stage_prog_data->param[uniforms++] = &zero;
>> +  stage_prog_data->param[param_offset + i] = &zero;
>>  }
>>
>>  fs_reg *
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
>> b/src/mesa/drivers/dri/i965/brw_fs.h
>> index 1a56c2a..9484e63 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.h
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>> @@ -291,7 +291,8 @@ public:
>>
>> struct brw_reg interp_reg(int location, int channel);
>>
>> -   virtual void setup_vec4_uniform_value(const gl_constant_value *values,
>> +   virtual void setup_vec4_uniform_value(unsigned param_offset,
>> + const gl_constant_value *values,
>>   unsigned n);
>>
>> int implied_mrf_writes(fs_inst *inst);
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
>> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> index ce4153d..7873d3f 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -244,10 +244,13 @@ fs_visitor::nir_setup_uniform(nir_variable *var)
>>* space for them here at the end of the parameter array.
>>*/
>>   var->data.driver_location = uniforms;
>> - param_size[uniforms] =
>> + unsigned size =
>
> const?

That line gets deleted 3 or 4 patches later so I'd rather not deal
with the merge conflicts.

>>  BRW_IMAGE_PARAM_SIZE * MAX2(storage->array_elements, 1);
>>
>> - setup_image_uniform_values(storage);
>> + setup_image_uniform_values(uniforms, storage);
>> +
>> + param_size[uniforms] = size;
>> + uniforms += size;
>>} else {
>>   unsigned slots = storage->type->component_slots();
>>   if (storage->array_elements)
>> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp 
>> b/src/mesa/drivers/dri/i965/brw_shader.cpp
>> index 48fbe8f..a7453fa 100644
>> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
>> @@ -1420,7 +1420,8 @@ 
>> backend_shader::assign_common_binding_table_offsets(uint32_t 
>> next_binding_table_
>>  }
>>
>>  void
>> -backend_shader::setup_image_uniform_values(const gl_uniform_storage 
>> *storage)
>> +backend_shader::setup_image_uniform_values(unsigned param_offset,
>> +   const gl_uniform_storage 
>> *storage)
>>  {
>> const unsigned stage = _mesa_program_enum_to_shader_stage(prog->Target);
>>
>> @@ -1431,18 +1432,19 @@ backend_shader::setup_image_uniform_values(const 
>> gl_uniform_storage *storage)
>>/* Upload the brw_image_param structure.  The order is expected to 
>> match
>> * the BRW_IMAGE_PARAM_*_OFFSET defines.
>> */
>> -  setup_vec4_uniform_value(
>> +  setup_vec4_uniform_value(param_offset,
>>   (const gl_constant_value *)¶m->surface_idx, 1);
>> -  setup_vec4_uniform_value(
>> +  setup_vec4_uniform_value(param_offset + 4,
>>   (const gl_constant_value *)param->offset, 2);
>> -  setup_vec4_uniform_value(
>> +  setup_vec4_uniform_value(param_offset + 8,
>>   (const gl_constant_value *)param->size, 3);
>> -  setup_vec4_uniform_value(
>> +  setup_vec4_uniform_value(param_offset + 12,
>>   (const gl_constant_value *)param->stride, 4);
>> -  setup_vec4_uniform_value(
>> +  setup_ve

[Mesa-dev] New stable-branch 10.6 candidate pushed

2015-08-20 Thread Emil Velikov
Hello list,

The candidate for the Mesa 10.6.5 is now available. Currently we have:
 - 20 queued
 - 16 nominated (outstanding)
 - and 3 rejected/obsolete patches

Apart from the usual nouveau, i965 and radeon driver fixes, the series
resolves a few serious issues with nouveau_vieux users and big endian platforms.


Take a look at section "Mesa stable queue" for more information.

Testing
---
The following results are against piglit 246791c51ec.


Changes - classic i965(snb)
---
None.


Changes - swrast classic

None.


Changes - gallium softpipe
--
None.


Changes - gallium llvmpipe (LLVM 3.6.2)
---
None.


Testing reports/general approval

Any testing reports (or general approval of the state of the branch)
will be greatly appreciated.


Trivial merge conflicts
---
None.


The plan is to have 10.6.5 this Monday(21th of August).

If you have any questions or comments that you would like to share
before the release, please go ahead.


Cheers,
Emil


Mesa stable queue
-

Nominated (16)
==

Boyan Ding (1):
  i915: Add XRGB format to intel_screen_make_configs

Brian Paul (1):
  configure: don't try to build gallium DRI drivers if --disable-dri is set

Chris Wilson (1):
  i965: Prevent coordinate overflow in intel_emit_linear_blit

David Heidelberg (1):
  st/nine: Require gcc >= 4.6

Emil Velikov (2):
  mapi: automake: inline glapi_gen_mapi define
  xmlpool: remove LOCALEDIR variable/fix bmake

Jason Ekstrand (1):
  i965/fs: Split VGRFs after lowering pull constants

Neil Roberts (2):
  i965: Swap the order of the vertex ID and edge flag attributes
  i965/bdw: Fix 3DSTATE_VF_INSTANCING when the edge flag is used

Rob Clark (1):
  xa: add xa_surface_from_handle2

Tapani Pälli (1):
  mesa: update fbo state in glTexStorage

Timothy Arceri (1):
  glsl: fix atomic buffer index for bindings other than 0

Tom Stellard (4):
  clover: Call clBuildProgram() notification function when build completes 
v2
  gallium/drivers: Add threadsafe wrappers for pipe_context v2
  clover: Use threadsafe wrappers for pipe_context v2
  clover: Properly initialize LLVM targets when linking with component libs



Queued (20)
===

Adam Jackson (1):
  glx: Fix __glXWireToEvent for BufferSwapComplete

Alex Deucher (2):
  radeonsi: add new OLAND pci id
  radeonsi: properly set the raster_config for KV

Emil Velikov (2):
  docs: add sha256 checksums for 10.6.4
  vc4: add missing nir include, to fix the build

Frank Binns (1):
  egl/x11: don't abort when creating a DRI2 drawable fails

Ilia Mirkin (3):
  nouveau: no need to do tnl wakeup, state updates are always hooked up
  gm107/ir: indirect handle goes first on maxwell also
  nv50,nvc0: take level into account when doing eng2d multi-layer blits

Jason Ekstrand (4):
  meta/copy_image: Stash off the scissor
  mesa/formats: Only do byteswapping for packed formats
  mesa/formats: Fix swizzle flipping for big-endian targets
  mesa/formats: Don't flip channels of null array formats

Marek Olšák (3):
  radeonsi: fix polygon offset scale
  r600g: fix polygon offset scale
  r600g: allow setting geometry shader sampler states

Neil Roberts (1):
  i965/bdw: Fix setting the instancing state for the SGVS element

Oded Gabbay (2):
  mesa: clear existing swizzle info before bitwise-OR
  mesa/formats: don't byteswap when building array formats

Renaud Gaubert (1):
  glsl: avoid compiler's segfault when processing operators with void 
arguments


Obsolete (1)


Oded Gabbay (1):
  mesa/formats: don't byteswap when building array formats


Rejected (2)


Oded Gabbay (1):
  mesa: _mesa_format_convert should be endian agnostic

Anuj Phogat (1):
  i965: Fix {src, dst}_pitch alignment check for XY_SRC_COPY_BLT

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


Re: [Mesa-dev] [PATCH] glsl: expose textureQueryLod in GLSL 4.00+ fragment shaders

2015-08-20 Thread Timothy Arceri
On Wed, 2015-08-19 at 18:44 -0400, Ilia Mirkin wrote:

Maybe quote or at least make mention of Issue 3 of the ARB_texture_query_lod
spec in the commit message so anyone searching git history can easily discover
why there are two differently named versions.

Otherwise Reviewed-by: Timothy Arceri 

> Signed-off-by: Ilia Mirkin 
> ---
>  src/glsl/builtin_functions.cpp | 119 --
> ---
>  1 file changed, 82 insertions(+), 37 deletions(-)
> 
> diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
> index 04ad617..9351f71 100644
> --- a/src/glsl/builtin_functions.cpp
> +++ b/src/glsl/builtin_functions.cpp
> @@ -136,6 +136,13 @@ v140(const _mesa_glsl_parse_state *state)
>  }
>  
>  static bool
> +v400_fs_only(const _mesa_glsl_parse_state *state)
> +{
> +   return state->is_version(400, 0) &&
> +  state->stage == MESA_SHADER_FRAGMENT;
> +}
> +
> +static bool
>  es31(const _mesa_glsl_parse_state *state)
>  {
> return state->is_version(0, 310);
> @@ -670,7 +677,7 @@ private:
>const glsl_type 
> *stream_type);
> B0(barrier)
>  
> -   B2(textureQueryLod);
> +   BA2(textureQueryLod);
> B1(textureQueryLevels);
> B1(dFdx);
> B1(dFdy);
> @@ -1951,40 +1958,77 @@ builtin_builder::create_builtins()
> add_function("barrier", _barrier(), NULL);
>  
> add_function("textureQueryLOD",
> -_textureQueryLod(glsl_type::sampler1D_type, 
>  glsl_type::float_type),
> -_textureQueryLod(glsl_type::isampler1D_type, 
> glsl_type::float_type),
> -_textureQueryLod(glsl_type::usampler1D_type, 
> glsl_type::float_type),
> -
> -_textureQueryLod(glsl_type::sampler2D_type, 
>  glsl_type::vec2_type),
> -_textureQueryLod(glsl_type::isampler2D_type, 
> glsl_type::vec2_type),
> -_textureQueryLod(glsl_type::usampler2D_type, 
> glsl_type::vec2_type),
> -
> -_textureQueryLod(glsl_type::sampler3D_type, 
>  glsl_type::vec3_type),
> -_textureQueryLod(glsl_type::isampler3D_type, 
> glsl_type::vec3_type),
> -_textureQueryLod(glsl_type::usampler3D_type, 
> glsl_type::vec3_type),
> -
> -_textureQueryLod(glsl_type::samplerCube_type, 
>  glsl_type::vec3_type),
> -_textureQueryLod(glsl_type::isamplerCube_type, 
> glsl_type::vec3_type),
> -_textureQueryLod(glsl_type::usamplerCube_type, 
> glsl_type::vec3_type),
> -
> -_textureQueryLod(glsl_type::sampler1DArray_type, 
>  glsl_type::float_type),
> -_textureQueryLod(glsl_type::isampler1DArray_type, 
> glsl_type::float_type),
> -_textureQueryLod(glsl_type::usampler1DArray_type, 
> glsl_type::float_type),
> -
> -_textureQueryLod(glsl_type::sampler2DArray_type, 
>  glsl_type::vec2_type),
> -_textureQueryLod(glsl_type::isampler2DArray_type, 
> glsl_type::vec2_type),
> -_textureQueryLod(glsl_type::usampler2DArray_type, 
> glsl_type::vec2_type),
> -
> -_textureQueryLod(glsl_type::samplerCubeArray_type, 
>  glsl_type::vec3_type),
> -_textureQueryLod(glsl_type::isamplerCubeArray_type, 
> glsl_type::vec3_type),
> -_textureQueryLod(glsl_type::usamplerCubeArray_type, 
> glsl_type::vec3_type),
> -
> -_textureQueryLod(glsl_type::sampler1DShadow_type, 
> glsl_type::float_type),
> -_textureQueryLod(glsl_type::sampler2DShadow_type, 
> glsl_type::vec2_type),
> -_textureQueryLod(glsl_type::samplerCubeShadow_type, 
> glsl_type::vec3_type),
> -_textureQueryLod(glsl_type::sampler1DArrayShadow_type, 
> glsl_type::float_type),
> -_textureQueryLod(glsl_type::sampler2DArrayShadow_type, 
> glsl_type::vec2_type),
> -_textureQueryLod(glsl_type::samplerCubeArrayShadow_type, 
> glsl_type::vec3_type),
> +_textureQueryLod(texture_query_lod, 
> glsl_type::sampler1D_type,  glsl_type::float_type),
> +_textureQueryLod(texture_query_lod, 
> glsl_type::isampler1D_type, glsl_type::float_type),
> +_textureQueryLod(texture_query_lod, 
> glsl_type::usampler1D_type, glsl_type::float_type),
> +
> +_textureQueryLod(texture_query_lod, 
> glsl_type::sampler2D_type,  glsl_type::vec2_type),
> +_textureQueryLod(texture_query_lod, 
> glsl_type::isampler2D_type, glsl_type::vec2_type),
> +_textureQueryLod(texture_query_lod, 
> glsl_type::usampler2D_type, glsl_type::vec2_type),
> +
> +_textureQueryLod(texture_query_lod, 
> glsl_type::sampler3D_type,  glsl_type::vec3_type),
> +_textureQueryLod(texture_query_lod, 
> glsl_type::isampler3D_type, glsl_type::vec3_type),
> +_textureQueryLod(texture_query_lod, 
> glsl_type::usampler3D_type, glsl_type::vec3_type),
> +
> + 

Re: [Mesa-dev] [PATCH] glsl: expose textureQueryLod in GLSL 4.00+ fragment shaders

2015-08-20 Thread Ilia Mirkin
So just stick something like

"""
From the ARB_texture_query_lod spec:

(3) The core specification uses the "Lod" spelling, not "LOD". Should
this extension be modified to use "Lod"?

  RESOLVED: The "Lod" spelling is the correct spelling for the core
  specification and the preferred spelling for use. However, use of
  "LOD" also exists, as the extension predated the core specification,
  so this extension won't remove use of "LOD".
"""

as the commit message? Fine by me. It seems excessive to put that into
builtin_functions.cpp... but if people feel strongly, I can do that
too.

On Thu, Aug 20, 2015 at 11:09 AM, Timothy Arceri  wrote:
> On Wed, 2015-08-19 at 18:44 -0400, Ilia Mirkin wrote:
>
> Maybe quote or at least make mention of Issue 3 of the ARB_texture_query_lod
> spec in the commit message so anyone searching git history can easily discover
> why there are two differently named versions.
>
> Otherwise Reviewed-by: Timothy Arceri 
>
>> Signed-off-by: Ilia Mirkin 
>> ---
>>  src/glsl/builtin_functions.cpp | 119 --
>> ---
>>  1 file changed, 82 insertions(+), 37 deletions(-)
>>
>> diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
>> index 04ad617..9351f71 100644
>> --- a/src/glsl/builtin_functions.cpp
>> +++ b/src/glsl/builtin_functions.cpp
>> @@ -136,6 +136,13 @@ v140(const _mesa_glsl_parse_state *state)
>>  }
>>
>>  static bool
>> +v400_fs_only(const _mesa_glsl_parse_state *state)
>> +{
>> +   return state->is_version(400, 0) &&
>> +  state->stage == MESA_SHADER_FRAGMENT;
>> +}
>> +
>> +static bool
>>  es31(const _mesa_glsl_parse_state *state)
>>  {
>> return state->is_version(0, 310);
>> @@ -670,7 +677,7 @@ private:
>>const glsl_type
>> *stream_type);
>> B0(barrier)
>>
>> -   B2(textureQueryLod);
>> +   BA2(textureQueryLod);
>> B1(textureQueryLevels);
>> B1(dFdx);
>> B1(dFdy);
>> @@ -1951,40 +1958,77 @@ builtin_builder::create_builtins()
>> add_function("barrier", _barrier(), NULL);
>>
>> add_function("textureQueryLOD",
>> -_textureQueryLod(glsl_type::sampler1D_type,
>>  glsl_type::float_type),
>> -_textureQueryLod(glsl_type::isampler1D_type,
>> glsl_type::float_type),
>> -_textureQueryLod(glsl_type::usampler1D_type,
>> glsl_type::float_type),
>> -
>> -_textureQueryLod(glsl_type::sampler2D_type,
>>  glsl_type::vec2_type),
>> -_textureQueryLod(glsl_type::isampler2D_type,
>> glsl_type::vec2_type),
>> -_textureQueryLod(glsl_type::usampler2D_type,
>> glsl_type::vec2_type),
>> -
>> -_textureQueryLod(glsl_type::sampler3D_type,
>>  glsl_type::vec3_type),
>> -_textureQueryLod(glsl_type::isampler3D_type,
>> glsl_type::vec3_type),
>> -_textureQueryLod(glsl_type::usampler3D_type,
>> glsl_type::vec3_type),
>> -
>> -_textureQueryLod(glsl_type::samplerCube_type,
>>  glsl_type::vec3_type),
>> -_textureQueryLod(glsl_type::isamplerCube_type,
>> glsl_type::vec3_type),
>> -_textureQueryLod(glsl_type::usamplerCube_type,
>> glsl_type::vec3_type),
>> -
>> -_textureQueryLod(glsl_type::sampler1DArray_type,
>>  glsl_type::float_type),
>> -_textureQueryLod(glsl_type::isampler1DArray_type,
>> glsl_type::float_type),
>> -_textureQueryLod(glsl_type::usampler1DArray_type,
>> glsl_type::float_type),
>> -
>> -_textureQueryLod(glsl_type::sampler2DArray_type,
>>  glsl_type::vec2_type),
>> -_textureQueryLod(glsl_type::isampler2DArray_type,
>> glsl_type::vec2_type),
>> -_textureQueryLod(glsl_type::usampler2DArray_type,
>> glsl_type::vec2_type),
>> -
>> -_textureQueryLod(glsl_type::samplerCubeArray_type,
>>  glsl_type::vec3_type),
>> -_textureQueryLod(glsl_type::isamplerCubeArray_type,
>> glsl_type::vec3_type),
>> -_textureQueryLod(glsl_type::usamplerCubeArray_type,
>> glsl_type::vec3_type),
>> -
>> -_textureQueryLod(glsl_type::sampler1DShadow_type,
>> glsl_type::float_type),
>> -_textureQueryLod(glsl_type::sampler2DShadow_type,
>> glsl_type::vec2_type),
>> -_textureQueryLod(glsl_type::samplerCubeShadow_type,
>> glsl_type::vec3_type),
>> -_textureQueryLod(glsl_type::sampler1DArrayShadow_type,
>> glsl_type::float_type),
>> -_textureQueryLod(glsl_type::sampler2DArrayShadow_type,
>> glsl_type::vec2_type),
>> -_textureQueryLod(glsl_type::samplerCubeArrayShadow_type,
>> glsl_type::vec3_type),
>> +_textureQueryLod(texture_query_lod,
>> glsl_type::sampler1D_type,  glsl_type::float_type),
>> +_textureQueryLod(texture_query_lod,
>> glsl_type::isampler1D_type, glsl_type::float_type),
>> +_textureQuer

Re: [Mesa-dev] New stable-branch 10.6 candidate pushed

2015-08-20 Thread Alex Deucher
On Thu, Aug 20, 2015 at 11:08 AM, Emil Velikov  wrote:
> Hello list,
>
> The candidate for the Mesa 10.6.5 is now available. Currently we have:
>  - 20 queued
>  - 16 nominated (outstanding)
>  - and 3 rejected/obsolete patches
>
> Apart from the usual nouveau, i965 and radeon driver fixes, the series
> resolves a few serious issues with nouveau_vieux users and big endian 
> platforms.
>
>
> Take a look at section "Mesa stable queue" for more information.
>
> Testing
> ---
> The following results are against piglit 246791c51ec.
>
>
> Changes - classic i965(snb)
> ---
> None.
>
>
> Changes - swrast classic
> 
> None.
>
>
> Changes - gallium softpipe
> --
> None.
>
>
> Changes - gallium llvmpipe (LLVM 3.6.2)
> ---
> None.
>
>
> Testing reports/general approval
> 
> Any testing reports (or general approval of the state of the branch)
> will be greatly appreciated.
>
>
> Trivial merge conflicts
> ---
> None.
>
>
> The plan is to have 10.6.5 this Monday(21th of August).
>
> If you have any questions or comments that you would like to share
> before the release, please go ahead.
>
>
> Cheers,
> Emil
>
>
> Mesa stable queue
> -
>
> Nominated (16)
> ==
>
> Boyan Ding (1):
>   i915: Add XRGB format to intel_screen_make_configs
>
> Brian Paul (1):
>   configure: don't try to build gallium DRI drivers if --disable-dri is 
> set
>
> Chris Wilson (1):
>   i965: Prevent coordinate overflow in intel_emit_linear_blit
>
> David Heidelberg (1):
>   st/nine: Require gcc >= 4.6
>
> Emil Velikov (2):
>   mapi: automake: inline glapi_gen_mapi define
>   xmlpool: remove LOCALEDIR variable/fix bmake
>
> Jason Ekstrand (1):
>   i965/fs: Split VGRFs after lowering pull constants
>
> Neil Roberts (2):
>   i965: Swap the order of the vertex ID and edge flag attributes
>   i965/bdw: Fix 3DSTATE_VF_INSTANCING when the edge flag is used
>
> Rob Clark (1):
>   xa: add xa_surface_from_handle2
>
> Tapani Pälli (1):
>   mesa: update fbo state in glTexStorage
>
> Timothy Arceri (1):
>   glsl: fix atomic buffer index for bindings other than 0
>
> Tom Stellard (4):
>   clover: Call clBuildProgram() notification function when build 
> completes v2
>   gallium/drivers: Add threadsafe wrappers for pipe_context v2
>   clover: Use threadsafe wrappers for pipe_context v2
>   clover: Properly initialize LLVM targets when linking with component 
> libs
>
>
>
> Queued (20)
> ===
>
> Adam Jackson (1):
>   glx: Fix __glXWireToEvent for BufferSwapComplete
>
> Alex Deucher (2):
>   radeonsi: add new OLAND pci id
>   radeonsi: properly set the raster_config for KV

Please drop "radeonsi: properly set the raster_config for KV" for stable.

Alex


>
> Emil Velikov (2):
>   docs: add sha256 checksums for 10.6.4
>   vc4: add missing nir include, to fix the build
>
> Frank Binns (1):
>   egl/x11: don't abort when creating a DRI2 drawable fails
>
> Ilia Mirkin (3):
>   nouveau: no need to do tnl wakeup, state updates are always hooked up
>   gm107/ir: indirect handle goes first on maxwell also
>   nv50,nvc0: take level into account when doing eng2d multi-layer blits
>
> Jason Ekstrand (4):
>   meta/copy_image: Stash off the scissor
>   mesa/formats: Only do byteswapping for packed formats
>   mesa/formats: Fix swizzle flipping for big-endian targets
>   mesa/formats: Don't flip channels of null array formats
>
> Marek Olšák (3):
>   radeonsi: fix polygon offset scale
>   r600g: fix polygon offset scale
>   r600g: allow setting geometry shader sampler states
>
> Neil Roberts (1):
>   i965/bdw: Fix setting the instancing state for the SGVS element
>
> Oded Gabbay (2):
>   mesa: clear existing swizzle info before bitwise-OR
>   mesa/formats: don't byteswap when building array formats
>
> Renaud Gaubert (1):
>   glsl: avoid compiler's segfault when processing operators with void 
> arguments
>
>
> Obsolete (1)
> 
>
> Oded Gabbay (1):
>   mesa/formats: don't byteswap when building array formats
>
>
> Rejected (2)
> 
>
> Oded Gabbay (1):
>   mesa: _mesa_format_convert should be endian agnostic
>
> Anuj Phogat (1):
>   i965: Fix {src, dst}_pitch alignment check for XY_SRC_COPY_BLT
>
> ___
> 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] glsl: check if return_deref in lower_subroutine_visitor::visit_leave isn't NULL

2015-08-20 Thread Emil Velikov
On 14 August 2015 at 13:49, Kai Wasserbäch  wrote:
> Fixes a crash in Piglit's
> spec@arb_shader_subroutine@lin...@no-mutual-recursion.vert for me.
>
> Signed-off-by: Kai Wasserbäch 
> ---
>
> Hey everyone,
> I ran the Piglit quick test suite afterwards and haven't observed any
> regressions over my previous quick run, but the crash went away. The test
> itself passes now! Instead of
>
> | Program received signal SIGSEGV, Segmentation fault.
> | (anonymous namespace)::lower_subroutine_visitor::visit_leave 
> (this=0x7fffdab0, ir=0xb1af58) at 
> ../../../../src/glsl/lower_subroutine.cpp:102
>
> I'm seeing the (expected)
>
> | Failed to link: error: function `void impl_b(int)' has static recursion.
> | error: function `void impl_a(int)' has static recursion.
> |
> | Failed to link vertex shader 
> /tests/spec/arb_shader_subroutine/linker/no-mutual-recursion.vert:
> | PIGLIT: {"result": "pass" }
>
> The builds used in both runs have been done in a clean pbuilder chroot (the
> same for both builds). Ie. there weren't any other differences between the two
> Piglit runs besides the addition of the patch in the latest Mesa build.
>
> If you accept this patch, please commit it for me, as I don't have commit
> access.
>
> Thanks,
> Kai
>
>
>  src/glsl/lower_subroutine.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/glsl/lower_subroutine.cpp b/src/glsl/lower_subroutine.cpp
> index b29912a..c1aed61 100644
> --- a/src/glsl/lower_subroutine.cpp
> +++ b/src/glsl/lower_subroutine.cpp
> @@ -98,7 +98,7 @@ lower_subroutine_visitor::visit_leave(ir_call *ir)
>else
>   last_branch = if_tree(equal(subr_to_int(var), lc), new_call, 
> last_branch);
>
> -  if (s > 0)
> +  if (return_deref && s > 0)
>  return_deref = return_deref->clone(mem_ctx, NULL);
> }
> if (last_branch)
Can anyone take a look at this simple one-liner ? Afaics Dave added
the pass earlier in 10.7-devel.

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


Re: [Mesa-dev] [PATCH 1/2] nvc0/ir: detect AND/SHR pairs and convert into EXTBF

2015-08-20 Thread Eric Anholt
Matt Turner  writes:

> On Tue, Aug 18, 2015 at 6:49 PM, Ilia Mirkin  wrote:
>> Some shaders appear to extract bits using shift/and combos. Detect
>> (some) of those and convert to EXTBF instead.
>
> What is EXTBF? Extract byte to float?
>
> I ask because Unigine Heaven has shaders that pack 3x byte-integers
> into one component of a vec4 and extracts them with shifts/ands and
> converts them to floats, and i965 could do the extraction and
> conversion in a single instruction. I'm curious if this is the same
> thing you're optimizing.
>
> I thought about adding an extract_byte(src, byte_num) operation, but
> i965's copy propagation caused me some headache and I shelved it.

I could use this one, as int, uint, and unorm unpacks.  Right now for
int/uint I'm recognizing the pattern in vc4_program.c (in a branch).
I'd be interested in writing the NIR bits if others are interested in
having this.


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


[Mesa-dev] [Bug 91691] Missing "gl.pc" file while using Scons for cross-compiling

2015-08-20 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=91691

--- Comment #1 from Emil Velikov  ---
Unfortunately scons does not produce any .pc files, not just gl.pc. It is
fixable, but I don't have any plans on tackling it. Patches welcome though :)

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/6] Copyright / license pet peeves

2015-08-20 Thread Ian Romanick
On 08/19/2015 08:51 PM, Matt Turner wrote:
> Patches 1 and 6 were too large for the mailing list, so I checked them
> in your tree. Patch 6 has some diffs that leave lines badly wrapped:
> 
> + * THE COPYRIGHT OWNER(S) AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> 
> other than that, all this looks like an improvement to me.
> 
> There are some instances of "VMWARE" and "THE COPYRIGHT OWNER(S)
> AND/OR ITS SUPPLIERS" that don't match the license
> (http://opensource.org/licenses/MIT) that I think should be changed.
> In particular, "VMWARE" is odd since they're not the copyright holder.

I left the odd wrapping on purpose.  With the bad wrapping, it makes the
diff script show a smaller set of changes in those files relative to the
"canonical" license.  I thought about putting the odd "DAMAGES OR OTHER"
on the previous line, but that would have made it much too long.

My real hope is that I can just replace those outliers with the
canonical license... leaving the odd wrapping will make that patch
smaller. :)

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


Re: [Mesa-dev] [PATCH 02/25] glsl: dont allow gl_PerVertex to be redeclared as an array of arrays

2015-08-20 Thread Ian Romanick
On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> V2: move single dimensional array detection into a helper
> 
> Signed-off-by: Timothy Arceri 
> Reviewed-by: Samuel Iglesias Gonsálvez 
> ---
>  src/glsl/ast.h  | 8 
>  src/glsl/ast_to_hir.cpp | 3 ++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index d8c6cea..ab94cb2 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -338,6 +338,14 @@ public:
>array_dimensions.push_tail(&dim->link);
> }
>  
> +   bool is_single_dimension()

I'd make this const... I believe all the list functions used here have
const versions.  With that change (or without it if it requires
additional list overloads), this patch is

Reviewed-by: Ian Romanick 

> +   {
> +  return (this->is_unsized_array && this->array_dimensions.is_empty()) ||
> + (!this->is_unsized_array &&
> +  this->array_dimensions.tail_pred->prev != NULL &&
> +  this->array_dimensions.tail_pred->prev->is_head_sentinel());
> +   }
> +
> virtual void print(void) const;
>  
> /* If true, this means that the array has an unsized outermost dimension. 
> */
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 06cd6a5..5ee02ed 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -5962,7 +5962,8 @@ ast_interface_block::hir(exec_list *instructions,
>   _mesa_shader_stage_to_string(state->stage));
>   }
>   if (this->instance_name == NULL ||
> - strcmp(this->instance_name, "gl_in") != 0 || 
> this->array_specifier == NULL) {
> + strcmp(this->instance_name, "gl_in") != 0 || 
> this->array_specifier == NULL ||
> + !this->array_specifier->is_single_dimension()) {
>  _mesa_glsl_error(&loc, state,
>   "gl_PerVertex input must be redeclared as "
>   "gl_in[]");
> 

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


Re: [Mesa-dev] [PATCH 01/25] glsl: calculate component size for arrays of arrays when varying packing disabled

2015-08-20 Thread Ian Romanick
On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> Signed-off-by: Timothy Arceri 
> Reviewed-by: Ilia Mirkin 
> ---
>  src/glsl/link_varyings.cpp | 13 ++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> index f7a7b8c..0d343d6 100644
> --- a/src/glsl/link_varyings.cpp
> +++ b/src/glsl/link_varyings.cpp
> @@ -956,9 +956,16 @@ varying_matches::record(ir_variable *producer_var, 
> ir_variable *consumer_var)
>   type = type->fields.array;
>}
>  
> -  slots = (type->is_array()
> -? (type->length * type->fields.array->matrix_columns)
> -: type->matrix_columns);
> +  if (type->is_array()) {
> + slots = 1;
> + while (type->is_array()) {
> +slots *= type->length;
> +type = type->fields.array;
> + }
> + slots *= type->matrix_columns;
> +  } else {
> + slots = var->type->matrix_columns;
> +  }

A comment that you are accumulating all the array sizes could be helpful
for future readers.  Either way, this patch is

Reviewed-by: Ian Romanick 

>this->matches[this->num_matches].num_components = 4 * slots;
> } else {
>this->matches[this->num_matches].num_components
> 

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


Re: [Mesa-dev] [PATCH 03/25] glsl: clean-up link uniform code

2015-08-20 Thread Ian Romanick
On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> These changes are also needed to allow linking of
> struct and interface arrays of arrays.
> ---
>  src/glsl/link_uniforms.cpp | 16 ++--
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> index 254086d..42f75e9 100644
> --- a/src/glsl/link_uniforms.cpp
> +++ b/src/glsl/link_uniforms.cpp
> @@ -72,6 +72,7 @@ void
>  program_resource_visitor::process(ir_variable *var)
>  {
> const glsl_type *t = var->type;
> +   const glsl_type *t_without_array = var->type->without_array();
> const bool row_major =
>var->data.matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR;
>  
> @@ -141,12 +142,8 @@ program_resource_visitor::process(ir_variable *var)
>char *name = ralloc_strdup(NULL, var->name);
>recursion(var->type, &name, strlen(name), row_major, NULL, false);
>ralloc_free(name);
> -   } else if (t->is_interface()) {
> -  char *name = ralloc_strdup(NULL, var->type->name);
> -  recursion(var->type, &name, strlen(name), row_major, NULL, false);
> -  ralloc_free(name);
> -   } else if (t->is_array() && t->fields.array->is_interface()) {
> -  char *name = ralloc_strdup(NULL, var->type->fields.array->name);
> +   } else if (t_without_array->is_interface()) {
> +  char *name = ralloc_strdup(NULL, t_without_array->name);
>recursion(var->type, &name, strlen(name), row_major, NULL, false);
>ralloc_free(name);
> } else {
> @@ -217,8 +214,8 @@ program_resource_visitor::recursion(const glsl_type *t, 
> char **name,
>   (*name)[name_length] = '\0';
>   this->leave_record(t, *name, row_major);
>}
> -   } else if (t->is_array() && (t->fields.array->is_record()
> -|| t->fields.array->is_interface())) {
> +   } else if (t->without_array()->is_record()
> +  || t->without_array()->is_interface()) {
>if (record_type == NULL && t->fields.array->is_record())
>   record_type = t->fields.array;

Is the part after the change still correct?  If t is an array of array
of struct, record_type will still point at NULL.  I guess this might be
addressed in a later patch...

>  
> @@ -810,8 +807,7 @@ link_update_uniform_buffer_variables(struct gl_shader 
> *shader)
>  
>if (var->type->is_record()) {
>   sentinel = '.';
> -  } else if (var->type->is_array()
> - && var->type->fields.array->is_record()) {
> +  } else if (var->type->without_array()->is_record()) {
>   sentinel = '[';
>}
>  
> 

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


Re: [Mesa-dev] [PATCH 03/25] glsl: clean-up link uniform code

2015-08-20 Thread Ian Romanick
On 08/20/2015 10:31 AM, Ian Romanick wrote:
> On 08/19/2015 09:37 PM, Timothy Arceri wrote:
>> These changes are also needed to allow linking of
>> struct and interface arrays of arrays.
>> ---
>>  src/glsl/link_uniforms.cpp | 16 ++--
>>  1 file changed, 6 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
>> index 254086d..42f75e9 100644
>> --- a/src/glsl/link_uniforms.cpp
>> +++ b/src/glsl/link_uniforms.cpp
>> @@ -72,6 +72,7 @@ void
>>  program_resource_visitor::process(ir_variable *var)
>>  {
>> const glsl_type *t = var->type;
>> +   const glsl_type *t_without_array = var->type->without_array();
>> const bool row_major =
>>var->data.matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR;
>>  
>> @@ -141,12 +142,8 @@ program_resource_visitor::process(ir_variable *var)
>>char *name = ralloc_strdup(NULL, var->name);
>>recursion(var->type, &name, strlen(name), row_major, NULL, false);
>>ralloc_free(name);
>> -   } else if (t->is_interface()) {
>> -  char *name = ralloc_strdup(NULL, var->type->name);
>> -  recursion(var->type, &name, strlen(name), row_major, NULL, false);
>> -  ralloc_free(name);
>> -   } else if (t->is_array() && t->fields.array->is_interface()) {
>> -  char *name = ralloc_strdup(NULL, var->type->fields.array->name);
>> +   } else if (t_without_array->is_interface()) {
>> +  char *name = ralloc_strdup(NULL, t_without_array->name);
>>recursion(var->type, &name, strlen(name), row_major, NULL, false);
>>ralloc_free(name);
>> } else {
>> @@ -217,8 +214,8 @@ program_resource_visitor::recursion(const glsl_type *t, 
>> char **name,
>>   (*name)[name_length] = '\0';
>>   this->leave_record(t, *name, row_major);
>>}
>> -   } else if (t->is_array() && (t->fields.array->is_record()
>> -|| t->fields.array->is_interface())) {
>> +   } else if (t->without_array()->is_record()
>> +  || t->without_array()->is_interface()) {
>>if (record_type == NULL && t->fields.array->is_record())
>>   record_type = t->fields.array;
> 
> Is the part after the change still correct?  If t is an array of array
> of struct, record_type will still point at NULL.  I guess this might be
> addressed in a later patch...

It looks like it's somewhat addressed in the next patch.  It's not clear
why this is better than just adding a "|| t->fields.array->is_array()"
clause to the right side of the &&.  Is that just the way it turned out
(which is fine), or am I missing something subtle (which should probably
be documented)?

>> @@ -810,8 +807,7 @@ link_update_uniform_buffer_variables(struct gl_shader 
>> *shader)
>>  
>>if (var->type->is_record()) {
>>   sentinel = '.';
>> -  } else if (var->type->is_array()
>> - && var->type->fields.array->is_record()) {
>> +  } else if (var->type->without_array()->is_record()) {
>>   sentinel = '[';
>>}
>>  
>>
> 
> ___
> 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 05/25] glsl: add support for initialising sampler AoA

2015-08-20 Thread Ian Romanick
This patch is

Reviewed-by: Ian Romanick 

On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> ---
>  src/glsl/link_uniform_initializers.cpp | 68 
> --
>  1 file changed, 41 insertions(+), 27 deletions(-)
> 
> diff --git a/src/glsl/link_uniform_initializers.cpp 
> b/src/glsl/link_uniform_initializers.cpp
> index 0cc79d9..d6a6ab7 100644
> --- a/src/glsl/link_uniform_initializers.cpp
> +++ b/src/glsl/link_uniform_initializers.cpp
> @@ -101,42 +101,54 @@ copy_constant_to_storage(union gl_constant_value 
> *storage,
>  }
>  
>  void
> -set_sampler_binding(gl_shader_program *prog, const char *name, int binding)
> +set_sampler_binding(void *mem_ctx, gl_shader_program *prog,
> +const glsl_type *type, const char *name, int *binding)
>  {
> -   struct gl_uniform_storage *const storage =
> -  get_storage(prog->UniformStorage, prog->NumUniformStorage, name);
>  
> -   if (storage == NULL) {
> -  assert(storage != NULL);
> -  return;
> -   }
> +   if (type->is_array() && type->fields.array->is_array()) {
> +  const glsl_type *const element_type = type->fields.array;
>  
> -   const unsigned elements = MAX2(storage->array_elements, 1);
> +  for (unsigned int i = 0; i < type->length; i++) {
> +  const char *element_name = ralloc_asprintf(mem_ctx, "%s[%d]", name, i);
>  
> -   /* Section 4.4.4 (Opaque-Uniform Layout Qualifiers) of the GLSL 4.20 spec
> -* says:
> -*
> -* "If the binding identifier is used with an array, the first element
> -* of the array takes the specified unit and each subsequent element
> -* takes the next consecutive unit."
> -*/
> -   for (unsigned int i = 0; i < elements; i++) {
> -  storage->storage[i].i = binding + i;
> -   }
> +  set_sampler_binding(mem_ctx, prog, element_type,
> + element_name, binding);
> +  }
> +   } else {
> +  struct gl_uniform_storage *const storage =
> + get_storage(prog->UniformStorage, prog->NumUniformStorage, name);
>  
> -   for (int sh = 0; sh < MESA_SHADER_STAGES; sh++) {
> -  gl_shader *shader = prog->_LinkedShaders[sh];
> +  if (storage == NULL) {
> + assert(storage != NULL);
> + return;
> +  }
> +
> +  const unsigned elements = MAX2(storage->array_elements, 1);
> +
> +  /* Section 4.4.4 (Opaque-Uniform Layout Qualifiers) of the GLSL 4.20 
> spec
> +   * says:
> +   *
> +   * "If the binding identifier is used with an array, the first 
> element
> +   * of the array takes the specified unit and each subsequent 
> element
> +   * takes the next consecutive unit."
> +   */
> +  for (unsigned int i = 0; i < elements; i++) {
> + storage->storage[i].i = (*binding)++;
> +  }
>  
> -  if (shader && storage->sampler[sh].active) {
> - for (unsigned i = 0; i < elements; i++) {
> -unsigned index = storage->sampler[sh].index + i;
> +  for (int sh = 0; sh < MESA_SHADER_STAGES; sh++) {
> +gl_shader *shader = prog->_LinkedShaders[sh];
>  
> -shader->SamplerUnits[index] = storage->storage[i].i;
> + if (shader && storage->sampler[sh].active) {
> +for (unsigned i = 0; i < elements; i++) {
> +   unsigned index = storage->sampler[sh].index + i;
> +
> +   shader->SamplerUnits[index] = storage->storage[i].i;
> +}
>   }
>}
> +  storage->initialized = true;
> }
> -
> -   storage->initialized = true;
>  }
>  
>  void
> @@ -270,7 +282,9 @@ link_set_uniform_initializers(struct gl_shader_program 
> *prog,
>  const glsl_type *const type = var->type;
>  
>  if (type->without_array()->is_sampler()) {
> -   linker::set_sampler_binding(prog, var->name, 
> var->data.binding);
> +   int binding = var->data.binding;
> +   linker::set_sampler_binding(mem_ctx, prog, var->type,
> +   var->name, &binding);
>  } else if (var->is_in_buffer_block()) {
> const glsl_type *const iface_type = var->get_interface_type();
>  
> 

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


Re: [Mesa-dev] [PATCH 06/25] glsl: fix binding validation for interface blocks

2015-08-20 Thread Ian Romanick
This patch is

Reviewed-by: Ian Romanick 

On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> V2: rebase on SSBO changes
> ---
>  src/glsl/ast_to_hir.cpp | 27 +--
>  1 file changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 5ee02ed..ee5ff9d 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2098,10 +2098,10 @@ validate_matrix_layout_for_type(struct 
> _mesa_glsl_parse_state *state,
>  static bool
>  validate_binding_qualifier(struct _mesa_glsl_parse_state *state,
> YYLTYPE *loc,
> -   ir_variable *var,
> +   const glsl_type *type,
> const ast_type_qualifier *qual)
>  {
> -   if (var->data.mode != ir_var_uniform && var->data.mode != 
> ir_var_shader_storage) {
> +   if (!qual->flags.q.uniform && !qual->flags.q.buffer) {
>_mesa_glsl_error(loc, state,
> "the \"binding\" qualifier only applies to uniforms 
> and "
> "shader storage buffer objects");
> @@ -2114,10 +2114,11 @@ validate_binding_qualifier(struct 
> _mesa_glsl_parse_state *state,
> }
>  
> const struct gl_context *const ctx = state->ctx;
> -   unsigned elements = var->type->is_array() ? var->type->length : 1;
> +   unsigned elements = type->is_array() ? type->length : 1;
> unsigned max_index = qual->binding + elements - 1;
> +   const glsl_type *base_type = type->without_array();
>  
> -   if (var->type->is_interface()) {
> +   if (base_type->is_interface()) {
>/* UBOs.  From page 60 of the GLSL 4.20 specification:
> * "If the binding point for any uniform block instance is less than 
> zero,
> *  or greater than or equal to the implementation-dependent maximum
> @@ -2128,7 +2129,7 @@ validate_binding_qualifier(struct 
> _mesa_glsl_parse_state *state,
> *
> * The implementation-dependent maximum is 
> GL_MAX_UNIFORM_BUFFER_BINDINGS.
> */
> -  if (var->data.mode == ir_var_uniform &&
> +  if (qual->flags.q.uniform &&
>   max_index >= ctx->Const.MaxUniformBufferBindings) {
>   _mesa_glsl_error(loc, state, "layout(binding = %d) for %d UBOs 
> exceeds "
>"the maximum number of UBO binding points (%d)",
> @@ -2136,6 +2137,7 @@ validate_binding_qualifier(struct 
> _mesa_glsl_parse_state *state,
>ctx->Const.MaxUniformBufferBindings);
>   return false;
>}
> +
>/* SSBOs. From page 67 of the GLSL 4.30 specification:
> * "If the binding point for any uniform or shader storage block 
> instance
> *  is less than zero, or greater than or equal to the
> @@ -2145,7 +2147,7 @@ validate_binding_qualifier(struct 
> _mesa_glsl_parse_state *state,
> *  N, all elements of the array from binding through binding + N – 1 
> must
> *  be within this range."
> */
> -  if (var->data.mode == ir_var_shader_storage &&
> +  if (qual->flags.q.buffer &&
>   max_index >= ctx->Const.MaxShaderStorageBufferBindings) {
>   _mesa_glsl_error(loc, state, "layout(binding = %d) for %d SSBOs 
> exceeds "
>"the maximum number of SSBO binding points (%d)",
> @@ -2153,8 +2155,7 @@ validate_binding_qualifier(struct 
> _mesa_glsl_parse_state *state,
>ctx->Const.MaxShaderStorageBufferBindings);
>   return false;
>}
> -   } else if (var->type->is_sampler() ||
> -  (var->type->is_array() && 
> var->type->fields.array->is_sampler())) {
> +   } else if (base_type->is_sampler()) {
>/* Samplers.  From page 63 of the GLSL 4.20 specification:
> * "If the binding is less than zero, or greater than or equal to the
> *  implementation-dependent maximum supported number of units, a
> @@ -2171,7 +2172,7 @@ validate_binding_qualifier(struct 
> _mesa_glsl_parse_state *state,
>  
>   return false;
>}
> -   } else if (var->type->contains_atomic()) {
> +   } else if (base_type->contains_atomic()) {
>assert(ctx->Const.MaxAtomicBufferBindings <= 
> MAX_COMBINED_ATOMIC_BUFFERS);
>if (unsigned(qual->binding) >= ctx->Const.MaxAtomicBufferBindings) {
>   _mesa_glsl_error(loc, state, "layout(binding = %d) exceeds the "
> @@ -2749,7 +2750,7 @@ apply_type_qualifier_to_variable(const struct 
> ast_type_qualifier *qual,
> }
>  
> if (qual->flags.q.explicit_binding &&
> -   validate_binding_qualifier(state, loc, var, qual)) {
> +   validate_binding_qualifier(state, loc, var->type, qual)) {
>var->data.explicit_binding = true;
>var->data.binding = qual->binding;
> }
> @@ -6058,6 +6059,8 @@ ast_interface_block::hir(exec_list *instructions,
>  num_variables,
>  packing,
>

Re: [Mesa-dev] [PATCH 07/25] glsl: add helper for calculating size of AoA

2015-08-20 Thread Ian Romanick
This patch is

Reviewed-by: Ian Romanick 

On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> V2: return 0 if not array rather than -1
> 
> Reviewed-by: Tapani Pälli 
> ---
>  src/glsl/glsl_types.h | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
> index e7c73da..469ee3e 100644
> --- a/src/glsl/glsl_types.h
> +++ b/src/glsl/glsl_types.h
> @@ -550,6 +550,25 @@ struct glsl_type {
> }
>  
> /**
> +* Return the total number of elements in an array including the elements
> +* in arrays of arrays.
> +*/
> +   unsigned arrays_of_arrays_size() const
> +   {
> +  if (!is_array())
> + return 0;
> +
> +  unsigned size = length;
> +  const glsl_type *base_type = fields.array;
> +
> +  while (base_type->is_array()) {
> + size = size * base_type->length;
> + base_type = base_type->fields.array;
> +  }
> +  return size;
> +   }
> +
> +   /**
>  * Return the amount of atomic counter storage required for a type.
>  */
> unsigned atomic_size() const
> 

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


Re: [Mesa-dev] [PATCH 08/25] glsl: validate binding qualifier for AoA

2015-08-20 Thread Ian Romanick
I might have been tempted to put this and the previous patch before
patch 6, but meh.

This patch is

Reviewed-by: Ian Romanick 

On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> Reviewed-by: Tapani Pälli 
> ---
>  src/glsl/ast_to_hir.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index ee5ff9d..54569a8 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2114,7 +2114,7 @@ validate_binding_qualifier(struct 
> _mesa_glsl_parse_state *state,
> }
>  
> const struct gl_context *const ctx = state->ctx;
> -   unsigned elements = type->is_array() ? type->length : 1;
> +   unsigned elements = type->is_array() ? type->arrays_of_arrays_size() : 1;
> unsigned max_index = qual->binding + elements - 1;
> const glsl_type *base_type = type->without_array();
>  
> 

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


Re: [Mesa-dev] [PATCH 02/25] glsl: dont allow gl_PerVertex to be redeclared as an array of arrays

2015-08-20 Thread Ian Romanick
On 08/20/2015 10:24 AM, Ian Romanick wrote:
> On 08/19/2015 09:37 PM, Timothy Arceri wrote:
>> V2: move single dimensional array detection into a helper
>>
>> Signed-off-by: Timothy Arceri 
>> Reviewed-by: Samuel Iglesias Gonsálvez 
>> ---
>>  src/glsl/ast.h  | 8 
>>  src/glsl/ast_to_hir.cpp | 3 ++-
>>  2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
>> index d8c6cea..ab94cb2 100644
>> --- a/src/glsl/ast.h
>> +++ b/src/glsl/ast.h
>> @@ -338,6 +338,14 @@ public:
>>array_dimensions.push_tail(&dim->link);
>> }
>>  
>> +   bool is_single_dimension()
> 
> I'd make this const... I believe all the list functions used here have
> const versions.  With that change (or without it if it requires
> additional list overloads), this patch is
> 
> Reviewed-by: Ian Romanick 
> 
>> +   {
>> +  return (this->is_unsized_array && this->array_dimensions.is_empty()) 
>> ||
>> + (!this->is_unsized_array &&
>> +  this->array_dimensions.tail_pred->prev != NULL &&
>> +  this->array_dimensions.tail_pred->prev->is_head_sentinel());

After looking at patch 9, I think I have a suggestion for this.  The
array is one dimension if this->array_dimensions has exactly one
element, correct?  In that case, I think this is more clear:

/* If the list is not empty and the first element is the only
 * element, the array is one-dimensional.
 */
return this->array_dimensions->get_head() != NULL &&
   this->array_dimensions->get_head()->get_next()->is_tail_sentinel());

>> +   }
>> +
>> virtual void print(void) const;
>>  
>> /* If true, this means that the array has an unsized outermost 
>> dimension. */
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index 06cd6a5..5ee02ed 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -5962,7 +5962,8 @@ ast_interface_block::hir(exec_list *instructions,
>>   _mesa_shader_stage_to_string(state->stage));
>>   }
>>   if (this->instance_name == NULL ||
>> - strcmp(this->instance_name, "gl_in") != 0 || 
>> this->array_specifier == NULL) {
>> + strcmp(this->instance_name, "gl_in") != 0 || 
>> this->array_specifier == NULL ||
>> + !this->array_specifier->is_single_dimension()) {
>>  _mesa_glsl_error(&loc, state,
>>   "gl_PerVertex input must be redeclared as "
>>   "gl_in[]");
>>
> 
> ___
> 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] glsl: expose textureQueryLod in GLSL 4.00+ fragment shaders

2015-08-20 Thread Marek Olšák
On Thu, Aug 20, 2015 at 5:15 PM, Ilia Mirkin  wrote:
> So just stick something like
>
> """
> From the ARB_texture_query_lod spec:
>
> (3) The core specification uses the "Lod" spelling, not "LOD". Should
> this extension be modified to use "Lod"?
>
>   RESOLVED: The "Lod" spelling is the correct spelling for the core
>   specification and the preferred spelling for use. However, use of
>   "LOD" also exists, as the extension predated the core specification,
>   so this extension won't remove use of "LOD".
> """
>
> as the commit message? Fine by me. It seems excessive to put that into
> builtin_functions.cpp... but if people feel strongly, I can do that
> too.

People tend to read code more than commit messages, so putting it in
the code is better.

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


Re: [Mesa-dev] [PATCH] glsl: expose textureQueryLod in GLSL 4.00+ fragment shaders

2015-08-20 Thread Ilia Mirkin
On Thu, Aug 20, 2015 at 2:01 PM, Marek Olšák  wrote:
> On Thu, Aug 20, 2015 at 5:15 PM, Ilia Mirkin  wrote:
>> So just stick something like
>>
>> """
>> From the ARB_texture_query_lod spec:
>>
>> (3) The core specification uses the "Lod" spelling, not "LOD". Should
>> this extension be modified to use "Lod"?
>>
>>   RESOLVED: The "Lod" spelling is the correct spelling for the core
>>   specification and the preferred spelling for use. However, use of
>>   "LOD" also exists, as the extension predated the core specification,
>>   so this extension won't remove use of "LOD".
>> """
>>
>> as the commit message? Fine by me. It seems excessive to put that into
>> builtin_functions.cpp... but if people feel strongly, I can do that
>> too.
>
> People tend to read code more than commit messages, so putting it in
> the code is better.

I don't really see what it adds to either the commit message or the
code... we don't have stuff in the code for like "xyz added by spec
bar". It's pretty obvious from the availability predicate... I don't
see a single other instance of this in builtin_functions.cpp.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 10/25] glsl: remove dead code in a single pass

2015-08-20 Thread Ian Romanick
On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> Currently only one ir assignment is removed for each var in a single
> dead code optimisation pass. This means if a var has more than one
> assignment, then it requires all the glsl optimisations to be run again
> for each additional assignment to be removed.
> Another pass is also required to remove the variable itself.
> 
> With this change all assignments and the variable are removed in a single
> pass.
> 
> Some of the arrays of arrays conformance tests that were looping
> through 8 dimensions ended up with a var with hundreds of assignments.
> 
> This change helps ES31-CTS.arrays_of_arrays.InteractionFunctionCalls1
> go from around 3 min 20 sec -> 2 min
> 
> ES31-CTS.arrays_of_arrays.InteractionFunctionCalls2 went from
> around 9 min 20 sec to 7 min 30 sec
> 
> I had difficulty getting the public shader-db to give a constent result
  consistent

> with or without this change but the results seemed unchanged at between
> 15-20 seconds.
> 
> Thomas Helland measured change with shader-db on his machine from
> approx 117 secs to 112 secs.
> 
> V2: Add assert to be sure references are counted before assignments.
> 
> Tested-By: Thomas Helland 
> ---
>  src/glsl/ir_variable_refcount.cpp | 28 +---
>  src/glsl/ir_variable_refcount.h   | 13 -
>  src/glsl/opt_dead_code.cpp| 33 ++---
>  src/glsl/opt_tree_grafting.cpp|  2 --
>  4 files changed, 59 insertions(+), 17 deletions(-)
> 
> diff --git a/src/glsl/ir_variable_refcount.cpp 
> b/src/glsl/ir_variable_refcount.cpp
> index e4d825c..4ca492e 100644
> --- a/src/glsl/ir_variable_refcount.cpp
> +++ b/src/glsl/ir_variable_refcount.cpp
> @@ -46,6 +46,16 @@ static void
>  free_entry(struct hash_entry *entry)
>  {
> ir_variable_refcount_entry *ivre = (ir_variable_refcount_entry *) 
> entry->data;
> +
> +   /* Free assignment list */
> +   while (!ivre->assign_list.is_empty()) {
> +  struct assignment_entry *assignment_entry =
> + exec_node_data(struct assignment_entry,
> +ivre->assign_list.head, link);
> +  assignment_entry->link.remove();
> +  free(assignment_entry);
> +   }

This can be simplified as:

   exec_node *n;
   while ((n = ivre->assign_list->pop_head()) != NULL) {
  struct assignment_entry *assignment_entry =
 exec_node_data(struct assignment_entry, n, link);
  free(assignment_entry);
   }


> +
> delete ivre;
>  }
>  
> @@ -59,7 +69,6 @@ 
> ir_variable_refcount_visitor::~ir_variable_refcount_visitor()
>  ir_variable_refcount_entry::ir_variable_refcount_entry(ir_variable *var)
>  {
> this->var = var;
> -   assign = NULL;
> assigned_count = 0;
> declaration = false;
> referenced_count = 0;
> @@ -125,8 +134,21 @@ ir_variable_refcount_visitor::visit_leave(ir_assignment 
> *ir)
> entry = this->get_variable_entry(ir->lhs->variable_referenced());
> if (entry) {
>entry->assigned_count++;
> -  if (entry->assign == NULL)
> -  entry->assign = ir;
> +
> +  /**
> +   * Build a list for dead code optimisation. Dont add assingment if it
   optimization  Don'tassignment

s/optimisation/optimization/ elsewhere (including the commit message) too.

> +   * was declared out of scope (outside the instruction stream). Also 
> dont
 
don't
> +   * bother adding any more to the list if there are more references then
than
> +   * assignments as this means the variable is used and won't be 
> optimised
> +   * out.
> +   */
> +  assert(entry->referenced_count >= entry->assigned_count);
> +  if (entry->referenced_count == entry->assigned_count) {
> + struct assignment_entry *assignment_entry =
> +(struct assignment_entry *)calloc(1, sizeof(*assignment_entry));
> + assignment_entry->assign = ir;
> + entry->assign_list.push_head(&assignment_entry->link);
> +  }
> }
>  
> return visit_continue;
> diff --git a/src/glsl/ir_variable_refcount.h b/src/glsl/ir_variable_refcount.h
> index c15e8110..5c74c31 100644
> --- a/src/glsl/ir_variable_refcount.h
> +++ b/src/glsl/ir_variable_refcount.h
> @@ -33,13 +33,24 @@
>  #include "ir_visitor.h"
>  #include "glsl_types.h"
>  
> +struct assignment_entry {
> +   exec_node link;
> +   ir_assignment *assign;
> +};
> +
>  class ir_variable_refcount_entry
>  {
>  public:
> ir_variable_refcount_entry(ir_variable *var);
>  
> ir_variable *var; /* The key: the variable's pointer. */
> -   ir_assignment *assign; /* An assignment to the variable, if any */
> +
> +   /**
> +* List of assignments to the variable, if any.
> +* This is intended to be used for dea

Re: [Mesa-dev] [PATCH 11/25] glsl: interleave constant propagation and folding

2015-08-20 Thread Ian Romanick
I had thought about doing something similar once upon a time, but I
never got around to it.  I'm glad you got some good performance data.
With the couple typos fixed, this patch is

Reviewed-by: Ian Romanick 

On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> The constant folding pass can take a long time to complete
> so rather than running throught the entire pass each time
 through

> a new constant is propagated (and vice versa) interleave them.
> 
> This change helps ES31-CTS.arrays_of_arrays.InteractionFunctionCalls1
> go from around 2 min -> 23 sec.
> ---
>  src/glsl/opt_constant_propagation.cpp | 46 
> +--
>  1 file changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/src/glsl/opt_constant_propagation.cpp 
> b/src/glsl/opt_constant_propagation.cpp
> index 10be8e8..ebe30d9 100644
> --- a/src/glsl/opt_constant_propagation.cpp
> +++ b/src/glsl/opt_constant_propagation.cpp
> @@ -110,6 +110,8 @@ public:
> virtual ir_visitor_status visit_enter(class ir_if *);
>  
> void add_constant(ir_assignment *ir);
> +   void constant_folding(ir_rvalue **rvalue);
> +   void constant_propagation(ir_rvalue **rvalue);
> void kill(ir_variable *ir, unsigned write_mask);
> void handle_if_block(exec_list *instructions);
> void handle_rvalue(ir_rvalue **rvalue);
> @@ -132,8 +134,39 @@ public:
>  
>  
>  void
> -ir_constant_propagation_visitor::handle_rvalue(ir_rvalue **rvalue)
> -{
> +ir_constant_propagation_visitor::constant_folding(ir_rvalue **rvalue) {
> +
> +   if (*rvalue == NULL || (*rvalue)->ir_type == ir_type_constant)
> +  return;
> +
> +   /* Note that we do rvalue visitoring on leaving.  So if an
 Note that we visit rvalues one leaving.

> +* expression has a non-constant operand, no need to go looking
> +* down it to find if it's constant.  This cuts the time of this
> +* pass down drastically.
> +*/
> +   ir_expression *expr = (*rvalue)->as_expression();
> +   if (expr) {
> +  for (unsigned int i = 0; i < expr->get_num_operands(); i++) {
> +  if (!expr->operands[i]->as_constant())
> + return;
> +  }
> +   }
> +
> +   /* Ditto for swizzles. */
> +   ir_swizzle *swiz = (*rvalue)->as_swizzle();
> +   if (swiz && !swiz->val->as_constant())
> +  return;
> +
> +   ir_constant *constant = (*rvalue)->constant_expression_value();
> +   if (constant) {
> +  *rvalue = constant;
> +  this->progress = true;
> +   }
> +}
> +
> +void
> +ir_constant_propagation_visitor::constant_propagation(ir_rvalue **rvalue) {
> +
> if (this->in_assignee || !*rvalue)
>return;
>  
> @@ -216,6 +249,13 @@ ir_constant_propagation_visitor::handle_rvalue(ir_rvalue 
> **rvalue)
> this->progress = true;
>  }
>  
> +void
> +ir_constant_propagation_visitor::handle_rvalue(ir_rvalue **rvalue)
> +{
> +   constant_propagation(rvalue);
> +   constant_folding(rvalue);
> +}
> +
>  ir_visitor_status
>  ir_constant_propagation_visitor::visit_enter(ir_function_signature *ir)
>  {
> @@ -243,6 +283,8 @@ 
> ir_constant_propagation_visitor::visit_enter(ir_function_signature *ir)
>  ir_visitor_status
>  ir_constant_propagation_visitor::visit_leave(ir_assignment *ir)
>  {
> +  constant_folding(&ir->rhs);
> +
> if (this->in_assignee)
>return visit_continue;
>  
> 

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


Re: [Mesa-dev] [PATCH 10/25] glsl: remove dead code in a single pass

2015-08-20 Thread Ilia Mirkin
On Thu, Aug 20, 2015 at 2:08 PM, Ian Romanick  wrote:
> s/optimisation/optimization/ elsewhere (including the commit message) too.

Does mesa standardise on US spelling?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 13/25] nir: wrapper for glsl_type arrays_of_arrays_size()

2015-08-20 Thread Ian Romanick
This patch is

Reviewed-by: Ian Romanick 

On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> Reviewed-by: Tapani Pälli 
> ---
>  src/glsl/nir/nir_types.cpp | 6 ++
>  src/glsl/nir/nir_types.h   | 2 ++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/src/glsl/nir/nir_types.cpp b/src/glsl/nir/nir_types.cpp
> index 940c676..da7015b 100644
> --- a/src/glsl/nir/nir_types.cpp
> +++ b/src/glsl/nir/nir_types.cpp
> @@ -106,6 +106,12 @@ glsl_get_length(const struct glsl_type *type)
> return type->is_matrix() ? type->matrix_columns : type->length;
>  }
>  
> +unsigned
> +glsl_get_aoa_size(const struct glsl_type *type)
> +{
> +   return type->arrays_of_arrays_size();
> +}
> +
>  const char *
>  glsl_get_struct_elem_name(const struct glsl_type *type, unsigned index)
>  {
> diff --git a/src/glsl/nir/nir_types.h b/src/glsl/nir/nir_types.h
> index a8ff8f2..5d16d3a 100644
> --- a/src/glsl/nir/nir_types.h
> +++ b/src/glsl/nir/nir_types.h
> @@ -59,6 +59,8 @@ unsigned glsl_get_matrix_columns(const struct glsl_type 
> *type);
>  
>  unsigned glsl_get_length(const struct glsl_type *type);
>  
> +unsigned glsl_get_aoa_size(const struct glsl_type *type);
> +
>  const char *glsl_get_struct_elem_name(const struct glsl_type *type,
>unsigned index);
>  
> 

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


Re: [Mesa-dev] [PATCH 10/25] glsl: remove dead code in a single pass

2015-08-20 Thread Ian Romanick
On 08/20/2015 11:12 AM, Ilia Mirkin wrote:
> On Thu, Aug 20, 2015 at 2:08 PM, Ian Romanick  wrote:
>> s/optimisation/optimization/ elsewhere (including the commit message) too.
> 
> Does mesa standardise on US spelling?

I did not realize that the British spelling was different.  I think we
mostly use color, favor, gray, etc., but I think that's more due to who
is writing the code than explicit coding standards.  I know that Paul
Berry used to use British spellings.

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


Re: [Mesa-dev] [Mesa-stable] New stable-branch 10.6 candidate pushed

2015-08-20 Thread Ben Widawsky
On Thu, Aug 20, 2015 at 04:08:33PM +0100, Emil Velikov wrote:
> Hello list,
> 

[snip]

> Neil Roberts (2):
>   i965: Swap the order of the vertex ID and edge flag attributes
>   i965/bdw: Fix 3DSTATE_VF_INSTANCING when the edge flag is used

I'm still working on these patches and I'd prefer if you held off on merging
them to stable for now (they aren't yet in master).

Thanks.

[snip]


-- 
Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 18/25] glsl: disable interface block AoA

2015-08-20 Thread Ian Romanick
On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> Desktop GL supports interface block AoA however AMD and Nvidia
> dont support it in their drivers curently so we can get away
> with disabling it for now.
> ---
>  src/glsl/ast_to_hir.cpp | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index af69ffd..c8e695a 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -6225,15 +6225,17 @@ ast_interface_block::hir(exec_list *instructions,
>  }
>   }
>  
> -  /* From section 4.3.9 (Interface Blocks) of the GLSL ES 3.10 spec:
> + /* From section 4.3.9 (Interface Blocks) of the GLSL ES 3.10 spec:
>*
>* * Arrays of arrays of blocks are not allowed
>*/
> - if (state->es_shader && block_array_type->is_array() &&
> + /* FIXME: Desktop GL allows interface AoA */
> + if (block_array_type->is_array() &&
>   block_array_type->fields.array->is_array()) {
>  _mesa_glsl_error(&loc, state,
> - "arrays of arrays interface blocks are "
> - "not allowed");
> + "arrays of arrays interface blocks are %s",
> + state->es_shader ? "not allowed" :
> + "not currently supported");

We should probably emit some extra text on desktop noting that this is
our bug, not an application bug.

>   }
>  
>   if (this->layout.flags.q.explicit_binding)
> 

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


Re: [Mesa-dev] [PATCH 23/25] glsl: Allow arrays of arrays in GLSL ES 3.10 and GLSL 4.30

2015-08-20 Thread Ian Romanick
On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> V2: add missing 's' to the extension name in error messages
>  and add decimal place in version string
> 
> Reviewed-by: Ilia Mirkin 
> ---
>  src/glsl/ast_to_hir.cpp   | 13 -
>  src/glsl/glsl_parser.yy   | 22 ++
>  src/glsl/glsl_parser_extras.h |  5 +
>  3 files changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index c8e695a..78e22cc 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -1996,12 +1996,15 @@ process_array_type(YYLTYPE *loc, const glsl_type 
> *base,
>*
>* "Only one-dimensional arrays may be declared."
>*/
> - if (!state->ARB_arrays_of_arrays_enable) {
> + if (!state->has_arrays_of_arrays()) {
> +const char *const requirement = state->es_shader
> +   ? "GLSL ES 3.10"
> +   : "GL_ARB_arrays_of_arrays or GLSL 4.30";
>  _mesa_glsl_error(loc, state,
> - "invalid array of `%s'"
> - "GL_ARB_arrays_of_arrays "
> - "required for defining arrays of arrays",
> - base->name);
> + "invalid array of `%s' "
> + "%s required for defining arrays of arrays",
> + base->name, requirement);
> +

Usually this is encapsulated in a check_foo_allowed method (e.g.,
check_explicit_uniform_location_allowed).

>  return glsl_type::error_type;
>   }
>}
> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> index 72606d2..396088f 100644
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> @@ -1965,10 +1965,13 @@ array_specifier:
>void *ctx = state;
>$$ = $1;
>  
> -  if (!state->ARB_arrays_of_arrays_enable) {
> - _mesa_glsl_error(& @1, state,
> -  "GL_ARB_arrays_of_arrays "
> -  "required for defining arrays of arrays");
> +  if (!state->has_arrays_of_arrays()) {
> + const char *const requirement = state->es_shader
> +? "GLSL ES 3.10"
> +: "GL_ARB_arrays_of_arrays or GLSL 4.30";
> + _mesa_glsl_error(& @1, state, 
> +  "%s required for defining arrays of arrays.",
> +  requirement);
>}
>$$->add_dimension(new(ctx) ast_expression(ast_unsized_array_dim, NULL,
>  NULL, NULL));
> @@ -1977,10 +1980,13 @@ array_specifier:
> {
>$$ = $1;
>  
> -  if (!state->ARB_arrays_of_arrays_enable) {
> - _mesa_glsl_error(& @1, state,
> -  "GL_ARB_arrays_of_arrays "
> -  "required for defining arrays of arrays");
> +  if (!state->has_arrays_of_arrays()) {
> + const char *const requirement = state->es_shader
> +? "GLSL ES 3.10"
> +: "GL_ARB_arrays_of_arrays or GLSL 4.30";
> + _mesa_glsl_error(& @1, state, 
> +  "%s required for defining arrays of arrays.",
> +  requirement);
>}
>  
>$$->add_dimension($3);
> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
> index 57a7555..91f8c95 100644
> --- a/src/glsl/glsl_parser_extras.h
> +++ b/src/glsl/glsl_parser_extras.h
> @@ -190,6 +190,11 @@ struct _mesa_glsl_parse_state {
>return true;
> }
>  
> +   bool has_arrays_of_arrays() const
> +   {
> +  return ARB_arrays_of_arrays_enable || is_version(430, 310);
> +   }
> +
> bool has_atomic_counters() const
> {
>return ARB_shader_atomic_counters_enable || is_version(420, 310);
> 

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


Re: [Mesa-dev] [PATCH 24/25] i965: enable ARB_arrays_of_arrays

2015-08-20 Thread Ian Romanick
With this change, what tests pass / fail in piglit and es31 conformance?
 I assume there are still some failures, or does everything work now?

On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> ---
>  src/mesa/drivers/dri/i965/intel_extensions.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c 
> b/src/mesa/drivers/dri/i965/intel_extensions.c
> index 0da528b..c0ff61d 100644
> --- a/src/mesa/drivers/dri/i965/intel_extensions.c
> +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
> @@ -176,6 +176,7 @@ intelInitExtensions(struct gl_context *ctx)
>  
> assert(brw->gen >= 4);
>  
> +   ctx->Extensions.ARB_arrays_of_arrays = true;
> ctx->Extensions.ARB_buffer_storage = true;
> ctx->Extensions.ARB_clear_texture = true;
> ctx->Extensions.ARB_clip_control = true;
> 

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


Re: [Mesa-dev] [PATCH 25/25] docs: Mark AoA as done for i965

2015-08-20 Thread Ian Romanick
Once all the other stuff lands, this patch is

Reviewed-by: Ian Romanick 

On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> ---
>  docs/GL3.txt  | 4 ++--
>  docs/relnotes/11.0.0.html | 1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/GL3.txt b/docs/GL3.txt
> index 54c0c5a..20627ae 100644
> --- a/docs/GL3.txt
> +++ b/docs/GL3.txt
> @@ -149,7 +149,7 @@ GL 4.2, GLSL 4.20:
>  
>  GL 4.3, GLSL 4.30:
>  
> -  GL_ARB_arrays_of_arrays  started (Timothy)
> +  GL_ARB_arrays_of_arrays  DONE (i965)
>GL_ARB_ES3_compatibility DONE (all drivers 
> that support GLSL 3.30)
>GL_ARB_clear_buffer_object   DONE (all drivers)
>GL_ARB_compute_shaderin progress (jljusten)
> @@ -203,7 +203,7 @@ GL 4.5, GLSL 4.50:
>  
>  These are the extensions cherry-picked to make GLES 3.1
>  GLES3.1, GLSL ES 3.1
> -  GL_ARB_arrays_of_arrays  started (Timothy)
> +  GL_ARB_arrays_of_arrays  DONE (i965)
>GL_ARB_compute_shaderin progress (jljusten)
>GL_ARB_draw_indirect DONE (i965, nvc0, 
> r600, radeonsi, llvmpipe, softpipe)
>GL_ARB_explicit_uniform_location DONE (all drivers 
> that support GLSL)
> diff --git a/docs/relnotes/11.0.0.html b/docs/relnotes/11.0.0.html
> index 447e35e..0ce7d71 100644
> --- a/docs/relnotes/11.0.0.html
> +++ b/docs/relnotes/11.0.0.html
> @@ -47,6 +47,7 @@ Note: some of the new features are only available with 
> certain drivers.
>  New hardware support for AMD GCN 1.2 GPUs: Tonga, Iceland, Carrizo, 
> Fiji
>  OpenGL 4.1 on radeonsi, nvc0
>  GL_AMD_vertex_shader_viewport_index on radeonsi
> +GL_ARB_arrays_of_arrays on i965
>  GL_ARB_conditional_render_inverted on r600, radeonsi
>  GL_ARB_derivative_control on radeonsi
>  GL_ARB_fragment_layer_viewport on radeonsi
> 

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


Re: [Mesa-dev] [PATCH 4/4] mesa/formats: refactor by globbing on types in switch statement

2015-08-20 Thread Emil Velikov
2015-08-12 0:07 GMT+01:00 Nanley Chery :
> From: Nanley Chery 
>
> Combine the adjacent cases which have the same GL type in the switch statemnt.
>
> Signed-off-by: Nanley Chery 
> ---
>  src/mesa/main/formats.c | 152 
> ++--
>  1 file changed, 17 insertions(+), 135 deletions(-)
>
> diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c
> index cb5ad21..9b9d79b 100644
> --- a/src/mesa/main/formats.c
> +++ b/src/mesa/main/formats.c
> @@ -1005,13 +1005,10 @@ 
> _mesa_uncompressed_format_to_type_and_comps(mesa_format format,
> case MESA_FORMAT_R8G8B8X8_UNORM:
> case MESA_FORMAT_B8G8R8X8_UNORM:
> case MESA_FORMAT_X8R8G8B8_UNORM:
> -  *datatype = GL_UNSIGNED_BYTE;
> -  *comps = 4;
> -  return;
> case MESA_FORMAT_BGR_UNORM8:
> case MESA_FORMAT_RGB_UNORM8:
>*datatype = GL_UNSIGNED_BYTE;
> -  *comps = 3;
> +  *comps = _mesa_format_num_components(format);
With the datatype aside what is stopping us from using a single
_mesa_format_num_components(format) ?

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


Re: [Mesa-dev] [PATCH 2/2] r600: Turn 'r600_shader_key' struct into union

2015-08-20 Thread Emil Velikov
Hi Edward,

On 19 August 2015 at 09:58, Edward O'Callaghan
 wrote:
> From: Edward O'Callaghan 
>
> This struct was getting a bit crowded, following the lead of
> radeonsi, mirror the idea of having sub-structures for each
> shader type. Turning 'r600_shader_key' into an union saves
> some trivial memory and CPU cycles for the shader keys.
>
> Signed-off-by: Edward O'Callaghan 
> ---
[snip]
> diff --git a/src/gallium/drivers/r600/r600_shader.h 
> b/src/gallium/drivers/r600/r600_shader.h
> index 5d05c81..e97 100644
> --- a/src/gallium/drivers/r600/r600_shader.h
> +++ b/src/gallium/drivers/r600/r600_shader.h
> @@ -95,13 +95,18 @@ struct r600_shader {
> struct r600_shader_array * arrays;
>  };
>
> -struct r600_shader_key {
> -   unsigned color_two_side:1;
> -   unsigned alpha_to_one:1;
> -   unsigned nr_cbufs:4;
> -   unsigned vs_as_es:1;
> -   unsigned vs_as_gs_a:1;
> -   unsigned vs_prim_id_out:8;
> +union r600_shader_key {
> +   struct {
> +   unsignedcolor_two_side:1;
> +   unsignedalpha_to_one:1;
> +   unsignednr_cbufs:4;
> +   } ps;
> +   struct {
> +   unsignedas_es:1; /* export shader */
> +   unsignedas_ls:1; /* local shader */
This seems like a new addition. Perhaps you should introduce it when
there is a user for it ?

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


Re: [Mesa-dev] [PATCH] egl/SCons: Fix Haiku build. Haiku is the only OS EGL builds on with SCons

2015-08-20 Thread Emil Velikov
Hi Alexander,

Please use a commit summary consistent with what we're using. I'm
thinking of "egl: scons: fix the haiku build, do not build the dri2
backend" but feel free to pick a better one.

On 19 August 2015 at 02:45, Alexander von Gluck IV
 wrote:
> ---
>  src/egl/SConscript |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/egl/SConscript b/src/egl/SConscript
> index 1b2a427..b05da84 100644
> --- a/src/egl/SConscript
> +++ b/src/egl/SConscript
> @@ -1,6 +1,7 @@
>  ###
>  # SConscript for EGL
>
> +# As of e7e29189e27b EGL is only built on Haiku with SCons
This has little value here imho. Please move it to the commit message.

With those addressed,
Reviewed-by: Emil Velikov 

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


[Mesa-dev] [PATCH 3/4] vc4: Add QIR/QPU support for the 8-bit vector instructions.

2015-08-20 Thread Eric Anholt
---
 src/gallium/drivers/vc4/vc4_program.c  | 20 
 src/gallium/drivers/vc4/vc4_qir.c  |  5 +
 src/gallium/drivers/vc4/vc4_qir.h  | 10 ++
 src/gallium/drivers/vc4/vc4_qpu_emit.c |  5 +
 4 files changed, 40 insertions(+)

diff --git a/src/gallium/drivers/vc4/vc4_program.c 
b/src/gallium/drivers/vc4/vc4_program.c
index ab2f885..b04c8d3 100644
--- a/src/gallium/drivers/vc4/vc4_program.c
+++ b/src/gallium/drivers/vc4/vc4_program.c
@@ -1062,6 +1062,26 @@ ntq_emit_alu(struct vc4_compile *c, nir_alu_instr *instr)
 qir_SUB(c, qir_uniform_ui(c, 0), src[0]));
 break;
 
+case nir_op_isadd_4x8:
+*dest = qir_V8ADDS(c, src[0], src[1]);
+break;
+
+case nir_op_issub_4x8:
+*dest = qir_V8SUBS(c, src[0], src[1]);
+break;
+
+case nir_op_imin_4x8:
+*dest = qir_V8MIN(c, src[0], src[1]);
+break;
+
+case nir_op_imax_4x8:
+*dest = qir_V8MAX(c, src[0], src[1]);
+break;
+
+case nir_op_imul_unorm_4x8:
+*dest = qir_V8MULD(c, src[0], src[1]);
+break;
+
 default:
 fprintf(stderr, "unknown NIR ALU inst: ");
 nir_print_instr(&instr->instr, stderr);
diff --git a/src/gallium/drivers/vc4/vc4_qir.c 
b/src/gallium/drivers/vc4/vc4_qir.c
index 254140a..016d6e2 100644
--- a/src/gallium/drivers/vc4/vc4_qir.c
+++ b/src/gallium/drivers/vc4/vc4_qir.c
@@ -40,6 +40,11 @@ static const struct qir_op_info qir_op_info[] = {
 [QOP_FSUB] = { "fsub", 1, 2 },
 [QOP_FMUL] = { "fmul", 1, 2 },
 [QOP_MUL24] = { "mul24", 1, 2 },
+[QOP_V8MULD] = {"v8muld", 1, 2 },
+[QOP_V8MIN] = {"v8min", 1, 2 },
+[QOP_V8MAX] = {"v8max", 1, 2 },
+[QOP_V8ADDS] = {"v8adds", 1, 2 },
+[QOP_V8SUBS] = {"v8subs", 1, 2 },
 [QOP_FMIN] = { "fmin", 1, 2 },
 [QOP_FMAX] = { "fmax", 1, 2 },
 [QOP_FMINABS] = { "fminabs", 1, 2 },
diff --git a/src/gallium/drivers/vc4/vc4_qir.h 
b/src/gallium/drivers/vc4/vc4_qir.h
index cade795..d032fab 100644
--- a/src/gallium/drivers/vc4/vc4_qir.h
+++ b/src/gallium/drivers/vc4/vc4_qir.h
@@ -66,6 +66,11 @@ enum qop {
 QOP_FADD,
 QOP_FSUB,
 QOP_FMUL,
+QOP_V8MULD,
+QOP_V8MIN,
+QOP_V8MAX,
+QOP_V8ADDS,
+QOP_V8SUBS,
 QOP_MUL24,
 QOP_FMIN,
 QOP_FMAX,
@@ -538,6 +543,11 @@ QIR_ALU1(MOV)
 QIR_ALU2(FADD)
 QIR_ALU2(FSUB)
 QIR_ALU2(FMUL)
+QIR_ALU2(V8MULD)
+QIR_ALU2(V8MIN)
+QIR_ALU2(V8MAX)
+QIR_ALU2(V8ADDS)
+QIR_ALU2(V8SUBS)
 QIR_ALU2(MUL24)
 QIR_ALU1(SEL_X_0_ZS)
 QIR_ALU1(SEL_X_0_ZC)
diff --git a/src/gallium/drivers/vc4/vc4_qpu_emit.c 
b/src/gallium/drivers/vc4/vc4_qpu_emit.c
index f324056..97f9ee0 100644
--- a/src/gallium/drivers/vc4/vc4_qpu_emit.c
+++ b/src/gallium/drivers/vc4/vc4_qpu_emit.c
@@ -204,6 +204,11 @@ vc4_generate_code(struct vc4_context *vc4, struct 
vc4_compile *c)
 A(NOT),
 
 M(FMUL),
+M(V8MULD),
+M(V8MIN),
+M(V8MAX),
+M(V8ADDS),
+M(V8SUBS),
 M(MUL24),
 };
 
-- 
2.1.4

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


[Mesa-dev] [PATCH 2/4] vc4: Don't re-pack unorm components we've already packed.

2015-08-20 Thread Eric Anholt
The  splats the first component to all the channels, so later channels
packing from it can be skipped.

total instructions in shared programs: 98170 -> 98168 (-0.00%)
instructions in affected programs: 257 -> 255 (-0.78%)
---
 src/gallium/drivers/vc4/vc4_program.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/vc4/vc4_program.c 
b/src/gallium/drivers/vc4/vc4_program.c
index 13c4721..ab2f885 100644
--- a/src/gallium/drivers/vc4/vc4_program.c
+++ b/src/gallium/drivers/vc4/vc4_program.c
@@ -840,13 +840,17 @@ ntq_emit_alu(struct vc4_compile *c, nir_alu_instr *instr)
 
 if (instr->op == nir_op_pack_unorm_4x8) {
 struct qreg result;
+struct qreg src0;
 for (int i = 0; i < 4; i++) {
 struct qreg src = ntq_get_src(c, instr->src[0].src,
   
instr->src[0].swizzle[i]);
-if (i == 0)
+if (i == 0) {
+src0 = src;
 result = qir_PACK__F(c, src);
-else
-result = qir_PACK_8_F(c, result, src, i);
+} else {
+if (memcmp(&src, &src0, sizeof(src)) != 0)
+result = qir_PACK_8_F(c, result, src, 
i);
+}
 }
 struct qreg *dest = ntq_get_dest(c, &instr->dest.dest);
 *dest = result;
-- 
2.1.4

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


[Mesa-dev] [PATCH 1/4] nir: Add opcodes for saturated vector math.

2015-08-20 Thread Eric Anholt
This corresponds to instructions used on vc4 for its blending inside of
shaders.  I've seen these opcodes on other architectures before, but I
think it's the first time these are needed in Mesa.
---
 src/glsl/nir/nir_opcodes.py   | 45 +++
 src/glsl/nir/nir_opt_algebraic.py |  6 ++
 2 files changed, 51 insertions(+)

diff --git a/src/glsl/nir/nir_opcodes.py b/src/glsl/nir/nir_opcodes.py
index df5b7e2..ff54823 100644
--- a/src/glsl/nir/nir_opcodes.py
+++ b/src/glsl/nir/nir_opcodes.py
@@ -460,6 +460,51 @@ binop("fmax", tfloat, "", "fmaxf(src0, src1)")
 binop("imax", tint, commutative + associative, "src1 > src0 ? src1 : src0")
 binop("umax", tunsigned, commutative + associative, "src1 > src0 ? src1 : 
src0")
 
+# Saturated vector add for 4 8bit ints.
+binop("isadd_4x8", tint, commutative + associative, """
+dst = 0;
+for (int i = 0; i < 32; i += 8) {
+   dst |= MIN2(((src0 >> i) & 0xff) + ((src1 >> i) & 0xff), 0xff) << i;
+}
+""")
+
+# Saturated vector subtract for 4 8bit ints.
+binop("issub_4x8", tint, "", """
+dst = 0;
+for (int i = 0; i < 32; i += 8) {
+   int src0_chan = (src0 >> i) & 0xff;
+   int src1_chan = (src1 >> i) & 0xff;
+   if (src0_chan > src1_chan)
+  dst |= (src0_chan - src1_chan) << i;
+}
+""")
+
+# vector min for 4 8bit ints.
+binop("imin_4x8", tint, commutative + associative, """
+dst = 0;
+for (int i = 0; i < 32; i += 8) {
+   dst |= MIN2((src0 >> i) & 0xff, (src1 >> i) & 0xff) << i;
+}
+""")
+
+# vector max for 4 8bit ints.
+binop("imax_4x8", tint, commutative + associative, """
+dst = 0;
+for (int i = 0; i < 32; i += 8) {
+   dst |= MAX2((src0 >> i) & 0xff, (src1 >> i) & 0xff) << i;
+}
+""")
+
+# unorm multiply: (a * b) / 255.
+binop("imul_unorm_4x8", tint, commutative + associative, """
+dst = 0;
+for (int i = 0; i < 32; i += 8) {
+   int src0_chan = (src0 >> i) & 0xff;
+   int src1_chan = (src1 >> i) & 0xff;
+   dst |= ((src0_chan * src1_chan) / 255) << i;
+}
+""")
+
 binop("fpow", tfloat, "", "powf(src0, src1)")
 
 binop_horiz("pack_half_2x16_split", 1, tunsigned, 1, tfloat, 1, tfloat,
diff --git a/src/glsl/nir/nir_opt_algebraic.py 
b/src/glsl/nir/nir_opt_algebraic.py
index 226e0a8..e1f2638 100644
--- a/src/glsl/nir/nir_opt_algebraic.py
+++ b/src/glsl/nir/nir_opt_algebraic.py
@@ -56,12 +56,16 @@ optimizations = [
(('iabs', ('ineg', a)), ('iabs', a)),
(('fadd', a, 0.0), a),
(('iadd', a, 0), a),
+   (('isadd_4x8', a, 0), a),
+   (('isadd_4x8', a, ~0), ~0),
(('fadd', ('fmul', a, b), ('fmul', a, c)), ('fmul', a, ('fadd', b, c))),
(('iadd', ('imul', a, b), ('imul', a, c)), ('imul', a, ('iadd', b, c))),
(('fadd', ('fneg', a), a), 0.0),
(('iadd', ('ineg', a), a), 0),
(('fmul', a, 0.0), 0.0),
(('imul', a, 0), 0),
+   (('imul_unorm_4x8', a, 0), 0),
+   (('imul_unorm_4x8', a, ~0), a),
(('fmul', a, 1.0), a),
(('imul', a, 1), a),
(('fmul', a, -1.0), ('fneg', a)),
@@ -200,6 +204,8 @@ optimizations = [
# Subtracts
(('fsub', a, ('fsub', 0.0, b)), ('fadd', a, b)),
(('isub', a, ('isub', 0, b)), ('iadd', a, b)),
+   (('issub_4x8', a, 0), a),
+   (('issub_4x8', a, ~0), 0),
(('fsub', a, b), ('fadd', a, ('fneg', b)), 'options->lower_sub'),
(('isub', a, b), ('iadd', a, ('ineg', b)), 'options->lower_sub'),
(('fneg', a), ('fsub', 0.0, a), 'options->lower_negate'),
-- 
2.1.4

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


[Mesa-dev] [PATCH 4/4] vc4: Convert blending to being done in 4x8 unorm normally.

2015-08-20 Thread Eric Anholt
We can't do this all the time, because you want blending to be done in
linear space, and sRGB would lose too much precision being done in 4x8.

total uniforms in shared programs: 32066 -> 32088 (0.07%)
uniforms in affected programs: 327 -> 349 (6.73%)
total instructions in shared programs: 98168 -> 94882 (-3.35%)
instructions in affected programs: 17234 -> 13948 (-19.07%)
---
 src/gallium/drivers/vc4/vc4_context.h |   5 +-
 src/gallium/drivers/vc4/vc4_nir_lower_blend.c | 286 +-
 src/gallium/drivers/vc4/vc4_qir.h |   2 +
 src/gallium/drivers/vc4/vc4_state.c   |   4 +-
 src/gallium/drivers/vc4/vc4_uniforms.c|  30 ++-
 5 files changed, 276 insertions(+), 51 deletions(-)

diff --git a/src/gallium/drivers/vc4/vc4_context.h 
b/src/gallium/drivers/vc4/vc4_context.h
index 654c46f..d3cbaeb 100644
--- a/src/gallium/drivers/vc4/vc4_context.h
+++ b/src/gallium/drivers/vc4/vc4_context.h
@@ -292,7 +292,10 @@ struct vc4_context {
 
 struct vc4_vertex_stateobj *vtx;
 
-struct pipe_blend_color blend_color;
+struct {
+struct pipe_blend_color f;
+uint8_t ub[4];
+} blend_color;
 struct pipe_stencil_ref stencil_ref;
 unsigned sample_mask;
 struct pipe_framebuffer_state framebuffer;
diff --git a/src/gallium/drivers/vc4/vc4_nir_lower_blend.c 
b/src/gallium/drivers/vc4/vc4_nir_lower_blend.c
index a372a6c..6af9be9 100644
--- a/src/gallium/drivers/vc4/vc4_nir_lower_blend.c
+++ b/src/gallium/drivers/vc4/vc4_nir_lower_blend.c
@@ -86,11 +86,11 @@ vc4_nir_srgb_encode(nir_builder *b, nir_ssa_def *linear)
 }
 
 static nir_ssa_def *
-vc4_blend_channel(nir_builder *b,
-  nir_ssa_def **src,
-  nir_ssa_def **dst,
-  unsigned factor,
-  int channel)
+vc4_blend_channel_f(nir_builder *b,
+nir_ssa_def **src,
+nir_ssa_def **dst,
+unsigned factor,
+int channel)
 {
 switch(factor) {
 case PIPE_BLENDFACTOR_ONE:
@@ -146,8 +146,75 @@ vc4_blend_channel(nir_builder *b,
 }
 
 static nir_ssa_def *
-vc4_blend_func(nir_builder *b, nir_ssa_def *src, nir_ssa_def *dst,
-   unsigned func)
+vc4_nir_set_packed_chan(nir_builder *b, nir_ssa_def *src0, nir_ssa_def *src1,
+int chan)
+{
+unsigned chan_mask = 0xff << (chan * 8);
+return nir_ior(b,
+   nir_iand(b, src0, nir_imm_int(b, ~chan_mask)),
+   nir_iand(b, src1, nir_imm_int(b, chan_mask)));
+}
+
+static nir_ssa_def *
+vc4_blend_channel_i(nir_builder *b,
+nir_ssa_def *src,
+nir_ssa_def *dst,
+nir_ssa_def *src_a,
+nir_ssa_def *dst_a,
+unsigned factor,
+int a_chan)
+{
+switch (factor) {
+case PIPE_BLENDFACTOR_ONE:
+return nir_imm_int(b, ~0);
+case PIPE_BLENDFACTOR_SRC_COLOR:
+return src;
+case PIPE_BLENDFACTOR_SRC_ALPHA:
+return src_a;
+case PIPE_BLENDFACTOR_DST_ALPHA:
+return dst_a;
+case PIPE_BLENDFACTOR_DST_COLOR:
+return dst;
+case PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE:
+return vc4_nir_set_packed_chan(b,
+   nir_imin_4x8(b,
+src_a,
+nir_inot(b, 
dst_a)),
+   nir_imm_int(b, ~0),
+   a_chan);
+case PIPE_BLENDFACTOR_CONST_COLOR:
+return vc4_nir_get_state_uniform(b, 
QUNIFORM_BLEND_CONST_COLOR_RGBA);
+case PIPE_BLENDFACTOR_CONST_ALPHA:
+return vc4_nir_get_state_uniform(b, 
QUNIFORM_BLEND_CONST_COLOR_);
+case PIPE_BLENDFACTOR_ZERO:
+return nir_imm_int(b, 0);
+case PIPE_BLENDFACTOR_INV_SRC_COLOR:
+return nir_inot(b, src);
+case PIPE_BLENDFACTOR_INV_SRC_ALPHA:
+return nir_inot(b, src_a);
+case PIPE_BLENDFACTOR_INV_DST_ALPHA:
+return nir_inot(b, dst_a);
+case PIPE_BLENDFACTOR_INV_DST_COLOR:
+return nir_inot(b, dst);
+case PIPE_BLENDFACTOR_INV_CONST_COLOR:
+return nir_inot(b, vc4_nir_get_state_uniform(b, 
QUNIFORM_BLEND_CONST_COLOR_RGBA));
+case PIPE_BLENDFACTOR_INV_CONST_ALPHA:
+return nir_inot(b, vc4_nir_get_state_uniform(b, 
QUNIFORM_BLEND_CONST_COLOR_));
+
+default:
+case PIPE_BLENDFACTOR_SRC1_COLOR:
+case PIPE_BLENDFACTOR_SRC1_ALPHA:
+case PIPE_BLENDFACTOR_INV_SRC1_COLOR:
+case PIPE_BLENDFACTOR_INV_SRC1_ALPHA:
+/* Unsupported. *

[Mesa-dev] [PATCH] i965: Use NIR by default for VS

2015-08-20 Thread Jason Ekstrand
Shader-db results for vec4 on i965:

   total instructions in shared programs: 1499894 -> 1502261 (0.16%)
   instructions in affected programs: 1414224 -> 1416591 (0.17%)
   helped:2434
   HURT:  10543
   GAINED:1
   LOST:  0

Shader-db results for vec4 on g4x:

   total instructions in shared programs: 1437411 -> 1439779 (0.16%)
   instructions in affected programs: 1362402 -> 1364770 (0.17%)
   helped:2434
   HURT:  10544
   GAINED:0
   LOST:  0

Shader-db results for vec4 on Iron Lake:

   total instructions in shared programs: 1437214 -> 1439593 (0.17%)
   instructions in affected programs: 1362205 -> 1364584 (0.17%)
   helped:2433
   HURT:  10544
   GAINED:1
   LOST:  0

Shader-db results for vec4 on Sandy Bridge:

   total instructions in shared programs: 2022092 -> 1941570 (-3.98%)
   instructions in affected programs: 1886838 -> 1806316 (-4.27%)
   helped:7510
   HURT:  10737
   GAINED:0
   LOST:  0

Shader-db results for vec4 on Ivy Bridge:

   total instructions in shared programs: 1853749 -> 1804960 (-2.63%)
   instructions in affected programs: 1686736 -> 1637947 (-2.89%)
   helped:6735
   HURT:  11101
   GAINED:0
   LOST:  0

Shader-db results for vec4 on Haswell:

   total instructions in shared programs: 1853749 -> 1804960 (-2.63%)
   instructions in affected programs: 1686736 -> 1637947 (-2.89%)
   helped:6735
   HURT:  11101
   GAINED:0
   LOST:  0

Signed-off-by: Jason Ekstrand 
---
 src/mesa/drivers/dri/i965/brw_shader.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp 
b/src/mesa/drivers/dri/i965/brw_shader.cpp
index 6b92806..14f647d 100644
--- a/src/mesa/drivers/dri/i965/brw_shader.cpp
+++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
@@ -122,7 +122,7 @@ brw_compiler_create(void *mem_ctx, const struct 
brw_device_info *devinfo)
compiler->glsl_compiler_options[MESA_SHADER_VERTEX].OptimizeForAOS = true;
compiler->glsl_compiler_options[MESA_SHADER_GEOMETRY].OptimizeForAOS = true;
 
-   if (compiler->scalar_vs || brw_env_var_as_boolean("INTEL_USE_NIR", false)) {
+   if (compiler->scalar_vs || brw_env_var_as_boolean("INTEL_USE_NIR", true)) {
   if (compiler->scalar_vs) {
  /* If we're using the scalar backend for vertex shaders, we need to
   * configure these accordingly.
@@ -135,7 +135,7 @@ brw_compiler_create(void *mem_ctx, const struct 
brw_device_info *devinfo)
   compiler->glsl_compiler_options[MESA_SHADER_VERTEX].NirOptions = 
nir_options;
}
 
-   if (brw_env_var_as_boolean("INTEL_USE_NIR", false)) {
+   if (brw_env_var_as_boolean("INTEL_USE_NIR", true)) {
   compiler->glsl_compiler_options[MESA_SHADER_GEOMETRY].NirOptions = 
nir_options;
}
 
-- 
2.4.3

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


Re: [Mesa-dev] [PATCH] i965: Use NIR by default for VS

2015-08-20 Thread Dave Airlie
> Shader-db results for vec4 on i965:

reads like a Johnny Cash cover of NIN.

sorry couldn't resist :-P

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


Re: [Mesa-dev] [PATCH 2/2] gallium/auxiliary: optimize rgb9e5 helper some more

2015-08-20 Thread Roland Scheidegger
Ping?

Am 09.08.2015 um 17:28 schrieb srol...@vmware.com:
> From: Roland Scheidegger 
> 
> I used this as some testing ground for investigating some compiler
> bits initially (e.g. lrint calls etc.), figured I could do much better
> in the end just for fun...
> This is mathematically equivalent, but uses some tricks to avoid
> doubles and also replaces some float math with ints. Good for another
> performance doubling or so. As a side note, some quick tests show that
> llvm's loop vectorizer would be able to properly vectorize this version
> (which it failed to do earlier due to doubles, producing a mess), giving
> another 3 times performance increase with sse2 (more with sse4.1), but this
> may not apply to mesa.
> ---
>  src/gallium/auxiliary/util/u_format_rgb9e5.h | 87 
> ++--
>  1 file changed, 42 insertions(+), 45 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/util/u_format_rgb9e5.h 
> b/src/gallium/auxiliary/util/u_format_rgb9e5.h
> index d11bfa8..21feba7 100644
> --- a/src/gallium/auxiliary/util/u_format_rgb9e5.h
> +++ b/src/gallium/auxiliary/util/u_format_rgb9e5.h
> @@ -74,62 +74,59 @@ typedef union {
> } field;
>  } rgb9e5;
>  
> -static inline float rgb9e5_ClampRange(float x)
> -{
> -   if (x > 0.0f) {
> -  if (x >= MAX_RGB9E5) {
> - return MAX_RGB9E5;
> -  } else {
> - return x;
> -  }
> -   } else {
> -  /* NaN gets here too since comparisons with NaN always fail! */
> -  return 0.0f;
> -   }
> -}
>  
> -/* Ok, FloorLog2 is not correct for the denorm and zero values, but we
> -   are going to do a max of this value with the minimum rgb9e5 exponent
> -   that will hide these problem cases. */
> -static inline int rgb9e5_FloorLog2(float x)
> +static inline int rgb9e5_ClampRange(float x)
>  {
> float754 f;
> -
> +   float754 max;
> f.value = x;
> -   return (f.field.biasedexponent - 127);
> +   max.value = MAX_RGB9E5;
> +
> +   if (f.raw > 0x7f80)
> +  /* catches neg, NaNs */
> +  return 0;
> +   else if (f.raw >= max.raw)
> +  return max.raw;
> +   else
> +  return f.raw;
>  }
>  
>  static inline unsigned float3_to_rgb9e5(const float rgb[3])
>  {
> rgb9e5 retval;
> -   float maxrgb;
> -   int rm, gm, bm;
> -   float rc, gc, bc;
> -   int exp_shared, maxm;
> +   int rm, gm, bm, exp_shared;
> float754 revdenom = {0};
> -
> -   rc = rgb9e5_ClampRange(rgb[0]);
> -   gc = rgb9e5_ClampRange(rgb[1]);
> -   bc = rgb9e5_ClampRange(rgb[2]);
> -
> -   maxrgb = MAX3(rc, gc, bc);
> -   exp_shared = MAX2(-RGB9E5_EXP_BIAS - 1, rgb9e5_FloorLog2(maxrgb)) + 1 + 
> RGB9E5_EXP_BIAS;
> +   float754 rc, bc, gc, maxrgb;
> +
> +   rc.raw = rgb9e5_ClampRange(rgb[0]);
> +   gc.raw = rgb9e5_ClampRange(rgb[1]);
> +   bc.raw = rgb9e5_ClampRange(rgb[2]);
> +   maxrgb.raw = MAX3(rc.raw, gc.raw, bc.raw);
> +
> +   /*
> +* Compared to what the spec suggests, instead of conditionally adjusting
> +* the exponent after the fact do it here by doing the equivalent of +0.5 
> -
> +* the int add will spill over into the exponent in this case.
> +*/
> +   maxrgb.raw += maxrgb.raw & (1 << (23-9));
> +   exp_shared = MAX2((maxrgb.raw >> 23), -RGB9E5_EXP_BIAS - 1 + 127) +
> +1 + RGB9E5_EXP_BIAS - 127;
> +   revdenom.field.biasedexponent = 127 - (exp_shared - RGB9E5_EXP_BIAS -
> +  RGB9E5_MANTISSA_BITS) + 1;
> assert(exp_shared <= RGB9E5_MAX_VALID_BIASED_EXP);
> -   assert(exp_shared >= 0);
> -   revdenom.field.biasedexponent = 127 - (exp_shared - RGB9E5_EXP_BIAS - 
> RGB9E5_MANTISSA_BITS);
> -
> -   maxm = (int) (maxrgb * revdenom.value + 0.5);
> -   if (maxm == MAX_RGB9E5_MANTISSA + 1) {
> -  revdenom.value *= 0.5f;
> -  exp_shared += 1;
> -  assert(exp_shared <= RGB9E5_MAX_VALID_BIASED_EXP);
> -   } else {
> -  assert(maxm <= MAX_RGB9E5_MANTISSA);
> -   }
> -
> -   rm = (int) (rc * revdenom.value + 0.5);
> -   gm = (int) (gc * revdenom.value + 0.5);
> -   bm = (int) (bc * revdenom.value + 0.5);
> +
> +   /*
> +* The spec uses strict round-up behavior (d3d10 disagrees, but in any 
> case
> +* must match what is done above for figuring out exponent).
> +* We avoid the doubles ((int) rc * revdenom + 0.5) by doing the rounding
> +* ourselves (revdenom was adjusted by +1, above).
> +*/
> +   rm = (int) (rc.value * revdenom.value);
> +   gm = (int) (gc.value * revdenom.value);
> +   bm = (int) (bc.value * revdenom.value);
> +   rm = (rm & 1) + (rm >> 1);
> +   gm = (gm & 1) + (gm >> 1);
> +   bm = (bm & 1) + (bm >> 1);
>  
> assert(rm <= MAX_RGB9E5_MANTISSA);
> assert(gm <= MAX_RGB9E5_MANTISSA);
> 

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


[Mesa-dev] [Bug 91020] Mesa's demo / tools won't compile since EGL changes

2015-08-20 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=91020

Julien Isorce  changed:

   What|Removed |Added

 CC||julien.iso...@gmail.com

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v2 1/3] nv50/ir: support different unordered_set implementations

2015-08-20 Thread Ilia Mirkin
I pushed this out without the Android.mk change:

http://cgit.freedesktop.org/mesa/mesa/commit/?id=2a4af36517333ef61d5f7ca2264fec3f49ee3662

I don't know anything about Android makefiles and didn't feel like I
knew enough about what was going on there to make any change. Feel
free to resubmit that separately.

Cheers,

  -ilia


On Sun, Jun 21, 2015 at 1:31 AM, Chih-Wei Huang  wrote:
> 2015-06-20 9:04 GMT+08:00 Chih-Wei Huang :
>> 2015-06-20 3:12 GMT+08:00 Emil Velikov :
>>> Hi Chih-Wei,
>>> On 19 June 2015 at 19:00, Chih-Wei Huang  wrote:
>>
 diff --git a/Android.common.mk b/Android.common.mk
 index d662d60..35dcda2 100644
 --- a/Android.common.mk
 +++ b/Android.common.mk
 @@ -77,7 +77,7 @@ LOCAL_CFLAGS += \
  endif

  LOCAL_CPPFLAGS += \
 -   $(if $(filter true,$(MESA_LOLLIPOP_BUILD)),-D_USING_LIBCXX) \
 +   $(if $(filter true,$(MESA_LOLLIPOP_BUILD)),-std=c++11) \
>>> Please expand like elsewhere in the build. Additionally this is a C++
>>> only flag, so LOCAL_CPPFLAGS does not sound like the right place.
>>> Shame that the Android folk did not like (f'd up) the standard
>>> CXXFLAGS.
>>
>> Seems you misread it.
>> LOCAL_CPPFLAGS is the C++ only flag.
>>
>>> ifeq MESA_LOLLIPOP_BUILD...
>>> LOCAL_C??FLAGS += \
>>> -std=c++11
>>> endif
>>
>> Personally I like the compact format.
>> But if you prefer the style, I can update it.
>
> After re-thinking the style, I hope to keep as it is.
> Actually the style is already accepted in my last patch.
> This patch only changed the unused -D_USING_LIBCXX
> to more appropriate -std=c++11.
>
> I consider the $(if ) operator of makefile to be
> analogous to ? : operator of C/C++.
> I know some people dislike ? : operator,
> but most programmers won't reject it
> since it make the code more elegant.
> I also see the ? : operator is used in
> Mesa's code extensively.
>
> In short, I think the patch is good
> and no plan to update it.
>
> Could you merge it?
>
> --
> Chih-Wei
> Android-x86 project
> http://www.android-x86.org
> ___
> 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 4/4] mesa/formats: refactor by globbing on types in switch statement

2015-08-20 Thread Nanley Chery
On Thu, Aug 20, 2015 at 11:34 AM, Emil Velikov 
wrote:

> 2015-08-12 0:07 GMT+01:00 Nanley Chery :
> > From: Nanley Chery 
> >
> > Combine the adjacent cases which have the same GL type in the switch
> statemnt.
> >
> > Signed-off-by: Nanley Chery 
> > ---
> >  src/mesa/main/formats.c | 152
> ++--
> >  1 file changed, 17 insertions(+), 135 deletions(-)
> >
> > diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c
> > index cb5ad21..9b9d79b 100644
> > --- a/src/mesa/main/formats.c
> > +++ b/src/mesa/main/formats.c
> > @@ -1005,13 +1005,10 @@
> _mesa_uncompressed_format_to_type_and_comps(mesa_format format,
> > case MESA_FORMAT_R8G8B8X8_UNORM:
> > case MESA_FORMAT_B8G8R8X8_UNORM:
> > case MESA_FORMAT_X8R8G8B8_UNORM:
> > -  *datatype = GL_UNSIGNED_BYTE;
> > -  *comps = 4;
> > -  return;
> > case MESA_FORMAT_BGR_UNORM8:
> > case MESA_FORMAT_RGB_UNORM8:
> >*datatype = GL_UNSIGNED_BYTE;
> > -  *comps = 3;
> > +  *comps = _mesa_format_num_components(format);
> With the datatype aside what is stopping us from using a single
> _mesa_format_num_components(format) ?
>
> I just checked w/ a gtest and we don't get the same output for the formats
listed below. The "Actual" value is the output of
_mesa_format_num_components() and the "Expected" value is the output of
_mesa_uncompressed_format_to_type_and_comps().

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_B4G4R4X4_UNORM

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_B5G5R5X1_UNORM

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_B10G10R10X2_UNORM

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_R10G10B10X2_UNORM

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 0
Expected: comps
Which is: 2
MESA_FORMAT_YCBCR

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 0
Expected: comps
Which is: 2
MESA_FORMAT_YCBCR_REV

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_RGBX_UNORM16

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_R8G8B8X8_SNORM

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_RGBX_SNORM16

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_B8G8R8X8_SRGB

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_X8R8G8B8_SRGB

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_R8G8B8X8_SRGB

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_X8B8G8R8_SRGB

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 2
Expected: comps
Which is: 1
MESA_FORMAT_Z32_FLOAT_S8X24_UINT

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_RGBX_FLOAT16

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_RGBX_FLOAT32

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_RGBX_UINT8

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_RGBX_UINT16

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_RGBX_UINT32

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_RGBX_SINT8

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_RGBX_SINT16

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_RGBX_SINT32

In fact, for this patch, the following behavioral differences exist:

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_X8B8G8R8_UNORM

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_R8G8B8X8_UNORM

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected: comps
Which is: 4
MESA_FORMAT_B8G8R8X8_UNORM

mesa_formats.cpp:59: Failure
Value of: _mesa_format_num_components(f)
  Actual: 3
Expected:

Re: [Mesa-dev] [PATCH] glsl: expose textureQueryLod in GLSL 4.00+ fragment shaders

2015-08-20 Thread Timothy Arceri
On Thu, 2015-08-20 at 14:06 -0400, Ilia Mirkin wrote:
> On Thu, Aug 20, 2015 at 2:01 PM, Marek Olšák 
> wrote:
> > On Thu, Aug 20, 2015 at 5:15 PM, Ilia Mirkin 
> > wrote:
> > > So just stick something like
> > > 
> > > """
> > > From the ARB_texture_query_lod spec:
> > > 
> > > (3) The core specification uses the "Lod" spelling, not
> > > "LOD". Should
> > > this extension be modified to use "Lod"?
> > > 
> > >   RESOLVED: The "Lod" spelling is the correct spelling for
> > > the core
> > >   specification and the preferred spelling for use. However,
> > > use of
> > >   "LOD" also exists, as the extension predated the core
> > > specification,
> > >   so this extension won't remove use of "LOD".
> > > """
> > > 
> > > as the commit message? Fine by me. It seems excessive to put that
> > > into
> > > builtin_functions.cpp... but if people feel strongly, I can do
> > > that
> > > too.
> > 
> > People tend to read code more than commit messages, so putting it
> > in
> > the code is better.
> 
> I don't really see what it adds to either the commit message or the
> code... we don't have stuff in the code for like "xyz added by spec
> bar". It's pretty obvious from the availability predicate... I don't
> see a single other instance of this in builtin_functions.cpp.

I don't necessarily think you need to put it in the code, but commit
messages are cheap. The code makes it obvious that this is the way it
should be done, but its not immediately obvious why this sillyness is
forced on us.

For example if its a spec bug that was later fixed but applications
still expect the old behaviour? or is it a bug that is noted but will
not be fixed?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] glsl: expose textureQueryLod in GLSL 4.00+ fragment shaders

2015-08-20 Thread Ilia Mirkin
On Thu, Aug 20, 2015 at 7:08 PM, Timothy Arceri  wrote:
> On Thu, 2015-08-20 at 14:06 -0400, Ilia Mirkin wrote:
>> On Thu, Aug 20, 2015 at 2:01 PM, Marek Olšák 
>> wrote:
>> > On Thu, Aug 20, 2015 at 5:15 PM, Ilia Mirkin 
>> > wrote:
>> > > So just stick something like
>> > >
>> > > """
>> > > From the ARB_texture_query_lod spec:
>> > >
>> > > (3) The core specification uses the "Lod" spelling, not
>> > > "LOD". Should
>> > > this extension be modified to use "Lod"?
>> > >
>> > >   RESOLVED: The "Lod" spelling is the correct spelling for
>> > > the core
>> > >   specification and the preferred spelling for use. However,
>> > > use of
>> > >   "LOD" also exists, as the extension predated the core
>> > > specification,
>> > >   so this extension won't remove use of "LOD".
>> > > """
>> > >
>> > > as the commit message? Fine by me. It seems excessive to put that
>> > > into
>> > > builtin_functions.cpp... but if people feel strongly, I can do
>> > > that
>> > > too.
>> >
>> > People tend to read code more than commit messages, so putting it
>> > in
>> > the code is better.
>>
>> I don't really see what it adds to either the commit message or the
>> code... we don't have stuff in the code for like "xyz added by spec
>> bar". It's pretty obvious from the availability predicate... I don't
>> see a single other instance of this in builtin_functions.cpp.
>
> I don't necessarily think you need to put it in the code, but commit
> messages are cheap. The code makes it obvious that this is the way it
> should be done, but its not immediately obvious why this sillyness is
> forced on us.
>
> For example if its a spec bug that was later fixed but applications
> still expect the old behaviour? or is it a bug that is noted but will
> not be fixed?

OK, well I don't really care about what goes into the commit message,
so I'm not going to argue the point, but I don't want to pollute code
with this stuff for no reason.

Marek, any objections?

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


Re: [Mesa-dev] [Mesa-stable] [PATCH] mesa: update fbo state in glTexStorage

2015-08-20 Thread Anuj Phogat
On Thu, Aug 20, 2015 at 12:52 AM, Tapani Pälli  wrote:
> We have to re-validate FBOs rendering to the texture like is done
> with TexImage and CopyTexImage.
>
> Signed-off-by: Tapani Pälli 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91673
> Cc: "10.6" 
> ---
>  src/mesa/main/texstorage.c | 15 +++
>  1 file changed, 15 insertions(+)
>
> diff --git a/src/mesa/main/texstorage.c b/src/mesa/main/texstorage.c
> index db2b63d..51f49b3 100644
> --- a/src/mesa/main/texstorage.c
> +++ b/src/mesa/main/texstorage.c
> @@ -189,6 +189,20 @@ clear_texture_fields(struct gl_context *ctx,
>  }
>
>
> +/**
> + * Update/re-validate framebuffer object.
> + */
> +static void
> +update_fbo_texture(struct gl_context *ctx, struct gl_texture_object *texObj)
> +{
> +   const unsigned numFaces = _mesa_num_tex_faces(texObj->Target);
> +   for (int level = 0; level < ARRAY_SIZE(texObj->Image[0]); level++) {
> +  for (unsigned face = 0; face < numFaces; face++)
> + _mesa_update_fbo_texture(ctx, texObj, face, level);
> +   }
> +}
> +
> +
>  GLboolean
>  _mesa_is_legal_tex_storage_format(struct gl_context *ctx, GLenum 
> internalformat)
>  {
> @@ -446,6 +460,7 @@ _mesa_texture_storage(struct gl_context *ctx, GLuint dims,
>
>_mesa_set_texture_view_state(ctx, texObj, target, levels);
>
> +  update_fbo_texture(ctx, texObj);
> }
>  }
>
> --
> 2.1.0
>
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-stable

LGTM. It'll be nice to have a piglit test for this bug.
Assuming no piglit regressions with the patch:

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


Re: [Mesa-dev] [PATCH] glsl: expose textureQueryLod in GLSL 4.00+ fragment shaders

2015-08-20 Thread Marek Olšák
On Fri, Aug 21, 2015 at 1:10 AM, Ilia Mirkin  wrote:
> On Thu, Aug 20, 2015 at 7:08 PM, Timothy Arceri  wrote:
>> On Thu, 2015-08-20 at 14:06 -0400, Ilia Mirkin wrote:
>>> On Thu, Aug 20, 2015 at 2:01 PM, Marek Olšák 
>>> wrote:
>>> > On Thu, Aug 20, 2015 at 5:15 PM, Ilia Mirkin 
>>> > wrote:
>>> > > So just stick something like
>>> > >
>>> > > """
>>> > > From the ARB_texture_query_lod spec:
>>> > >
>>> > > (3) The core specification uses the "Lod" spelling, not
>>> > > "LOD". Should
>>> > > this extension be modified to use "Lod"?
>>> > >
>>> > >   RESOLVED: The "Lod" spelling is the correct spelling for
>>> > > the core
>>> > >   specification and the preferred spelling for use. However,
>>> > > use of
>>> > >   "LOD" also exists, as the extension predated the core
>>> > > specification,
>>> > >   so this extension won't remove use of "LOD".
>>> > > """
>>> > >
>>> > > as the commit message? Fine by me. It seems excessive to put that
>>> > > into
>>> > > builtin_functions.cpp... but if people feel strongly, I can do
>>> > > that
>>> > > too.
>>> >
>>> > People tend to read code more than commit messages, so putting it
>>> > in
>>> > the code is better.
>>>
>>> I don't really see what it adds to either the commit message or the
>>> code... we don't have stuff in the code for like "xyz added by spec
>>> bar". It's pretty obvious from the availability predicate... I don't
>>> see a single other instance of this in builtin_functions.cpp.
>>
>> I don't necessarily think you need to put it in the code, but commit
>> messages are cheap. The code makes it obvious that this is the way it
>> should be done, but its not immediately obvious why this sillyness is
>> forced on us.
>>
>> For example if its a spec bug that was later fixed but applications
>> still expect the old behaviour? or is it a bug that is noted but will
>> not be fixed?
>
> OK, well I don't really care about what goes into the commit message,
> so I'm not going to argue the point, but I don't want to pollute code
> with this stuff for no reason.
>
> Marek, any objections?

No. Feel free to commit.

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


Re: [Mesa-dev] [PATCH] glsl: check if return_deref in lower_subroutine_visitor::visit_leave isn't NULL

2015-08-20 Thread Dave Airlie
> Can anyone take a look at this simple one-liner ? Afaics Dave added
> the pass earlier in 10.7-devel.

Looks good, pushed it, thanks.

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


[Mesa-dev] [PATCH] st/mesa: fix assignments with 4-operand arguments (i.e. BFI)

2015-08-20 Thread Ilia Mirkin
Signed-off-by: Ilia Mirkin 
Cc: "10.6" 
---
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index c8e1b8f..76a77f0 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -2816,7 +2816,7 @@ glsl_to_tgsi_visitor::visit(ir_assignment *ir)
*/
   glsl_to_tgsi_instruction *inst, *new_inst;
   inst = (glsl_to_tgsi_instruction *)this->instructions.get_tail();
-  new_inst = emit_asm(ir, inst->op, l, inst->src[0], inst->src[1], 
inst->src[2]);
+  new_inst = emit_asm(ir, inst->op, l, inst->src[0], inst->src[1], 
inst->src[2], inst->src[3]);
   new_inst->saturate = inst->saturate;
   inst->dead_mask = inst->dst[0].writemask;
} else {
-- 
2.4.6

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


[Mesa-dev] [PATCH] st/mesa: pass through 4th opcode argument in bitmap/pixel visitors

2015-08-20 Thread Ilia Mirkin
Signed-off-by: Ilia Mirkin 
---
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 76a77f0..eb47685 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -4404,12 +4404,12 @@ get_pixel_transfer_visitor(struct st_fragment_program 
*fp,
 * new visitor. */
foreach_in_list(glsl_to_tgsi_instruction, inst, &original->instructions) {
   glsl_to_tgsi_instruction *newinst;
-  st_src_reg src_regs[3];
+  st_src_reg src_regs[4];
 
   if (inst->dst[0].file == PROGRAM_OUTPUT)
  prog->OutputsWritten |= BITFIELD64_BIT(inst->dst[0].index);
 
-  for (int i = 0; i < 3; i++) {
+  for (int i = 0; i < 4; i++) {
  src_regs[i] = inst->src[i];
  if (src_regs[i].file == PROGRAM_INPUT &&
  src_regs[i].index == VARYING_SLOT_COL0) {
@@ -4420,7 +4420,7 @@ get_pixel_transfer_visitor(struct st_fragment_program *fp,
 prog->InputsRead |= BITFIELD64_BIT(src_regs[i].index);
   }
 
-  newinst = v->emit_asm(NULL, inst->op, inst->dst[0], src_regs[0], 
src_regs[1], src_regs[2]);
+  newinst = v->emit_asm(NULL, inst->op, inst->dst[0], src_regs[0], 
src_regs[1], src_regs[2], src_regs[3]);
   newinst->tex_target = inst->tex_target;
   newinst->sampler_array_size = inst->sampler_array_size;
}
@@ -4489,18 +4489,18 @@ get_bitmap_visitor(struct st_fragment_program *fp,
 * new visitor. */
foreach_in_list(glsl_to_tgsi_instruction, inst, &original->instructions) {
   glsl_to_tgsi_instruction *newinst;
-  st_src_reg src_regs[3];
+  st_src_reg src_regs[4];
 
   if (inst->dst[0].file == PROGRAM_OUTPUT)
  prog->OutputsWritten |= BITFIELD64_BIT(inst->dst[0].index);
 
-  for (int i = 0; i < 3; i++) {
+  for (int i = 0; i < 4; i++) {
  src_regs[i] = inst->src[i];
  if (src_regs[i].file == PROGRAM_INPUT)
 prog->InputsRead |= BITFIELD64_BIT(src_regs[i].index);
   }
 
-  newinst = v->emit_asm(NULL, inst->op, inst->dst[0], src_regs[0], 
src_regs[1], src_regs[2]);
+  newinst = v->emit_asm(NULL, inst->op, inst->dst[0], src_regs[0], 
src_regs[1], src_regs[2], src_regs[3]);
   newinst->tex_target = inst->tex_target;
   newinst->sampler_array_size = inst->sampler_array_size;
}
-- 
2.4.6

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


[Mesa-dev] [PATCH] glsl: use bitfield_insert instead of and + shift + or for packing

2015-08-20 Thread Ilia Mirkin
It is fairly tricky to detect the proper conditions for using bitfield
insert, but easy to just use it up front. This removes a lot of
instructions on nvc0 when invoking the packing builtins.

Signed-off-by: Ilia Mirkin 
---

Not sure if all backends will benefit from this, but nvc0 definitely
does, since it has a "insert bitfield" instruction which handles
everything all-in-one (the combined offset/bits argument takes the
position of the third source, which requires no extra logic when
they're both immediate).

It would be just as easy to use the 3-arg bfi op (no masking necessary
since we know that we start with a clean value), but gallium doesn't
support it. (Not for the least of reasons is that nvc0 doesn't support
it and I'm the one that added BFI to TGSI.)

Unpacking could use BFE, really the snorm logic would greatly benefit
from using an IBFE (thus avoiding the shifts back and forth),
but... later.

 src/glsl/ir_optimization.h |  4 +++-
 src/glsl/lower_packing_builtins.cpp| 24 +++-
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  3 +++
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/src/glsl/ir_optimization.h b/src/glsl/ir_optimization.h
index eef107e..b955874 100644
--- a/src/glsl/ir_optimization.h
+++ b/src/glsl/ir_optimization.h
@@ -66,7 +66,9 @@ enum lower_packing_builtins_op {
LOWER_UNPACK_SNORM_4x8   = 0x0200,
 
LOWER_PACK_UNORM_4x8 = 0x0400,
-   LOWER_UNPACK_UNORM_4x8   = 0x0800
+   LOWER_UNPACK_UNORM_4x8   = 0x0800,
+
+   LOWER_PACK_USE_BFI   = 0x1000,
 };
 
 bool do_common_optimization(exec_list *ir, bool linked,
diff --git a/src/glsl/lower_packing_builtins.cpp 
b/src/glsl/lower_packing_builtins.cpp
index a6fb8a8..be17e1d 100644
--- a/src/glsl/lower_packing_builtins.cpp
+++ b/src/glsl/lower_packing_builtins.cpp
@@ -225,6 +225,14 @@ private:
   "tmp_pack_uvec2_to_uint");
   factory.emit(assign(u, uvec2_rval));
 
+  if (op_mask & LOWER_PACK_USE_BFI) {
+ return bitfield_insert(
+bit_and(swizzle_x(u), constant(0xu)),
+swizzle_y(u),
+constant(16),
+constant(16));
+  }
+
   /* return (u.y << 16) | (u.x & 0x); */
   return bit_or(lshift(swizzle_y(u), constant(16u)),
 bit_and(swizzle_x(u), constant(0xu)));
@@ -242,9 +250,23 @@ private:
{
   assert(uvec4_rval->type == glsl_type::uvec4_type);
 
-  /* uvec4 u = UVEC4_RVAL; */
   ir_variable *u = factory.make_temp(glsl_type::uvec4_type,
   "tmp_pack_uvec4_to_uint");
+
+  if (op_mask & LOWER_PACK_USE_BFI) {
+ /* uvec4 u = UVEC4_RVAL; */
+ factory.emit(assign(u, uvec4_rval));
+
+ return bitfield_insert(
+bitfield_insert(
+   bitfield_insert(
+  bit_and(swizzle_x(u), constant(0xffu)),
+  swizzle_y(u), constant(8), constant(8)),
+   swizzle_z(u), constant(16), constant(8)),
+swizzle_w(u), constant(24), constant(8));
+  }
+
+  /* uvec4 u = UVEC4_RVAL & 0xff */
   factory.emit(assign(u, bit_and(uvec4_rval, constant(0xffu;
 
   /* return (u.w << 24) | (u.z << 16) | (u.y << 8) | u.x; */
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index eb47685..d3c7238 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -5995,6 +5995,9 @@ st_link_shader(struct gl_context *ctx, struct 
gl_shader_program *prog)
LOWER_PACK_HALF_2x16 |
LOWER_UNPACK_HALF_2x16;
 
+ if (ctx->Extensions.ARB_gpu_shader5)
+lower_inst |= LOWER_PACK_USE_BFI;
+
  lower_packing_builtins(ir, lower_inst);
   }
 
-- 
2.4.6

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


[Mesa-dev] [PATCH] glsl: provide the option of using BFE for unpack builting lowering

2015-08-20 Thread Ilia Mirkin
This greatly improves generated code, especially for the snorm variants,
since it is able to get rid of the lshift/rshift for sext, as well as
replacing each shift + mask with a single op.

Signed-off-by: Ilia Mirkin 
---

This actually slightly pessimizes nvc0 generated code for the unorm
case because it is no longer able to detect that it can instruct cvt
to read a byte directly, but that will be fixed in the backend later.

 src/glsl/ir_builder.cpp|   6 ++
 src/glsl/ir_builder.h  |   1 +
 src/glsl/ir_optimization.h |   1 +
 src/glsl/lower_packing_builtins.cpp| 112 -
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp |   3 +-
 5 files changed, 105 insertions(+), 18 deletions(-)

diff --git a/src/glsl/ir_builder.cpp b/src/glsl/ir_builder.cpp
index cd03859..c9cf124 100644
--- a/src/glsl/ir_builder.cpp
+++ b/src/glsl/ir_builder.cpp
@@ -567,6 +567,12 @@ csel(operand a, operand b, operand c)
 }
 
 ir_expression *
+bitfield_extract(operand a, operand b, operand c)
+{
+   return expr(ir_triop_bitfield_extract, a, b, c);
+}
+
+ir_expression *
 bitfield_insert(operand a, operand b, operand c, operand d)
 {
void *mem_ctx = ralloc_parent(a.val);
diff --git a/src/glsl/ir_builder.h b/src/glsl/ir_builder.h
index f76453f..b483ebf 100644
--- a/src/glsl/ir_builder.h
+++ b/src/glsl/ir_builder.h
@@ -200,6 +200,7 @@ ir_expression *interpolate_at_sample(operand a, operand b);
 ir_expression *fma(operand a, operand b, operand c);
 ir_expression *lrp(operand x, operand y, operand a);
 ir_expression *csel(operand a, operand b, operand c);
+ir_expression *bitfield_extract(operand a, operand b, operand c);
 ir_expression *bitfield_insert(operand a, operand b, operand c, operand d);
 
 ir_swizzle *swizzle(operand a, int swizzle, int components);
diff --git a/src/glsl/ir_optimization.h b/src/glsl/ir_optimization.h
index b955874..265b223 100644
--- a/src/glsl/ir_optimization.h
+++ b/src/glsl/ir_optimization.h
@@ -69,6 +69,7 @@ enum lower_packing_builtins_op {
LOWER_UNPACK_UNORM_4x8   = 0x0800,
 
LOWER_PACK_USE_BFI   = 0x1000,
+   LOWER_PACK_USE_BFE   = 0x2000,
 };
 
 bool do_common_optimization(exec_list *ir, bool linked,
diff --git a/src/glsl/lower_packing_builtins.cpp 
b/src/glsl/lower_packing_builtins.cpp
index be17e1d..e77e5d9 100644
--- a/src/glsl/lower_packing_builtins.cpp
+++ b/src/glsl/lower_packing_builtins.cpp
@@ -118,6 +118,7 @@ public:
  *rvalue = split_unpack_half_2x16(op0);
  break;
   case LOWER_PACK_UNPACK_NONE:
+  default:
  assert(!"not reached");
  break;
   }
@@ -307,6 +308,39 @@ private:
}
 
/**
+* \brief Unpack a uint32 into two int16's.
+*
+* Specifically each 16-bit value is sign-extended to the full width of an
+* int32 on return.
+*/
+   ir_rvalue *
+   unpack_uint_to_ivec2(ir_rvalue *uint_rval)
+   {
+  assert(uint_rval->type == glsl_type::uint_type);
+
+  if (!(op_mask & LOWER_PACK_USE_BFE)) {
+ return rshift(lshift(u2i(unpack_uint_to_uvec2(uint_rval)),
+  constant(16u)),
+   constant(16u));
+  }
+
+  ir_variable *i = factory.make_temp(glsl_type::int_type,
+  "tmp_unpack_uint_to_ivec2_i");
+  factory.emit(assign(i, u2i(uint_rval)));
+
+  /* ivec2 i2; */
+  ir_variable *i2 = factory.make_temp(glsl_type::ivec4_type,
+   "tmp_unpack_uint_to_ivec4_i2");
+
+  factory.emit(assign(i2, bitfield_extract(i, constant(0), constant(16)),
+  WRITEMASK_X));
+  factory.emit(assign(i2, bitfield_extract(i, constant(16), constant(16)),
+  WRITEMASK_Y));
+
+  return deref(i2).val;
+   }
+
+   /**
 * \brief Unpack a uint32 into four uint8's.
 *
 * Interpret the given uint32 as a uint8 4-tuple where the uint32's least
@@ -327,21 +361,69 @@ private:
   ir_variable *u4 = factory.make_temp(glsl_type::uvec4_type,
"tmp_unpack_uint_to_uvec4_u4");
 
-  /* u4.x = u & 0xffu; */
-  factory.emit(assign(u4, bit_and(u, constant(0xffu)), WRITEMASK_X));
+  if (op_mask & LOWER_PACK_USE_BFE) {
+ factory.emit(assign(u4, bitfield_extract(u, constant(0), constant(8)),
+ WRITEMASK_X));
+ factory.emit(assign(u4, bitfield_extract(u, constant(8), constant(8)),
+ WRITEMASK_Y));
+ factory.emit(assign(u4, bitfield_extract(u, constant(16), 
constant(8)),
+ WRITEMASK_Z));
+ factory.emit(assign(u4, bitfield_extract(u, constant(24), 
constant(8)),
+ WRITEMASK_W));
+  } else {
+ /* u4.x = u & 0xffu; */
+ factory.emit(assign(u4, bit_and(u, constant(0xffu)), WRITEMASK_X));
+
+ /* u4.y = (u >>

Re: [Mesa-dev] [PATCH] glsl: provide the option of using BFE for unpack builting lowering

2015-08-20 Thread Ilia Mirkin
On Thu, Aug 20, 2015 at 9:58 PM, Ilia Mirkin  wrote:
> This greatly improves generated code, especially for the snorm variants,
> since it is able to get rid of the lshift/rshift for sext, as well as
> replacing each shift + mask with a single op.
>
> Signed-off-by: Ilia Mirkin 
> ---
>
> This actually slightly pessimizes nvc0 generated code for the unorm
> case because it is no longer able to detect that it can instruct cvt
> to read a byte directly, but that will be fixed in the backend later.
>
>  src/glsl/ir_builder.cpp|   6 ++
>  src/glsl/ir_builder.h  |   1 +
>  src/glsl/ir_optimization.h |   1 +
>  src/glsl/lower_packing_builtins.cpp| 112 
> -
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp |   3 +-
>  5 files changed, 105 insertions(+), 18 deletions(-)
>
> diff --git a/src/glsl/ir_builder.cpp b/src/glsl/ir_builder.cpp
> index cd03859..c9cf124 100644
> --- a/src/glsl/ir_builder.cpp
> +++ b/src/glsl/ir_builder.cpp
> @@ -567,6 +567,12 @@ csel(operand a, operand b, operand c)
>  }
>
>  ir_expression *
> +bitfield_extract(operand a, operand b, operand c)
> +{
> +   return expr(ir_triop_bitfield_extract, a, b, c);
> +}
> +
> +ir_expression *
>  bitfield_insert(operand a, operand b, operand c, operand d)
>  {
> void *mem_ctx = ralloc_parent(a.val);
> diff --git a/src/glsl/ir_builder.h b/src/glsl/ir_builder.h
> index f76453f..b483ebf 100644
> --- a/src/glsl/ir_builder.h
> +++ b/src/glsl/ir_builder.h
> @@ -200,6 +200,7 @@ ir_expression *interpolate_at_sample(operand a, operand 
> b);
>  ir_expression *fma(operand a, operand b, operand c);
>  ir_expression *lrp(operand x, operand y, operand a);
>  ir_expression *csel(operand a, operand b, operand c);
> +ir_expression *bitfield_extract(operand a, operand b, operand c);
>  ir_expression *bitfield_insert(operand a, operand b, operand c, operand d);
>
>  ir_swizzle *swizzle(operand a, int swizzle, int components);
> diff --git a/src/glsl/ir_optimization.h b/src/glsl/ir_optimization.h
> index b955874..265b223 100644
> --- a/src/glsl/ir_optimization.h
> +++ b/src/glsl/ir_optimization.h
> @@ -69,6 +69,7 @@ enum lower_packing_builtins_op {
> LOWER_UNPACK_UNORM_4x8   = 0x0800,
>
> LOWER_PACK_USE_BFI   = 0x1000,
> +   LOWER_PACK_USE_BFE   = 0x2000,
>  };
>
>  bool do_common_optimization(exec_list *ir, bool linked,
> diff --git a/src/glsl/lower_packing_builtins.cpp 
> b/src/glsl/lower_packing_builtins.cpp
> index be17e1d..e77e5d9 100644
> --- a/src/glsl/lower_packing_builtins.cpp
> +++ b/src/glsl/lower_packing_builtins.cpp
> @@ -118,6 +118,7 @@ public:
>   *rvalue = split_unpack_half_2x16(op0);
>   break;
>case LOWER_PACK_UNPACK_NONE:
> +  default:
>   assert(!"not reached");
>   break;
>}
> @@ -307,6 +308,39 @@ private:
> }
>
> /**
> +* \brief Unpack a uint32 into two int16's.
> +*
> +* Specifically each 16-bit value is sign-extended to the full width of an
> +* int32 on return.
> +*/
> +   ir_rvalue *
> +   unpack_uint_to_ivec2(ir_rvalue *uint_rval)
> +   {
> +  assert(uint_rval->type == glsl_type::uint_type);
> +
> +  if (!(op_mask & LOWER_PACK_USE_BFE)) {
> + return rshift(lshift(u2i(unpack_uint_to_uvec2(uint_rval)),
> +  constant(16u)),
> +   constant(16u));
> +  }
> +
> +  ir_variable *i = factory.make_temp(glsl_type::int_type,
> +  "tmp_unpack_uint_to_ivec2_i");
> +  factory.emit(assign(i, u2i(uint_rval)));
> +
> +  /* ivec2 i2; */
> +  ir_variable *i2 = factory.make_temp(glsl_type::ivec4_type,
> +   "tmp_unpack_uint_to_ivec4_i2");

This should, of course, be an ivec2, as the comment indicates. Oops!

> +
> +  factory.emit(assign(i2, bitfield_extract(i, constant(0), constant(16)),
> +  WRITEMASK_X));
> +  factory.emit(assign(i2, bitfield_extract(i, constant(16), 
> constant(16)),
> +  WRITEMASK_Y));
> +
> +  return deref(i2).val;
> +   }
> +
> +   /**
>  * \brief Unpack a uint32 into four uint8's.
>  *
>  * Interpret the given uint32 as a uint8 4-tuple where the uint32's least
> @@ -327,21 +361,69 @@ private:
>ir_variable *u4 = factory.make_temp(glsl_type::uvec4_type,
> "tmp_unpack_uint_to_uvec4_u4");
>
> -  /* u4.x = u & 0xffu; */
> -  factory.emit(assign(u4, bit_and(u, constant(0xffu)), WRITEMASK_X));
> +  if (op_mask & LOWER_PACK_USE_BFE) {
> + factory.emit(assign(u4, bitfield_extract(u, constant(0), 
> constant(8)),
> + WRITEMASK_X));
> + factory.emit(assign(u4, bitfield_extract(u, constant(8), 
> constant(8)),
> + WRITEMASK_Y));
> + factory.emit(assign(u4, bitfie

Re: [Mesa-dev] [Mesa-stable] [PATCH] mesa: update fbo state in glTexStorage

2015-08-20 Thread Tapani Pälli



On 08/21/2015 02:20 AM, Anuj Phogat wrote:

On Thu, Aug 20, 2015 at 12:52 AM, Tapani Pälli  wrote:

We have to re-validate FBOs rendering to the texture like is done
with TexImage and CopyTexImage.

Signed-off-by: Tapani Pälli 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91673
Cc: "10.6" 
---
  src/mesa/main/texstorage.c | 15 +++
  1 file changed, 15 insertions(+)

diff --git a/src/mesa/main/texstorage.c b/src/mesa/main/texstorage.c
index db2b63d..51f49b3 100644
--- a/src/mesa/main/texstorage.c
+++ b/src/mesa/main/texstorage.c
@@ -189,6 +189,20 @@ clear_texture_fields(struct gl_context *ctx,
  }


+/**
+ * Update/re-validate framebuffer object.
+ */
+static void
+update_fbo_texture(struct gl_context *ctx, struct gl_texture_object *texObj)
+{
+   const unsigned numFaces = _mesa_num_tex_faces(texObj->Target);
+   for (int level = 0; level < ARRAY_SIZE(texObj->Image[0]); level++) {
+  for (unsigned face = 0; face < numFaces; face++)
+ _mesa_update_fbo_texture(ctx, texObj, face, level);
+   }
+}
+
+
  GLboolean
  _mesa_is_legal_tex_storage_format(struct gl_context *ctx, GLenum 
internalformat)
  {
@@ -446,6 +460,7 @@ _mesa_texture_storage(struct gl_context *ctx, GLuint dims,

_mesa_set_texture_view_state(ctx, texObj, target, levels);

+  update_fbo_texture(ctx, texObj);
 }
  }

--
2.1.0

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


LGTM. It'll be nice to have a piglit test for this bug.
Assuming no piglit regressions with the patch:


Yes, I'll write one based on the test program in the bug.


Reviewed-by: Anuj Phogat 


Thanks!

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


Re: [Mesa-dev] [PATCH 24/25] i965: enable ARB_arrays_of_arrays

2015-08-20 Thread Timothy Arceri
On Thu, 2015-08-20 at 11:31 -0700, Ian Romanick wrote:
> With this change, what tests pass / fail in piglit and es31 conformance?

All es31 conformance tests pass (there was one failing waiting on an SSBO
patch to land).

All piglit tests besides interface tests and some indirect sampler tests which
I believe requires the same (or related) fix as will be needed for fixing
indirect indexing of struct arrays of samplers.
I was hoping to fix these after this landed.

I also have some more unpolished piglit tests locally that I'm slowly trying
to polish off and submit but there are currently no extra issues here.

Obviously since AoA touches just about everything I may have missed something
but I think this is a pretty solid start, the Nvidia drivers still fall over
on some of my piglit tests for example.

>  I assume there are still some failures, or does everything work now?
> 
> On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> > ---
> >  src/mesa/drivers/dri/i965/intel_extensions.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c 
> > b/src/mesa/drivers/dri/i965/intel_extensions.c
> > index 0da528b..c0ff61d 100644
> > --- a/src/mesa/drivers/dri/i965/intel_extensions.c
> > +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
> > @@ -176,6 +176,7 @@ intelInitExtensions(struct gl_context *ctx)
> >  
> > assert(brw->gen >= 4);
> >  
> > +   ctx->Extensions.ARB_arrays_of_arrays = true;
> > ctx->Extensions.ARB_buffer_storage = true;
> > ctx->Extensions.ARB_clear_texture = true;
> > ctx->Extensions.ARB_clip_control = true;
> > 
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 08/25] glsl: validate binding qualifier for AoA

2015-08-20 Thread Timothy Arceri
On Thu, 2015-08-20 at 10:46 -0700, Ian Romanick wrote:
> I might have been tempted to put this and the previous patch before
> patch 6, but meh.

Yeah I did it this way as 6 is really a fix independent of AoA and I wanted to
be able to push it without waiting on the AoA patches to be fully reviewed (I
sent it separately a couple of time without getting a review). 

For the record I've pushed it so it will be in Mesa 11.0. I'll push these ones
when I land the rest of the series.

> 
> This patch is
> 
> Reviewed-by: Ian Romanick 
> 
> On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> > Reviewed-by: Tapani Pälli 
> > ---
> >  src/glsl/ast_to_hir.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> > index ee5ff9d..54569a8 100644
> > --- a/src/glsl/ast_to_hir.cpp
> > +++ b/src/glsl/ast_to_hir.cpp
> > @@ -2114,7 +2114,7 @@ validate_binding_qualifier(struct 
> > _mesa_glsl_parse_state *state,
> > }
> >  
> > const struct gl_context *const ctx = state->ctx;
> > -   unsigned elements = type->is_array() ? type->length : 1;
> > +   unsigned elements = type->is_array() ? type->arrays_of_arrays_size() : 
> > 1;
> > unsigned max_index = qual->binding + elements - 1;
> > const glsl_type *base_type = type->without_array();
> >  
> > 
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 03/25] glsl: clean-up link uniform code

2015-08-20 Thread Timothy Arceri
On Thu, 2015-08-20 at 10:39 -0700, Ian Romanick wrote:
> On 08/20/2015 10:31 AM, Ian Romanick wrote:
> > On 08/19/2015 09:37 PM, Timothy Arceri wrote:
> > > These changes are also needed to allow linking of
> > > struct and interface arrays of arrays.
> > > ---
> > >  src/glsl/link_uniforms.cpp | 16 ++--
> > >  1 file changed, 6 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> > > index 254086d..42f75e9 100644
> > > --- a/src/glsl/link_uniforms.cpp
> > > +++ b/src/glsl/link_uniforms.cpp
> > > @@ -72,6 +72,7 @@ void
> > >  program_resource_visitor::process(ir_variable *var)
> > >  {
> > > const glsl_type *t = var->type;
> > > +   const glsl_type *t_without_array = var->type->without_array();
> > > const bool row_major =
> > >var->data.matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR;
> > >  
> > > @@ -141,12 +142,8 @@ program_resource_visitor::process(ir_variable *var)
> > >char *name = ralloc_strdup(NULL, var->name);
> > >recursion(var->type, &name, strlen(name), row_major, NULL, 
> > > false);
> > >ralloc_free(name);
> > > -   } else if (t->is_interface()) {
> > > -  char *name = ralloc_strdup(NULL, var->type->name);
> > > -  recursion(var->type, &name, strlen(name), row_major, NULL, 
> > > false);
> > > -  ralloc_free(name);
> > > -   } else if (t->is_array() && t->fields.array->is_interface()) {
> > > -  char *name = ralloc_strdup(NULL, var->type->fields.array->name);
> > > +   } else if (t_without_array->is_interface()) {
> > > +  char *name = ralloc_strdup(NULL, t_without_array->name);
> > >recursion(var->type, &name, strlen(name), row_major, NULL, 
> > > false);
> > >ralloc_free(name);
> > > } else {
> > > @@ -217,8 +214,8 @@ program_resource_visitor::recursion(const glsl_type 
> > > *t, char **name,
> > >   (*name)[name_length] = '\0';
> > >   this->leave_record(t, *name, row_major);
> > >}
> > > -   } else if (t->is_array() && (t->fields.array->is_record()
> > > -|| t->fields.array->is_interface())) {
> > > +   } else if (t->without_array()->is_record()
> > > +  || t->without_array()->is_interface()) {
> > >if (record_type == NULL && t->fields.array->is_record())
> > >   record_type = t->fields.array;
> > 
> > Is the part after the change still correct?  If t is an array of array
> > of struct, record_type will still point at NULL.  I guess this might be
> > addressed in a later patch...
> 
> It looks like it's somewhat addressed in the next patch.  It's not clear
> why this is better than just adding a "|| t->fields.array->is_array()"
> clause to the right side of the &&.  Is that just the way it turned out
> (which is fine), or am I missing something subtle (which should probably
> be documented)?

Please correct me if I'm missing something but I don't think the next patch
changes anything in regards to setting record_type. The change in the
following patch is about treating all but the innermost array for AoA like we
currently would treat nested struct arrays.

Ilia also questioned this code in an earlier series so maybe I should add a
comment. Having record_type point at NULL shouldn't matter for AoA of structs
because we should just end up back in this same piece of code i.e it will
eventually get set for the innermost array by the recurrsive calls. 

Adding || t->fields.array->is_array() would set record_type to the outermost
arrays element type which I don't think is what we want right? The outer
arrays are meant to be treated as separate uniforms, we only care about the
innermost array.

> 
> > > @@ -810,8 +807,7 @@ link_update_uniform_buffer_variables(struct 
> > > gl_shader *shader)
> > >  
> > >if (var->type->is_record()) {
> > >   sentinel = '.';
> > > -  } else if (var->type->is_array()
> > > - && var->type->fields.array->is_record()) {
> > > +  } else if (var->type->without_array()->is_record()) {
> > >   sentinel = '[';
> > >}
> > >  
> > > 
> > 
> > ___
> > 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