[Mesa-dev] [PATCH 0/4 v2] Fix implicit conversion

2011-07-27 Thread Chad Versace
These fixes are candidates for the 7.10 and 7.11 branches.

(v1 of the series contained an Eclipse reformatting bomb).

Chad Versace (4):
  glsl: Document parameters_lists_match()
  glsl: Refactor ast_function.cpp:type_compare() into a glsl_type
method
  glsl: Fix illegal implicit type conversions
  glsl: Fix implicit conversions in array constructors

 src/glsl/ast_function.cpp |4 ++-
 src/glsl/glsl_types.cpp   |   61 +++
 src/glsl/glsl_types.h |   11 ++
 src/glsl/ir_function.cpp  |   77 +++-
 4 files changed, 88 insertions(+), 65 deletions(-)

-- 
1.7.6

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


[Mesa-dev] [PATCH 1/4] glsl: Document parameters_lists_match()

2011-07-27 Thread Chad Versace
The function's parameter do not indicate which parameter list belongs to
the function signature and which is the actual parameter list. So clarify
this in a Doxygen comment.

CC: Ian Romanick 
Signed-off-by: Chad Versace 
---
 src/glsl/ir_function.cpp |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/src/glsl/ir_function.cpp b/src/glsl/ir_function.cpp
index 0f2f1a0..6795988 100644
--- a/src/glsl/ir_function.cpp
+++ b/src/glsl/ir_function.cpp
@@ -85,6 +85,12 @@ type_compare(const glsl_type *a, const glsl_type *b)
 }
 
 
+/**
+ * Called by matching_signature().
+ *
+ * \param list_a Parameters of the function definition.
+ * \param list_b Actual parameters passed to the function.
+ */
 static int
 parameter_lists_match(const exec_list *list_a, const exec_list *list_b)
 {
-- 
1.7.6

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


[Mesa-dev] [PATCH 2/4] glsl: Refactor ast_function.cpp:type_compare() into a glsl_type method

2011-07-27 Thread Chad Versace
On a subsequent commit, ast_function_expression::hir() will need access to
type_compare(), but it is defined in a different file.  So refactor
type_compare() into glsl_type::implicit_conversion_compare().

CC: Ian Romanick 
Signed-off-by: Chad Versace 
---
 src/glsl/glsl_types.cpp  |   59 ++
 src/glsl/glsl_types.h|   11 +++
 src/glsl/ir_function.cpp |   71 -
 3 files changed, 77 insertions(+), 64 deletions(-)

diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index a5e21bb..5a93180 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -523,3 +523,62 @@ glsl_type::component_slots() const
   return 0;
}
 }
+
+int glsl_type::implicit_conversion_compare(const glsl_type *desired) const {
+   /* If the types are the same, they trivially match. */
+   if (this == desired)
+  return 0;
+
+   switch (desired->base_type) {
+   case GLSL_TYPE_UINT:
+   case GLSL_TYPE_INT:
+   case GLSL_TYPE_BOOL:
+  /* There is no implicit conversion to or from integer types or bool. */
+  if ((this->is_integer() != desired->is_integer())
+ || (this->is_boolean() != desired->is_boolean()))
+return -1;
+
+  /* FALLTHROUGH */
+
+   case GLSL_TYPE_FLOAT:
+  if ((this->vector_elements != desired->vector_elements)
+ || (this->matrix_columns != desired->matrix_columns))
+return -1;
+
+  return 1;
+
+   case GLSL_TYPE_SAMPLER:
+   case GLSL_TYPE_STRUCT:
+  /* Samplers and structures must match exactly. */
+  return -1;
+
+   case GLSL_TYPE_ARRAY: {
+  if ((this->base_type != GLSL_TYPE_ARRAY)
+   || (this->length != desired->length))
+return -1;
+
+  /* From GLSL 1.50 spec, page 27 (page 33 of the PDF):
+   *"There are no implicit array or structure conversions."
+   *
+   * If the comparison of the array element types detects that a conversion
+   * would be required, the array types do not match.
+   */
+  const glsl_type *this2 = this->fields.array;
+  const glsl_type *desired2 = desired->fields.array;
+  if (this2->implicit_conversion_compare(desired2) == 0)
+return 0;
+  else
+return -1;
+   }
+
+   case GLSL_TYPE_VOID:
+   case GLSL_TYPE_ERROR:
+   default:
+  /* These are all error conditions.  It is invalid for a parameter to
+   * a function to be declared as error, void, or a function.
+   */
+  return -1;
+   }
+
+   assert(0);
+}
diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
index 87f57e7..d791eee 100644
--- a/src/glsl/glsl_types.h
+++ b/src/glsl/glsl_types.h
@@ -224,6 +224,17 @@ struct glsl_type {
 */
unsigned component_slots() const;
 
+   /**
+* \brief Check if an implicit type conversion is possible or necessary.
+*
+* If the types are identical, so no conversion is necessary, return 0.
+* If an implicit conversion is legal, return 1.
+* If an implicit conversion is illegal, return -1.
+*
+* See page 20 (26 of the pdf) of the GLSL 1.20 spec, Section 4.1.10 
Implicit
+* Conversions.
+*/
+   int implicit_conversion_compare(const glsl_type *desired) const;
 
/**
 * Query whether or not a type is a scalar (non-vector and non-matrix).
diff --git a/src/glsl/ir_function.cpp b/src/glsl/ir_function.cpp
index 6795988..98f9f51 100644
--- a/src/glsl/ir_function.cpp
+++ b/src/glsl/ir_function.cpp
@@ -24,67 +24,6 @@
 #include "glsl_types.h"
 #include "ir.h"
 
-int
-type_compare(const glsl_type *a, const glsl_type *b)
-{
-   /* If the types are the same, they trivially match.
-*/
-   if (a == b)
-  return 0;
-
-   switch (a->base_type) {
-   case GLSL_TYPE_UINT:
-   case GLSL_TYPE_INT:
-   case GLSL_TYPE_BOOL:
-  /* There is no implicit conversion to or from integer types or bool.
-   */
-  if ((a->is_integer() != b->is_integer())
- || (a->is_boolean() != b->is_boolean()))
-return -1;
-
-  /* FALLTHROUGH */
-
-   case GLSL_TYPE_FLOAT:
-  if ((a->vector_elements != b->vector_elements)
- || (a->matrix_columns != b->matrix_columns))
-return -1;
-
-  return 1;
-
-   case GLSL_TYPE_SAMPLER:
-   case GLSL_TYPE_STRUCT:
-  /* Samplers and structures must match exactly.
-   */
-  return -1;
-
-   case GLSL_TYPE_ARRAY:
-  if ((b->base_type != GLSL_TYPE_ARRAY)
- || (a->length != b->length))
-return -1;
-
-  /* From GLSL 1.50 spec, page 27 (page 33 of the PDF):
-   *"There are no implicit array or structure conversions."
-   *
-   * If the comparison of the array element types detects that a conversion
-   * would be required, the array types do not match.
-   */
-  return (type_compare(a->fields.array, b->fields.array) == 0) ? 0 : -1;
-
-   case GLSL_TYPE_VOID:
-   case GLSL_TYPE_ERROR:
-   default:
-  /* These are all error conditions.  It is invalid for a parameter to
-   

[Mesa-dev] [PATCH 3/4] glsl: Fix illegal implicit type conversions

2011-07-27 Thread Chad Versace
glsl_type::implicit_conversion_compare() allowed the following illegal
implicit conversions:
bool -> float
bvecN -> vecN

int   -> uint
ivecN -> uvecN
uint  -> int
uvecN -> ivecN

Fixes Piglit
spec/glsl-1.20/compiler/built-in-functions/outerProduct-bvec*.vert.

If cherry-picked, the following commit is required:
glsl: Refactor ast_function.cpp:type_compare() into a glsl_type method

Note: This is a candidate for the 7.10 and 7.11 branches.
CC: Ian Romanick 
Signed-off-by: Chad Versace 
---
 src/glsl/glsl_types.cpp |   20 +++-
 1 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index 5a93180..1bb11bb 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -533,20 +533,22 @@ int glsl_type::implicit_conversion_compare(const 
glsl_type *desired) const {
case GLSL_TYPE_UINT:
case GLSL_TYPE_INT:
case GLSL_TYPE_BOOL:
-  /* There is no implicit conversion to or from integer types or bool. */
-  if ((this->is_integer() != desired->is_integer())
- || (this->is_boolean() != desired->is_boolean()))
+  /* There is no implicit conversion among any of the above types. */
+  if (this->base_type == desired->base_type
+   && this->vector_elements == desired->vector_elements
+   && this->matrix_columns == desired->matrix_columns)
+return 1;
+  else
 return -1;
 
-  /* FALLTHROUGH */
-
case GLSL_TYPE_FLOAT:
-  if ((this->vector_elements != desired->vector_elements)
- || (this->matrix_columns != desired->matrix_columns))
+  if ((this->is_float() || this->is_integer())
+   && this->vector_elements == desired->vector_elements
+   && this->matrix_columns == desired->matrix_columns)
+return 1;
+  else
 return -1;
 
-  return 1;
-
case GLSL_TYPE_SAMPLER:
case GLSL_TYPE_STRUCT:
   /* Samplers and structures must match exactly. */
-- 
1.7.6

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


Re: [Mesa-dev] [PATCH 0/4] Fix implicit conversions

2011-07-27 Thread Chad Versace
On 07/26/2011 11:35 PM, Chad Versace wrote:
> These fixes are candidates for the stable branches.
> 
> Chad Versace (4):
>   glsl: Document parameters_lists_match()
>   glsl: Refactor ast_function.cpp:type_compare() into a glsl_type
> method
>   glsl: Fix illegal implicit type conversions
>   glsl: Fix implicit conversions in array constructors
> 
>  src/glsl/ast_function.cpp |4 +-
>  src/glsl/glsl_types.cpp   |  306 
> ++---
>  src/glsl/glsl_types.h |   11 ++
>  src/glsl/ir_function.cpp  |   77 ++--
>  4 files changed, 177 insertions(+), 221 deletions(-)
> 

Oops... ignore this series. It contains an Eclipse reformatting bomb. v2 will 
be posted soon.

-- 
Chad Versace
c...@chad-versace.us



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


[Mesa-dev] [PATCH 4/4] glsl: Fix implicit conversions in array constructors

2011-07-27 Thread Chad Versace
Array constructors obey narrower conversion rules than other constructors,
but process_array_constructor() was incorrectly applying the broader
rules. From page 33 (page 39 of the pdf) of the GLSL 1.20 spec, Section
5.4.4 Array Constructors:
Each argument must be the same type as the element type of the array,
or be a type that can be converted to the element type of the array
according to Section 4.1.10 “Implicit Conversions.”

Fixes Piglit
spec/glsl-1.20/compiler/structure-and-array-operations/array-ctor-implicit-conversion-bvec*-vec*.vert.

If cherry-picked, the following commits are required:
glsl: Refactor ast_function.cpp:type_compare() into a glsl_type method
glsl: Fix illegal implicit type conversions

Note: This is a candidate for the 7.10 and 7.11 branches.
CC: Ian Romanick 
Signed-off-by: Chad Versace 
---
 src/glsl/ast_function.cpp |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index bdb73f4..70961ab 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -448,7 +448,9 @@ process_array_constructor(exec_list *instructions,
glsl_type::get_instance(GLSL_TYPE_FLOAT,
ir->type->vector_elements,
ir->type->matrix_columns);
-result = convert_component(ir, desired_type);
+if (result->type->implicit_conversion_compare(desired_type) >= 0) {
+   result = convert_component(ir, desired_type);
+}
   }
 
   if (result->type != constructor_type->element_type()) {
-- 
1.7.6

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


Re: [Mesa-dev] [PATCH] egl/gallium: fix build without softpipe and llvmpipe

2011-07-27 Thread Benjamin Franzke
Looks good to me too, and since there were no complaints, pushed.
Thanks.

2011/7/25 Tobias Droste :
> Signed-off-by: Tobias Droste 
> Acked-by: Jakob Bornecrantz 
> Reviewed-by: Marek Olšák 
> ---
>  src/gallium/targets/egl-static/Makefile |   12 +---
>  1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/targets/egl-static/Makefile 
> b/src/gallium/targets/egl-static/Makefile
> index 69e7eec..5b7b330 100644
> --- a/src/gallium/targets/egl-static/Makefile
> +++ b/src/gallium/targets/egl-static/Makefile
> @@ -141,10 +141,18 @@ egl_LIBS += \
>        $(TOP)/src/gallium/drivers/svga/libsvga.a
>  endif
>
> -# swrast
> +# softpipe
> +ifneq ($(findstring softpipe,$(GALLIUM_DRIVERS_DIRS)),)
>  egl_CPPFLAGS += -DGALLIUM_SOFTPIPE -DGALLIUM_RBUG -DGALLIUM_TRACE
>  egl_LIBS += $(TOP)/src/gallium/drivers/softpipe/libsoftpipe.a
>  egl_SYS += -lm
> +endif
> +
> +# llvmpipe
> +ifneq ($(findstring llvmpipe,$(GALLIUM_DRIVERS_DIRS)),)
> +egl_CPPFLAGS += -DGALLIUM_LLVMPIPE
> +egl_LIBS += $(TOP)/src/gallium/drivers/llvmpipe/libllvmpipe.a
> +endif
>
>  # sort to remove duplicates
>  egl_CPPFLAGS := $(sort $(egl_CPPFLAGS))
> @@ -158,8 +166,6 @@ st_GL_SYS := -lm -lpthread $(DLOPEN_LIBS)
>
>  # LLVM
>  ifeq ($(MESA_LLVM),1)
> -egl_CPPFLAGS += -DGALLIUM_LLVMPIPE
> -egl_LIBS += $(TOP)/src/gallium/drivers/llvmpipe/libllvmpipe.a
>  egl_SYS += $(LLVM_LIBS)
>  LDFLAGS += $(LLVM_LDFLAGS)
>
> --
> 1.7.3.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/4] gallium: extend resource_resolve to accommodate BlitFramebuffer

2011-07-27 Thread Christoph Bumiller
On 07/27/2011 01:25 AM, Roland Scheidegger wrote:
>> On 07/26/2011 01:49 AM, Roland Scheidegger wrote:
>>> (Strange thought sent that before - mail client going crazy...)
>>>
 Resolving color buffers is pretty well defined (for standard msaa
 at
 least). I have no idea how that's supposed to be defined for depth
 and stencil values. Frankly I'm not sure what glBlitFramebuffer is
 supposed to do here, maybe it's defined somewhere but even
 applying
 the term "resolve" to a depth buffer seems very iffy. At the very
 least it needs to be documented in the gallium docs what
 "resolving"
 a depth/stencil buffer really means.
>>>
>>> Hmm actually it must be like ReadPixels. So it is "recommended"
>>> that implementations just use the centermost sample, but this is
>>> not required. In particular "any function using depth/stencil
>>> values" is valid.
>>> Taking this to the extreme, this means just returning 0 is valid (f
>>> = 0*sample0 + 0*sample1...) though probably not recommended...
>>> Averaging would be allowed as would be any other crazy stuff. In
>>> any case calling this step, whatever it does, "resolve" seems
>>> abusive with results possibly quite implementation dependent. I
>>> have no idea what nv50 does here though I guess given the loose
>>> definition it should certainly fit the requirements.
>>>
>> But that's great, it means drivers can't do anything wrong here.
>>
>> And either way, GL demands that you can "resolve" it so you better
>> support it. nv50 gallium would follow the behaviour of the binary
>> driver, which is take the value from a single sample.
>> Should hw for some reason not be able to do this (I can't imagine
>> how),
>> well, they can return black. Or whatever the st would do instead.
> If this is really useful to anyone, I agree we probably should be able to 
> handle that in the driver (though using the name "resolve" for that operation 
> still irks me but I could live with it). I'd like to hear from others though, 
> certainly I don't really know if it's that useful. State tracker could 
> resolve through using a shader (ok that requires d3d101 feature but it's 
> likely generally a feature which is way more useful than any of this stuff), 
> though certainly a driver could do that internally as well.
> 
>> Now again about the rest of the new arguments:
>>
>> pipe_resource *aux (from my first reply):
>> The are multisampling implementations that store extra sample data
>> required for resolve in a separate buffer, nv's coverage sampling for
>> example.
>> I think the pipe driver can store a link to such a buffer in the
>> associated colour resource, so, forget about that one.
> Yes I think if it can be hidden it should be hidden, unless we've got some 
> way to deal with such features in some good generic way (which seems hard to 
> do though coverage sampling isn't so special anymore since AMD does the same 
> too nowadays).
> 
>>
>> Now the important ones:
>>
>> mask: If depth resolve is supported, you are not allowed to clobber
>> stencil. You have to allow the pipe driver to not do double the work.
> Yes. If we're going to support depth "resolve" there this should be included.
> 
>>
>> dst_level:
>> If the user resolves to the (n != 0)-th level of a texture, you
>> certainly want this; doing an extra copy of several MiB only because
>> you
>> don't like the looks of an extra parameter in the function
>> declaration
>> is insane from any angle.
> Since source is always level 0, why would you want to resolve to a different 
> than 0 level anyway? I cannot imagine why that would be useful. But maybe my 
> imagination is limited.
> There's likely a reason d3d10 decided it didn't want to bother driver writers 
> with this stuff...
> 
>>
>> dstx, dsty, box:
>> BlitFramebuffer honours scissor state. What if the user is scissoring
>> away half of the destination ? Do you really want to push through the
>> other half, all that to temporary storage, and then again blit
>> several
>> MiB of data over, just because you don't like to clutter the
>> interface ?
> Seems an acceptable tradeoff to me if it's a rare case. It's not like the 
> blit of half a buffer is going to kill your framerate anyway these days. But 
> again I don't know if it's really used that way...
> Sort of reminds me of drivers which tried to scissor away things which 
> weren't going to be visible on screen. Just made things way more complex with 
> very little benefits.
> But well if you include any of these parameters you can just as well include 
> dst_level too because it won't be a simple
> resolve-whole-surface anymore.
> 
>>
>> yflip:
>> When the user is able to resolve to the window system buffer (if mesa
>> is
>> kind enough to choose the same component ordering, or if the "same
>> format" requirement is interpreted loosely - this is certainly
>> possible
>> with the binary drivers), then you have to do this flip.
>> And you don't want to ALWAYS do double the work.

Re: [Mesa-dev] [PATCH 2/3] glsl: Clarify and document type_compare()

2011-07-27 Thread Paul Berry
On 26 July 2011 16:08, Chad Versace  wrote:
> The parameters were named 'a' and 'b', but there was no documentation
> indicating which was being converted to which. So rename 'a' -> 'to' and
> 'b' -> 'from'.
>
> Also add a Doxygen comment.
>
> Signed-off-by: Chad Versace 
> ---
>  src/glsl/ir_function.cpp |   27 +--
>  1 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/src/glsl/ir_function.cpp b/src/glsl/ir_function.cpp
> index 6795988..e6ba359 100644
> --- a/src/glsl/ir_function.cpp
> +++ b/src/glsl/ir_function.cpp
> @@ -24,29 +24,36 @@
>  #include "glsl_types.h"
>  #include "ir.h"
>
> +/**
> + * \brief Check if an implicit type conversion is possible.
> + *
> + * Called by type_compare().
> + *
> + * \return If conversion is possible, 1. Else, -1.

Actually, if I'm not mistaken, it's:
0 if the types match exactly.
1 if the types don't match, but a conversion is possible.
-1 if the types don't match, and a conversion is impossible.

> + */
>  int
> -type_compare(const glsl_type *a, const glsl_type *b)
> +type_compare(const glsl_type *to, const glsl_type *from)
>  {
>    /* If the types are the same, they trivially match.
>     */
> -   if (a == b)
> +   if (to == from)
>       return 0;
>
> -   switch (a->base_type) {
> +   switch (to->base_type) {
>    case GLSL_TYPE_UINT:
>    case GLSL_TYPE_INT:
>    case GLSL_TYPE_BOOL:
>       /* There is no implicit conversion to or from integer types or bool.
>        */
> -      if ((a->is_integer() != b->is_integer())
> -         || (a->is_boolean() != b->is_boolean()))
> +      if ((to->is_integer() != from->is_integer())
> +         || (to->is_boolean() != from->is_boolean()))
>         return -1;
>
>       /* FALLTHROUGH */
>
>    case GLSL_TYPE_FLOAT:
> -      if ((a->vector_elements != b->vector_elements)
> -         || (a->matrix_columns != b->matrix_columns))
> +      if ((to->vector_elements != from->vector_elements)
> +         || (to->matrix_columns != from->matrix_columns))
>         return -1;
>
>       return 1;
> @@ -58,8 +65,8 @@ type_compare(const glsl_type *a, const glsl_type *b)
>       return -1;
>
>    case GLSL_TYPE_ARRAY:
> -      if ((b->base_type != GLSL_TYPE_ARRAY)
> -         || (a->length != b->length))
> +      if ((from->base_type != GLSL_TYPE_ARRAY)
> +         || (to->length != from->length))
>         return -1;
>
>       /* From GLSL 1.50 spec, page 27 (page 33 of the PDF):
> @@ -68,7 +75,7 @@ type_compare(const glsl_type *a, const glsl_type *b)
>        * If the comparison of the array element types detects that a 
> conversion
>        * would be required, the array types do not match.
>        */
> -      return (type_compare(a->fields.array, b->fields.array) == 0) ? 0 : -1;
> +      return (type_compare(to->fields.array, from->fields.array) == 0) ? 0 : 
> -1;
>
>    case GLSL_TYPE_VOID:
>    case GLSL_TYPE_ERROR:
> --
> 1.7.6
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>

I realize this is a bit of yak shaving, but I'm not thrilled about
type_compare's three possible return values.  I'd prefer to move
responsibility for checking whether the types match exactly into the
caller.  Then type_compare would only need two return values, so it
could return a boolean, and we could name the function in such a way
that the meaning of the boolean was obvious.  For example:

bool is_conversion_possible(const glsl_type *to, const glsl_type *from);

Any chance we could do this bit of clean-up while we're in the neighborhood?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3 v2] i965: Check actual tile offsets in Gen4 miptree workaround.

2011-07-27 Thread Eric Anholt
On Tue, 26 Jul 2011 12:10:20 -0700, Chad Versace  wrote:
Non-text part: multipart/mixed
Non-text part: multipart/signed
> On 07/25/2011 05:06 PM, Kenneth Graunke wrote:
> > The apitrace dump in bug #34009 managed to fool the draw_offset check
> > into thinking that we were tile aligned when we weren't.  This led to an
> > assertion failure in brw_update_renderbuffer_surface with tile_y != 0.
> > 
> > Simply compute tile_x and tile_y and check those, as that way both
> > places are checking the same thing.
> > 
> > NOTE: This is a candidate for stable release branches.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=34009
> > Cc: Chad Versace 
> > Cc: Eric Anholt 
> > Signed-off-by: Kenneth Graunke 
> > ---
> >  src/mesa/drivers/dri/intel/intel_fbo.c |   28 ++--
> >  1 files changed, 26 insertions(+), 2 deletions(-)
> > 
> > v2 doesn't break the faked packed/depth stencil on Ivybridge (there is no
> > irb->region on the wrapper), and introduces a function (since I need to add
> > more conditions in a third patch.)
> > 
> > diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c 
> > b/src/mesa/drivers/dri/intel/intel_fbo.c
> > index 35be325..90bfe96 100644
> > --- a/src/mesa/drivers/dri/intel/intel_fbo.c
> > +++ b/src/mesa/drivers/dri/intel/intel_fbo.c
> > @@ -647,6 +647,22 @@ intel_renderbuffer_tile_offsets(struct 
> > intel_renderbuffer *irb,
> > }
> >  }
> >  
> > +#ifndef I915
> > +static bool
> > +need_tile_offset_workaround(struct brw_context *brw,
> > +   struct intel_renderbuffer *irb)
> > +{
> > +   uint32_t tile_x, tile_y;
> > +
> > +   intel_renderbuffer_tile_offsets(irb, &tile_x, &tile_y);
> > +
> > +   if (!brw->has_surface_tile_offset)
> > +  return tile_x != 0 || tile_y != 0;
> > +
> > +   return false;
> > +}
> > +#endif
> > +
> >  /**
> >   * Called by glFramebufferTexture[123]DEXT() (and other places) to
> >   * prepare for rendering into texture memory.  This might be called
> > @@ -699,9 +715,17 @@ intel_render_texture(struct gl_context * ctx,
> > intel_renderbuffer_set_draw_offset(irb, intel_image, att->Zoffset);
> > intel_image->used_as_render_target = GL_TRUE;
> >  
> > +   bool is_depthbuffer = irb->Base._BaseFormat == GL_DEPTH_COMPONENT ||
> > +irb->Base._BaseFormat == GL_DEPTH_STENCIL;
> > +
> > +   /* Resolve combined depth/stencil wrapper to get at the real depth 
> > buffer.
> > +* Notably, we don't need the original irb after this point.
> > +*/
> > +   if (is_depthbuffer)
> > +  irb = intel_get_renderbuffer(fb, BUFFER_DEPTH);
> > +
> >  #ifndef I915
> > -   if (!brw_context(ctx)->has_surface_tile_offset &&
> > -   (irb->draw_offset & 4095) != 0) {
> 
> Why did you remove `(irb->draw_offset & 4095) != 0`? It's not evident from 
> the patch.

That was trying to check whether there was a within-tile offset, but
that doesn't work.  tile_x and tile_y are the actual within-tile
offsets.


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


[Mesa-dev] [PATCH] glsl: improve the accuracy of the radians() builtin function

2011-07-27 Thread Paul Berry
The constant used in the radians() function didn't have enough
precision, causing a relative error of 1.676e-5, which is far worse
than the precision of 32-bit floats.  This patch reduces the relative
error to 1.14e-9, which is the best we can do in 32 bits.

Fixes piglit tests {fs,vs}-radians-{float,vec2,vec3,vec4}.
---
 src/glsl/builtins/ir/radians |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/glsl/builtins/ir/radians b/src/glsl/builtins/ir/radians
index 6a0f5d2..a419101 100644
--- a/src/glsl/builtins/ir/radians
+++ b/src/glsl/builtins/ir/radians
@@ -2,20 +2,20 @@
(signature float
  (parameters
(declare (in) float arg0))
- ((return (expression float * (var_ref arg0) (constant float 
(0.017453))
+ ((return (expression float * (var_ref arg0) (constant float 
(0.0174532925))
 
(signature vec2
  (parameters
(declare (in) vec2 arg0))
- ((return (expression vec2 * (var_ref arg0) (constant float (0.017453))
+ ((return (expression vec2 * (var_ref arg0) (constant float 
(0.0174532925))
 
(signature vec3
  (parameters
(declare (in) vec3 arg0))
- ((return (expression vec3 * (var_ref arg0) (constant float (0.017453))
+ ((return (expression vec3 * (var_ref arg0) (constant float 
(0.0174532925))
 
(signature vec4
  (parameters
(declare (in) vec4 arg0))
- ((return (expression vec4 * (var_ref arg0) (constant float (0.017453))
+ ((return (expression vec4 * (var_ref arg0) (constant float 
(0.0174532925))
 ))
-- 
1.7.6

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


Re: [Mesa-dev] [PATCH 2/4] gallium: extend resource_resolve to accommodate BlitFramebuffer

2011-07-27 Thread Roland Scheidegger
> On 07/27/2011 01:25 AM, Roland Scheidegger wrote:
> >> On 07/26/2011 01:49 AM, Roland Scheidegger wrote:
> >>> (Strange thought sent that before - mail client going crazy...)
> >>>
>  Resolving color buffers is pretty well defined (for standard
>  msaa
>  at
>  least). I have no idea how that's supposed to be defined for
>  depth
>  and stencil values. Frankly I'm not sure what glBlitFramebuffer
>  is
>  supposed to do here, maybe it's defined somewhere but even
>  applying
>  the term "resolve" to a depth buffer seems very iffy. At the
>  very
>  least it needs to be documented in the gallium docs what
>  "resolving"
>  a depth/stencil buffer really means.
> >>>
> >>> Hmm actually it must be like ReadPixels. So it is "recommended"
> >>> that implementations just use the centermost sample, but this is
> >>> not required. In particular "any function using depth/stencil
> >>> values" is valid.
> >>> Taking this to the extreme, this means just returning 0 is valid
> >>> (f
> >>> = 0*sample0 + 0*sample1...) though probably not recommended...
> >>> Averaging would be allowed as would be any other crazy stuff. In
> >>> any case calling this step, whatever it does, "resolve" seems
> >>> abusive with results possibly quite implementation dependent. I
> >>> have no idea what nv50 does here though I guess given the loose
> >>> definition it should certainly fit the requirements.
> >>>
> >> But that's great, it means drivers can't do anything wrong here.
> >>
> >> And either way, GL demands that you can "resolve" it so you better
> >> support it. nv50 gallium would follow the behaviour of the binary
> >> driver, which is take the value from a single sample.
> >> Should hw for some reason not be able to do this (I can't imagine
> >> how),
> >> well, they can return black. Or whatever the st would do instead.
> > If this is really useful to anyone, I agree we probably should be
> > able to handle that in the driver (though using the name "resolve"
> > for that operation still irks me but I could live with it). I'd
> > like to hear from others though, certainly I don't really know if
> > it's that useful. State tracker could resolve through using a
> > shader (ok that requires d3d101 feature but it's likely generally
> > a feature which is way more useful than any of this stuff), though
> > certainly a driver could do that internally as well.
> > 
> >> Now again about the rest of the new arguments:
> >>
> >> pipe_resource *aux (from my first reply):
> >> The are multisampling implementations that store extra sample data
> >> required for resolve in a separate buffer, nv's coverage sampling
> >> for
> >> example.
> >> I think the pipe driver can store a link to such a buffer in the
> >> associated colour resource, so, forget about that one.
> > Yes I think if it can be hidden it should be hidden, unless we've
> > got some way to deal with such features in some good generic way
> > (which seems hard to do though coverage sampling isn't so special
> > anymore since AMD does the same too nowadays).
> > 
> >>
> >> Now the important ones:
> >>
> >> mask: If depth resolve is supported, you are not allowed to
> >> clobber
> >> stencil. You have to allow the pipe driver to not do double the
> >> work.
> > Yes. If we're going to support depth "resolve" there this should be
> > included.
> > 
> >>
> >> dst_level:
> >> If the user resolves to the (n != 0)-th level of a texture, you
> >> certainly want this; doing an extra copy of several MiB only
> >> because
> >> you
> >> don't like the looks of an extra parameter in the function
> >> declaration
> >> is insane from any angle.
> > Since source is always level 0, why would you want to resolve to a
> > different than 0 level anyway? I cannot imagine why that would be
> > useful. But maybe my imagination is limited.
> > There's likely a reason d3d10 decided it didn't want to bother
> > driver writers with this stuff...
> > 
> >>
> >> dstx, dsty, box:
> >> BlitFramebuffer honours scissor state. What if the user is
> >> scissoring
> >> away half of the destination ? Do you really want to push through
> >> the
> >> other half, all that to temporary storage, and then again blit
> >> several
> >> MiB of data over, just because you don't like to clutter the
> >> interface ?
> > Seems an acceptable tradeoff to me if it's a rare case. It's not
> > like the blit of half a buffer is going to kill your framerate
> > anyway these days. But again I don't know if it's really used that
> > way...
> > Sort of reminds me of drivers which tried to scissor away things
> > which weren't going to be visible on screen. Just made things way
> > more complex with very little benefits.
> > But well if you include any of these parameters you can just as
> > well include dst_level too because it won't be a simple
> > resolve-whole-surface anymore.
> > 
> >>
> >> yflip:
> >> When the user is able to resolve to the window system buffer (if
> >> mesa
> >> 

[Mesa-dev] [Bug 39604] New: [regression bisected] ES2 conformance test copy_texture fails

2011-07-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=39604

   Summary: [regression bisected] ES2 conformance test
copy_texture fails
   Product: Mesa
   Version: git
  Platform: Other
OS/Version: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Mesa core
AssignedTo: mesa-dev@lists.freedesktop.org
ReportedBy: i...@freedesktop.org
CC: bri...@vmware.com


On both swrast and i965 the ES2 conformance test copy_texture fails as shown
below.  The test previously passed on master and passes on 7.11.  I bisected
the failure to:

commit 5874890c26f434f54e9218b83fae4eb8175c24e9
Author: Brian Paul 
Date:   Tue Jul 19 20:03:05 2011 -0600

mesa: stop using ctx->Driver.CopyTexImage1D/2D() hooks


[idr@mumford piglit]$ bin/GTF -noimagefileio -id=45
-run=/home/idr/devel/graphics/GTF_ES/GTF_ES/glsl/GTF/GL2FixedTests/copy_texture/copy_texture.test
Running GTF (Graphics Test Framework)
egl_vendor: Mesa Project
egl_version: 1.4
ConfigID: 45 RGBA bits( 8, 8, 8, 0) Depth bits: 24 Stencil bits: 8
Mesa warning: couldn't open libtxc_dxtn.so, software DXTn
compression/decompression unavailable
Mesa: Initializing x86-64 optimizations
gtf_version: 2.9
mode: egl
arg: bin/GTF -noimagefileio -id=45
-run=/home/idr/devel/graphics/GTF_ES/GTF_ES/glsl/GTF/GL2FixedTests/copy_texture/copy_texture.test
gl_renderer: Software Rasterizer
gl_shading_language_version: OpenGL ES GLSL ES 1.0.16
gl_version: OpenGL ES 2.0 Mesa 7.12-devel
gl_vendor: Mesa Project
gl_extensions: GL_EXT_blend_minmax GL_EXT_multi_draw_arrays
GL_EXT_texture_filter_anisotropic GL_OES_depth24 GL_OES_element_index_uint
GL_OES_fbo_render_mipmap GL_OES_mapbuffer GL_OES_rgb8_rgba8
GL_OES_standard_derivatives GL_OES_stencil8 GL_OES_texture_3D
GL_OES_texture_npot GL_OES_depth_texture GL_OES_packed_depth_stencil
GL_EXT_texture_type_2_10_10_10_REV 
stamp: 20110727123343
Zero epsilon: 0.000122, RGBA epsilon: ( 0.004044, 0.004044, 0.004044, 0.000122)
copy_texture: Start
#+ Color Buffer Format is GL_RGB. Texture Format is GL_LUMINANCE. Color drawn
is (0.626, 0.25, 0.704, 0.716). Color read is (1, 1, 1, 1). Expected color is
(0.626, 0.626, 0.626, 1). Epsilon is (0.0668, 0.0668, 0.0668, 0.000122).
copy_texture: total = 1, failure = 1, #1

Regression FAILED 1 of 1 tests.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- 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


[Mesa-dev] [PATCH] glsl: improve the accuracy of the asin() builtin function.

2011-07-27 Thread Paul Berry
The previous formula for asin(x) was algebraically equivalent to:

sign(x)*(pi/2 - sqrt(1-|x|)*(A + B|x| + C|x|^2))

where A, B, and C were arbitrary constants determined by a curve fit.

This formula had a worst case absolute error of 0.00448, an unbounded
worst case relative error, and a discontinuity near x=0.

Changed the formula to:

sign(x)*(pi/2 - sqrt(1-|x|)*(pi/2 + (pi/4-1)|x| + A|x|^2 + B|x|^3))

where A and B are arbitrary constants determined by a curve fit.  This
has a worst case absolute error of 0.00039, a worst case relative
error of 0.000405, and no discontinuities.

I don't expect a significant performance degradation, since the extra
multiply-accumulate should be fast compared to the sqrt() computation.
---
 src/glsl/builtins/ir/asin |   68 ++--
 1 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/src/glsl/builtins/ir/asin b/src/glsl/builtins/ir/asin
index e230ad6..45d9e67 100644
--- a/src/glsl/builtins/ir/asin
+++ b/src/glsl/builtins/ir/asin
@@ -5,23 +5,26 @@
  ((return (expression float *
   (expression float sign (var_ref x))
   (expression float -
-   (expression float *
-(constant float (3.1415926))
-(constant float (0.5)))
+   (constant float (1.5707964))
(expression float *
 (expression float sqrt
  (expression float -
   (constant float (1.0))
   (expression float abs (var_ref x
 (expression float +
- (constant float (1.5707288))
+ (constant float (1.5707964))
  (expression float *
   (expression float abs (var_ref x))
   (expression float +
-   (constant float (-0.2121144))
+   (constant float (-0.21460183))
(expression float *
-(constant float (0.0742610))
-(expression float abs (var_ref x
+(expression float abs (var_ref x))
+ (expression float +
+  (constant float (0.086566724))
+  (expression float *
+   (expression float abs (var_ref x))
+   (constant float (-0.03102955))
+
 
(signature vec2
  (parameters
@@ -29,23 +32,26 @@
  ((return (expression vec2 *
   (expression vec2 sign (var_ref x))
   (expression vec2 -
-   (expression float *
-(constant float (3.1415926))
-(constant float (0.5)))
+   (constant float (1.5707964))
(expression vec2 *
 (expression vec2 sqrt
  (expression vec2 -
   (constant float (1.0))
   (expression vec2 abs (var_ref x
 (expression vec2 +
- (constant float (1.5707288))
+ (constant float (1.5707964))
  (expression vec2 *
   (expression vec2 abs (var_ref x))
   (expression vec2 +
-   (constant float (-0.2121144))
+   (constant float (-0.21460183))
(expression vec2 *
-(constant float (0.0742610))
-(expression vec2 abs (var_ref x
+(expression vec2 abs (var_ref x))
+ (expression vec2 +
+  (constant float (0.086566724))
+  (expression vec2 *
+   (expression vec2 abs (var_ref x))
+   (constant float (-0.03102955))
+
 
(signature vec3
  (parameters
@@ -53,23 +59,26 @@
  ((return (expression vec3 *
   (expression vec3 sign (var_ref x))
   (expression vec3 -
-   (expression float *
-(constant float (3.1415926))
-(constant float (0.5)))
+   (constant float (1.5707964))
(expression vec3 *
 (expression vec3 sqrt
  (expression vec3 -
   (constant float (1.0))
   (expression vec3 abs (var_ref x
 (expression vec3 +
- (constant float (1.5707288))
+ (constant float (1.5707964))
  (expression vec3 *
   (expression vec3 abs (var_ref x))
   (expression vec3 +
-   (constant float (-0.2121144))
+   (constant float (-0.21460183))
(expression vec3 *
-(constant float (0.0742610))
-(expression vec3 abs (var_ref x
+(expression vec3 abs (var_ref x))
+ (expression vec3 +
+   

Re: [Mesa-dev] [PATCH] Add dependency generation for Mesa and GLSL dricore objects.

2011-07-27 Thread Christopher James Halse Rogers
On Fri, 2011-07-22 at 18:49 -0700, Eric Anholt wrote:
> ---
>  src/glsl/Makefile |1 +
>  src/mesa/Makefile |5 +++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/glsl/Makefile b/src/glsl/Makefile
> index 005b51d..c20a6c9 100644
> --- a/src/glsl/Makefile
> +++ b/src/glsl/Makefile
> @@ -164,6 +164,7 @@ depend: $(ALL_SOURCES) Makefile
>   rm -f depend
>   touch depend
>   $(MKDEP) $(MKDEP_OPTIONS) $(INCLUDES) $(ALL_SOURCES) 2> /dev/null
> + $(MKDEP) $(MKDEP_OPTIONS) -a -p $(DRICORE_OBJ_DIR)/ $(INCLUDES) 
> $(ALL_SOURCES) 2> /dev/null
>  
>  # Remove .o and backup files
>  clean: clean-dricore
> diff --git a/src/mesa/Makefile b/src/mesa/Makefile
> index a903a26..88f31b6 100644
> --- a/src/mesa/Makefile
> +++ b/src/mesa/Makefile
> @@ -12,11 +12,10 @@ DRICORE_OBJ_DIR := objs-dricore
>  include sources.mak
>  
>  # adjust object dirs
> +DRICORE_OBJECTS := $(addprefix $(DRICORE_OBJ_DIR)/, $(MESA_OBJECTS))
>  MESA_OBJECTS := $(addprefix $(MESA_OBJ_DIR)/, $(MESA_OBJECTS))
>  MESA_GALLIUM_OBJECTS := $(addprefix $(MESA_OBJ_DIR)/, 
> $(MESA_GALLIUM_OBJECTS))
>  
> -DRICORE_OBJECTS := $(addprefix $(DRICORE_OBJ_DIR)/, $(MESA_OBJECTS))
> -
>  # define preprocessor flags
>  MESA_CPPFLAGS := $(API_DEFINES) $(DEFINES)
>  
> @@ -124,6 +123,8 @@ depend: $(ALL_SOURCES)
>   @ touch depend
>   @$(MKDEP) $(MKDEP_OPTIONS) -p$(MESA_OBJ_DIR)/ $(MESA_CPPFLAGS) \
>   $(ALL_SOURCES) > /dev/null 2>/dev/null
> + @$(MKDEP) $(MKDEP_OPTIONS) -a -p$(DRICORE_OBJ_DIR)/ $(MESA_CPPFLAGS) \
> + $(ALL_SOURCES) > /dev/null 2>/dev/null
>  
>  ##
>  # Installation rules

Reviewed-By: Christopher James Halse Rogers



signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] glsl: improve the accuracy of the atan(x, y) builtin function.

2011-07-27 Thread Paul Berry
The previous formula for atan(x,y) returned a value of +/- pi whenever
|x|<0.0001, and used a formula based on atan(y/x) otherwise.  This
broke in cases where both x and y were small (e.g. atan(1e-5, 1e-5)).

This patch modifies the formula so that it returns a value of +/- pi
whenever |x|<1e-8*|y|, and uses the formula based on atan(y/x)
otherwise.
---
 src/glsl/builtins/ir/atan |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/src/glsl/builtins/ir/atan b/src/glsl/builtins/ir/atan
index cfecc1f..7b5ea13 100644
--- a/src/glsl/builtins/ir/atan
+++ b/src/glsl/builtins/ir/atan
@@ -54,7 +54,9 @@
 )
 (
   (declare () float r)
-  (if (expression bool > (expression float abs (var_ref x)) (constant 
float (0.000100))) (
+  (if (expression bool >
+   (expression float abs (var_ref x))
+   (expression float * (constant float (1.0e-8)) (expression float abs 
(var_ref y (
 (assign (x) (var_ref r) (call atan ((expression float / (var_ref y) 
(var_ref x)
 (if (expression bool < (var_ref x) (constant float (0.00)) ) (
   (if (expression bool >= (var_ref y) (constant float (0.00)) )
-- 
1.7.6

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


Re: [Mesa-dev] [PATCH] i965/fs: Respect ARB_color_buffer_float clamping.

2011-07-27 Thread Kenneth Graunke
On 07/25/2011 07:08 PM, Eric Anholt wrote:
> This was done in the old codegen path, but not the new one.
> ---
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |   21 +++--
>  1 files changed, 15 insertions(+), 6 deletions(-)

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


[Mesa-dev] [PATCH] i965/gen4: Fix message parameter loading for 1D TXD sampling.

2011-07-27 Thread Kenneth Graunke
We were neglecting to load dvdx and dvdy.  v is not optional.

Fixes piglit tests tex-grad-0[12345].frag on Broadwater/Crestline.

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 9632aae..c6ed02c 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -633,14 +633,14 @@ fs_visitor::emit_texture_gen4(ir_texture *ir, fs_reg dst, 
fs_reg coordinate,
   for (int i = 0; i < ir->lod_info.grad.dPdx->type->vector_elements; i++) {
 emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen), dPdx);
 dPdx.reg_offset++;
-mlen++;
   }
+  mlen += MAX2(ir->lod_info.grad.dPdx->type->vector_elements, 2);
 
   for (int i = 0; i < ir->lod_info.grad.dPdy->type->vector_elements; i++) {
 emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen), dPdy);
 dPdy.reg_offset++;
-mlen++;
   }
+  mlen += MAX2(ir->lod_info.grad.dPdy->type->vector_elements, 2);
} else {
   /* Oh joy.  gen4 doesn't have SIMD8 non-shadow-compare bias/lod
* instructions.  We'll need to do SIMD16 here.
-- 
1.7.6

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


[Mesa-dev] [PATCH v3] i965: Check actual tile offsets in Gen4 miptree workaround.

2011-07-27 Thread Kenneth Graunke
The purpose of the (irb->draw_offset & 4095) != 0 check was to ensure
that we don't have XYy offsets into a tile, since Gen4 hardware doesn't
support that.  However, it's insufficient: there are cases where
draw_offset & 4095 is 0 but we still have a Y-offset.  This leads to an
assertion failure in brw_update_renderbuffer_surface with tile_y != 0.

Instead, simply call intel_renderbuffer_tile_offsets to compute the
actual X/Y offsets and check if either are non-zero.  This makes both
the workaround and the assertion check the same things.

Fixes piglit test fbo-generatemipmap-formats, and should also fix
bugs #34009 and #39487.

NOTE: This is a candidate for stable release branches.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=34009
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=39487
Cc: Chad Versace 
Cc: Eric Anholt 
Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/intel/intel_fbo.c |   26 --
 1 files changed, 24 insertions(+), 2 deletions(-)

v3: The depth wrapper chasing apparently broke on Gen4.  Hopefully this one
should work on all platforms...

diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c 
b/src/mesa/drivers/dri/intel/intel_fbo.c
index 35be325..4c0b378 100644
--- a/src/mesa/drivers/dri/intel/intel_fbo.c
+++ b/src/mesa/drivers/dri/intel/intel_fbo.c
@@ -647,6 +647,29 @@ intel_renderbuffer_tile_offsets(struct intel_renderbuffer 
*irb,
}
 }
 
+#ifndef I915
+static bool
+need_tile_offset_workaround(struct brw_context *brw,
+   struct intel_renderbuffer *irb)
+{
+   uint32_t tile_x, tile_y;
+
+   /* intel_renderbuffer_tile_offsets needs a non-NULL region, but
+* combined depth/stencil wrappers don't have one.  Resolve irb to
+* the actual depth buffer first.
+*/
+   if (irb->wrapped_depth != NULL)
+  irb = irb->wrapped_depth;
+
+   intel_renderbuffer_tile_offsets(irb, &tile_x, &tile_y);
+
+   if (!brw->has_surface_tile_offset)
+  return tile_x != 0 || tile_y != 0;
+
+   return false;
+}
+#endif
+
 /**
  * Called by glFramebufferTexture[123]DEXT() (and other places) to
  * prepare for rendering into texture memory.  This might be called
@@ -700,8 +723,7 @@ intel_render_texture(struct gl_context * ctx,
intel_image->used_as_render_target = GL_TRUE;
 
 #ifndef I915
-   if (!brw_context(ctx)->has_surface_tile_offset &&
-   (irb->draw_offset & 4095) != 0) {
+   if (need_tile_offset_workaround(brw_context(ctx), irb)) {
   /* Original gen4 hardware couldn't draw to a non-tile-aligned
* destination in a miptree unless you actually setup your
* renderbuffer as a miptree and used the fragile
-- 
1.7.6

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


Re: [Mesa-dev] [PATCH] glsl: improve the accuracy of the radians() builtin function

2011-07-27 Thread Kenneth Graunke
On 07/27/2011 11:08 AM, Paul Berry wrote:
> The constant used in the radians() function didn't have enough
> precision, causing a relative error of 1.676e-5, which is far worse
> than the precision of 32-bit floats.  This patch reduces the relative
> error to 1.14e-9, which is the best we can do in 32 bits.
> 
> Fixes piglit tests {fs,vs}-radians-{float,vec2,vec3,vec4}.

Reviewed-by: Kenneth Graunke 

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


Re: [Mesa-dev] [PATCH] glsl: improve the accuracy of the asin() builtin function.

2011-07-27 Thread Paul Berry
I forgot to mention:

Fixes piglit tests {vs,fs}-asin-float and {vs,fs}-atan-*

On 27 July 2011 15:15, Paul Berry  wrote:
> The previous formula for asin(x) was algebraically equivalent to:
>
> sign(x)*(pi/2 - sqrt(1-|x|)*(A + B|x| + C|x|^2))
>
> where A, B, and C were arbitrary constants determined by a curve fit.
>
> This formula had a worst case absolute error of 0.00448, an unbounded
> worst case relative error, and a discontinuity near x=0.
>
> Changed the formula to:
>
> sign(x)*(pi/2 - sqrt(1-|x|)*(pi/2 + (pi/4-1)|x| + A|x|^2 + B|x|^3))
>
> where A and B are arbitrary constants determined by a curve fit.  This
> has a worst case absolute error of 0.00039, a worst case relative
> error of 0.000405, and no discontinuities.
>
> I don't expect a significant performance degradation, since the extra
> multiply-accumulate should be fast compared to the sqrt() computation.
> ---
>  src/glsl/builtins/ir/asin |   68 ++--
>  1 files changed, 40 insertions(+), 28 deletions(-)
>
> diff --git a/src/glsl/builtins/ir/asin b/src/glsl/builtins/ir/asin
> index e230ad6..45d9e67 100644
> --- a/src/glsl/builtins/ir/asin
> +++ b/src/glsl/builtins/ir/asin
> @@ -5,23 +5,26 @@
>      ((return (expression float *
>               (expression float sign (var_ref x))
>               (expression float -
> -               (expression float *
> -                (constant float (3.1415926))
> -                (constant float (0.5)))
> +               (constant float (1.5707964))
>                (expression float *
>                 (expression float sqrt
>                  (expression float -
>                   (constant float (1.0))
>                   (expression float abs (var_ref x
>                 (expression float +
> -                 (constant float (1.5707288))
> +                 (constant float (1.5707964))
>                  (expression float *
>                   (expression float abs (var_ref x))
>                   (expression float +
> -                   (constant float (-0.2121144))
> +                   (constant float (-0.21460183))
>                    (expression float *
> -                    (constant float (0.0742610))
> -                    (expression float abs (var_ref x
> +                    (expression float abs (var_ref x))
> +                     (expression float +
> +                      (constant float (0.086566724))
> +                      (expression float *
> +                       (expression float abs (var_ref x))
> +                       (constant float (-0.03102955))
> +                    
>
>    (signature vec2
>      (parameters
> @@ -29,23 +32,26 @@
>      ((return (expression vec2 *
>               (expression vec2 sign (var_ref x))
>               (expression vec2 -
> -               (expression float *
> -                (constant float (3.1415926))
> -                (constant float (0.5)))
> +               (constant float (1.5707964))
>                (expression vec2 *
>                 (expression vec2 sqrt
>                  (expression vec2 -
>                   (constant float (1.0))
>                   (expression vec2 abs (var_ref x
>                 (expression vec2 +
> -                 (constant float (1.5707288))
> +                 (constant float (1.5707964))
>                  (expression vec2 *
>                   (expression vec2 abs (var_ref x))
>                   (expression vec2 +
> -                   (constant float (-0.2121144))
> +                   (constant float (-0.21460183))
>                    (expression vec2 *
> -                    (constant float (0.0742610))
> -                    (expression vec2 abs (var_ref x
> +                    (expression vec2 abs (var_ref x))
> +                     (expression vec2 +
> +                      (constant float (0.086566724))
> +                      (expression vec2 *
> +                       (expression vec2 abs (var_ref x))
> +                       (constant float (-0.03102955))
> +                    
>
>    (signature vec3
>      (parameters
> @@ -53,23 +59,26 @@
>      ((return (expression vec3 *
>               (expression vec3 sign (var_ref x))
>               (expression vec3 -
> -               (expression float *
> -                (constant float (3.1415926))
> -                (constant float (0.5)))
> +               (constant float (1.5707964))
>                (expression vec3 *
>                 (expression vec3 sqrt
>                  (expression vec3 -
>                   (constant float (1.0))
>                   (expression vec3 abs (var_ref x
>                 (expression vec3 +
> -                 (constant float (1.5707288))
> +                 (constant float (1.5707964))
>                  (expression vec3 *
>                   (expression vec3 abs (var_ref x))
>                   (expression vec3 +
> -                   (constant float (-0.2121144))
> +                   (

Re: [Mesa-dev] RFC: ctx->Driver.Map/UnmapTextureImage() hooks

2011-07-27 Thread Eric Anholt
On Sat, 23 Jul 2011 11:58:22 -0600, Brian Paul  wrote:
> On Sat, Jul 23, 2011 at 9:14 AM, Eric Anholt  wrote:
> > On Fri, 22 Jul 2011 14:06:48 -0600, Brian Paul  wrote:
> >> On 07/22/2011 01:32 PM, Eric Anholt wrote:
> >> > On Thu, 23 Jun 2011 19:08:51 -0600, Brian Paul  wrote:
> >> >>
> >> >> I'd like to overhaul the part of Mesa related to texture memory
> >> >> reading/writing.
> >> >
> >> > OK, I'm taking a look at map-texture-image-v4.  I like what I'm seeing
> >> > overall, I just want to be sure that this isn't something that gets
> >> > squash-merged.  There's going to be breakage, and I want to be able to
> >> > bisect into it.
> >> >
> >> > In the metaops code, please use glBufferData instead of
> >> > glBufferSubData.  If you BufferSubData, I have to block on the GPU if it
> >> > was using that buffer already.
> >>
> >> It looks like we'd have to change that in several other places too.
> >> Can we do that change later?
> >>
> >>
> >> > In the comments for void (*MapTextureImage), please note what the units
> >> > of rowStride are.  I see that's present in swrast later, but I think the
> >> > mtypes.h and dd.h files are used for reference a lot (I do, certainly).
> >>
> >> Will do.  The parameter comments in s_texture.c are out of date too.
> >>
> >>
> >> > c029312ad62039904818a8b1765c6bcdf50044df is huge, and it doesn't even
> >> > build.  Ouch.  I think there's some room for splitting some of this up
> >> > so that we can get a nice series.
> >>
> >> Where's the build breakage?  I don't remember that.
> >>
> >> This was originally a long series of sometimes ugly WIP patches.  At
> >> one point I had a git mishap and trashed some of the intermediate
> >> patches.  I agree that splitting up this commit would be good, but it
> >> would be a lot of work that I don't really have time for.
> >>
> >> It would be great if you could do a full piglit run with the branch
> >> and check for i965/i915 regressions.  I'm not aware of any with swrast
> >> or gallium.  I'd help diagnose any regressions.
> >
> > The piglit run was in bad shape and then hung the GPU, something that
> > piglit hasn't done for me in a long time.
> >
> > I think I'm going to need to split up the commits to make progress.
> 
> OK, I'll try to find some time to do a piglit run on my i945 and see
> what's up (I don't have a i965 handy).  I may not get to it for a few
> days though.

I'm cutting up your patches into a reasonable series so that we can
actually review and bisect the code.  So far I've run my mti-tested
branch on snb, and mti is your code I'm rebasing onto it as I cut chunks
out.


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


Re: [Mesa-dev] RFC: ctx->Driver.Map/UnmapTextureImage() hooks

2011-07-27 Thread Brian Paul
On Wed, Jul 27, 2011 at 7:29 PM, Eric Anholt  wrote:
> On Sat, 23 Jul 2011 11:58:22 -0600, Brian Paul  wrote:
>> On Sat, Jul 23, 2011 at 9:14 AM, Eric Anholt  wrote:
>> > On Fri, 22 Jul 2011 14:06:48 -0600, Brian Paul  wrote:
>> >> On 07/22/2011 01:32 PM, Eric Anholt wrote:
>> >> > On Thu, 23 Jun 2011 19:08:51 -0600, Brian Paul  
>> >> > wrote:
>> >> >>
>> >> >> I'd like to overhaul the part of Mesa related to texture memory
>> >> >> reading/writing.
>> >> >
>> >> > OK, I'm taking a look at map-texture-image-v4.  I like what I'm seeing
>> >> > overall, I just want to be sure that this isn't something that gets
>> >> > squash-merged.  There's going to be breakage, and I want to be able to
>> >> > bisect into it.
>> >> >
>> >> > In the metaops code, please use glBufferData instead of
>> >> > glBufferSubData.  If you BufferSubData, I have to block on the GPU if it
>> >> > was using that buffer already.
>> >>
>> >> It looks like we'd have to change that in several other places too.
>> >> Can we do that change later?
>> >>
>> >>
>> >> > In the comments for void (*MapTextureImage), please note what the units
>> >> > of rowStride are.  I see that's present in swrast later, but I think the
>> >> > mtypes.h and dd.h files are used for reference a lot (I do, certainly).
>> >>
>> >> Will do.  The parameter comments in s_texture.c are out of date too.
>> >>
>> >>
>> >> > c029312ad62039904818a8b1765c6bcdf50044df is huge, and it doesn't even
>> >> > build.  Ouch.  I think there's some room for splitting some of this up
>> >> > so that we can get a nice series.
>> >>
>> >> Where's the build breakage?  I don't remember that.
>> >>
>> >> This was originally a long series of sometimes ugly WIP patches.  At
>> >> one point I had a git mishap and trashed some of the intermediate
>> >> patches.  I agree that splitting up this commit would be good, but it
>> >> would be a lot of work that I don't really have time for.
>> >>
>> >> It would be great if you could do a full piglit run with the branch
>> >> and check for i965/i915 regressions.  I'm not aware of any with swrast
>> >> or gallium.  I'd help diagnose any regressions.
>> >
>> > The piglit run was in bad shape and then hung the GPU, something that
>> > piglit hasn't done for me in a long time.
>> >
>> > I think I'm going to need to split up the commits to make progress.
>>
>> OK, I'll try to find some time to do a piglit run on my i945 and see
>> what's up (I don't have a i965 handy).  I may not get to it for a few
>> days though.
>
> I'm cutting up your patches into a reasonable series so that we can
> actually review and bisect the code.  So far I've run my mti-tested
> branch on snb, and mti is your code I'm rebasing onto it as I cut chunks
> out.

Thanks for doing that.

I found that the map-texture-image-v4 branch is missing some stuff
that I lost during my git accident.  I think I'm nearly done restoring
it.  I'll push it to the branch asap, maybe tonight.

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


[Mesa-dev] [Bug 29012] gallium: drivers should respect force_s3tc_enable option (and maybe other) as classic driver

2011-07-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=29012

Bryan Cain  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution||FIXED

--- Comment #4 from Bryan Cain  2011-07-27 20:37:59 
PDT ---
This is fixed by commits 95739f19c and 860c51d82711 in master for all Gallium
drivers using u_format.  I've also pushed them to the 7.11 branch.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- 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


[Mesa-dev] [PATCH 0/5] Fix implicit type conversion errors

2011-07-27 Thread Chad Versace
Fixes Piglit tests:
spec/glsl-1.20/compiler/built-in-functions/outerProduct-bvec*.vert

spec/glsl-1.20/compiler/structure-and-array-operations/array-ctor-implicit-conversion-bool-float.vert

spec/glsl-1.20/compiler/structure-and-array-operations/array-ctor-implicit-conversion-bvec*-vec*.vert

Chad Versace (5):
  glsl: Add method glsl_type::can_implicitly_convert_to()
  glsl: Fix implicit conversions in non-constructor function calls
  glsl: Remove ir_function.cpp:type_compare()
  glsl: Fix conversions in array constructors
  glsl: Clarify ir_function::matching_sigature()

 src/glsl/ast_function.cpp |   12 -
 src/glsl/glsl_types.cpp   |   15 +
 src/glsl/glsl_types.h |   40 ++
 src/glsl/ir_function.cpp  |  132 +++-
 4 files changed, 110 insertions(+), 89 deletions(-)

-- 
1.7.6

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


[Mesa-dev] [PATCH 1/5] glsl: Add method glsl_type::can_implicitly_convert_to()

2011-07-27 Thread Chad Versace
This method checks if a source type is identical to or can be implicitly
converted to a target type according to the GLSL 1.20 spec, Section 4.1.10
Implicit Conversions.

The following commits use the method for a bugfix:
glsl: Fix implicit conversions in non-constructor function calls
glsl: Fix implicit conversions in array constructors

Note: This is a candidate for the 7.10 and 7.11 branches.
CC: Ian Romanick 
CC: Kenneth Graunke 
Signed-off-by: Chad Versace 
---
 src/glsl/glsl_types.cpp |   30 ++
 src/glsl/glsl_types.h   |   40 
 2 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index a5e21bb..9cfdcab 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -523,3 +523,33 @@ glsl_type::component_slots() const
   return 0;
}
 }
+
+bool
+glsl_type::can_implicitly_convert_to(const glsl_type *desired) const
+{
+   if (this == desired)
+  return true;
+
+   /* There is no conversion among matrix types. */
+   if (this->matrix_columns != 1 || desired->matrix_columns != 1)
+  return false;
+
+   switch (desired->base_type) {
+   case GLSL_TYPE_UINT:
+   case GLSL_TYPE_INT:
+   case GLSL_TYPE_BOOL:
+   case GLSL_TYPE_SAMPLER:
+   case GLSL_TYPE_STRUCT:
+   case GLSL_TYPE_VOID:
+   case GLSL_TYPE_ERROR:
+  return false;
+
+   case GLSL_TYPE_FLOAT:
+  return this->is_integer()
+&& this->vector_elements == desired->vector_elements;
+
+   default:
+  assert(0);
+  return false;
+   }
+}
diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
index 87f57e7..6add33e 100644
--- a/src/glsl/glsl_types.h
+++ b/src/glsl/glsl_types.h
@@ -224,6 +224,46 @@ struct glsl_type {
 */
unsigned component_slots() const;
 
+   /**
+* \brief Can this type be implicitly converted to another?
+*
+* \return True if the types are identical or if this type can be converted
+* to \c desired according to Section 4.1.10 of the GLSL spec.
+*
+* \verbatim
+* From page 25 (31 of the pdf) of the GLSL 1.50 spec, Section 4.1.10
+* Implicit Conversions:
+*
+* In some situations, an expression and its type will be implicitly
+* converted to a different type. The following table shows all allowed
+* implicit conversions:
+*
+* Type of expression | Can be implicitly converted to
+* --
+* int  float
+* uint
+*
+* ivec2vec2
+* uvec2
+*
+* ivec3vec3
+* uvec3
+*
+* ivec4vec4
+* uvec4
+*
+* There are no implicit array or structure conversions. For example,
+* an array of int cannot be implicitly converted to an array of float.
+* There are no implicit conversions between signed and unsigned
+* integers.  When an implicit conversion is done, it is not a
+* re-interpretation of the expression's bit pattern, but a conversion
+* of its value to an equivalent value in the new type. For example,
+* the integer value -5 will be converted to the floating-point value
+* -5.0. Integer values having more bits of precision than a floating
+* point mantissa will lose precision when converted to float.
+* \endverbatim
+*/
+   bool can_implicitly_convert_to(const glsl_type *desired) const;
 
/**
 * Query whether or not a type is a scalar (non-vector and non-matrix).
-- 
1.7.6

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


[Mesa-dev] [PATCH 2/5] glsl: Fix implicit conversions in non-constructor function calls

2011-07-27 Thread Chad Versace
Context
---
In ast_function_expression::hir(), parameter_lists_match() checks if the
function call's actual parameter list matches the signature's parameter
list, where the match may require implicit conversion of some arguments.
To check if an implicit conversion exists between individual arguments,
type_compare() is used.

Problems

type_compare() allowed the following illegal implicit conversions:
bool -> float
bvecN -> vecN

int -> uint
ivecN -> uvecN

uint -> int
uvecN -> ivecN

Change
--
type_compare() is buggy, so replace it with 
glsl_type::can_be_implicitly_converted_to().
This comprises a rewrite of parameter_lists_match().

Fixes Piglit spec/glsl-1.20/compiler/built-in-functions/outerProduct-bvec*.vert

Note: This is a candidate for the 7.10 and 7.11 branches.
CC: Ian Romanick 
CC: Kenneth Graunke 
Signed-off-by: Chad Versace 
---
 src/glsl/glsl_types.cpp  |   23 +++
 src/glsl/ir_function.cpp |   52 +
 2 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index 9cfdcab..3b97aa9 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -531,25 +531,10 @@ glsl_type::can_implicitly_convert_to(const glsl_type 
*desired) const
   return true;
 
/* There is no conversion among matrix types. */
-   if (this->matrix_columns != 1 || desired->matrix_columns != 1)
+   if (this->matrix_columns > 1 || desired->matrix_columns > 1)
   return false;
 
-   switch (desired->base_type) {
-   case GLSL_TYPE_UINT:
-   case GLSL_TYPE_INT:
-   case GLSL_TYPE_BOOL:
-   case GLSL_TYPE_SAMPLER:
-   case GLSL_TYPE_STRUCT:
-   case GLSL_TYPE_VOID:
-   case GLSL_TYPE_ERROR:
-  return false;
-
-   case GLSL_TYPE_FLOAT:
-  return this->is_integer()
-&& this->vector_elements == desired->vector_elements;
-
-   default:
-  assert(0);
-  return false;
-   }
+   return desired->is_float()
+  && this->is_integer()
+  && this->vector_elements == desired->vector_elements;
 }
diff --git a/src/glsl/ir_function.cpp b/src/glsl/ir_function.cpp
index 0f2f1a0..56d50a8 100644
--- a/src/glsl/ir_function.cpp
+++ b/src/glsl/ir_function.cpp
@@ -85,12 +85,25 @@ type_compare(const glsl_type *a, const glsl_type *b)
 }
 
 
+/**
+ * \brief Check if two parameter lists match.
+ *
+ * \param list_a Parameters of the function definition.
+ * \param list_b Actual parameters passed to the function.
+ * \return If an exact match, return 0.
+ * If an inexact match requiring implicit conversion, return 1.
+ * If not a match, return -1.
+ * \see matching_signature()
+ */
 static int
 parameter_lists_match(const exec_list *list_a, const exec_list *list_b)
 {
const exec_node *node_a = list_a->head;
const exec_node *node_b = list_b->head;
-   int total_score = 0;
+
+   /* This is set to true if there is an inexact match requiring an implicit
+* conversion. */
+   bool inexact_match = false;
 
for (/* empty */
; !node_a->is_tail_sentinel()
@@ -106,12 +119,11 @@ parameter_lists_match(const exec_list *list_a, const 
exec_list *list_b)
   const ir_variable *const param = (ir_variable *) node_a;
   const ir_instruction *const actual = (ir_instruction *) node_b;
 
-  /* Determine whether or not the types match.  If the types are an
-   * exact match, the match score is zero.  If the types don't match
-   * but the actual parameter can be coerced to the type of the declared
-   * parameter, the match score is one.
-   */
-  int score;
+  if (param->type == actual->type)
+continue;
+
+  /* Try to find an implicit conversion from actual to param. */
+  inexact_match = true;
   switch ((enum ir_variable_mode)(param->mode)) {
   case ir_var_auto:
   case ir_var_uniform:
@@ -125,29 +137,28 @@ parameter_lists_match(const exec_list *list_a, const 
exec_list *list_b)
 
   case ir_var_const_in:
   case ir_var_in:
-score = type_compare(param->type, actual->type);
-break;
+if (actual->type->can_implicitly_convert_to(param->type))
+   continue;
+else
+   return -1;
 
   case ir_var_out:
-score = type_compare(actual->type, param->type);
-break;
+if (param->type->can_implicitly_convert_to(actual->type))
+   continue;
+else
+   return -1;
 
   case ir_var_inout:
 /* Since there are no bi-directional automatic conversions (e.g.,
  * there is int -> float but no float -> int), inout parameters must
  * be exact matches.
  */
-score = (type_compare(actual->type, param->type) == 0) ? 0 : -1;
-break;
+return -1;
 
   default:
 assert(false);
-  }
-
-  if (score < 0)
 return -1;
-
-  total_score += score;
+  }
}
 
/* If all of the parameters from the othe

[Mesa-dev] [PATCH 3/5] glsl: Remove ir_function.cpp:type_compare()

2011-07-27 Thread Chad Versace
The function is no longer used and has been replaced by
glsl_type::can_implicitly_convert_to().

Note: This is a candidate for the 7.10 and 7.11 branches.
CC: Ian Romanick 
CC: Kenneth Graunke 
Signed-off-by: Chad Versace 
---
 src/glsl/ir_function.cpp |   61 --
 1 files changed, 0 insertions(+), 61 deletions(-)

diff --git a/src/glsl/ir_function.cpp b/src/glsl/ir_function.cpp
index 56d50a8..e25b094 100644
--- a/src/glsl/ir_function.cpp
+++ b/src/glsl/ir_function.cpp
@@ -24,67 +24,6 @@
 #include "glsl_types.h"
 #include "ir.h"
 
-int
-type_compare(const glsl_type *a, const glsl_type *b)
-{
-   /* If the types are the same, they trivially match.
-*/
-   if (a == b)
-  return 0;
-
-   switch (a->base_type) {
-   case GLSL_TYPE_UINT:
-   case GLSL_TYPE_INT:
-   case GLSL_TYPE_BOOL:
-  /* There is no implicit conversion to or from integer types or bool.
-   */
-  if ((a->is_integer() != b->is_integer())
- || (a->is_boolean() != b->is_boolean()))
-return -1;
-
-  /* FALLTHROUGH */
-
-   case GLSL_TYPE_FLOAT:
-  if ((a->vector_elements != b->vector_elements)
- || (a->matrix_columns != b->matrix_columns))
-return -1;
-
-  return 1;
-
-   case GLSL_TYPE_SAMPLER:
-   case GLSL_TYPE_STRUCT:
-  /* Samplers and structures must match exactly.
-   */
-  return -1;
-
-   case GLSL_TYPE_ARRAY:
-  if ((b->base_type != GLSL_TYPE_ARRAY)
- || (a->length != b->length))
-return -1;
-
-  /* From GLSL 1.50 spec, page 27 (page 33 of the PDF):
-   *"There are no implicit array or structure conversions."
-   *
-   * If the comparison of the array element types detects that a conversion
-   * would be required, the array types do not match.
-   */
-  return (type_compare(a->fields.array, b->fields.array) == 0) ? 0 : -1;
-
-   case GLSL_TYPE_VOID:
-   case GLSL_TYPE_ERROR:
-   default:
-  /* These are all error conditions.  It is invalid for a parameter to
-   * a function to be declared as error, void, or a function.
-   */
-  return -1;
-   }
-
-   /* This point should be unreachable.
-*/
-   assert(0);
-}
-
-
 /**
  * \brief Check if two parameter lists match.
  *
-- 
1.7.6

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


[Mesa-dev] [PATCH 4/5] glsl: Fix conversions in array constructors

2011-07-27 Thread Chad Versace
Array constructors obey narrower conversion rules than other constructors
[1] --- they use the implicit conversion rules [2] instead of the scalar
constructor conversions [3].  But process_array_constructor() was
incorrectly applying the broader rules.

To fix this, first check (with glsl_type::can_be_implicitly_converted_to)
if an implicit conversion is legal before performing the conversion.

[1] GLSL 1.50 spec, Section 5.4.4 Array Constructors, page 52 (58 of pdf)
[2] GLSL 1.50 spec, Section 4.1.10 Implicit Conversions, page 25 (31 of pdf)
[3] GLSL 1.50 spec, Section 5.4.1 Conversion, page 48 (54 of pdf)

Fixes Piglit tests:

spec/glsl-1.20/compiler/structure-and-array-operations/array-ctor-implicit-conversion-bool-float.vert

spec/glsl-1.20/compiler/structure-and-array-operations/array-ctor-implicit-conversion-bvec*-vec*.vert

Note: This is a candidate for the 7.10 and 7.11 branches.
CC: Ian Romanick 
CC: Kenneth Graunke 
Signed-off-by: Chad Versace 
---
 src/glsl/ast_function.cpp |   12 ++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index bdb73f4..9bee3eb 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -442,13 +442,21 @@ process_array_constructor(exec_list *instructions,
   ir_rvalue *ir = (ir_rvalue *) n;
   ir_rvalue *result = ir;
 
-  /* Apply implicit conversions (not the scalar constructor rules!) */
+  /* Apply implicit conversions (not the scalar constructor rules!). See
+   * the spec quote above. */
   if (constructor_type->element_type()->is_float()) {
 const glsl_type *desired_type =
glsl_type::get_instance(GLSL_TYPE_FLOAT,
ir->type->vector_elements,
ir->type->matrix_columns);
-result = convert_component(ir, desired_type);
+if (result->type->can_implicitly_convert_to(desired_type)) {
+   /* Even though convert_component() implements the scalar
+* conversion rules, not the implicit conversion rules, its safe
+* to use it here because we already checked that the implicit
+* conversion is legal.
+*/
+   result = convert_component(ir, desired_type);
+}
   }
 
   if (result->type != constructor_type->element_type()) {
-- 
1.7.6

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


[Mesa-dev] [PATCH 5/5] glsl: Clarify ir_function::matching_sigature()

2011-07-27 Thread Chad Versace
The function used a variable named 'score', which was an outright lie.
A signature matches or it doesn't; there is no fuzzy scoring.

CC: Ian Romanick 
CC: Kenneth Graunke 
Signed-off-by: Chad Versace 
---
 src/glsl/ir_function.cpp |   19 ---
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/glsl/ir_function.cpp b/src/glsl/ir_function.cpp
index e25b094..5a2c9bb 100644
--- a/src/glsl/ir_function.cpp
+++ b/src/glsl/ir_function.cpp
@@ -134,18 +134,23 @@ ir_function::matching_signature(const exec_list 
*actual_parameters)
   ir_function_signature *const sig =
 (ir_function_signature *) iter.get();
 
-  const int score = parameter_lists_match(& sig->parameters,
- actual_parameters);
-
-  /* If we found an exact match, simply return it */
-  if (score == 0)
+  switch (parameter_lists_match(& sig->parameters, actual_parameters)) {
+  case 0:
+/* Exact match. */
 return sig;
-
-  if (score > 0) {
+  case 1:
+/* Inexact match that requires implicit conversion. */
 if (match == NULL)
match = sig;
 else
multiple_inexact_matches = true;
+continue;
+  case -1:
+/* Not a match. */
+continue;
+  default:
+assert(false);
+return NULL;
   }
}
 
-- 
1.7.6

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