date:20150819

Jason Ekstrand  writes:

> Previously, setting up image uniforms relied on being called after
> fs_visitor::uniforms was set and with fs_visitor::uniforms not allocating
> space for it.  This made sense in an ir_visitor world because the visitor
> assigns locations and uploads data as it walks through the variables.  In
> NIR it also happened to work because nir_lower_io assumed zero space for
> images.  In the near future, we will be able to reserve space using
> nir_lower_io and these invariants will be broken.
>
> This commit makes setup_image_uniforms take a pointer to the location in
> the prog_data params array that indicates where the image uniforms should
> be stored.  This way it can be called from anywhere in the shader setup.
>
> v2: Fix nir_setup_uniform to correctly correspond with this change
> (rebase fail)
>
> Cc: Francisco Jerez 
> Cc: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 12 
>  src/mesa/drivers/dri/i965/brw_fs.h   |  3 ---
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp |  6 --
>  src/mesa/drivers/dri/i965/brw_shader.cpp | 28 +---
>  src/mesa/drivers/dri/i965/brw_shader.h   |  5 ++---
>  5 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 82cb499..d1550e6 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -942,18 +942,6 @@ fs_visitor::import_uniforms(fs_visitor *v)
> this->param_size = v->param_size;
>  }
>  
> -void
> -fs_visitor::setup_vector_uniform_values(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];
> -
> -   for (unsigned i = n; i < 4; ++i)
> -  stage_prog_data->param[uniforms++] = &zero;
> -}
> -
>  fs_reg *
>  fs_visitor::emit_fragcoord_interpolation(bool pixel_center_integer,
>   bool origin_upper_left)
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index 975183e..28fcfa3 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -291,9 +291,6 @@ public:
>  
> struct brw_reg interp_reg(int location, int channel);
>  
> -   virtual void setup_vector_uniform_values(const gl_constant_value *values,
> -unsigned n);
> -
> int implied_mrf_writes(fs_inst *inst);
>  
> virtual void dump_instructions();
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index ce4153d..c8ea649 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -244,10 +244,12 @@ 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 =
>  BRW_IMAGE_PARAM_SIZE * MAX2(storage->array_elements, 1);
>  
> - setup_image_uniform_values(storage);
> + setup_image_uniform_values(stage_prog_data->param + 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 6b92806..b31ae9b 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -1419,8 +1419,22 @@ 
> backend_shader::assign_common_binding_table_offsets(uint32_t 
> next_binding_table_
> /* prog_data->base.binding_table.size will be set by 
> brw_mark_surface_used. */
>  }
>  
> +static void
> +setup_vector_uniform_values(const gl_constant_value ***dst,
> +const gl_constant_value *values, unsigned n)
> +{
> +   static const gl_constant_value zero = { 0 };
> +
> +   for (unsigned i = 0; i < n; ++i)
> +  *((*dst)++) = &values[i];
> +
> +   for (unsigned i = n; i < 4; ++i)
> +  *((*dst)++) = &zero;
> +}
> +
>  void
> -backend_shader::setup_image_uniform_values(const gl_uniform_storage *storage)
> +backend_shader::setup_image_uniform_values(const gl_constant_value 
> **prog_param,
> +   const gl_uniform_storage *storage)
>  {
> const unsigned stage = _mesa_program_enum_to_shader_stage(prog->Target);
>  
> @@ -1431,17 +1445,17 @@ 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_vector_uniform_values(
> +

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

2015-08-19 Thread Marek Olšák

For the series:

Reviewed-by: Marek Olšák 

Marek

On Wed, Aug 19, 2015 at 10:58 AM, 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 
> ---
>  src/gallium/drivers/r600/r600_pipe.h |  4 ++--
>  src/gallium/drivers/r600/r600_shader.c   | 30 
> ++--
>  src/gallium/drivers/r600/r600_shader.h   | 21 +++
>  src/gallium/drivers/r600/r600_state_common.c | 26 
>  4 files changed, 43 insertions(+), 38 deletions(-)
>
> diff --git a/src/gallium/drivers/r600/r600_pipe.h 
> b/src/gallium/drivers/r600/r600_pipe.h
> index 9b66105..384ba80 100644
> --- a/src/gallium/drivers/r600/r600_pipe.h
> +++ b/src/gallium/drivers/r600/r600_pipe.h
> @@ -90,7 +90,7 @@
>
>  struct r600_context;
>  struct r600_bytecode;
> -struct r600_shader_key;
> +union  r600_shader_key;
>
>  /* This is an atom containing GPU commands that never change.
>   * This is supposed to be copied directly into the CS. */
> @@ -643,7 +643,7 @@ void r600_resource_copy_region(struct pipe_context *ctx,
>  /* r600_shader.c */
>  int r600_pipe_shader_create(struct pipe_context *ctx,
> struct r600_pipe_shader *shader,
> -   struct r600_shader_key key);
> +   union r600_shader_key key);
>
>  void r600_pipe_shader_destroy(struct pipe_context *ctx, struct 
> r600_pipe_shader *shader);
>
> diff --git a/src/gallium/drivers/r600/r600_shader.c 
> b/src/gallium/drivers/r600/r600_shader.c
> index 8d1f95a..6cbfd1b 100644
> --- a/src/gallium/drivers/r600/r600_shader.c
> +++ b/src/gallium/drivers/r600/r600_shader.c
> @@ -62,7 +62,7 @@ The compiler must issue the source argument to slots z, y, 
> and x
>
>  static int r600_shader_from_tgsi(struct r600_context *rctx,
>  struct r600_pipe_shader *pipeshader,
> -struct r600_shader_key key);
> +union r600_shader_key key);
>
>
>  static void r600_add_gpr_array(struct r600_shader *ps, int start_gpr,
> @@ -133,7 +133,7 @@ static int store_shader(struct pipe_context *ctx,
>
>  int r600_pipe_shader_create(struct pipe_context *ctx,
> struct r600_pipe_shader *shader,
> -   struct r600_shader_key key)
> +   union r600_shader_key key)
>  {
> struct r600_context *rctx = (struct r600_context *)ctx;
> struct r600_pipe_shader_selector *sel = shader->selector;
> @@ -141,7 +141,7 @@ int r600_pipe_shader_create(struct pipe_context *ctx,
> bool dump = r600_can_dump_shader(&rctx->screen->b, sel->tokens);
> unsigned use_sb = !(rctx->screen->b.debug_flags & DBG_NO_SB);
> unsigned sb_disasm = use_sb || (rctx->screen->b.debug_flags & 
> DBG_SB_DISASM);
> -   unsigned export_shader = key.vs_as_es;
> +   unsigned export_shader = key.vs.as_es;
>
> shader->shader.bc.isa = rctx->isa;
>
> @@ -1802,7 +1802,7 @@ static int emit_gs_ring_writes(struct r600_shader_ctx 
> *ctx, bool ind)
>
>  static int r600_shader_from_tgsi(struct r600_context *rctx,
>  struct r600_pipe_shader *pipeshader,
> -struct r600_shader_key key)
> +union r600_shader_key key)
>  {
> struct r600_screen *rscreen = rctx->screen;
> struct r600_shader *shader = &pipeshader->shader;
> @@ -1816,7 +1816,7 @@ static int r600_shader_from_tgsi(struct r600_context 
> *rctx,
> unsigned opcode;
> int i, j, k, r = 0;
> int next_param_base = 0, next_clip_base;
> -   int max_color_exports = MAX2(key.nr_cbufs, 1);
> +   int max_color_exports = MAX2(key.ps.nr_cbufs, 1);
> /* Declarations used by llvm code */
> bool use_llvm = false;
> bool indirect_gprs;
> @@ -1830,8 +1830,8 @@ static int r600_shader_from_tgsi(struct r600_context 
> *rctx,
> ctx.shader = shader;
> ctx.native_integers = true;
>
> -   shader->vs_as_gs_a = key.vs_as_gs_a;
> -   shader->vs_as_es = key.vs_as_es;
> +   shader->vs_as_gs_a = key.vs.as_gs_a;
> +   shader->vs_as_es = key.vs.as_es;
>
> r600_bytecode_init(ctx.bc, rscreen->b.chip_class, rscreen->b.family,
>rscreen->has_compressed_msaa_texturing);
> @@ -1844,9 +1844,9 @@ static int r600_shader_from_tgsi(struct r600_context 
> *rctx,
> shader->processor_type = ctx.type;
> ctx.bc->type = shader->processor_type;
>
> -   ring_outputs = key.vs_as_es || (ctx.type == TGSI_PROCESSOR_GEOMETRY);
> +   ring_outputs = key.vs.as_es || (ctx.type == TGSI_PROCESSOR_GEOMETRY);
>
> -   if (

Re: [Mesa-dev] Can we use anonymous unions?

2015-08-19 Thread Marek Olšák

On Wed, Aug 19, 2015 at 2:47 AM, Jason Ekstrand  wrote:
> On Tue, Aug 18, 2015 at 5:36 PM, Ian Romanick  wrote:
>> On 08/18/2015 10:08 AM, Connor Abbott wrote:
>>> On Tue, Aug 18, 2015 at 9:56 AM, Kenneth Graunke  
>>> wrote:
 Hey,

 I was thinking about using an anonymous union.  Specifically, something
 like:

 struct shader {
...
union {
   struct geometry_shader_info gs;
   struct tess_eval_shader_info tes;
   ...
};
 };

 Are those acceptable in Mesa?  I don't think we've traditionally used
 them, but I'm not sure why.  Apparently they're part of C11, though not
 part of C99.  However, GCC allows them, presumably Clang, and it looks
 like MSVC 2005 supports them in C:

 https://msdn.microsoft.com/en-us/library/y9zewe0d%28v=vs.80%29.aspx

 We might be able to use these to avoid some of our fun
 thing->base.Base.base.program.Base.base shenanigans...

 --Ken

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

>>>
>>> Well, we already use anonymous unions in various places in NIR (for
>>> example, the definition of nir_dest and nir_src), so they should be ok
>>> for core Mesa. IIRC there was a problem with combining anonymous
>>> unions and C99-style initializers, but it doesn't sound like you'll be
>>> too worried about that.
>>
>> Hm... is that why I get thousands of uninitialized field warnings?  Or
>> are NIR_SRC_INIT and NIR_DEST_INIT just missing some things?
>
> That depends on how you look at it.  You could say that they are
> missing things.  However, the correct initialization for either a
> nir_src or a nir_dest is effectively a memset to 0.  Since C (and C++)
> guarantee that missing fields in a structure initializer (or compound
> literal) are initialized to 0 as long as one of them is, we get what
> we want without having to have the entire struct layout full of zeros.
> Also, since there is a union or two in there, it makes everything
> easier.

Yes, using {} (C99 only) or {0} initializers is a common way to
initialize structures to zero. If it causes any warnings, they should
be suppressed.

Marek
___
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-19 Thread Dave Airlie

On 19 August 2015 at 18: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.

it will save them later, at this point I don't it does save anything,

since the key is 16-bits now, and is 11-bits after this change, which
means it's probably a 32-bit anyways :-)

Still a nice cleanup.

Dave.
>
> Signed-off-by: Edward O'Callaghan 
> ---
>  src/gallium/drivers/r600/r600_pipe.h |  4 ++--
>  src/gallium/drivers/r600/r600_shader.c   | 30 
> ++--
>  src/gallium/drivers/r600/r600_shader.h   | 21 +++
>  src/gallium/drivers/r600/r600_state_common.c | 26 
>  4 files changed, 43 insertions(+), 38 deletions(-)
>
> diff --git a/src/gallium/drivers/r600/r600_pipe.h 
> b/src/gallium/drivers/r600/r600_pipe.h
> index 9b66105..384ba80 100644
> --- a/src/gallium/drivers/r600/r600_pipe.h
> +++ b/src/gallium/drivers/r600/r600_pipe.h
> @@ -90,7 +90,7 @@
>
>  struct r600_context;
>  struct r600_bytecode;
> -struct r600_shader_key;
> +union  r600_shader_key;
>
>  /* This is an atom containing GPU commands that never change.
>   * This is supposed to be copied directly into the CS. */
> @@ -643,7 +643,7 @@ void r600_resource_copy_region(struct pipe_context *ctx,
>  /* r600_shader.c */
>  int r600_pipe_shader_create(struct pipe_context *ctx,
> struct r600_pipe_shader *shader,
> -   struct r600_shader_key key);
> +   union r600_shader_key key);
>
>  void r600_pipe_shader_destroy(struct pipe_context *ctx, struct 
> r600_pipe_shader *shader);
>
> diff --git a/src/gallium/drivers/r600/r600_shader.c 
> b/src/gallium/drivers/r600/r600_shader.c
> index 8d1f95a..6cbfd1b 100644
> --- a/src/gallium/drivers/r600/r600_shader.c
> +++ b/src/gallium/drivers/r600/r600_shader.c
> @@ -62,7 +62,7 @@ The compiler must issue the source argument to slots z, y, 
> and x
>
>  static int r600_shader_from_tgsi(struct r600_context *rctx,
>  struct r600_pipe_shader *pipeshader,
> -struct r600_shader_key key);
> +union r600_shader_key key);
>
>
>  static void r600_add_gpr_array(struct r600_shader *ps, int start_gpr,
> @@ -133,7 +133,7 @@ static int store_shader(struct pipe_context *ctx,
>
>  int r600_pipe_shader_create(struct pipe_context *ctx,
> struct r600_pipe_shader *shader,
> -   struct r600_shader_key key)
> +   union r600_shader_key key)
>  {
> struct r600_context *rctx = (struct r600_context *)ctx;
> struct r600_pipe_shader_selector *sel = shader->selector;
> @@ -141,7 +141,7 @@ int r600_pipe_shader_create(struct pipe_context *ctx,
> bool dump = r600_can_dump_shader(&rctx->screen->b, sel->tokens);
> unsigned use_sb = !(rctx->screen->b.debug_flags & DBG_NO_SB);
> unsigned sb_disasm = use_sb || (rctx->screen->b.debug_flags & 
> DBG_SB_DISASM);
> -   unsigned export_shader = key.vs_as_es;
> +   unsigned export_shader = key.vs.as_es;
>
> shader->shader.bc.isa = rctx->isa;
>
> @@ -1802,7 +1802,7 @@ static int emit_gs_ring_writes(struct r600_shader_ctx 
> *ctx, bool ind)
>
>  static int r600_shader_from_tgsi(struct r600_context *rctx,
>  struct r600_pipe_shader *pipeshader,
> -struct r600_shader_key key)
> +union r600_shader_key key)
>  {
> struct r600_screen *rscreen = rctx->screen;
> struct r600_shader *shader = &pipeshader->shader;
> @@ -1816,7 +1816,7 @@ static int r600_shader_from_tgsi(struct r600_context 
> *rctx,
> unsigned opcode;
> int i, j, k, r = 0;
> int next_param_base = 0, next_clip_base;
> -   int max_color_exports = MAX2(key.nr_cbufs, 1);
> +   int max_color_exports = MAX2(key.ps.nr_cbufs, 1);
> /* Declarations used by llvm code */
> bool use_llvm = false;
> bool indirect_gprs;
> @@ -1830,8 +1830,8 @@ static int r600_shader_from_tgsi(struct r600_context 
> *rctx,
> ctx.shader = shader;
> ctx.native_integers = true;
>
> -   shader->vs_as_gs_a = key.vs_as_gs_a;
> -   shader->vs_as_es = key.vs_as_es;
> +   shader->vs_as_gs_a = key.vs.as_gs_a;
> +   shader->vs_as_es = key.vs.as_es;
>
> r600_bytecode_init(ctx.bc, rscreen->b.chip_class, rscreen->b.family,
>rscreen->has_compressed_msaa_texturing);
> @@ -1844,9 +1844,9 @@ static int r600_shader_from_tgsi(struct r600_context 
> *rctx,
> shader->processor_type = ctx.type;
> ctx.bc->type = shader->processor_type;
>
> -   ring_outpu

Re: [Mesa-dev] [PATCH 09/25] glsl: Add support for image binding qualifiers.

Timothy Arceri  writes:

> On Mon, 2015-08-17 at 19:45 +0300, Francisco Jerez wrote:
>> Support for binding an image to an image unit explicitly in the shader
>> source is required by both GLSL 4.2 and GLSL ES 3.1, but not by the
>> original ARB_shader_image_load_store extension.
>> ---
>>  src/glsl/ast_to_hir.cpp| 12 +++-
>>  src/glsl/link_uniform_initializers.cpp | 24 +---
>>  2 files changed, 28 insertions(+), 8 deletions(-)
>> 
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index 06cd6a5..0bf7a1f 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -2181,10 +2181,20 @@ validate_binding_qualifier(struct 
>> _mesa_glsl_parse_state *state,
>>  
>>   return false;
>>}
>> +   } else if (state->is_version(420, 310) &&
>> +  var->type->without_array()->is_image()) {
>> +  assert(ctx->Const.MaxImageUnits <= MAX_IMAGE_UNITS);
>> +  if (max_index >= ctx->Const.MaxImageUnits) {
>> + _mesa_glsl_error(loc, state, "Image binding %d exceeds the "
>> +  " maximum number of image units (%d)", max_index,
>> +  ctx->Const.MaxImageUnits);
>> + return false;
>> +  }
>> +
>> } else {
>>_mesa_glsl_error(loc, state,
>> "the \"binding\" qualifier only applies to uniform "
>> -   "blocks, samplers, atomic counters, or arrays 
>> thereof");
>> +   "blocks, opaque variables, or arrays thereof");
>>return false;
>> }
>>  
>> diff --git a/src/glsl/link_uniform_initializers.cpp 
>> b/src/glsl/link_uniform_initializers.cpp
>> index d61ae91..7c6269b 100644
>> --- a/src/glsl/link_uniform_initializers.cpp
>> +++ b/src/glsl/link_uniform_initializers.cpp
>> @@ -101,7 +101,7 @@ copy_constant_to_storage(union gl_constant_value 
>> *storage,
>>  }
>>  
>>  void
>> -set_sampler_binding(gl_shader_program *prog, const char *name, int binding)
>> +set_opaque_binding(gl_shader_program *prog, const char *name, int binding)
>
> I guess you could add a comment the atomics are handled elsewhere but not a
> big deal.
>
> Reviewed-by: Timothy Arceri 
>

| --- a/src/glsl/link_uniform_initializers.cpp
| +++ b/src/glsl/link_uniform_initializers.cpp
| @@ -100,6 +100,11 @@ copy_constant_to_storage(union gl_constant_value 
*storage,
| }
|  }
| 
| +/**
| + * Initialize an opaque uniform from the value of an explicit binding
| + * qualifier specified in the shader.  Atomic counters are different because
| + * they have no storage and should be handled elsewhere.
| + */
|  void
|  set_opaque_binding(gl_shader_program *prog, const char *name, int binding)
|  {

Does that look okay for you?

Thanks.

>
>>  {
>> struct gl_uniform_storage *const storage =
>>get_storage(prog->UniformStorage, prog->NumUniformStorage, name);
>> @@ -127,11 +127,20 @@ set_sampler_binding(gl_shader_program *prog, const 
>> char *name, int binding)
>> for (int sh = 0; sh < MESA_SHADER_STAGES; sh++) {
>>gl_shader *shader = prog->_LinkedShaders[sh];
>>  
>> -  if (shader && storage->sampler[sh].active) {
>> - for (unsigned i = 0; i < elements; i++) {
>> -unsigned index = storage->sampler[sh].index + i;
>> +  if (shader) {
>> + if (storage->type->base_type == GLSL_TYPE_SAMPLER &&
>> + storage->sampler[sh].active) {
>> +for (unsigned i = 0; i < elements; i++) {
>> +   const unsigned index = storage->sampler[sh].index + i;
>> +   shader->SamplerUnits[index] = storage->storage[i].i;
>> +}
>>  
>> -shader->SamplerUnits[index] = storage->storage[i].i;
>> + } else if (storage->type->base_type == GLSL_TYPE_IMAGE &&
>> +storage->image[sh].active) {
>> +for (unsigned i = 0; i < elements; i++) {
>> +   const unsigned index = storage->image[sh].index + i;
>> +   shader->ImageUnits[index] = storage->storage[i].i;
>> +}
>>   }
>>}
>> }
>> @@ -267,8 +276,9 @@ link_set_uniform_initializers(struct gl_shader_program 
>> *prog,
>>   if (var->data.explicit_binding) {
>>  const glsl_type *const type = var->type;
>>  
>> -if (type->without_array()->is_sampler()) {
>> -   linker::set_sampler_binding(prog, var->name, var
>> ->data.binding);
>> +if (type->without_array()->is_sampler() ||
>> +type->without_array()->is_image()) {
>> +   linker::set_opaque_binding(prog, var->name, var
>> ->data.binding);
>>  } else if (var->is_in_buffer_block()) {
>> const glsl_type *const iface_type = var
>> ->get_interface_type();
>>  


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

Re: [Mesa-dev] [PATCH] ABI-check: Use more portable bash invocation.

2015-08-19 Thread Eero Tamminen


Hi,

On 08/18/2015 08:59 PM, Matt Turner wrote:

On Tue, Jul 21, 2015 at 2:08 PM, Vinson Lee  wrote:

Fixes 'make check' on FreeBSD.

Signed-off-by: Vinson Lee 
---


Seems fine to me.

Reviewed-by: Matt Turner 

At the same time, I don't have any idea if the scripts actually rely
on bash itself. I think I've heard that Debian has a tool to detect
bash-dependencies in shell scripts that might be of some use.


Debian "devscripts" package contains "checkbashisms".

I've used it to make my own scripts POSIX / Busybox shell compliant and 
it has worked fine for me.



- Eero

PS. About only Bashism that is hard to do in POSIX shell is array 
accesses.  E.g. most things where string slices are used, can also be 
done with POSIX string operations.  For example:

extension=${str##*\.}
if [ $extension = "gz" ]; then
...

(Diligent shell programmer would have as first check:
if [ "$extension" = "$str" ]; then
# no extension
return
fi

)

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

Re: [Mesa-dev] [PATCH 2/2][RFC] docs: Add the 2015 ARB extensions

2015-08-19 Thread Eero Tamminen


Hi,

On 08/19/2015 03:18 AM, Ian Romanick wrote:

On 08/12/2015 02:23 PM, Thomas Helland wrote:

2015-08-12 18:56 GMT+02:00 Kenneth Graunke :

On Wednesday, August 12, 2015 06:32:50 PM Thomas Helland wrote:

2015-08-12 17:48 GMT+02:00 Ilia Mirkin :

On Tue, Aug 11, 2015 at 1:48 PM, Thomas Helland
 wrote:

Signed-off-by: Thomas Helland 
---
This adds a section for the extensions nvidia has chosen to
call the "GL ARB 2015 Extensions" unveiled at SIGGRAPH.


There are ARB extensions released every year (or more often, not
sure)... we don't track all ARB extensions. Why are these so special
vs e.g. the ones released along with GL 4.5 but that weren't included
in the spec? Or any of the other ones...



Well. They're not really special I guess. This just follows from the
discussion that went down on irc between me, glennk, fredrikh, ++.


Should GL3.txt just become extension-implementation-status.txt and
list all non-vendor-specific extensions? So far it has stuck to actual
GL versions (and more recently GLES).



We can keep it GL / GLES versions only. Or we can extend it to a
extension-implementation-status.txt thing. Or we can split it
into two different files. I really don't care to much either way.

If we end up adding these extensions to the file then a rename
and adding other ARB's is probably the way to go. There are
positive and negative sides to both approaches, and its not
my call to decide how, and if, we want this. It gives a nice overview
but at the same time it has PR- and "needs-to-be-kept-updated"-
implications that we may not want. I'm all ears for suggestions.

-Thomas


I like the idea of adding an "ARB Extensions" section and listing all
the ARB extensions that aren't part of a particular GL version - simply
in addition to the existing content, rather than reorganizing it.

GL3.txt has been a misnomer for a while, but I don't care whether we
rename it or not; it doesn't bother me.

--Ken


I've assembled a list of extensions I *think* are not demanded by
any current openGL specs, but I may have missed some.
(I find it weird that I VAO's in any of the specs, for example)
I could add all of them to a separate section to track them,
or I can leave it as is and drop this patch. Up to you guys.

2.  GLX_ARB_get_proc_address
4.  WGL_ARB_buffer_region
8.  WGL_ARB_extensions_string
9.  WGL_ARB_pixel_format
10. WGL_ARB_make_current_read
11. WGL_ARB_pbuffer



15. GL_ARB_vertex_blend
16. GL_ARB_matrix_palette


Ilia sent a list of extensions that are required for 2.1 and 3.0.  There
are a bunch of additional extensions, like the two above and
GL_ARB_compatibility below that should be listed in GL3.txt.  We never
want these extensions ever.  If we add them to that file, someone will
do a bunch of work and submit a bunch of patches.


Maybe at the end of the doc there could be a list of extensions that 
Mesa will never implement? (potentially with short description why: 
"obsolete", "nobody uses", "bad idea" etc)



- Eero


20. WGL_ARB_render_texture
24. GL_ARB_shadow_ambient
36. GL_ARB_fragment_program_shadow
42. GL_ARB_pixel_buffer_object
43. GL_ARB_depth_buffer_float
45. GL_ARB_framebuffer_object
46. GL_ARB_framebuffer_sRGB
 GLX_ARB_framebuffer_sRGB
 WGL_ARB_framebuffer_sRGB
48. GL_ARB_half_float_vertex
50. GL_ARB_map_buffer_range
52. GL_ARB_texture_compression_rgtc
53. GL_ARB_texture_rg
54. GL_ARB_vertex_array_object
55. WGL_ARB_create_context
56. GLX_ARB_create_context
58. GL_ARB_compatibility
60. GL_ARB_shader_texture_lod
74. WGL_ARB_create_context_profile
75. GLX_ARB_create_context_profile
76. GL_ARB_shading_language_include


We can also omit this one.  Nobody ever implemented this extension.
It's one of those things that started as a good idea, but it got really,
really out of hand.


101.GLX_ARB_create_context_robustness
102.WGL_ARB_create_context_robustness
103.GL_ARB_cl_event
104.GL_ARB_debug_output
105.GL_ARB_robustness
106.GL_ARB_shader_stencil_export
118.GL_KHR_texture_compression_astc_hdr
 GL_KHR_texture_compression_astc_ldr


Isn't ASTC LDR required for OpenGL ES 3.2?


126.GL_ARB_robustness_isolation
142.GLX_ARB_robustness_application_isolation
 GLX_ARB_robustness_share_group_isolation
143.WGL_ARB_robustness_application_isolation
 WGL_ARB_robustness_share_group_isolation
152.GL_ARB_bindless_texture
153.GL_ARB_compute_variable_group_size
154.GL_ARB_indirect_parameters
155.GL_ARB_seamless_cubemap_per_texture
156.GL_ARB_shader_draw_parameters
157.GL_ARB_shader_group_vote
158.GL_ARB_sparse_texture
171.GL_ARB_pipeline_statistics_query
172.GL_ARB_sparse_buffer

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

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.

> 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


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

[Mesa-dev] [PATCH 20.5/25] mesa: Add ES31 API tag for the extension table.

I'll mark the OES_shader_image_atomic extension entry with this tag to
make sure that we don't expose it on earlier GLES API versions
accidentally, because according to the extension:

 "OpenGL ES 3.1 and GLSL ES 3.10 are required."
---
 src/mesa/main/extensions.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
index d934d19..017de2d 100644
--- a/src/mesa/main/extensions.c
+++ b/src/mesa/main/extensions.c
@@ -50,6 +50,7 @@ enum {
ES1 = 1 << API_OPENGLES,
ES2 = 1 << API_OPENGLES2,
ES3 = 1 << (API_OPENGL_LAST + 1),
+   ES31 = 1 << (API_OPENGL_LAST + 2),
 };
 
 /**
@@ -773,6 +774,8 @@ _mesa_make_extension_string(struct gl_context *ctx)
unsigned api_set = (1 << ctx->API);
if (_mesa_is_gles3(ctx))
   api_set |= ES3;
+   if (_mesa_is_gles31(ctx))
+  api_set |= ES31;
 
/* Check if the MESA_EXTENSION_MAX_YEAR env var is set */
{
@@ -854,6 +857,8 @@ _mesa_get_extension_count(struct gl_context *ctx)
unsigned api_set = (1 << ctx->API);
if (_mesa_is_gles3(ctx))
   api_set |= ES3;
+   if (_mesa_is_gles31(ctx))
+  api_set |= ES31;
 
/* only count once */
if (ctx->Extensions.Count != 0)
@@ -880,6 +885,8 @@ _mesa_get_enabled_extension(struct gl_context *ctx, GLuint 
index)
unsigned api_set = (1 << ctx->API);
if (_mesa_is_gles3(ctx))
   api_set |= ES3;
+   if (_mesa_is_gles31(ctx))
+  api_set |= ES31;
 
base = (GLboolean*) &ctx->Extensions;
n = 0;
-- 
2.4.6

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

Re: [Mesa-dev] big endian: r600g

2015-08-19 Thread Dragomir Ivanov

Best thing to do is to hire some developer to do the work for you. If it
knows Mesa well, this can be cheaper and faster than doing it yourself. I
know Timothy Arceri  ( t_arc...@yahoo.com.au)  was doing some Mesa work for
pay in the past.

On Wed, Aug 19, 2015 at 10:34 AM, Christian Zigotzky  wrote:

> Hi Michel,
>
> Thank you for your answer. I don't have an idea anymore. Could you tell me
> which files we have to modify? Is it possible to port the AMD Catalyst
> driver to the PowerPC platform? If yes do you know how can I in touch with
> AMD? Please help us. We want to release new PowerPC desktop systems but
> without 3D acceleration it wouldn't be so nice.
>
> Cheers,
>
> Christian
>
> On 19 August 2015 at 09:12 AM, Michel Dänzer wrote:
>
>> On 19.08.2015 15:02, Christian Zigotzky wrote:
>>
>>> Could you please adapt the r600g driver for the changed PIPE_FORMAT_*
>>> semantics for big endian machines, please?
>>>
>>> All Power Mac and AMIGA one users can't use AMD Radeon HD7xxx or higher
>>> for 3D acceleration. The r600g driver needs to be adapted to changes in
>>> the way the Gallium3D infrastructure deals with big endian hosts.
>>>
>>> Bug report: https://bugs.freedesktop.org/show_bug.cgi?id=72877
>>>
>> I'm afraid this kind of request isn't going to get you anywhere. The
>> problem is that there's nobody working on Mesa who cares about those
>> platforms. You'd need to find somebody who cares about those platforms
>> and who can work on fixing the issues.
>>
>>
>>
> ___
> 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] big endian: r600g

2015-08-19 Thread Martin Peres


On 19/08/15 14:52, Dragomir Ivanov wrote:
Best thing to do is to hire some developer to do the work for you. If 
it knows Mesa well, this can be cheaper and faster than doing it 
yourself. I know Timothy Arceri  ( t_arc...@yahoo.com.au 
)  was doing some Mesa work for pay in 
the past.


I guess Collabora or another open source service company is who you 
should be contacting for such specific task.

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

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

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

--- Comment #3 from Dongseong Hwang  ---
(In reply to Tapani Pälli from comment #2)
> With a bit of testing it seems things work fine if glTexStorage2D call is
> moved to happen right after first glBindTexture call (this is also how is
> being done in some Piglit tests). Maybe this will help you get forward while
> the bug is being fixed.

That's good hint for chromium.

Mesa crashes on updating immutable texture bound to FBO.
So following code doesn't crash

GLuint tex_;
GLuint fbo_;
glGenTextures(1, &tex_);
glBindTexture(GL_TEXTURE_2D, tex_);

glGenFramebuffers(1, &fbo_);
glBindFramebuffer(GL_FRAMEBUFFER, fbo_);
glFramebufferTexture2D(GL_FRAMEBUFFER,
   GL_COLOR_ATTACHMENT0,
   GL_TEXTURE_2D,
   tex_,
   0);

glBindTexture(GL_TEXTURE_2D, tex_);
glTexStorage2D(GL_TEXTURE_2D, 2, GL_RGBA8, 2, 2);

// hack to prevent crash
glFramebufferTexture2D(GL_FRAMEBUFFER,
   GL_COLOR_ATTACHMENT0,
   GL_TEXTURE_2D,
   0,
   0);

// not SEGFAULT lol
glTexSubImage2D(GL_TEXTURE_2D,
0,
0, 0,
2, 2,
GL_RGBA, GL_UNSIGNED_BYTE,
source_pixels);

glFramebufferTexture2D(GL_FRAMEBUFFER,
   GL_COLOR_ATTACHMENT0,
   GL_TEXTURE_2D,
   tex_,
   0);

-- 
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/es3.1: Allow GL_COMPUTE_WORK_GROUP_SIZE for OpenGL ES 3.1

From: Marta Lofstedt 

According to OpenGL ES specification section 7.12,
GL_COMPUTE_WORK_GROUP_SIZE, is supported by the
glGetProgramiv function.

Signed-off-by: Marta Lofstedt 
---
 src/mesa/main/shaderapi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index b227c17..0e0e0d6 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -756,7 +756,7 @@ get_programiv(struct gl_context *ctx, GLuint program, 
GLenum pname,
   return;
case GL_COMPUTE_WORK_GROUP_SIZE: {
   int i;
-  if (!_mesa_is_desktop_gl(ctx) || !ctx->Extensions.ARB_compute_shader)
+  if (!_mesa_has_compute_shaders(ctx))
  break;
   if (!shProg->LinkStatus) {
  _mesa_error(ctx, GL_INVALID_OPERATION, "glGetProgramiv(program not "
-- 
1.9.1

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

[Mesa-dev] [PATCH] mesa/es3.1: Enable getting MAX_COMPUTE_WORK_GROUP_ values for OpenGL ES 3.1

From: Marta Lofstedt 

According to the OpenGL ES 3.1 specification chapter 17, the
MAX_COMPUTE_WORK_GROUP_COUNT and MAX_COMPUTE_WORK_GROUP_SIZE
is available for glGetIntegeri_v.

Signed-off-by: Marta Lofstedt 
---
 src/mesa/main/get.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index 307a5ff..c691997 100644
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -2049,7 +2049,7 @@ find_value_indexed(const char *func, GLenum pname, GLuint 
index, union value *v)
   return TYPE_INT;
 
case GL_MAX_COMPUTE_WORK_GROUP_COUNT:
-  if (!_mesa_is_desktop_gl(ctx) || !ctx->Extensions.ARB_compute_shader)
+  if (!_mesa_has_compute_shaders(ctx))
  goto invalid_enum;
   if (index >= 3)
  goto invalid_value;
@@ -2057,7 +2057,7 @@ find_value_indexed(const char *func, GLenum pname, GLuint 
index, union value *v)
   return TYPE_INT;
 
case GL_MAX_COMPUTE_WORK_GROUP_SIZE:
-  if (!_mesa_is_desktop_gl(ctx) || !ctx->Extensions.ARB_compute_shader)
+  if (!_mesa_has_compute_shaders(ctx))
  goto invalid_enum;
   if (index >= 3)
  goto invalid_value;
-- 
1.9.1

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

Re: [Mesa-dev] [PATCH v3 2/5] glsl: add support for the imageSize builtin

Martin Peres  writes:

> The code is heavily inspired from Francisco Jerez's code supporting the
> image_load_store extension.
>
> Backends willing to support this builtin should handle
> __intrinsic_image_size.
>
> v2: Based on the review of Ilia Mirkin
> - Enable the extension for GLES 3.1
> - Fix indentation
> - Fix the return type (float to int, number of components for CubeImages)
> - Add a warning related to GLES 3.1
>
> v3: Based on the review of Francisco Jerez
> - Refactor the code to share both add_image_function and _image with the other
>   image-related functions
>
> Signed-off-by: Martin Peres 
> ---
>  src/glsl/builtin_functions.cpp | 109 
> +++--
>  1 file changed, 93 insertions(+), 16 deletions(-)
>
> diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
> index 2175c66..5d0a825 100644
> --- a/src/glsl/builtin_functions.cpp
> +++ b/src/glsl/builtin_functions.cpp
> @@ -399,6 +399,13 @@ shader_image_load_store(const _mesa_glsl_parse_state 
> *state)
>  }
>  
>  static bool
> +shader_image_size(const _mesa_glsl_parse_state *state)
> +{
> +   return (state->is_version(430, 310) ||
> +   state->ARB_shader_image_size_enable);
> +}
> +
> +static bool
>  gs_streams(const _mesa_glsl_parse_state *state)
>  {
> return gpu_shader5(state) && gs_only(state);
> @@ -492,6 +499,11 @@ private:
> /** Create a new function and add the given signatures. */
> void add_function(const char *name, ...);
>  
> +   typedef ir_function_signature 
> *(builtin_builder::*image_prototype_ctr)(const glsl_type *image_type,
> +  
> const char *intrinsic_name,
> +  
> unsigned num_arguments,
> +  
> unsigned flags);
> +
> enum image_function_flags {
>IMAGE_FUNCTION_EMIT_STUB = (1 << 0),
>IMAGE_FUNCTION_RETURNS_VOID = (1 << 1),
> @@ -507,6 +519,7 @@ private:
>  */
> void add_image_function(const char *name,
> const char *intrinsic_name,
> +   image_prototype_ctr prototype,
> unsigned num_arguments,
> unsigned flags);
>  
> @@ -708,7 +721,12 @@ private:
> const char *intrinsic_name,
> unsigned num_arguments,
> unsigned flags);
> -   ir_function_signature *_image(const glsl_type *image_type,
> +   ir_function_signature *_image_size_prototype(const glsl_type *image_type,
> +   const char *intrinsic_name,
> +   unsigned num_arguments,
> +   unsigned flags);

The indentation looks weird.  Mixing tabs and spaces?

> +   ir_function_signature *_image(image_prototype_ctr prototype,
> + const glsl_type *image_type,
>   const char *intrinsic_name,
>   unsigned num_arguments,
>   unsigned flags);
> @@ -2552,6 +2570,7 @@ builtin_builder::add_function(const char *name, ...)
>  void
>  builtin_builder::add_image_function(const char *name,
>  const char *intrinsic_name,
> +image_prototype_ctr prototype,
>  unsigned num_arguments,
>  unsigned flags)
>  {
> @@ -2590,12 +2609,13 @@ builtin_builder::add_image_function(const char *name,
>glsl_type::uimage2DMS_type,
>glsl_type::uimage2DMSArray_type
> };
> +
> ir_function *f = new(mem_ctx) ir_function(name);
>  
> for (unsigned i = 0; i < ARRAY_SIZE(types); ++i) {
>if (types[i]->sampler_type != GLSL_TYPE_FLOAT ||
>(flags & IMAGE_FUNCTION_SUPPORTS_FLOAT_DATA_TYPE))
> - f->add_signature(_image(types[i], intrinsic_name,
> + f->add_signature(_image(prototype, types[i], intrinsic_name,
>   num_arguments, flags));
> }
>  
> @@ -2608,43 +2628,57 @@ builtin_builder::add_image_functions(bool glsl)
> const unsigned flags = (glsl ? IMAGE_FUNCTION_EMIT_STUB : 0);
>  
> add_image_function(glsl ? "imageLoad" : "__intrinsic_image_load",
> -  "__intrinsic_image_load", 0,
> -  (flags | IMAGE_FUNCTION_HAS_VECTOR_DATA_TYPE |
> +   "__intrinsic_image_load",
> +   &builtin_builder::_image_prototype, 0,
> +   (flags | IMAGE_FUNCTION_HAS_VECTOR_DATA_TYPE |
> IMAGE_FUNCTION_SUPPORTS_FLOAT_DATA_TYPE |
> IMAGE_FUNCTION_READ_ONLY));
>

Re: [Mesa-dev] [PATCH v2] i965: Refactor image uniform setup

2015-08-19 Thread Jason Ekstrand

On Aug 19, 2015 2:56 AM, "Francisco Jerez"  wrote:
>
> Jason Ekstrand  writes:
>
> > Previously, setting up image uniforms relied on being called after
> > fs_visitor::uniforms was set and with fs_visitor::uniforms not
allocating
> > space for it.  This made sense in an ir_visitor world because the
visitor
> > assigns locations and uploads data as it walks through the variables.
In
> > NIR it also happened to work because nir_lower_io assumed zero space for
> > images.  In the near future, we will be able to reserve space using
> > nir_lower_io and these invariants will be broken.
> >
> > This commit makes setup_image_uniforms take a pointer to the location in
> > the prog_data params array that indicates where the image uniforms
should
> > be stored.  This way it can be called from anywhere in the shader setup.
> >
> > v2: Fix nir_setup_uniform to correctly correspond with this change
> > (rebase fail)
> >
> > Cc: Francisco Jerez 
> > Cc: Kenneth Graunke 
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs.cpp | 12 
> >  src/mesa/drivers/dri/i965/brw_fs.h   |  3 ---
> >  src/mesa/drivers/dri/i965/brw_fs_nir.cpp |  6 --
> >  src/mesa/drivers/dri/i965/brw_shader.cpp | 28
+---
> >  src/mesa/drivers/dri/i965/brw_shader.h   |  5 ++---
> >  5 files changed, 27 insertions(+), 27 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > index 82cb499..d1550e6 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > @@ -942,18 +942,6 @@ fs_visitor::import_uniforms(fs_visitor *v)
> > this->param_size = v->param_size;
> >  }
> >
> > -void
> > -fs_visitor::setup_vector_uniform_values(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];
> > -
> > -   for (unsigned i = n; i < 4; ++i)
> > -  stage_prog_data->param[uniforms++] = &zero;
> > -}
> > -
> >  fs_reg *
> >  fs_visitor::emit_fragcoord_interpolation(bool pixel_center_integer,
> >   bool origin_upper_left)
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
b/src/mesa/drivers/dri/i965/brw_fs.h
> > index 975183e..28fcfa3 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs.h
> > +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> > @@ -291,9 +291,6 @@ public:
> >
> > struct brw_reg interp_reg(int location, int channel);
> >
> > -   virtual void setup_vector_uniform_values(const gl_constant_value
*values,
> > -unsigned n);
> > -
> > int implied_mrf_writes(fs_inst *inst);
> >
> > virtual void dump_instructions();
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > index ce4153d..c8ea649 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > @@ -244,10 +244,12 @@ 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 =
> >  BRW_IMAGE_PARAM_SIZE * MAX2(storage->array_elements, 1);
> >
> > - setup_image_uniform_values(storage);
> > + setup_image_uniform_values(stage_prog_data->param + 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 6b92806..b31ae9b 100644
> > --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> > @@ -1419,8 +1419,22 @@
backend_shader::assign_common_binding_table_offsets(uint32_t
next_binding_table_
> > /* prog_data->base.binding_table.size will be set by
brw_mark_surface_used. */
> >  }
> >
> > +static void
> > +setup_vector_uniform_values(const gl_constant_value ***dst,
> > +const gl_constant_value *values, unsigned
n)
> > +{
> > +   static const gl_constant_value zero = { 0 };
> > +
> > +   for (unsigned i = 0; i < n; ++i)
> > +  *((*dst)++) = &values[i];
> > +
> > +   for (unsigned i = n; i < 4; ++i)
> > +  *((*dst)++) = &zero;
> > +}
> > +
> >  void
> > -backend_shader::setup_image_uniform_values(const gl_uniform_storage
*storage)
> > +backend_shader::setup_image_uniform_values(const gl_constant_value
**prog_param,
> > +   const gl_uniform_storage
*storage)
> >  {
> > const unsigned stage =
_mesa_program_enum_to_shader_stage(prog->Target);
> >
> > @@ -1431,17 +1445,17 @@
backend_shader::setup_image_uniform_values(

Re: [Mesa-dev] [PATCH v2] i965: Refactor image uniform setup

Jason Ekstrand  writes:

> On Aug 19, 2015 2:56 AM, "Francisco Jerez"  wrote:
>>
>> Jason Ekstrand  writes:
>>
>> > Previously, setting up image uniforms relied on being called after
>> > fs_visitor::uniforms was set and with fs_visitor::uniforms not
> allocating
>> > space for it.  This made sense in an ir_visitor world because the
> visitor
>> > assigns locations and uploads data as it walks through the variables.
> In
>> > NIR it also happened to work because nir_lower_io assumed zero space for
>> > images.  In the near future, we will be able to reserve space using
>> > nir_lower_io and these invariants will be broken.
>> >
>> > This commit makes setup_image_uniforms take a pointer to the location in
>> > the prog_data params array that indicates where the image uniforms
> should
>> > be stored.  This way it can be called from anywhere in the shader setup.
>> >
>> > v2: Fix nir_setup_uniform to correctly correspond with this change
>> > (rebase fail)
>> >
>> > Cc: Francisco Jerez 
>> > Cc: Kenneth Graunke 
>> > ---
>> >  src/mesa/drivers/dri/i965/brw_fs.cpp | 12 
>> >  src/mesa/drivers/dri/i965/brw_fs.h   |  3 ---
>> >  src/mesa/drivers/dri/i965/brw_fs_nir.cpp |  6 --
>> >  src/mesa/drivers/dri/i965/brw_shader.cpp | 28
> +---
>> >  src/mesa/drivers/dri/i965/brw_shader.h   |  5 ++---
>> >  5 files changed, 27 insertions(+), 27 deletions(-)
>> >
>> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> > index 82cb499..d1550e6 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> > @@ -942,18 +942,6 @@ fs_visitor::import_uniforms(fs_visitor *v)
>> > this->param_size = v->param_size;
>> >  }
>> >
>> > -void
>> > -fs_visitor::setup_vector_uniform_values(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];
>> > -
>> > -   for (unsigned i = n; i < 4; ++i)
>> > -  stage_prog_data->param[uniforms++] = &zero;
>> > -}
>> > -
>> >  fs_reg *
>> >  fs_visitor::emit_fragcoord_interpolation(bool pixel_center_integer,
>> >   bool origin_upper_left)
>> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> b/src/mesa/drivers/dri/i965/brw_fs.h
>> > index 975183e..28fcfa3 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_fs.h
>> > +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>> > @@ -291,9 +291,6 @@ public:
>> >
>> > struct brw_reg interp_reg(int location, int channel);
>> >
>> > -   virtual void setup_vector_uniform_values(const gl_constant_value
> *values,
>> > -unsigned n);
>> > -
>> > int implied_mrf_writes(fs_inst *inst);
>> >
>> > virtual void dump_instructions();
>> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> > index ce4153d..c8ea649 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> > @@ -244,10 +244,12 @@ 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 =
>> >  BRW_IMAGE_PARAM_SIZE * MAX2(storage->array_elements, 1);
>> >
>> > - setup_image_uniform_values(storage);
>> > + setup_image_uniform_values(stage_prog_data->param + 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 6b92806..b31ae9b 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
>> > +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
>> > @@ -1419,8 +1419,22 @@
> backend_shader::assign_common_binding_table_offsets(uint32_t
> next_binding_table_
>> > /* prog_data->base.binding_table.size will be set by
> brw_mark_surface_used. */
>> >  }
>> >
>> > +static void
>> > +setup_vector_uniform_values(const gl_constant_value ***dst,
>> > +const gl_constant_value *values, unsigned
> n)
>> > +{
>> > +   static const gl_constant_value zero = { 0 };
>> > +
>> > +   for (unsigned i = 0; i < n; ++i)
>> > +  *((*dst)++) = &values[i];
>> > +
>> > +   for (unsigned i = n; i < 4; ++i)
>> > +  *((*dst)++) = &zero;
>> > +}
>> > +
>> >  void
>> > -backend_shader::setup_image_uniform_values(const gl_uniform_storage
> *storage)
>> > +backend_shader::setup_image_uniform_values(const gl_constant_value
> **prog_param,
>> > +   const gl_uniform_storage
>

Re: [Mesa-dev] [PATCH v2] i965: Refactor image uniform setup

2015-08-19 Thread Jason Ekstrand

On Aug 19, 2015 6:45 AM, "Francisco Jerez"  wrote:
>
> Jason Ekstrand  writes:
>
> > On Aug 19, 2015 2:56 AM, "Francisco Jerez" 
wrote:
> >>
> >> Jason Ekstrand  writes:
> >>
> >> > Previously, setting up image uniforms relied on being called after
> >> > fs_visitor::uniforms was set and with fs_visitor::uniforms not
> > allocating
> >> > space for it.  This made sense in an ir_visitor world because the
> > visitor
> >> > assigns locations and uploads data as it walks through the variables.
> > In
> >> > NIR it also happened to work because nir_lower_io assumed zero space
for
> >> > images.  In the near future, we will be able to reserve space using
> >> > nir_lower_io and these invariants will be broken.
> >> >
> >> > This commit makes setup_image_uniforms take a pointer to the
location in
> >> > the prog_data params array that indicates where the image uniforms
> > should
> >> > be stored.  This way it can be called from anywhere in the shader
setup.
> >> >
> >> > v2: Fix nir_setup_uniform to correctly correspond with this change
> >> > (rebase fail)
> >> >
> >> > Cc: Francisco Jerez 
> >> > Cc: Kenneth Graunke 
> >> > ---
> >> >  src/mesa/drivers/dri/i965/brw_fs.cpp | 12 
> >> >  src/mesa/drivers/dri/i965/brw_fs.h   |  3 ---
> >> >  src/mesa/drivers/dri/i965/brw_fs_nir.cpp |  6 --
> >> >  src/mesa/drivers/dri/i965/brw_shader.cpp | 28
> > +---
> >> >  src/mesa/drivers/dri/i965/brw_shader.h   |  5 ++---
> >> >  5 files changed, 27 insertions(+), 27 deletions(-)
> >> >
> >> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> > b/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> > index 82cb499..d1550e6 100644
> >> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> > @@ -942,18 +942,6 @@ fs_visitor::import_uniforms(fs_visitor *v)
> >> > this->param_size = v->param_size;
> >> >  }
> >> >
> >> > -void
> >> > -fs_visitor::setup_vector_uniform_values(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];
> >> > -
> >> > -   for (unsigned i = n; i < 4; ++i)
> >> > -  stage_prog_data->param[uniforms++] = &zero;
> >> > -}
> >> > -
> >> >  fs_reg *
> >> >  fs_visitor::emit_fragcoord_interpolation(bool pixel_center_integer,
> >> >   bool origin_upper_left)
> >> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> > b/src/mesa/drivers/dri/i965/brw_fs.h
> >> > index 975183e..28fcfa3 100644
> >> > --- a/src/mesa/drivers/dri/i965/brw_fs.h
> >> > +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> >> > @@ -291,9 +291,6 @@ public:
> >> >
> >> > struct brw_reg interp_reg(int location, int channel);
> >> >
> >> > -   virtual void setup_vector_uniform_values(const gl_constant_value
> > *values,
> >> > -unsigned n);
> >> > -
> >> > int implied_mrf_writes(fs_inst *inst);
> >> >
> >> > virtual void dump_instructions();
> >> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> >> > index ce4153d..c8ea649 100644
> >> > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> >> > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> >> > @@ -244,10 +244,12 @@ 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 =
> >> >  BRW_IMAGE_PARAM_SIZE * MAX2(storage->array_elements, 1);
> >> >
> >> > - setup_image_uniform_values(storage);
> >> > + setup_image_uniform_values(stage_prog_data->param +
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 6b92806..b31ae9b 100644
> >> > --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> >> > +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> >> > @@ -1419,8 +1419,22 @@
> > backend_shader::assign_common_binding_table_offsets(uint32_t
> > next_binding_table_
> >> > /* prog_data->base.binding_table.size will be set by
> > brw_mark_surface_used. */
> >> >  }
> >> >
> >> > +static void
> >> > +setup_vector_uniform_values(const gl_constant_value ***dst,
> >> > +const gl_constant_value *values,
unsigned
> > n)
> >> > +{
> >> > +   static const gl_constant_value zero = { 0 };
> >> > +
> >> > +   for (unsigned i = 0; i < n; ++i)
> >> > +  *((*dst)++) = &values[i];
> >> > +
> >> > +   for (unsigned i = n; i < 4; ++i)
> >> > +  *

Re: [Mesa-dev] [PATCH v3 3/5] nir: convert the glsl intrinsic image_size to nir_intrinsic_image_size

2015-08-19 Thread Martin Peres


On 19/08/15 16:47, Francisco Jerez wrote:

Martin Peres  writes:


v2, review from Francisco Jerez:
  - make the destination variable as large as what the nir instrinsic
defines (4) instead of the size of the return variable of glsl. This
is still safe for the already existing code because all the intrinsics
affected returned the same amount of components as expected by glsl IR.
In the case of image_size, it is not possible to do so because the
returned number of component depends on the image type and this case
is not well handled by nir.

Signed-off-by: Martin Peres 
---
  src/glsl/nir/glsl_to_nir.cpp  | 21 +++--
  src/glsl/nir/nir_intrinsics.h |  2 ++
  2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
index 77327b6..3063243 100644
--- a/src/glsl/nir/glsl_to_nir.cpp
+++ b/src/glsl/nir/glsl_to_nir.cpp
@@ -641,6 +641,8 @@ nir_visitor::visit(ir_call *ir)
   op = nir_intrinsic_image_atomic_comp_swap;
} else if (strcmp(ir->callee_name(), "__intrinsic_memory_barrier") == 
0) {
   op = nir_intrinsic_memory_barrier;
+  } else if (strcmp(ir->callee_name(), "__intrinsic_image_size") == 0) {
+ op = nir_intrinsic_image_size;
} else {
   unreachable("not reached");
}
@@ -666,7 +668,8 @@ nir_visitor::visit(ir_call *ir)
case nir_intrinsic_image_atomic_or:
case nir_intrinsic_image_atomic_xor:
case nir_intrinsic_image_atomic_exchange:
-  case nir_intrinsic_image_atomic_comp_swap: {
+  case nir_intrinsic_image_atomic_comp_swap:
+  case nir_intrinsic_image_size: {
   nir_ssa_undef_instr *instr_undef =
  nir_ssa_undef_instr_create(shader, 1);
   nir_instr_insert_after_cf_list(this->cf_node_list,
@@ -681,6 +684,17 @@ nir_visitor::visit(ir_call *ir)
   instr->variables[0] = evaluate_deref(&instr->instr, image);
   param = param->get_next();
  
+ /* Set the intrinsic destination. */

+ if (ir->return_deref) {
+const nir_intrinsic_info *info;
+info = &nir_intrinsic_infos[instr->intrinsic];

I'm a fan of initializing things at the same point where they are
defined.  You only use it once though, you may want to drop the
declaration altogether.


It exceeded the 80-char limit. I guess I can put the assignation on the 
following line.





+nir_ssa_dest_init(&instr->instr, &instr->dest,
+  info->dest_components, NULL);
+ }
+
+ if (op == nir_intrinsic_image_size)
+break;
+
   /* Set the address argument, extending the coordinate vector to four
* components.
*/
@@ -721,11 +735,6 @@ nir_visitor::visit(ir_call *ir)
  instr->src[3] = evaluate_rvalue((ir_dereference *)param);
  param = param->get_next();
   }
-
- /* Set the intrinsic destination. */
- if (ir->return_deref)
-nir_ssa_dest_init(&instr->instr, &instr->dest,
-  ir->return_deref->type->vector_elements, NULL);
   break;
}
case nir_intrinsic_memory_barrier:
diff --git a/src/glsl/nir/nir_intrinsics.h b/src/glsl/nir/nir_intrinsics.h
index bc6e6b8..6c7a61a 100644
--- a/src/glsl/nir/nir_intrinsics.h
+++ b/src/glsl/nir/nir_intrinsics.h
@@ -123,6 +123,8 @@ INTRINSIC(image_atomic_or, 3, ARR(4, 1, 1), true, 1, 1, 0, 
0)
  INTRINSIC(image_atomic_xor, 3, ARR(4, 1, 1), true, 1, 1, 0, 0)
  INTRINSIC(image_atomic_exchange, 3, ARR(4, 1, 1), true, 1, 1, 0, 0)
  INTRINSIC(image_atomic_comp_swap, 4, ARR(4, 1, 1, 1), true, 1, 1, 0, 0)
+INTRINSIC(image_size, 0, ARR(), true, 4, 1, 0,
+  NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
  

Looks good to me,
Reviewed-by: Francisco Jerez 


Thanks for the review, I will address the comments on the previous patch!




  #define SYSTEM_VALUE(name, components) \
 INTRINSIC(load_##name, 0, ARR(), true, components, 0, 0, \
--
2.5.0

___
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 v3 3/5] nir: convert the glsl intrinsic image_size to nir_intrinsic_image_size

Martin Peres  writes:

> v2, review from Francisco Jerez:
>  - make the destination variable as large as what the nir instrinsic
>defines (4) instead of the size of the return variable of glsl. This
>is still safe for the already existing code because all the intrinsics
>affected returned the same amount of components as expected by glsl IR.
>In the case of image_size, it is not possible to do so because the
>returned number of component depends on the image type and this case
>is not well handled by nir.
>
> Signed-off-by: Martin Peres 
> ---
>  src/glsl/nir/glsl_to_nir.cpp  | 21 +++--
>  src/glsl/nir/nir_intrinsics.h |  2 ++
>  2 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
> index 77327b6..3063243 100644
> --- a/src/glsl/nir/glsl_to_nir.cpp
> +++ b/src/glsl/nir/glsl_to_nir.cpp
> @@ -641,6 +641,8 @@ nir_visitor::visit(ir_call *ir)
>   op = nir_intrinsic_image_atomic_comp_swap;
>} else if (strcmp(ir->callee_name(), "__intrinsic_memory_barrier") == 
> 0) {
>   op = nir_intrinsic_memory_barrier;
> +  } else if (strcmp(ir->callee_name(), "__intrinsic_image_size") == 0) {
> + op = nir_intrinsic_image_size;
>} else {
>   unreachable("not reached");
>}
> @@ -666,7 +668,8 @@ nir_visitor::visit(ir_call *ir)
>case nir_intrinsic_image_atomic_or:
>case nir_intrinsic_image_atomic_xor:
>case nir_intrinsic_image_atomic_exchange:
> -  case nir_intrinsic_image_atomic_comp_swap: {
> +  case nir_intrinsic_image_atomic_comp_swap:
> +  case nir_intrinsic_image_size: {
>   nir_ssa_undef_instr *instr_undef =
>  nir_ssa_undef_instr_create(shader, 1);
>   nir_instr_insert_after_cf_list(this->cf_node_list,
> @@ -681,6 +684,17 @@ nir_visitor::visit(ir_call *ir)
>   instr->variables[0] = evaluate_deref(&instr->instr, image);
>   param = param->get_next();
>  
> + /* Set the intrinsic destination. */
> + if (ir->return_deref) {
> +const nir_intrinsic_info *info;
> +info = &nir_intrinsic_infos[instr->intrinsic];

I'm a fan of initializing things at the same point where they are
defined.  You only use it once though, you may want to drop the
declaration altogether.

> +nir_ssa_dest_init(&instr->instr, &instr->dest,
> +  info->dest_components, NULL);
> + }
> +
> + if (op == nir_intrinsic_image_size)
> +break;
> +
>   /* Set the address argument, extending the coordinate vector to four
>* components.
>*/
> @@ -721,11 +735,6 @@ nir_visitor::visit(ir_call *ir)
>  instr->src[3] = evaluate_rvalue((ir_dereference *)param);
>  param = param->get_next();
>   }
> -
> - /* Set the intrinsic destination. */
> - if (ir->return_deref)
> -nir_ssa_dest_init(&instr->instr, &instr->dest,
> -  ir->return_deref->type->vector_elements, NULL);
>   break;
>}
>case nir_intrinsic_memory_barrier:
> diff --git a/src/glsl/nir/nir_intrinsics.h b/src/glsl/nir/nir_intrinsics.h
> index bc6e6b8..6c7a61a 100644
> --- a/src/glsl/nir/nir_intrinsics.h
> +++ b/src/glsl/nir/nir_intrinsics.h
> @@ -123,6 +123,8 @@ INTRINSIC(image_atomic_or, 3, ARR(4, 1, 1), true, 1, 1, 
> 0, 0)
>  INTRINSIC(image_atomic_xor, 3, ARR(4, 1, 1), true, 1, 1, 0, 0)
>  INTRINSIC(image_atomic_exchange, 3, ARR(4, 1, 1), true, 1, 1, 0, 0)
>  INTRINSIC(image_atomic_comp_swap, 4, ARR(4, 1, 1, 1), true, 1, 1, 0, 0)
> +INTRINSIC(image_size, 0, ARR(), true, 4, 1, 0,
> +  NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
>  

Looks good to me,
Reviewed-by: Francisco Jerez 

>  #define SYSTEM_VALUE(name, components) \
> INTRINSIC(load_##name, 0, ARR(), true, components, 0, 0, \
> -- 
> 2.5.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

Re: [Mesa-dev] [PATCH v3 2/5] glsl: add support for the imageSize builtin

2015-08-19 Thread Martin Peres


On 19/08/15 16:36, Francisco Jerez wrote:

Martin Peres  writes:


The code is heavily inspired from Francisco Jerez's code supporting the
image_load_store extension.

Backends willing to support this builtin should handle
__intrinsic_image_size.

v2: Based on the review of Ilia Mirkin
- Enable the extension for GLES 3.1
- Fix indentation
- Fix the return type (float to int, number of components for CubeImages)
- Add a warning related to GLES 3.1

v3: Based on the review of Francisco Jerez
- Refactor the code to share both add_image_function and _image with the other
   image-related functions

Signed-off-by: Martin Peres 
---
  src/glsl/builtin_functions.cpp | 109 +++--
  1 file changed, 93 insertions(+), 16 deletions(-)

diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
index 2175c66..5d0a825 100644
--- a/src/glsl/builtin_functions.cpp
+++ b/src/glsl/builtin_functions.cpp
@@ -399,6 +399,13 @@ shader_image_load_store(const _mesa_glsl_parse_state 
*state)
  }
  
  static bool

+shader_image_size(const _mesa_glsl_parse_state *state)
+{
+   return (state->is_version(430, 310) ||
+   state->ARB_shader_image_size_enable);
+}
+
+static bool
  gs_streams(const _mesa_glsl_parse_state *state)
  {
 return gpu_shader5(state) && gs_only(state);
@@ -492,6 +499,11 @@ private:
 /** Create a new function and add the given signatures. */
 void add_function(const char *name, ...);
  
+   typedef ir_function_signature *(builtin_builder::*image_prototype_ctr)(const glsl_type *image_type,

+  
const char *intrinsic_name,
+  
unsigned num_arguments,
+  
unsigned flags);
+
 enum image_function_flags {
IMAGE_FUNCTION_EMIT_STUB = (1 << 0),
IMAGE_FUNCTION_RETURNS_VOID = (1 << 1),
@@ -507,6 +519,7 @@ private:
  */
 void add_image_function(const char *name,
 const char *intrinsic_name,
+   image_prototype_ctr prototype,
 unsigned num_arguments,
 unsigned flags);
  
@@ -708,7 +721,12 @@ private:

 const char *intrinsic_name,
 unsigned num_arguments,
 unsigned flags);
-   ir_function_signature *_image(const glsl_type *image_type,
+   ir_function_signature *_image_size_prototype(const glsl_type *image_type,
+   const char *intrinsic_name,
+   unsigned num_arguments,
+   unsigned flags);

The indentation looks weird.  Mixing tabs and spaces?


No, I just forgot to re-align after changing the name. Sorry. It is fixed.



+   ir_function_signature *_image(image_prototype_ctr prototype,
+ const glsl_type *image_type,
   const char *intrinsic_name,
   unsigned num_arguments,
   unsigned flags);
@@ -2552,6 +2570,7 @@ builtin_builder::add_function(const char *name, ...)
  void
  builtin_builder::add_image_function(const char *name,
  const char *intrinsic_name,
+image_prototype_ctr prototype,
  unsigned num_arguments,
  unsigned flags)
  {
@@ -2590,12 +2609,13 @@ builtin_builder::add_image_function(const char *name,
glsl_type::uimage2DMS_type,
glsl_type::uimage2DMSArray_type
 };
+
 ir_function *f = new(mem_ctx) ir_function(name);
  
 for (unsigned i = 0; i < ARRAY_SIZE(types); ++i) {

if (types[i]->sampler_type != GLSL_TYPE_FLOAT ||
(flags & IMAGE_FUNCTION_SUPPORTS_FLOAT_DATA_TYPE))
- f->add_signature(_image(types[i], intrinsic_name,
+ f->add_signature(_image(prototype, types[i], intrinsic_name,
   num_arguments, flags));
 }
  
@@ -2608,43 +2628,57 @@ builtin_builder::add_image_functions(bool glsl)

 const unsigned flags = (glsl ? IMAGE_FUNCTION_EMIT_STUB : 0);
  
 add_image_function(glsl ? "imageLoad" : "__intrinsic_image_load",

-  "__intrinsic_image_load", 0,
-  (flags | IMAGE_FUNCTION_HAS_VECTOR_DATA_TYPE |
+   "__intrinsic_image_load",
+   &builtin_builder::_image_prototype, 0,
+   (flags | IMAGE_FUNCTION_HAS_VECTOR_DATA_TYPE |
 IMAGE_FUNCTION_SUPPORTS_FLOAT_DATA_TYPE |
 IMAGE_FUNCTION_READ_ONLY));
  
 add_image_function(glsl ? "imageStore"

Re: [Mesa-dev] [PATCH v3 4/5] i965: handle nir_intrinsic_image_size

Martin Peres  writes:

> v2, Review from Francisco Jerez:
> - avoid the camelCase for the booleans
> - init the booleans using the sampler type
> - force the initialization of all the components of the output register
>
> Signed-off-by: Martin Peres 
> ---
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 48 
> 
>  1 file changed, 48 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index ce4153d..cc0a5a6 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -1406,6 +1406,54 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, 
> nir_intrinsic_instr *instr
>break;
> }
>  
> +   case nir_intrinsic_image_size: {
> +  /* Get the referenced image variable and type. */
> +  const nir_variable *var = instr->variables[0]->var;
> +  const glsl_type *type = var->type->without_array();
> +  const brw_reg_type base_type = get_image_base_type(type);
> +
> +  /* Get the size of the image. */
> +  const fs_reg image = get_nir_image_deref(instr->variables[0]);
> +  const fs_reg size = offset(image, bld, BRW_IMAGE_PARAM_SIZE_OFFSET);
> +
> +  /*
> +   * For 1DArray image types, the array index is stored in the Z 
> component.
> +   * Fix this by swizzling the Z component to the Y component.
> +   */
> +  const bool is_1d_array_image =
> +  (type->sampler_dimensionality == GLSL_SAMPLER_DIM_1D &&
> +   type->sampler_array);
> +
> +  /*
> +   * For CubeMapArray images, we should count the number of cubes instead
> +   * of the number of faces. Fix it by dividing the (Z component) by 6.
> +   */
> +  const bool is_cube_map_array_image =
> +  (type->sampler_dimensionality == GLSL_SAMPLER_DIM_CUBE &&
> +   type->sampler_array);
> +
> +  /* Copy all the components. */
> +  const nir_intrinsic_info *info = 
> &nir_intrinsic_infos[instr->intrinsic];
> +  for (int c = 0; c < info->dest_components; ++c) {
> + if (c > type->coordinate_components()) {

Off by one, "c > type->coordinate_components()" is what you probably
meant to avoid reading past the end of the size array.

> + bld.MOV(offset(retype(dest, base_type), bld, c),

base_type is the base data type of the image, i.e. float for
floating-point or fixed-point normalized image formats.  imageSize
always returns integer results regardless of the image data type, so
BRW_REGISTER_TYPE_D is what you want (feel free to factor out the
offset(retype(dest...)...) expression to a local variable if it gets too
long).  I guess you were only testing imageSize with integer formats?

With these and the comments from other people addressed this patch is:

Reviewed-by: Francisco Jerez 


> + fs_reg(1));
> + } else if (c == 1 && is_1d_array_image) {
> +bld.MOV(offset(retype(dest, base_type), bld, c),
> +offset(size, bld, 2));
> + } else if (c == 2 && is_cube_map_array_image) {
> +bld.emit(SHADER_OPCODE_INT_QUOTIENT,
> + offset(retype(dest, base_type), bld, c),
> + offset(size, bld, c), fs_reg(6));
> + } else {
> +bld.MOV(offset(retype(dest, base_type), bld, c),
> +offset(size, bld, c));
> + }
> +   }
> +
> +  break;
> +   }
> +
> case nir_intrinsic_load_front_face:
>bld.MOV(retype(dest, BRW_REGISTER_TYPE_D),
>*emit_frontfacing_interpolation());
> -- 
> 2.5.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

Re: [Mesa-dev] [PATCH 5/5] i965: enable GL_ARB_shader_image_size

Martin Peres  writes:

> Signed-off-by: Martin Peres 
> ---
>  docs/GL3.txt | 4 ++--
>  docs/relnotes/11.0.0.html| 1 +
>  src/mesa/drivers/dri/i965/intel_extensions.c | 1 +
>  3 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/docs/GL3.txt b/docs/GL3.txt
> index 54c0c5a..5319e06 100644
> --- a/docs/GL3.txt
> +++ b/docs/GL3.txt
> @@ -163,7 +163,7 @@ GL 4.3, GLSL 4.30:
>GL_ARB_multi_draw_indirect   DONE (i965, nvc0, 
> r600, radeonsi, llvmpipe, softpipe)
>GL_ARB_program_interface_query   DONE (all drivers)
>GL_ARB_robust_buffer_access_behavior not started
> -  GL_ARB_shader_image_size in progress (Martin 
> Peres)
> +  GL_ARB_shader_image_size DONE (i965)
>GL_ARB_shader_storage_buffer_object  in progress (Iago 
> Toral, Samuel Iglesias)
>GL_ARB_stencil_texturing DONE (i965/gen8+, 
> nv50, nvc0, r600, radeonsi, llvmpipe, softpipe)
>GL_ARB_texture_buffer_range  DONE (nv50, nvc0, 
> i965, r600, radeonsi, llvmpipe)
> @@ -211,7 +211,7 @@ GLES3.1, GLSL ES 3.1
>GL_ARB_program_interface_query   DONE (all drivers)
>GL_ARB_shader_atomic_countersDONE (i965)
>GL_ARB_shader_image_load_store   in progress (curro)
> -  GL_ARB_shader_image_size in progress (Martin 
> Peres)
> +  GL_ARB_shader_image_size DONE (i965)
>GL_ARB_shader_storage_buffer_object  in progress (Iago 
> Toral, Samuel Iglesias)
>GL_ARB_shading_language_packing  DONE (all drivers)
>GL_ARB_separate_shader_objects   DONE (all drivers)
> diff --git a/docs/relnotes/11.0.0.html b/docs/relnotes/11.0.0.html
> index 2d80198..45a1689 100644
> --- a/docs/relnotes/11.0.0.html
> +++ b/docs/relnotes/11.0.0.html
> @@ -53,6 +53,7 @@ Note: some of the new features are only available with 
> certain drivers.
>  GL_ARB_gpu_shader5 on radeonsi
>  GL_ARB_gpu_shader_fp64 on llvmpipe, radeonsi
>  GL_ARB_shader_image_load_store on i965
> +GL_ARB_shader_image_size on i965
>  GL_ARB_shader_stencil_export on llvmpipe
>  GL_ARB_shader_subroutine on core profile all drivers
>  GL_ARB_tessellation_shader on nvc0, radeonsi
> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c 
> b/src/mesa/drivers/dri/i965/intel_extensions.c
> index 0da528b..4365b71 100644
> --- a/src/mesa/drivers/dri/i965/intel_extensions.c
> +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
> @@ -326,6 +326,7 @@ intelInitExtensions(struct gl_context *ctx)
>ctx->Extensions.ARB_gpu_shader5 = true;
>ctx->Extensions.ARB_shader_atomic_counters = true;
>ctx->Extensions.ARB_shader_image_load_store = true;
> +  ctx->Extensions.ARB_shader_image_size = true;
>ctx->Extensions.ARB_texture_compression_bptc = true;
>ctx->Extensions.ARB_texture_view = true;
>  
> -- 
> 2.5.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Looks good to me,
Reviewed-by: Francisco Jerez 


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

Re: [Mesa-dev] [PATCH 01/10] i965: Use constant miptree pointer in tex surface setup

Topi Pohjolainen  writes:

> as it is only used for reading.
>
> Signed-off-by: Topi Pohjolainen 
> ---
>  src/mesa/drivers/dri/i965/brw_context.h   | 2 +-
>  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 2 +-
>  src/mesa/drivers/dri/i965/gen8_surface_state.c| 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 3553f6e..ae29798 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -990,7 +990,7 @@ struct brw_context
>uint32_t surf_index);
>  
>void (*emit_texture_surface_state)(struct brw_context *brw,
> - struct intel_mipmap_tree *mt,
> + const struct intel_mipmap_tree *mt,
>   GLenum target,
>   unsigned min_layer,
>   unsigned max_layer,
> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c 
> b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> index 15ab2b0..494bc22 100644
> --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> @@ -265,7 +265,7 @@ gen7_emit_buffer_surface_state(struct brw_context *brw,
>  
>  static void
>  gen7_emit_texture_surface_state(struct brw_context *brw,
> -struct intel_mipmap_tree *mt,
> +const struct intel_mipmap_tree *mt,
>  GLenum target,
>  unsigned min_layer, unsigned max_layer,
>  unsigned min_level, unsigned max_level,
> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c 
> b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> index b2d1a57..c595ec3 100644
> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> @@ -184,7 +184,7 @@ gen8_emit_buffer_surface_state(struct brw_context *brw,
>  
>  static void
>  gen8_emit_texture_surface_state(struct brw_context *brw,
> -struct intel_mipmap_tree *mt,
> +const struct intel_mipmap_tree *mt,
>  GLenum target,
>  unsigned min_layer, unsigned max_layer,
>  unsigned min_level, unsigned max_level,
> -- 
> 1.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reviewed-by: Francisco Jerez 



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

Re: [Mesa-dev] [PATCH 02/10] i965: Reduce the scope of input in buffer tex setup

Topi Pohjolainen  writes:

> Signed-off-by: Topi Pohjolainen 

Reviewed-by: Francisco Jerez 

> ---
>  src/mesa/drivers/dri/i965/brw_context.h   | 4 ++--
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  | 8 +++-
>  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 2 +-
>  src/mesa/drivers/dri/i965/gen8_surface_state.c| 2 +-
>  4 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> b/src/mesa/drivers/dri/i965/brw_context.h
> index ae29798..da018bf 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1710,8 +1710,8 @@ void brw_create_constant_surface(struct brw_context 
> *brw,
>   uint32_t size,
>   uint32_t *out_offset,
>   bool dword_pitch);
> -void brw_update_buffer_texture_surface(struct gl_context *ctx,
> -   unsigned unit,
> +void brw_update_buffer_texture_surface(struct brw_context *brw,
> +   struct gl_texture_object *tObj,
> uint32_t *surf_offset);
>  void
>  brw_update_sol_surface(struct brw_context *brw,
> 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 72aad96..73aa719 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -276,12 +276,10 @@ gen4_emit_buffer_surface_state(struct brw_context *brw,
>  }
>  
>  void
> -brw_update_buffer_texture_surface(struct gl_context *ctx,
> -  unsigned unit,
> +brw_update_buffer_texture_surface(struct brw_context *brw,
> +  struct gl_texture_object *tObj,
>uint32_t *surf_offset)
>  {
> -   struct brw_context *brw = brw_context(ctx);
> -   struct gl_texture_object *tObj = ctx->Texture.Unit[unit]._Current;
> struct intel_buffer_object *intel_obj =
>intel_buffer_object(tObj->BufferObject);
> uint32_t size = tObj->BufferSize;
> @@ -323,7 +321,7 @@ brw_update_texture_surface(struct gl_context *ctx,
>  
> /* BRW_NEW_TEXTURE_BUFFER */
> if (tObj->Target == GL_TEXTURE_BUFFER) {
> -  brw_update_buffer_texture_surface(ctx, unit, surf_offset);
> +  brw_update_buffer_texture_surface(brw, tObj, surf_offset);
>return;
> }
>  
> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c 
> b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> index 494bc22..6aa8299 100644
> --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> @@ -357,7 +357,7 @@ gen7_update_texture_surface(struct gl_context *ctx,
> struct gl_texture_object *obj = ctx->Texture.Unit[unit]._Current;
>  
> if (obj->Target == GL_TEXTURE_BUFFER) {
> -  brw_update_buffer_texture_surface(ctx, unit, surf_offset);
> +  brw_update_buffer_texture_surface(brw, obj, surf_offset);
>  
> } else {
>struct intel_texture_object *intel_obj = intel_texture_object(obj);
> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c 
> b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> index c595ec3..11defd1 100644
> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> @@ -308,7 +308,7 @@ gen8_update_texture_surface(struct gl_context *ctx,
> struct gl_texture_object *obj = ctx->Texture.Unit[unit]._Current;
>  
> if (obj->Target == GL_TEXTURE_BUFFER) {
> -  brw_update_buffer_texture_surface(ctx, unit, surf_offset);
> +  brw_update_buffer_texture_surface(brw, obj, surf_offset);
>  
> } else {
>struct gl_texture_image *firstImage = obj->Image[0][obj->BaseLevel];
> -- 
> 1.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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 91687] Crash when creating new context after destroying the old one using indirect rendering

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

--- Comment #1 from Guilherme  ---
Created attachment 117784
  --> https://bugs.freedesktop.org/attachment.cgi?id=117784&action=edit
gdb stack trace

-- 
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 03/10] i965: Move tex buffer dispatch into hw independent setup

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"?).

> + 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.
| }

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


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 91687] Crash when creating new context after destroying the old one using indirect rendering

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

--- Comment #3 from Guilherme  ---
As the crash is a seg fault on update_framebuffer_size because surface=NULL, I
tried to just make a check if the surface is NULL. Then it does not crash
anywhere else, but nothing is rendered.

-- 
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] [Bug 91687] Crash when creating new context after destroying the old one using indirect rendering

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

--- Comment #2 from Guilherme  ---
Created attachment 117785
  --> https://bugs.freedesktop.org/attachment.cgi?id=117785&action=edit
xtrace with glx commands

-- 
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] [Bug 91687] Crash when creating new context after destroying the old one using indirect rendering

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

Bug ID: 91687
Summary: Crash when creating new context after destroying the
old one using indirect rendering
Product: Mesa
Version: 10.5
Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
Severity: critical
Priority: medium
Component: Mesa core
Assignee: mesa-dev@lists.freedesktop.org
Reporter: gqm...@gmail.com
QA Contact: mesa-dev@lists.freedesktop.org

Created attachment 117783
--> https://bugs.freedesktop.org/attachment.cgi?id=117783&action=edit
VTK/Qt Python program that crashes

I tested that on mesa Ubuntu 14.04 (mesa 10.3.2) and 15.10 (mesa 10.5.2). I'm
not sure if it is reproducible. The attached traces were gotten on Ubuntu
14.04.

The crash happens when running the attached python program crash_mesa_vtk_qt.py
using indirect rendering (specifically I need to run it inside a chroot that
has a very old mesa version)

It uses vtk and Qt, I'm sorry I could not come up with a more isolated example,
but I'm not very familiar with opengl programming.

In summary, that is what is happening:

1 - A vtk render window is created and rendered the first time (that makes vtk
create a new glx context)

2 - The vtk render window is set to render offscreen (it is not using OSMesa).
That makes vtk destroy the current context and create a new one associated to a
new X offscreen window.

3 - The window is rendered and I could even generate a png file.

4 - The vtk render window is set to render on screen again. That makes vtk
destroy the current glx context and create a new one associated to the first X
window (the on screen window).

5 - When rendering the vtk window again, X server crashes (stack trace
attached).

Looking into vtk code it does not seem it is doing anything wrong. The
offscreen related code can be found on:

https://gitlab.kitware.com/vtk/vtk/blob/v6.2.0/Rendering/OpenGL/vtkXOpenGLRenderWindow.cxx#L800

https://gitlab.kitware.com/vtk/vtk/blob/v6.2.0/Rendering/OpenGL/vtkOpenGLRenderWindow.cxx#L1856

https://gitlab.kitware.com/vtk/vtk/blob/v6.2.0/Rendering/OpenGL/vtkXOpenGLRenderWindow.cxx#L943

https://gitlab.kitware.com/vtk/vtk/blob/v6.2.0/Rendering/OpenGL/vtkOpenGLRenderWindow.cxx#L1856

--
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 04/10] i965/gen4: Use tex object format instead of the miptree format

Topi Pohjolainen  writes:

> Equivalent logic for newer generations (>= 7) use the tex object
> format instead. This patch prepares for merging the decision
> making for all generations.
>
> Signed-off-by: Topi Pohjolainen 

Reviewed-by: Francisco Jerez 

> ---
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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 dca67e8..26e9122 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -322,7 +322,7 @@ brw_update_texture_surface(struct gl_context *ctx,
> surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
> 6 * 4, 32, surf_offset);
>  
> -   uint32_t tex_format = translate_tex_format(brw, mt->format,
> +   uint32_t tex_format = translate_tex_format(brw, intelObj->_Format,
>sampler->sRGBDecode);
>  
> if (for_gather) {
> -- 
> 1.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

Re: [Mesa-dev] [PATCH 05/10] i965: Move tex miptree and format resolving into dispatcher

Topi Pohjolainen  writes:

> All hardware platforms have this in common, so do it in the
> hardware independent dispatcher.
>
> v2 (Matt): Removed extra whitespace.
> v3: Non-trivial rebase
>
> Reviewed-by: Matt Turner  (v1)
> Reviewed-by: Kenneth Graunke  (v1)
> Signed-off-by: Topi Pohjolainen 

I had already reviewed this patch, not sure why you dropped it, anyway:
Reviewed-by: Francisco Jerez 

> ---
>  src/mesa/drivers/dri/i965/brw_context.h   |  4 +++-
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  | 26 
> ---
>  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 10 +++--
>  src/mesa/drivers/dri/i965/gen8_surface_state.c| 14 +++-
>  4 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> b/src/mesa/drivers/dri/i965/brw_context.h
> index da018bf..ef9bb87 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -981,7 +981,9 @@ struct brw_context
> struct
> {
>void (*update_texture_surface)(struct gl_context *ctx,
> - unsigned unit,
> + const struct intel_mipmap_tree *mt,
> + struct gl_texture_object *tex_obj,
> + uint32_t tex_format,
>   uint32_t *surf_offset,
>   bool for_gather);
>uint32_t (*update_renderbuffer_surface)(struct brw_context *brw,
> 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 26e9122..c14f00a 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -308,23 +308,19 @@ brw_update_buffer_texture_surface(struct brw_context 
> *brw,
>  
>  static void
>  brw_update_texture_surface(struct gl_context *ctx,
> -   unsigned unit,
> +   const struct intel_mipmap_tree *mt,
> +   struct gl_texture_object *tObj,
> +   uint32_t tex_format,
> uint32_t *surf_offset,
> bool for_gather)
>  {
> struct brw_context *brw = brw_context(ctx);
> -   struct gl_texture_object *tObj = ctx->Texture.Unit[unit]._Current;
> struct intel_texture_object *intelObj = intel_texture_object(tObj);
> -   struct intel_mipmap_tree *mt = intelObj->mt;
> -   struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
> uint32_t *surf;
>  
> surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
> 6 * 4, 32, surf_offset);
>  
> -   uint32_t tex_format = translate_tex_format(brw, intelObj->_Format,
> -  sampler->sRGBDecode);
> -
> if (for_gather) {
>/* Sandybridge's gather4 message is broken for integer formats.
> * To work around this, we pretend the surface is UNORM for
> @@ -828,8 +824,22 @@ update_stage_texture_surfaces(struct brw_context *brw,
>   continue;
>}
>  
> +  const struct gl_texture_image *first_img = 
> tex->Image[0][tex->BaseLevel];
> +  const struct intel_texture_object *itex = intel_texture_object(tex);
> +  const struct intel_mipmap_tree *mt = itex->mt;
> +  const struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx,
> + unit);
> +  uint32_t format = translate_tex_format(brw, itex->_Format,
> + sampler->sRGBDecode);
> +
> +  if (tex->StencilSampling && first_img->_BaseFormat == 
> GL_DEPTH_STENCIL) {
> + mt = mt->stencil_mt;
> + format = BRW_SURFACEFORMAT_R8_UINT;
> + assert(brw->gen >= 8);
> +  }
> +
>/* _NEW_TEXTURE */
> -  brw->vtbl.update_texture_surface(ctx, unit,
> +  brw->vtbl.update_texture_surface(ctx, mt, tex, format,
> surf_offset + s, for_gather);
> }
>  }
> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c 
> b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> index 6aa8299..26d080f 100644
> --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> @@ -349,20 +349,19 @@ gen7_emit_texture_surface_state(struct brw_context *brw,
>  
>  static void
>  gen7_update_texture_surface(struct gl_context *ctx,
> -unsigned unit,
> +const struct intel_mipmap_tree *mt,
> +struct gl_texture_object *obj,
> +uint32_t format,
>  uint32_t *surf_offset,
>  bool for_gather)
>  {
> struct brw_context *brw = brw_context(ctx);
> -   struct gl_textu

Re: [Mesa-dev] [PATCH 06/10] i965/gen7: Move tex gather format override into common dispatcher

Topi Pohjolainen  writes:

> Signed-off-by: Topi Pohjolainen 

Reviewed-by: Francisco Jerez 

> ---
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 4 
>  1 file changed, 4 insertions(+)
>
> 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 c14f00a..b68b136 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -832,6 +832,10 @@ update_stage_texture_surfaces(struct brw_context *brw,
>uint32_t format = translate_tex_format(brw, itex->_Format,
>   sampler->sRGBDecode);
>  
> +  if (brw->gen == 7 && for_gather &&
> +  format == BRW_SURFACEFORMAT_R32G32_FLOAT)
> + format = BRW_SURFACEFORMAT_R32G32_FLOAT_LD;
> +
>if (tex->StencilSampling && first_img->_BaseFormat == 
> GL_DEPTH_STENCIL) {
>   mt = mt->stencil_mt;
>   format = BRW_SURFACEFORMAT_R8_UINT;
> -- 
> 1.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature
___
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

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.

> ---
>  src/mesa/drivers/dri/i965/brw_context.h   |  2 +-
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  | 14 --
>  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 13 +
>  src/mesa/drivers/dri/i965/gen8_surface_state.c| 13 +
>  4 files changed, 15 insertions(+), 27 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> b/src/mesa/drivers/dri/i965/brw_context.h
> index ef9bb87..061600f 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -983,7 +983,7 @@ struct brw_context
>void (*update_texture_surface)(struct gl_context *ctx,
>   const struct intel_mipmap_tree *mt,
>   struct gl_texture_object *tex_obj,
> - uint32_t tex_format,
> + uint32_t tex_format, int swizzle,
>   uint32_t *surf_offset,
>   bool for_gather);
>uint32_t (*update_renderbuffer_surface)(struct brw_context *brw,
> 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 b68b136..4afdc23 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -310,7 +310,7 @@ static void
>  brw_update_texture_surface(struct gl_context *ctx,
> const struct intel_mipmap_tree *mt,
> struct gl_texture_object *tObj,
> -   uint32_t tex_format,
> +   uint32_t tex_format, int swizzle /* unused */,
> uint32_t *surf_offset,
> bool for_gather)
>  {
> @@ -842,8 +842,18 @@ update_stage_texture_surfaces(struct brw_context *brw,
>   assert(brw->gen >= 8);
>}
>  
> +  /* Handling GL_ALPHA as a surface format override breaks 1.30+ style
> +   * texturing functions that return a float, as our code generation 
> always
> +   * selects the .x channel (which would always be 0).
> +   */
> +  const bool alpha_depth = tex->DepthMode == GL_ALPHA &&
> + (first_img->_BaseFormat == GL_DEPTH_COMPONENT ||
> +  first_img->_BaseFormat == GL_DEPTH_STENCIL);
> +  const unsigned swizzle = unlikely(alpha_depth) ?
> + SWIZZLE_XYZW : brw_get_texture_swizzle(ctx, tex);
> +
>/* _NEW_TEXTURE */
> -  brw->vtbl.update_texture_surface(ctx, mt, tex, format,
> +  brw->vtbl.update_texture_surface(ctx, mt, tex, format, swizzle,
> surf_offset + s, for_gather);
> }
>  }
> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c 
> b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> index 26d080f..0de43a4 100644
> --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> @@ -351,7 +351,7 @@ static void
>  gen7_update_texture_surface(struct gl_context *ctx,
>  const struct intel_mipmap_tree *mt,
>  struct gl_texture_object *obj,
> -uint32_t format,
> +uint32_t format, int swizzle,
>  uint32_t *surf_offset,
>  bool for_gather)
>  {
> @@ -368,17 +368,6 @@ gen7_update_texture_surface(struct gl_context *ctx,
>const unsigned depth = (obj->Immutable && obj->Target != GL_TEXTURE_3D 
> ?
>obj->NumLayers : mt->logical_depth0);
>  
> -  /* Handling GL_ALPHA as a surface format override breaks 1.30+ style
> -   * texturing functions that return a float, as our code generation 
> always
> -   * selects the .x channel (which would always be 0).
> -   */
> -  struct gl_texture_image *firstImage = obj->Image[0][obj->BaseLevel];
> -  const bool alpha_depth = obj->DepthMode == GL_ALPHA &&
> - (firstImage->_BaseFormat == GL_DEPTH_COMPONENT ||
> -  firstImage->_BaseFormat == GL_DEPTH_STENCIL);
> -  const unsigned swizzle = (unlikely(alpha_depth) ? SWIZZLE_XYZW :
> -brw_get_texture_swizzle(&brw->ctx, obj));
> -
>if (for_gather && format == BRW_SURFACEFORMAT_R32G32_FLOAT)
>   format = BRW_SURFACEFORMAT_R32G32_FLOAT_LD;
>  
> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c 
> b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> index b6766ef..c4eed08 100644
>

Re: [Mesa-dev] [PATCH v3 4/5] i965: handle nir_intrinsic_image_size

Francisco Jerez  writes:

> Martin Peres  writes:
>
>> v2, Review from Francisco Jerez:
>> - avoid the camelCase for the booleans
>> - init the booleans using the sampler type
>> - force the initialization of all the components of the output register
>>
>> Signed-off-by: Martin Peres 
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 48 
>> 
>>  1 file changed, 48 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
>> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> index ce4153d..cc0a5a6 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -1406,6 +1406,54 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, 
>> nir_intrinsic_instr *instr
>>break;
>> }
>>  
>> +   case nir_intrinsic_image_size: {
>> +  /* Get the referenced image variable and type. */
>> +  const nir_variable *var = instr->variables[0]->var;
>> +  const glsl_type *type = var->type->without_array();
>> +  const brw_reg_type base_type = get_image_base_type(type);
>> +
>> +  /* Get the size of the image. */
>> +  const fs_reg image = get_nir_image_deref(instr->variables[0]);
>> +  const fs_reg size = offset(image, bld, BRW_IMAGE_PARAM_SIZE_OFFSET);
>> +
>> +  /*
>> +   * For 1DArray image types, the array index is stored in the Z 
>> component.
>> +   * Fix this by swizzling the Z component to the Y component.
>> +   */
>> +  const bool is_1d_array_image =
>> +  (type->sampler_dimensionality == GLSL_SAMPLER_DIM_1D &&
>> +   type->sampler_array);
>> +
>> +  /*
>> +   * For CubeMapArray images, we should count the number of cubes 
>> instead
>> +   * of the number of faces. Fix it by dividing the (Z component) by 6.
>> +   */
>> +  const bool is_cube_map_array_image =
>> +  (type->sampler_dimensionality == GLSL_SAMPLER_DIM_CUBE &&
>> +   type->sampler_array);
>> +
>> +  /* Copy all the components. */
>> +  const nir_intrinsic_info *info = 
>> &nir_intrinsic_infos[instr->intrinsic];
>> +  for (int c = 0; c < info->dest_components; ++c) {
>> + if (c > type->coordinate_components()) {
>
> Off by one, "c > type->coordinate_components()" is what you probably

Typo -- ">=" is of course what I meant to write. :P

> meant to avoid reading past the end of the size array.
>
>> + bld.MOV(offset(retype(dest, base_type), bld, c),
>
> base_type is the base data type of the image, i.e. float for
> floating-point or fixed-point normalized image formats.  imageSize
> always returns integer results regardless of the image data type, so
> BRW_REGISTER_TYPE_D is what you want (feel free to factor out the
> offset(retype(dest...)...) expression to a local variable if it gets too
> long).  I guess you were only testing imageSize with integer formats?
>
> With these and the comments from other people addressed this patch is:
>
> Reviewed-by: Francisco Jerez 
>
>
>> + fs_reg(1));
>> + } else if (c == 1 && is_1d_array_image) {
>> +bld.MOV(offset(retype(dest, base_type), bld, c),
>> +offset(size, bld, 2));
>> + } else if (c == 2 && is_cube_map_array_image) {
>> +bld.emit(SHADER_OPCODE_INT_QUOTIENT,
>> + offset(retype(dest, base_type), bld, c),
>> + offset(size, bld, c), fs_reg(6));
>> + } else {
>> +bld.MOV(offset(retype(dest, base_type), bld, c),
>> +offset(size, bld, c));
>> + }
>> +   }
>> +
>> +  break;
>> +   }
>> +
>> case nir_intrinsic_load_front_face:
>>bld.MOV(retype(dest, BRW_REGISTER_TYPE_D),
>>*emit_frontfacing_interpolation());
>> -- 
>> 2.5.0
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

Re: [Mesa-dev] [PATCH 20.5/25] mesa: Add ES31 API tag for the extension table.

2015-08-19 Thread Tapani Pälli


Reviewed-by: Tapani Palli 

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

I'll mark the OES_shader_image_atomic extension entry with this tag to
make sure that we don't expose it on earlier GLES API versions
accidentally, because according to the extension:

  "OpenGL ES 3.1 and GLSL ES 3.10 are required."
---
  src/mesa/main/extensions.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
index d934d19..017de2d 100644
--- a/src/mesa/main/extensions.c
+++ b/src/mesa/main/extensions.c
@@ -50,6 +50,7 @@ enum {
 ES1 = 1 << API_OPENGLES,
 ES2 = 1 << API_OPENGLES2,
 ES3 = 1 << (API_OPENGL_LAST + 1),
+   ES31 = 1 << (API_OPENGL_LAST + 2),
  };
  
  /**

@@ -773,6 +774,8 @@ _mesa_make_extension_string(struct gl_context *ctx)
 unsigned api_set = (1 << ctx->API);
 if (_mesa_is_gles3(ctx))
api_set |= ES3;
+   if (_mesa_is_gles31(ctx))
+  api_set |= ES31;
  
 /* Check if the MESA_EXTENSION_MAX_YEAR env var is set */

 {
@@ -854,6 +857,8 @@ _mesa_get_extension_count(struct gl_context *ctx)
 unsigned api_set = (1 << ctx->API);
 if (_mesa_is_gles3(ctx))
api_set |= ES3;
+   if (_mesa_is_gles31(ctx))
+  api_set |= ES31;
  
 /* only count once */

 if (ctx->Extensions.Count != 0)
@@ -880,6 +885,8 @@ _mesa_get_enabled_extension(struct gl_context *ctx, GLuint 
index)
 unsigned api_set = (1 << ctx->API);
 if (_mesa_is_gles3(ctx))
api_set |= ES3;
+   if (_mesa_is_gles31(ctx))
+  api_set |= ES31;
  
 base = (GLboolean*) &ctx->Extensions;

 n = 0;


___
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

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

Bug ID: 91691
   Summary: Missing "gl.pc" file while using Scons for
cross-compiling
   Product: Mesa
   Version: 9.1
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Other
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: yugioh...@1s.fr
QA Contact: mesa-dev@lists.freedesktop.org

Hello,

I remark that when I cross compile mesa (linux -> windows), I do not have the
"gl.pc" file useful for pkg-config.
It is probably because I am using "scons" to build mesa for windows whereas I
am using "autotools" to build mesa for linux.
The building process is not the same in both case.

Is there a way to generate the "gl.pc" file when I use scons to build mesa or I
need to create it manually?

Thank you.
Best regards.

-- 
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] [Bug 91691] Missing "gl.pc" file while using Scons for cross-compiling

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

YuGiOhJCJ  changed:

   What|Removed |Added

 CC||yugioh...@1s.fr

-- 
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 11/25] glsl: Implement GLSL ES restriction on images being either readonly or writeonly.

2015-08-19 Thread Tapani Pälli


Patches 11,12,13:
Reviewed-by: Tapani Pälli 

On 08/17/2015 07:45 PM, Francisco Jerez wrote:

---
  src/glsl/ast_to_hir.cpp | 18 ++
  1 file changed, 18 insertions(+)

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 4d279f6..ff57ec3 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2469,6 +2469,24 @@ apply_image_qualifier_to_variable(const struct 
ast_type_qualifier *qual,
  
   var->data.image_format = GL_NONE;

}
+
+  /* From page 70 of the GLSL ES 3.1 specification:
+   *
+   * "Except for image variables qualified with the format qualifiers
+   *  r32f, r32i, and r32ui, image variables must specify either memory
+   *  qualifier readonly or the memory qualifier writeonly."
+   */
+  if (state->es_shader &&
+  var->data.image_format != GL_R32F &&
+  var->data.image_format != GL_R32I &&
+  var->data.image_format != GL_R32UI &&
+  !var->data.image_read_only &&
+  !var->data.image_write_only) {
+ _mesa_glsl_error(loc, state, "image variables of format other than "
+  "r32f, r32i or r32ui must be qualified `readonly' or 
"
+  "`writeonly'");
+  }
+
 } else if (qual->flags.q.read_only ||
qual->flags.q.write_only ||
qual->flags.q.coherent ||


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

[Mesa-dev] [PATCH] mesa: Allow query of GL_VERTEX_BINDING_BUFFER

From: Marta Lofstedt 

According to OpenGL ES 3.1 specification table : 20.2 and
OpenGL specification 4.4 table 23.4. The glGetIntegeri_v
functions should report the name  of the buffer bound
when called with GL_VERTEX_BINDING_BUFFER.

Signed-off-by: Marta Lofstedt 
---
 src/mesa/main/get.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index 307a5ff..ae2d0b7 100644
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -1989,6 +1989,14 @@ find_value_indexed(const char *func, GLenum pname, 
GLuint index, union value *v)
   v->value_int = 
ctx->Array.VAO->VertexBinding[VERT_ATTRIB_GENERIC(index)].Stride;
   return TYPE_INT;
 
+   case GL_VERTEX_BINDING_BUFFER:
+  if (!_mesa_is_desktop_gl(ctx) && !_mesa_is_gles31(ctx))
+  goto invalid_enum;
+  if (index >= ctx->Const.Program[MESA_SHADER_VERTEX].MaxAttribs)
+  goto invalid_value;
+  v->value_int = 
ctx->Array.VAO->VertexBinding[VERT_ATTRIB_GENERIC(index)].BufferObj->Name;
+  return TYPE_INT;
+
/* ARB_shader_image_load_store */
case GL_IMAGE_BINDING_NAME: {
   struct gl_texture_object *t;
-- 
1.9.1

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

[Mesa-dev] [PATCH 0/3] Enable misc. Vertex bindings for OpenGL ES 3.1

These patches enable some Vertex Binding functionality for OpenGL ES 3.1,
that is currently limited to desktop GL.

Marta Lofstedt (3):
  mesa/es3.1 : Align OpenGL ES 3.1 glBindVertexBuffer error handling
with OpenGL Core
  mesa/es3.1: Allow query of Vertex bindings for GLES 3.1
  mesa/es3.1: Enable GL_ARB_vertex_attrib_binding functionality for GLES
3.1

 src/mesa/main/bufferobj.c |  2 +-
 src/mesa/main/get.c   |  7 ---
 src/mesa/main/varray.c| 14 +++---
 3 files changed, 12 insertions(+), 11 deletions(-)

-- 
1.9.1

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

[Mesa-dev] [PATCH 2/3] mesa/es3.1: Allow query of Vertex bindings for GLES 3.1

From: Marta Lofstedt 

Signed-off-by: Marta Lofstedt 
---
 src/mesa/main/get.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index ae2d0b7..b6b7d6c 100644
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -1966,7 +1966,8 @@ find_value_indexed(const char *func, GLenum pname, GLuint 
index, union value *v)
   return TYPE_INT64;
 
case GL_VERTEX_BINDING_DIVISOR:
-  if (!_mesa_is_desktop_gl(ctx) || !ctx->Extensions.ARB_instanced_arrays)
+  if ((!_mesa_is_desktop_gl(ctx) || !ctx->Extensions.ARB_instanced_arrays) 
&&
+  !_mesa_is_gles31(ctx))
   goto invalid_enum;
   if (index >= ctx->Const.Program[MESA_SHADER_VERTEX].MaxAttribs)
   goto invalid_value;
@@ -1974,7 +1975,7 @@ find_value_indexed(const char *func, GLenum pname, GLuint 
index, union value *v)
   return TYPE_INT;
 
case GL_VERTEX_BINDING_OFFSET:
-  if (!_mesa_is_desktop_gl(ctx))
+  if (!_mesa_is_desktop_gl(ctx) && !_mesa_is_gles31(ctx))
   goto invalid_enum;
   if (index >= ctx->Const.Program[MESA_SHADER_VERTEX].MaxAttribs)
   goto invalid_value;
@@ -1982,7 +1983,7 @@ find_value_indexed(const char *func, GLenum pname, GLuint 
index, union value *v)
   return TYPE_INT;
 
case GL_VERTEX_BINDING_STRIDE:
-  if (!_mesa_is_desktop_gl(ctx))
+  if (!_mesa_is_desktop_gl(ctx) && !_mesa_is_gles31(ctx))
   goto invalid_enum;
   if (index >= ctx->Const.Program[MESA_SHADER_VERTEX].MaxAttribs)
   goto invalid_value;
-- 
1.9.1

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

[Mesa-dev] [PATCH 3/3] mesa/es3.1: Enable GL_ARB_vertex_attrib_binding functionality for GLES 3.1

From: Marta Lofstedt 

Signed-off-by: Marta Lofstedt 
---
 src/mesa/main/varray.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
index 3bab985..3ddf795 100644
--- a/src/mesa/main/varray.c
+++ b/src/mesa/main/varray.c
@@ -899,12 +899,12 @@ get_vertex_array_attrib(struct gl_context *ctx,
   }
   goto error;
case GL_VERTEX_ATTRIB_BINDING:
-  if (_mesa_is_desktop_gl(ctx)) {
+  if (_mesa_is_desktop_gl(ctx) || _mesa_is_gles31(ctx)) {
  return array->VertexBinding - VERT_ATTRIB_GENERIC0;
   }
   goto error;
case GL_VERTEX_ATTRIB_RELATIVE_OFFSET:
-  if (_mesa_is_desktop_gl(ctx)) {
+  if (_mesa_is_desktop_gl(ctx) || _mesa_is_gles31(ctx)) {
  return array->RelativeOffset;
   }
   goto error;
@@ -1676,7 +1676,7 @@ vertex_array_vertex_buffer(struct gl_context *ctx, struct 
gl_vertex_array_object
   return;
}
 
-   if (ctx->API == API_OPENGL_CORE && ctx->Version >= 44 &&
+   if (((ctx->API == API_OPENGL_CORE && ctx->Version >= 44) || 
_mesa_is_gles31(ctx)) &&
stride > ctx->Const.MaxVertexAttribStride) {
   _mesa_error(ctx, GL_INVALID_VALUE, "%s(stride=%d > "
   "GL_MAX_VERTEX_ATTRIB_STRIDE)", func, stride);
@@ -1726,7 +1726,7 @@ _mesa_BindVertexBuffer(GLuint bindingIndex, GLuint 
buffer, GLintptr offset,
 *"An INVALID_OPERATION error is generated if no vertex array object
 * is bound."
 */
-   if (ctx->API == API_OPENGL_CORE &&
+   if ((ctx->API == API_OPENGL_CORE || _mesa_is_gles31(ctx)) &&
ctx->Array.VAO == ctx->Array.DefaultVAO) {
   _mesa_error(ctx, GL_INVALID_OPERATION,
   "glBindVertexBuffer(No array object bound)");
@@ -1948,7 +1948,7 @@ vertex_attrib_format(GLuint attribIndex, GLint size, 
GLenum type,
 * is an oversight.  In the OpenGL 4.3 (Core Profile) spec, it applies
 * to all three functions.
 */
-   if (ctx->API == API_OPENGL_CORE &&
+   if ((ctx->API == API_OPENGL_CORE || _mesa_is_gles31(ctx)) &&
ctx->Array.VAO == ctx->Array.DefaultVAO) {
   _mesa_error(ctx, GL_INVALID_OPERATION,
   "%s(No array object bound)", func);
@@ -2136,7 +2136,7 @@ _mesa_VertexAttribBinding(GLuint attribIndex, GLuint 
bindingIndex)
 *"An INVALID_OPERATION error is generated if no vertex array object
 * is bound."
 */
-   if (ctx->API == API_OPENGL_CORE &&
+   if ((ctx->API == API_OPENGL_CORE || _mesa_is_gles31(ctx)) &&
ctx->Array.VAO == ctx->Array.DefaultVAO) {
   _mesa_error(ctx, GL_INVALID_OPERATION,
   "glVertexAttribBinding(No array object bound)");
@@ -2210,7 +2210,7 @@ _mesa_VertexBindingDivisor(GLuint bindingIndex, GLuint 
divisor)
 *"An INVALID_OPERATION error is generated if no vertex array object
 * is bound."
 */
-   if (ctx->API == API_OPENGL_CORE &&
+   if ((ctx->API == API_OPENGL_CORE || _mesa_is_gles31(ctx)) &&
ctx->Array.VAO == ctx->Array.DefaultVAO) {
   _mesa_error(ctx, GL_INVALID_OPERATION,
   "glVertexBindingDivisor(No array object bound)");
-- 
1.9.1

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

[Mesa-dev] [PATCH 1/3] mesa/es3.1 : Align OpenGL ES 3.1 glBindVertexBuffer error handling with OpenGL Core

From: Marta Lofstedt 

According to OpenGL ES 3.1 specification 10.3.1:
"An INVALID_OPERATION error is generated if buffer is not zero
or a name returned from a previous call to GenBuffers,
or if such a name has since been deleted with DeleteBuffers."
This error check was previously limited to OpenGL Core.

Signed-off-by: Marta Lofstedt 
---
 src/mesa/main/bufferobj.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index e17b41c..2ca0064 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -919,7 +919,7 @@ _mesa_handle_bind_buffer_gen(struct gl_context *ctx,
 {
struct gl_buffer_object *buf = *buf_handle;
 
-   if (!buf && ctx->API == API_OPENGL_CORE) {
+   if (!buf && (ctx->API == API_OPENGL_CORE || _mesa_is_gles31(ctx))) {
   _mesa_error(ctx, GL_INVALID_OPERATION, "%s(non-gen name)", caller);
   return false;
}
-- 
1.9.1

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

[Mesa-dev] [PATCH 1/2] mesa/formats: only do type and component lookup for uncompressed formats

From: Nanley Chery 

Only uncompressed formats have a non-void type and actual components per pixel.
Rename _mesa_format_to_type_and_comps to
_mesa_uncompressed_format_to_type_and_comps and require callers to check if
the format is not compressed.

v2. include compressed format cases to avoid gcc warnings.

Signed-off-by: Nanley Chery 
---
 src/mesa/main/format_utils.c |  2 +-
 src/mesa/main/formats.c  | 89 +---
 src/mesa/main/formats.h  |  2 +-
 src/mesa/main/mipmap.c   |  2 +-
 4 files changed, 46 insertions(+), 49 deletions(-)

diff --git a/src/mesa/main/format_utils.c b/src/mesa/main/format_utils.c
index 810bb16..5fdabd5 100644
--- a/src/mesa/main/format_utils.c
+++ b/src/mesa/main/format_utils.c
@@ -602,7 +602,7 @@ _mesa_format_to_array(mesa_format format, GLenum *type, int 
*num_components,
 
*normalized = !_mesa_is_format_integer(format);
 
-   _mesa_format_to_type_and_comps(format, type, &format_components);
+   _mesa_uncompressed_format_to_type_and_comps(format, type, 
&format_components);
 
switch (_mesa_get_format_layout(format)) {
case MESA_FORMAT_LAYOUT_ARRAY:
diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c
index baeb1bf..24c16ee 100644
--- a/src/mesa/main/formats.c
+++ b/src/mesa/main/formats.c
@@ -870,9 +870,9 @@ _mesa_format_row_stride(mesa_format format, GLsizei width)
 
 
 /**
- * Debug/test: check that all formats are handled in the
- * _mesa_format_to_type_and_comps() function.  When new pixel formats
- * are added to Mesa, that function needs to be updated.
+ * Debug/test: check that all uncompressed formats are handled in the
+ * _mesa_uncompressed_format_to_type_and_comps() function. When new pixel
+ * formats are added to Mesa, that function needs to be updated.
  * This is a no-op after the first call.
  */
 static void
@@ -886,7 +886,8 @@ check_format_to_type_and_comps(void)
   /* This function will emit a problem/warning if the format is
* not handled.
*/
-  _mesa_format_to_type_and_comps(f, &datatype, &comps);
+  if (!_mesa_is_format_compressed(f))
+ _mesa_uncompressed_format_to_type_and_comps(f, &datatype, &comps);
}
 }
 
@@ -982,11 +983,11 @@ _mesa_test_formats(void)
 
 
 /**
- * Return datatype and number of components per texel for the given 
mesa_format.
- * Only used for mipmap generation code.
+ * Return datatype and number of components per texel for the given
+ * uncompressed mesa_format. Only used for mipmap generation code.
  */
 void
-_mesa_format_to_type_and_comps(mesa_format format,
+_mesa_uncompressed_format_to_type_and_comps(mesa_format format,
GLenum *datatype, GLuint *comps)
 {
switch (format) {
@@ -1220,44 +1221,6 @@ _mesa_format_to_type_and_comps(mesa_format format,
   *comps = 2;
   return;
 
-   case MESA_FORMAT_RGB_FXT1:
-   case MESA_FORMAT_RGBA_FXT1:
-   case MESA_FORMAT_RGB_DXT1:
-   case MESA_FORMAT_RGBA_DXT1:
-   case MESA_FORMAT_RGBA_DXT3:
-   case MESA_FORMAT_RGBA_DXT5:
-   case MESA_FORMAT_SRGB_DXT1:
-   case MESA_FORMAT_SRGBA_DXT1:
-   case MESA_FORMAT_SRGBA_DXT3:
-   case MESA_FORMAT_SRGBA_DXT5:
-   case MESA_FORMAT_R_RGTC1_UNORM:
-   case MESA_FORMAT_R_RGTC1_SNORM:
-   case MESA_FORMAT_RG_RGTC2_UNORM:
-   case MESA_FORMAT_RG_RGTC2_SNORM:
-   case MESA_FORMAT_L_LATC1_UNORM:
-   case MESA_FORMAT_L_LATC1_SNORM:
-   case MESA_FORMAT_LA_LATC2_UNORM:
-   case MESA_FORMAT_LA_LATC2_SNORM:
-   case MESA_FORMAT_ETC1_RGB8:
-   case MESA_FORMAT_ETC2_RGB8:
-   case MESA_FORMAT_ETC2_SRGB8:
-   case MESA_FORMAT_ETC2_RGBA8_EAC:
-   case MESA_FORMAT_ETC2_SRGB8_ALPHA8_EAC:
-   case MESA_FORMAT_ETC2_R11_EAC:
-   case MESA_FORMAT_ETC2_RG11_EAC:
-   case MESA_FORMAT_ETC2_SIGNED_R11_EAC:
-   case MESA_FORMAT_ETC2_SIGNED_RG11_EAC:
-   case MESA_FORMAT_ETC2_RGB8_PUNCHTHROUGH_ALPHA1:
-   case MESA_FORMAT_ETC2_SRGB8_PUNCHTHROUGH_ALPHA1:
-   case MESA_FORMAT_BPTC_RGBA_UNORM:
-   case MESA_FORMAT_BPTC_SRGB_ALPHA_UNORM:
-   case MESA_FORMAT_BPTC_RGB_SIGNED_FLOAT:
-   case MESA_FORMAT_BPTC_RGB_UNSIGNED_FLOAT:
-  /* XXX generate error instead? */
-  *datatype = GL_UNSIGNED_BYTE;
-  *comps = 0;
-  return;
-
case MESA_FORMAT_RGBA_FLOAT32:
   *datatype = GL_FLOAT;
   *comps = 4;
@@ -1556,12 +1519,46 @@ _mesa_format_to_type_and_comps(mesa_format format,
   assert(0);
   return;
 
+   case MESA_FORMAT_RGB_FXT1:
+   case MESA_FORMAT_RGBA_FXT1:
+   case MESA_FORMAT_RGB_DXT1:
+   case MESA_FORMAT_RGBA_DXT1:
+   case MESA_FORMAT_RGBA_DXT3:
+   case MESA_FORMAT_RGBA_DXT5:
+   case MESA_FORMAT_SRGB_DXT1:
+   case MESA_FORMAT_SRGBA_DXT1:
+   case MESA_FORMAT_SRGBA_DXT3:
+   case MESA_FORMAT_SRGBA_DXT5:
+   case MESA_FORMAT_R_RGTC1_UNORM:
+   case MESA_FORMAT_R_RGTC1_SNORM:
+   case MESA_FORMAT_RG_RGTC2_UNORM:
+   case MESA_FORMAT_RG_RGTC2_SNORM:
+   case MESA_FORMAT_L_LATC1_UNORM:
+   case MESA_FORMAT_L_LATC1_SNORM:
+   case MESA_FORMAT_LA_LATC2_UNORM:
+   case MESA_FORMAT_LA_LATC2_SNORM:
+

[Mesa-dev] [PATCH 2/2] mesa/formats: make format testing a gtest

From: Nanley Chery 

We currently check that our format info table is sane during context
initialization in debug builds. Perform this check during
`make check` instead. This enables format testing in release builds
and removes the requirement of an exhuastive switch for
_mesa_uncompressed_format_to_type_and_comps().

Signed-off-by: Nanley Chery 
---
 src/mesa/main/context.c  |   4 -
 src/mesa/main/formats.c  | 157 ++-
 src/mesa/main/tests/Makefile.am  |   1 +
 src/mesa/main/tests/mesa_formats.cpp | 130 +
 4 files changed, 137 insertions(+), 155 deletions(-)
 create mode 100644 src/mesa/main/tests/mesa_formats.cpp

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 7451e5a..6a40c0a 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -402,10 +402,6 @@ one_time_init( struct gl_context *ctx )
 PACKAGE_VERSION, __DATE__, __TIME__);
   }
 #endif
-
-#ifdef DEBUG
-  _mesa_test_formats();
-#endif
}
 
/* per-API one-time init */
diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c
index 24c16ee..e58b917 100644
--- a/src/mesa/main/formats.c
+++ b/src/mesa/main/formats.c
@@ -81,6 +81,7 @@ static const struct gl_format_info *
 _mesa_get_format_info(mesa_format format)
 {
const struct gl_format_info *info = &format_info[format];
+   STATIC_ASSERT(ARRAY_SIZE(format_info) == MESA_FORMAT_COUNT);
assert(info->Name == format);
return info;
 }
@@ -869,118 +870,6 @@ _mesa_format_row_stride(mesa_format format, GLsizei width)
 }
 
 
-/**
- * Debug/test: check that all uncompressed formats are handled in the
- * _mesa_uncompressed_format_to_type_and_comps() function. When new pixel
- * formats are added to Mesa, that function needs to be updated.
- * This is a no-op after the first call.
- */
-static void
-check_format_to_type_and_comps(void)
-{
-   mesa_format f;
-
-   for (f = MESA_FORMAT_NONE + 1; f < MESA_FORMAT_COUNT; f++) {
-  GLenum datatype = 0;
-  GLuint comps = 0;
-  /* This function will emit a problem/warning if the format is
-   * not handled.
-   */
-  if (!_mesa_is_format_compressed(f))
- _mesa_uncompressed_format_to_type_and_comps(f, &datatype, &comps);
-   }
-}
-
-/**
- * Do sanity checking of the format info table.
- */
-void
-_mesa_test_formats(void)
-{
-   GLuint i;
-
-   STATIC_ASSERT(ARRAY_SIZE(format_info) == MESA_FORMAT_COUNT);
-
-   for (i = 0; i < MESA_FORMAT_COUNT; i++) {
-  const struct gl_format_info *info = _mesa_get_format_info(i);
-  assert(info);
-
-  assert(info->Name == i);
-
-  if (info->Name == MESA_FORMAT_NONE)
- continue;
-
-  if (info->BlockWidth == 1 && info->BlockHeight == 1) {
- if (info->RedBits > 0) {
-GLuint t = info->RedBits + info->GreenBits
-   + info->BlueBits + info->AlphaBits;
-assert(t / 8 <= info->BytesPerBlock);
-(void) t;
- }
-  }
-
-  assert(info->DataType == GL_UNSIGNED_NORMALIZED ||
- info->DataType == GL_SIGNED_NORMALIZED ||
- info->DataType == GL_UNSIGNED_INT ||
- info->DataType == GL_INT ||
- info->DataType == GL_FLOAT ||
- /* Z32_FLOAT_X24S8 has DataType of GL_NONE */
- info->DataType == GL_NONE);
-
-  if (info->BaseFormat == GL_RGB) {
- assert(info->RedBits > 0);
- assert(info->GreenBits > 0);
- assert(info->BlueBits > 0);
- assert(info->AlphaBits == 0);
- assert(info->LuminanceBits == 0);
- assert(info->IntensityBits == 0);
-  }
-  else if (info->BaseFormat == GL_RGBA) {
- assert(info->RedBits > 0);
- assert(info->GreenBits > 0);
- assert(info->BlueBits > 0);
- assert(info->AlphaBits > 0);
- assert(info->LuminanceBits == 0);
- assert(info->IntensityBits == 0);
-  }
-  else if (info->BaseFormat == GL_RG) {
- assert(info->RedBits > 0);
- assert(info->GreenBits > 0);
- assert(info->BlueBits == 0);
- assert(info->AlphaBits == 0);
- assert(info->LuminanceBits == 0);
- assert(info->IntensityBits == 0);
-  }
-  else if (info->BaseFormat == GL_RED) {
- assert(info->RedBits > 0);
- assert(info->GreenBits == 0);
- assert(info->BlueBits == 0);
- assert(info->AlphaBits == 0);
- assert(info->LuminanceBits == 0);
- assert(info->IntensityBits == 0);
-  }
-  else if (info->BaseFormat == GL_LUMINANCE) {
- assert(info->RedBits == 0);
- assert(info->GreenBits == 0);
- assert(info->BlueBits == 0);
- assert(info->AlphaBits == 0);
- assert(info->LuminanceBits > 0);
- assert(info->IntensityBits == 0);
-  }
-  else if (info->BaseFormat == GL_INTENSITY) {
- assert(info->RedBits == 0);
- assert(info->GreenBits == 0);

[Mesa-dev] [PATCH v2] mesa/formats: refactor by removing compressed formats

From: Nanley Chery 

All compressed formats return GL_FALSE and there isn't any evidence to
support that this behaviour would change. Remove all switch cases for
compressed formats.

v2. Since the exhaustive switch is removed, add a gtest to ensure
all formats are handled.

Cc: Brian Paul 
Cc: Chad Versace 
Signed-off-by: Nanley Chery 
---
 src/mesa/drivers/dri/i915/intel_pixel_read.c |  2 +-
 src/mesa/drivers/dri/i915/intel_tex_image.c  |  2 +-
 src/mesa/main/formats.c  | 57 ++--
 src/mesa/main/formats.h  |  2 +-
 src/mesa/main/readpix.c  |  2 +-
 src/mesa/main/tests/mesa_formats.cpp |  7 +++-
 src/mesa/main/texgetimage.c  |  2 +-
 src/mesa/main/texstore.c |  2 +-
 src/mesa/state_tracker/st_cb_readpixels.c|  2 +-
 src/mesa/state_tracker/st_cb_texture.c   |  6 +--
 src/mesa/state_tracker/st_format.c   |  2 +-
 src/mesa/swrast/s_drawpix.c  |  2 +-
 12 files changed, 29 insertions(+), 59 deletions(-)

diff --git a/src/mesa/drivers/dri/i915/intel_pixel_read.c 
b/src/mesa/drivers/dri/i915/intel_pixel_read.c
index 149e921..e6fa8f2 100644
--- a/src/mesa/drivers/dri/i915/intel_pixel_read.c
+++ b/src/mesa/drivers/dri/i915/intel_pixel_read.c
@@ -91,7 +91,7 @@ do_blit_readpixels(struct gl_context * ctx,
 
if (ctx->_ImageTransferState ||
!_mesa_format_matches_format_and_type(irb->mt->format, format, type,
- false)) {
+ false, NULL)) {
   DBG("%s - bad format for blit\n", __func__);
   return false;
}
diff --git a/src/mesa/drivers/dri/i915/intel_tex_image.c 
b/src/mesa/drivers/dri/i915/intel_tex_image.c
index 0a213e9..5ab60d1 100644
--- a/src/mesa/drivers/dri/i915/intel_tex_image.c
+++ b/src/mesa/drivers/dri/i915/intel_tex_image.c
@@ -134,7 +134,7 @@ try_pbo_upload(struct gl_context *ctx,
}
 
if (!_mesa_format_matches_format_and_type(intelImage->mt->format,
- format, type, false)) {
+ format, type, false, NULL)) {
   DBG("%s: format mismatch (upload to %s with format 0x%x, type 0x%x)\n",
  __func__, _mesa_get_format_name(intelImage->mt->format),
  format, type);
diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c
index e58b917..51ac76d 100644
--- a/src/mesa/main/formats.c
+++ b/src/mesa/main/formats.c
@@ -1432,15 +1432,18 @@ _mesa_uncompressed_format_to_type_and_comps(mesa_format 
format,
 GLboolean
 _mesa_format_matches_format_and_type(mesa_format mesa_format,
 GLenum format, GLenum type,
- GLboolean swapBytes)
+GLboolean swapBytes, GLenum *error)
 {
const GLboolean littleEndian = _mesa_little_endian();
+   GLenum local_error = GL_NO_ERROR;
 
/* Note: When reading a GL format/type combination, the format lists channel
 * assignments from most significant channel in the type to least
 * significant.  A type with _REV indicates that the assignments are
 * swapped, so they are listed from least significant to most significant.
 *
+* Compressed formats will fall through and return GL_FALSE.
+*
 * For sanity, please keep this switch statement ordered the same as the
 * enums in formats.h.
 */
@@ -1701,26 +1704,6 @@ _mesa_format_matches_format_and_type(mesa_format 
mesa_format,
case MESA_FORMAT_S_UINT8:
   return format == GL_STENCIL_INDEX && type == GL_UNSIGNED_BYTE;
 
-   case MESA_FORMAT_SRGB_DXT1:
-   case MESA_FORMAT_SRGBA_DXT1:
-   case MESA_FORMAT_SRGBA_DXT3:
-   case MESA_FORMAT_SRGBA_DXT5:
-  return GL_FALSE;
-
-   case MESA_FORMAT_RGB_FXT1:
-   case MESA_FORMAT_RGBA_FXT1:
-   case MESA_FORMAT_RGB_DXT1:
-   case MESA_FORMAT_RGBA_DXT1:
-   case MESA_FORMAT_RGBA_DXT3:
-   case MESA_FORMAT_RGBA_DXT5:
-  return GL_FALSE;
-
-   case MESA_FORMAT_BPTC_RGBA_UNORM:
-   case MESA_FORMAT_BPTC_SRGB_ALPHA_UNORM:
-   case MESA_FORMAT_BPTC_RGB_SIGNED_FLOAT:
-   case MESA_FORMAT_BPTC_RGB_UNSIGNED_FLOAT:
-  return GL_FALSE;
-
case MESA_FORMAT_RGBA_FLOAT32:
   return format == GL_RGBA && type == GL_FLOAT && !swapBytes;
case MESA_FORMAT_RGBA_FLOAT16:
@@ -1917,31 +1900,6 @@ _mesa_format_matches_format_and_type(mesa_format 
mesa_format,
   return format == GL_RGBA && type == GL_UNSIGNED_SHORT &&
  !swapBytes;
 
-   case MESA_FORMAT_R_RGTC1_UNORM:
-   case MESA_FORMAT_R_RGTC1_SNORM:
-   case MESA_FORMAT_RG_RGTC2_UNORM:
-   case MESA_FORMAT_RG_RGTC2_SNORM:
-  return GL_FALSE;
-
-   case MESA_FORMAT_L_LATC1_UNORM:
-   case MESA_FORMAT_L_LATC1_SNORM:
-   case MESA_FORMAT_LA_LATC2_UNORM:
-   case MESA_FORMAT_LA_LATC2_SNORM:
-  return GL_FALSE;
-
-   case MESA_FORMAT_ETC1_RGB8:
-   case MESA_FORMAT_ETC2_RGB8:
-   case MESA_FORMAT_ETC2_SRGB8:
-   cas

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