Re: [Mesa-dev] [PATCH] gallium/util: Don't use __builtin_clrsb in util_last_bit().

[Erik Faye-Lund]

On Tue, Feb 3, 2015 at 7:58 PM, Matt Turner matts...@gmail.com wrote:
 On Tue, Feb 3, 2015 at 6:13 AM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Tue, Feb 3, 2015 at 2:28 AM, Matt Turner matts...@gmail.com wrote:
 Unclear circumstances lead to undefined symbols on x86.

 Bugzilla: https://bugs.gentoo.org/show_bug.cgi?id=536916

 A comment[1] on the gentoo bug-tracker suggest doing something along
 these lines instead (untested)...

 [1]: https://bugs.gentoo.org/show_bug.cgi?id=536916#c5

 That was me, and it didn't seem to solve his problem.

Wow, I should have checked that, sorry for the noise :P
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] nir/validate: Validate that only float ALU outputs are saturated

[Erik Faye-Lund]

On Tue, Feb 3, 2015 at 9:43 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 ---
  src/glsl/nir/nir_validate.c | 8 
  1 file changed, 8 insertions(+)

 diff --git a/src/glsl/nir/nir_validate.c b/src/glsl/nir/nir_validate.c
 index 7c801b2..89dfdf8 100644
 --- a/src/glsl/nir/nir_validate.c
 +++ b/src/glsl/nir/nir_validate.c
 @@ -239,6 +239,14 @@ validate_alu_dest(nir_alu_dest *dest, validate_state 
 *state)
  * register/SSA value
  */
 assert(is_packed || !(dest-write_mask  ~((1  dest_size) - 1)));
 +
 +   /* validate that saturate is only ever used on instructions with
 +* destinations of type float
 +*/
 +   nir_alu_instr *alu = nir_instr_as_alu(state-instr);
 +   assert(nir_op_infos[alu-op].output_type == nir_type_float ||
 +  !dest-saturate);

I think this can end up generating a warning on builds with asserts
disabled due to alu being written but never read. Perhaps just do
nir_instr_as_alu(state-instr)-op directly in the expression? It's
a tad less readable, though :/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 01/11] mesa: Returns a GL_INVALID_VALUE error on several glGet* APIs when max length is negative

[Erik Faye-Lund]

On Tue, Jan 20, 2015 at 3:54 AM, Ian Romanick i...@freedesktop.org wrote:
 On 01/19/2015 04:35 AM, Erik Faye-Lund wrote:
 On Mon, Jan 19, 2015 at 1:31 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Mon, Jan 19, 2015 at 12:32 PM, Eduardo Lima Mitev el...@igalia.com 
 wrote:
 The manual page for glGetAttachedShaders, glGetShaderSource, 
 glGetActiveUniform and
 glGetActiveUniform state that a GL_INVALID_VALUE is returned if the 
 maximum length
 argument is less than zero. For reference, see:
 https://www.opengl.org/sdk/docs/man3/xhtml/glGetAttachedShaders.xml,
 https://www.khronos.org/opengles/sdk/docs/man31/html/glGetAttachedShaders.xhtml,
 https://www.opengl.org/sdk/docs/man3/xhtml/glGetShaderSource.xml,
 https://www.khronos.org/opengles/sdk/docs/man31/html/glGetShaderSource.xhtml,
 https://www.opengl.org/sdk/docs/man3/xhtml/glGetActiveUniform.xml,
 https://www.khronos.org/opengles/sdk/docs/man31/html/glGetActiveUniform.xhtml,
 https://www.opengl.org/sdk/docs/man3/xhtml/glGetActiveAttrib.xml,
 https://www.khronos.org/opengles/sdk/docs/man31/html/glGetActiveAttrib.xhtml.

 This fixes 4 dEQP test:
 * dEQP-GLES3.functional.negative_api.state.get_attached_shaders
 * dEQP-GLES3.functional.negative_api.state.get_shader_source
 * dEQP-GLES3.functional.negative_api.state.get_active_uniform
 * dEQP-GLES3.functional.negative_api.state.get_active_attrib

 These tests are about GLES3, but I cannot find such behavior specified
 in the OpenGL ES 3.0 specification for GetAttachedShaders nor
 GetProgramBinary. I stopped checking after those two, because I felt I
 saw a pattern ;)

 The man pages you linked to are for desktop-OpenGL, which *does*
 specify such an error; OpenGL 4.5 spec says An INVALID_VALUE error is
 generated if maxCount is negative about GetAttachedShaders (and a
 similar for GetProgramBinary()).

 However, the GLES 3 man pages also list such an error:
 https://www.khronos.org/opengles/sdk/docs/man3/html/glGetAttachedShaders.xhtml

 Generally speaking, the manual pages are not considered to dictate
 behavior, only to be a programmer convenience. And they have
 historically been full of errors. However, since desktop GL *does*
 specify these errors *and* the man-pages document them, this does look
 like a spec-error for GLES 3 to me. But I think we should get a
 clarification from Khronos before assuming so, otherwise we won't be
 in conformance. Ian, any thoughts?

 By the way, this error is also properly defined for OpenGL ES 3.1, so
 I'm feeling even more confident that it's a spec-bug in OpenGL ES 3.0.

 There's a general statement in the Errors section that passing a
 negative value for a GLsizei or GLsizeiptr is always an INVALID_VALUE
 error.  That exists in all specs back to OpenGL 1.0. :)  Since it's in a
 different place, it is often overlooked (like when we wrote these
 functions in Mesa).

Thanks a lot for pointing that out. Yeah, you're right, this is not
missing from the GLES 3.0 spec, section 2.5 says:


Several error generation conditions are implicit in the description of every GL
command:
...
* If a negative number is provided where an argument of type sizei or
sizeiptr is specified, the error INVALID_VALUE is generated.


So yeah, worry withdrawn!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 07/11] glsl: error out on empty declarations

[Erik Faye-Lund]

On Mon, Jan 19, 2015 at 12:32 PM, Eduardo Lima Mitev el...@igalia.com wrote:
 From: Iago Toral Quiroga ito...@igalia.com

 So far we have only been emitting a warning.

 Fixes the following 2 dEQP tests:
 dEQP-GLES3.functional.shaders.arrays.invalid.empty_declaration_without_var_name_vertex
 dEQP-GLES3.functional.shaders.arrays.invalid.empty_declaration_without_var_name_fragment
 ---
  src/glsl/ast_to_hir.cpp | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
 index c52e4af..5ed8b0e 100644
 --- a/src/glsl/ast_to_hir.cpp
 +++ b/src/glsl/ast_to_hir.cpp
 @@ -3406,7 +3406,7 @@ ast_declarator_list::hir(exec_list *instructions,
 lowp
  };

 -_mesa_glsl_warning(loc, state,
 +_mesa_glsl_error(loc, state,
 empty declaration with precision qualifier, 
 to set the default precision, use 
 `precision %s %s;',
 @@ -3414,7 +3414,7 @@ ast_declarator_list::hir(exec_list *instructions,
 type_name);
   }
} else if (this-type-specifier-structure == NULL) {
 - _mesa_glsl_warning(loc, state, empty declaration);
 + _mesa_glsl_error(loc, state, empty declaration);
}
 }


OpenGL ES 3.1 specifies this as allowed, as far as I can tell:

Section 4.11:
Empty declarations are allowed. E.g.

int; // No effect
struct S {int x;}; // Defines a struct S

OpenGL ES 3.0 seems to not say, which generally should be interpreted
as allowed. In fact, no such error is listed in section 10 of the
spec, which is the list of errors required to detect.

So I think these dEQP-tests are bogus.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 08/11] glsl: GLSL ES identifiers cannot exceed 1024 characters

[Erik Faye-Lund]

On Mon, Jan 19, 2015 at 12:32 PM, Eduardo Lima Mitev el...@igalia.com wrote:
 From: Iago Toral Quiroga ito...@igalia.com

 Fixes the following 2 dEQP tests:
 dEQP-GLES3.functional.shaders.keywords.invalid_identifiers.max_length_vertex
 dEQP-GLES3.functional.shaders.keywords.invalid_identifiers.max_length_fragment
 ---
  src/glsl/glsl_parser.yy | 7 +++
  1 file changed, 7 insertions(+)

 diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
 index 7fb8c38..165419c 100644
 --- a/src/glsl/glsl_parser.yy
 +++ b/src/glsl/glsl_parser.yy
 @@ -362,6 +362,13 @@ any_identifier:
 IDENTIFIER
 | TYPE_IDENTIFIER
 | NEW_IDENTIFIER
 +   {
 +  if (state-es_shader  strlen($1)  1024) {
 + _mesa_glsl_error( @1, state,
 +  Identifier `%s' exceeds 
 +  1024 characters, $1);
 +  }
 +   }

The limit of 1024 characters for identifiers is only specified for
OpenGL ES Shading Language versions 3.00 and up, not for version 1.00
as OpenGL ES 2.0 uses. Perhaps a spec bug?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 01/11] mesa: Returns a GL_INVALID_VALUE error on several glGet* APIs when max length is negative

[Erik Faye-Lund]

On Mon, Jan 19, 2015 at 1:31 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Mon, Jan 19, 2015 at 12:32 PM, Eduardo Lima Mitev el...@igalia.com wrote:
 The manual page for glGetAttachedShaders, glGetShaderSource, 
 glGetActiveUniform and
 glGetActiveUniform state that a GL_INVALID_VALUE is returned if the maximum 
 length
 argument is less than zero. For reference, see:
 https://www.opengl.org/sdk/docs/man3/xhtml/glGetAttachedShaders.xml,
 https://www.khronos.org/opengles/sdk/docs/man31/html/glGetAttachedShaders.xhtml,
 https://www.opengl.org/sdk/docs/man3/xhtml/glGetShaderSource.xml,
 https://www.khronos.org/opengles/sdk/docs/man31/html/glGetShaderSource.xhtml,
 https://www.opengl.org/sdk/docs/man3/xhtml/glGetActiveUniform.xml,
 https://www.khronos.org/opengles/sdk/docs/man31/html/glGetActiveUniform.xhtml,
 https://www.opengl.org/sdk/docs/man3/xhtml/glGetActiveAttrib.xml,
 https://www.khronos.org/opengles/sdk/docs/man31/html/glGetActiveAttrib.xhtml.

 This fixes 4 dEQP test:
 * dEQP-GLES3.functional.negative_api.state.get_attached_shaders
 * dEQP-GLES3.functional.negative_api.state.get_shader_source
 * dEQP-GLES3.functional.negative_api.state.get_active_uniform
 * dEQP-GLES3.functional.negative_api.state.get_active_attrib

 These tests are about GLES3, but I cannot find such behavior specified
 in the OpenGL ES 3.0 specification for GetAttachedShaders nor
 GetProgramBinary. I stopped checking after those two, because I felt I
 saw a pattern ;)

 The man pages you linked to are for desktop-OpenGL, which *does*
 specify such an error; OpenGL 4.5 spec says An INVALID_VALUE error is
 generated if maxCount is negative about GetAttachedShaders (and a
 similar for GetProgramBinary()).

 However, the GLES 3 man pages also list such an error:
 https://www.khronos.org/opengles/sdk/docs/man3/html/glGetAttachedShaders.xhtml

 Generally speaking, the manual pages are not considered to dictate
 behavior, only to be a programmer convenience. And they have
 historically been full of errors. However, since desktop GL *does*
 specify these errors *and* the man-pages document them, this does look
 like a spec-error for GLES 3 to me. But I think we should get a
 clarification from Khronos before assuming so, otherwise we won't be
 in conformance. Ian, any thoughts?

By the way, this error is also properly defined for OpenGL ES 3.1, so
I'm feeling even more confident that it's a spec-bug in OpenGL ES 3.0.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 01/11] mesa: Returns a GL_INVALID_VALUE error on several glGet* APIs when max length is negative

[Erik Faye-Lund]

On Mon, Jan 19, 2015 at 12:32 PM, Eduardo Lima Mitev el...@igalia.com wrote:
 The manual page for glGetAttachedShaders, glGetShaderSource, 
 glGetActiveUniform and
 glGetActiveUniform state that a GL_INVALID_VALUE is returned if the maximum 
 length
 argument is less than zero. For reference, see:
 https://www.opengl.org/sdk/docs/man3/xhtml/glGetAttachedShaders.xml,
 https://www.khronos.org/opengles/sdk/docs/man31/html/glGetAttachedShaders.xhtml,
 https://www.opengl.org/sdk/docs/man3/xhtml/glGetShaderSource.xml,
 https://www.khronos.org/opengles/sdk/docs/man31/html/glGetShaderSource.xhtml,
 https://www.opengl.org/sdk/docs/man3/xhtml/glGetActiveUniform.xml,
 https://www.khronos.org/opengles/sdk/docs/man31/html/glGetActiveUniform.xhtml,
 https://www.opengl.org/sdk/docs/man3/xhtml/glGetActiveAttrib.xml,
 https://www.khronos.org/opengles/sdk/docs/man31/html/glGetActiveAttrib.xhtml.

 This fixes 4 dEQP test:
 * dEQP-GLES3.functional.negative_api.state.get_attached_shaders
 * dEQP-GLES3.functional.negative_api.state.get_shader_source
 * dEQP-GLES3.functional.negative_api.state.get_active_uniform
 * dEQP-GLES3.functional.negative_api.state.get_active_attrib

These tests are about GLES3, but I cannot find such behavior specified
in the OpenGL ES 3.0 specification for GetAttachedShaders nor
GetProgramBinary. I stopped checking after those two, because I felt I
saw a pattern ;)

The man pages you linked to are for desktop-OpenGL, which *does*
specify such an error; OpenGL 4.5 spec says An INVALID_VALUE error is
generated if maxCount is negative about GetAttachedShaders (and a
similar for GetProgramBinary()).

However, the GLES 3 man pages also list such an error:
https://www.khronos.org/opengles/sdk/docs/man3/html/glGetAttachedShaders.xhtml

Generally speaking, the manual pages are not considered to dictate
behavior, only to be a programmer convenience. And they have
historically been full of errors. However, since desktop GL *does*
specify these errors *and* the man-pages document them, this does look
like a spec-error for GLES 3 to me. But I think we should get a
clarification from Khronos before assuming so, otherwise we won't be
in conformance. Ian, any thoughts?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Expected wide line rendering with clipping

[Erik Faye-Lund]

On Mon, Feb 9, 2015 at 12:35 PM, Roland Scheidegger srol...@vmware.com wrote:
 Am 09.02.2015 um 09:53 schrieb Iago Toral:
 On Fri, 2015-02-06 at 21:27 +0100, Roland Scheidegger wrote:
 Am 06.02.2015 um 13:11 schrieb Iago Toral:
 Hi,

 Eduardo and I have been looking into a few dEQP test failures that deal
 with wide line rendering. There are a few of them that fail because of
 how clipping is implemented for this case.

 The problem in these cases seems to be that the hw renders the wide line
 as a parallelogram so that if an edge of the parallelogram is partially
 clipped, the other parallel edge will also be clipped at the same
 height/width so that the resulting wide line stays a parallelogram. The
 dEQP renders as if a wide line where a collection of 1px lines, so
 cliping one edge of the resulting line does not have implications for
 the other edge.

 This ASCII art illustrates the problem (* represent line pixels, |
 represents the viewport's rightmost edge):

  Expected by dEQP  i965 rendering
  |  |
 *|  |
**|  |
   ***|  |
  |  |
  |  |
  |  |

 We can make the rendering result match dEQP's expectation by enabling
 the GuardBand in the clip stage (GEN6_CLIP_GB_TEST). This is being
 disabled by the driver for gen7 in this scenario because the underlying
 framebuffer surface is larger than the viewport, and per the comment in
 the code, in gen7 that would make it render beyond the viewport limits
 in that surface, while it notes that gen8 hw fixes this. So I guess that
 we do not want to do this in any case in gen7.

 Then, I have been reviewing the OpenGL specs to see if they clarify what
 should happen when clipping wide lines and I think the spec does not
 make a clear point, since when it talks about line clipping it does not
 cover the case of wide lines specifically:

 13.5. PRIMITIVE CLIPPING
 ...
 If part of the line segment lies in the volume and part lies outside,
 then the line segment is clipped and new vertex coordinates are computed
 for one or both vertices. A clipped line segment endpoint lies on both
 the original line segment and the boundary of the clip volume.
 ...
 

 The above description is clear for 1px lines, but where should the new
 vertex be generated exactly for wide lines, where many points could be
 at the boundary of the clip volume? Is any point valid? (that would mean
 the dEQP test is bogus because there are multiple accepted renderings),
 should the new vertex be exactly at the center of the line? (that would
 make both deqp and intel rendering bogus).

 Then there is also this comment in 14.5.2.2 Wide Lines inside the
 non-AA line rasterization chapter:

 Non-antialiased line segments of width other than one are rasterized by
 off-setting them in the minor direction (for an x-major line, the minor
 direction is y, and for a y-major line, the minor direction is x) and
 replicating fragments in the minor direction (see figure 14.3). Let w be
 the width rounded to the nearest integer (if w = 0, then it is as if w =
 1). If the line segment has endpoints given by (x 0 , y 0 ) and (x 1 , y
 1 ) in window coordinates, the segment with endpoints (x 0 , y 0 − (w −
 1)/2) and (x 1 , y 1 − (w − 1)/2) is rasterized, but instead of a single
 fragment, a column of fragments of height w (a row of fragments of
 length w for a y-major segment) is produced at each x (y for y-major)
 location. The lowest fragment of this column is the fragment that would
 be produced by rasterizing the segment of width 1 with the modified
 coordinates.

 This suggests that wide line rendering should be equivalent to
 rendering multiple lines of width=1 and if we actually did that then the
 expected rendering result after clipping would be what dEQP expects.

 I think the whole thing isn't 100% clear. Does anyone have any thoughts
 on what should be considered correct behavior in this case?

 Iago



 I am quite sure that i965 rendering is what (at least legacy GL)
 expects. This is because the wide-line expansion is defined to be
 happening at rasterization time, i.e. past clipping. Thus, at clipping
 time, no matter if a line is wide or not, it is still really a line,
 hence clipping will produce a single new endpoint at the viewport edge
 and in rasterization this is then what will be used as the endpoint.

 That makes sense, but in that case I think i965 rendering would still be
 incorrect, the expected result would look something like this:

   |
   |
 **|
***|
   |
   |
   |

 However, as far as I can tell, since i965 hardware renders wide lines as
 parallelograms, it can't render something like this.

 Actually, the rendering result looks more like something we would obtain
 if we did line expansion before clipping:

   |
  

Re: [Mesa-dev] [PATCH 2/2] i965/skl: Layout a 1D miptree horizontally

[Erik Faye-Lund]

On Wed, Feb 18, 2015 at 6:02 PM, Ian Romanick i...@freedesktop.org wrote:
 On 02/17/2015 06:03 AM, Neil Roberts wrote:
 +   /* When this layout is used the horizontal alignment is fixed at 64 and 
 the
 +* hardware ignores the value given in the surface state
 +*/
 +   const unsigned int align_w = 64;
 +
 +   mt-total_height = mt-physical_height0;
 +
 +   if (mt-compressed) {
 +  mt-total_height = ALIGN(mt-physical_height0, mt-align_h);
 +   }

 There aren't any compressed formats that support 1D textures, so I don't
 think this can occur.  Does the bspec say anything about compressed 1D
 textures?

This might not be valid for this hardware generation, but ASTC
supports compressed 1D textures...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] i965/skl: Layout a 1D miptree horizontally

[Erik Faye-Lund]

On Thu, Feb 19, 2015 at 1:12 AM, Matt Turner matts...@gmail.com wrote:
 On Wed, Feb 18, 2015 at 3:34 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Wed, Feb 18, 2015 at 6:02 PM, Ian Romanick i...@freedesktop.org wrote:
 On 02/17/2015 06:03 AM, Neil Roberts wrote:
 +   /* When this layout is used the horizontal alignment is fixed at 64 
 and the
 +* hardware ignores the value given in the surface state
 +*/
 +   const unsigned int align_w = 64;
 +
 +   mt-total_height = mt-physical_height0;
 +
 +   if (mt-compressed) {
 +  mt-total_height = ALIGN(mt-physical_height0, mt-align_h);
 +   }

 There aren't any compressed formats that support 1D textures, so I don't
 think this can occur.  Does the bspec say anything about compressed 1D
 textures?

 This might not be valid for this hardware generation, but ASTC
 supports compressed 1D textures...

 I don't see that. It [0] mentions TexImage1D in an area that explains
 what regular GL 4.2 allows but the next paragraph explains that it
 doesn't apply to ASTC. I don't see text anywhere else that indicates
 that 1D is supported. Am I looking at the wrong extension or missing
 something?

 [0] https://www.opengl.org/registry/specs/KHR/texture_compression_astc_hdr.txt

No, you're right, I wasn't being careful enough when checking. My bad,
sorry for the noise.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] util: port _mesa_strto[df] to C

[Erik Faye-Lund]

On Tue, Mar 17, 2015 at 12:32 AM, Ian Romanick i...@freedesktop.org wrote:
 On 03/15/2015 12:05 PM, Erik Faye-Lund wrote:
 _mesa_strtod and _mesa_strtof are only used from the GLSL compiler,

 It's also used in the ARB_vertex_program / ARB_fragment_program
 assembler in src/prog.

Oh, right. Thanks for pointing that out.

 so the locale doesn't need to be initialized before the first context
 gets initialized. So let's use explicit initialization from the
 one-time init code instead of depending on a C++ compiler to initialize
 at image-load time.

 This is fairly close to the way Chia-I originally had it:

 http://lists.freedesktop.org/archives/mesa-dev/2014-April/058215.html

 Some discussion of alternate methods started:

 http://lists.freedesktop.org/archives/mesa-dev/2014-May/058861.html

Thanks for pointing me at this discussion, very useful.

 I'm a little concerned that having the initialization in Mesa and the
 function accessible to both Mesa and Gallium that we may set ourselves
 up for problems later.

Doesn't really sound like a ground-shattering risk to me. But perhaps
adding an assert verifying that initialization was done could offset
that risk?

 It also occurs to me that the neither the old code nor the new code ever
 call freelocale.

In OpenGL ES 2.0 and OpenGL 4.1, you have glReleaseShaderCompiler
which is intended for this kind of work. But I'm not sure a single
leak of a locale is really worth the implementation-effort.

 I think that's easier to fix with the static object
 method (using a destructor).

 I guess I'm kind of ambivalent about the change.

Yeah, especially initialization having to be done in three different
locations causes me to start losing some confidence that this is a
good idea.

 Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
 ---

 Because of the recent discussion on libc++ and Mesa, I thought I'd
 have a look into what parts of mesa depended on libc++, and I spotted
 this file.

 In this case, it was rather trivial to port the code to plain C, making
 it dead obvious that it doesn't depend on libc++. I'm not proposing all
 C++ gets this treatment, but in this case it seems like a pretty
 straight-forward way to make it obvious that this code does not depend
 on libc++.

  src/mesa/main/context.c   |  3 +++
  src/util/Makefile.sources |  2 +-
  src/util/{strtod.cpp = strtod.c} | 14 --
  src/util/strtod.h |  3 +++
  4 files changed, 15 insertions(+), 7 deletions(-)
  rename src/util/{strtod.cpp = strtod.c} (89%)

 diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
 index 22c2341..de6a016 100644
 --- a/src/mesa/main/context.c
 +++ b/src/mesa/main/context.c
 @@ -119,6 +119,7 @@
  #include shared.h
  #include shaderobj.h
  #include util/simple_list.h
 +#include util/strtod.h
  #include state.h
  #include stencil.h
  #include texcompress_s3tc.h
 @@ -398,6 +399,8 @@ one_time_init( struct gl_context *ctx )
assert( sizeof(GLint) == 4 );
assert( sizeof(GLuint) == 4 );

 +  _mesa_locale_init();
 +
_mesa_one_time_init_extension_overrides();

_mesa_get_cpu_features();
 diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
 index 560ea83..f930790 100644
 --- a/src/util/Makefile.sources
 +++ b/src/util/Makefile.sources
 @@ -17,7 +17,7 @@ MESA_UTIL_FILES :=  \
   set.c \
   set.h \
   simple_list.h \
 - strtod.cpp \
 + strtod.c \
   strtod.h \
   texcompress_rgtc_tmp.h \
   u_atomic.h
 diff --git a/src/util/strtod.cpp b/src/util/strtod.c
 similarity index 89%
 rename from src/util/strtod.cpp
 rename to src/util/strtod.c
 index 2b4dd98..a4a60e0 100644
 --- a/src/util/strtod.cpp
 +++ b/src/util/strtod.c
 @@ -30,18 +30,20 @@
  #include locale.h
  #ifdef HAVE_XLOCALE_H
  #include xlocale.h
 +static locale_t loc;
  #endif
  #endif

  #include strtod.h


 +void
 +_mesa_locale_init(void)
 +{
  #if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
 -static struct locale_initializer {
 -   locale_initializer() { loc = newlocale(LC_CTYPE_MASK, C, NULL); }
 -   locale_t loc;
 -} loc_init;
 +   loc = newlocale(LC_CTYPE_MASK, C, NULL);
  #endif
 +}

  /**
   * Wrapper around strtod which uses the C locale so the decimal
 @@ -51,7 +53,7 @@ double
  _mesa_strtod(const char *s, char **end)
  {
  #if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
 -   return strtod_l(s, end, loc_init.loc);
 +   return strtod_l(s, end, loc);
  #else
 return strtod(s, end);
  #endif
 @@ -66,7 +68,7 @@ float
  _mesa_strtof(const char *s, char **end)
  {
  #if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
 -   return strtof_l(s, end, loc_init.loc);
 +   return strtof_l(s, end, loc);
  #elif defined(HAVE_STRTOF)
 return strtof(s, end);
  #else
 diff --git a/src/util/strtod.h b/src/util/strtod.h
 index 02c25dd..b7e2beb 100644
 --- a/src/util/strtod.h
 +++ b/src/util/strtod.h
 @@ -31,6 +31,9 @@
  extern C {
  #endif

 +extern void
 +_mesa_locale_init(void);
 +
  extern double

[Mesa-dev] [PATCH] util: port _mesa_strto[df] to C

[Erik Faye-Lund]

_mesa_strtod and _mesa_strtof are only used from the GLSL compiler,
so the locale doesn't need to be initialized before the first context
gets initialized. So let's use explicit initialization from the
one-time init code instead of depending on a C++ compiler to initialize
at image-load time.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---

Because of the recent discussion on libc++ and Mesa, I thought I'd
have a look into what parts of mesa depended on libc++, and I spotted
this file.

In this case, it was rather trivial to port the code to plain C, making
it dead obvious that it doesn't depend on libc++. I'm not proposing all
C++ gets this treatment, but in this case it seems like a pretty
straight-forward way to make it obvious that this code does not depend
on libc++.

 src/mesa/main/context.c   |  3 +++
 src/util/Makefile.sources |  2 +-
 src/util/{strtod.cpp = strtod.c} | 14 --
 src/util/strtod.h |  3 +++
 4 files changed, 15 insertions(+), 7 deletions(-)
 rename src/util/{strtod.cpp = strtod.c} (89%)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 22c2341..de6a016 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -119,6 +119,7 @@
 #include shared.h
 #include shaderobj.h
 #include util/simple_list.h
+#include util/strtod.h
 #include state.h
 #include stencil.h
 #include texcompress_s3tc.h
@@ -398,6 +399,8 @@ one_time_init( struct gl_context *ctx )
   assert( sizeof(GLint) == 4 );
   assert( sizeof(GLuint) == 4 );
 
+  _mesa_locale_init();
+
   _mesa_one_time_init_extension_overrides();
 
   _mesa_get_cpu_features();
diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
index 560ea83..f930790 100644
--- a/src/util/Makefile.sources
+++ b/src/util/Makefile.sources
@@ -17,7 +17,7 @@ MESA_UTIL_FILES :=\
set.c \
set.h \
simple_list.h \
-   strtod.cpp \
+   strtod.c \
strtod.h \
texcompress_rgtc_tmp.h \
u_atomic.h
diff --git a/src/util/strtod.cpp b/src/util/strtod.c
similarity index 89%
rename from src/util/strtod.cpp
rename to src/util/strtod.c
index 2b4dd98..a4a60e0 100644
--- a/src/util/strtod.cpp
+++ b/src/util/strtod.c
@@ -30,18 +30,20 @@
 #include locale.h
 #ifdef HAVE_XLOCALE_H
 #include xlocale.h
+static locale_t loc;
 #endif
 #endif
 
 #include strtod.h
 
 
+void
+_mesa_locale_init(void)
+{
 #if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
-static struct locale_initializer {
-   locale_initializer() { loc = newlocale(LC_CTYPE_MASK, C, NULL); }
-   locale_t loc;
-} loc_init;
+   loc = newlocale(LC_CTYPE_MASK, C, NULL);
 #endif
+}
 
 /**
  * Wrapper around strtod which uses the C locale so the decimal
@@ -51,7 +53,7 @@ double
 _mesa_strtod(const char *s, char **end)
 {
 #if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
-   return strtod_l(s, end, loc_init.loc);
+   return strtod_l(s, end, loc);
 #else
return strtod(s, end);
 #endif
@@ -66,7 +68,7 @@ float
 _mesa_strtof(const char *s, char **end)
 {
 #if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
-   return strtof_l(s, end, loc_init.loc);
+   return strtof_l(s, end, loc);
 #elif defined(HAVE_STRTOF)
return strtof(s, end);
 #else
diff --git a/src/util/strtod.h b/src/util/strtod.h
index 02c25dd..b7e2beb 100644
--- a/src/util/strtod.h
+++ b/src/util/strtod.h
@@ -31,6 +31,9 @@
 extern C {
 #endif
 
+extern void
+_mesa_locale_init(void);
+
 extern double
 _mesa_strtod(const char *s, char **end);
 
-- 
2.1.0

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

Re: [Mesa-dev] [PATCH] util: port _mesa_strto[df] to C

[Erik Faye-Lund]

On Mon, Mar 16, 2015 at 10:13 PM, Emil Velikov emil.l.veli...@gmail.com wrote:
 On 15/03/15 19:05, Erik Faye-Lund wrote:
 _mesa_strtod and _mesa_strtof are only used from the GLSL compiler,
 so the locale doesn't need to be initialized before the first context
 gets initialized. So let's use explicit initialization from the
 one-time init code instead of depending on a C++ compiler to initialize
 at image-load time.

 Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
 ---

 Because of the recent discussion on libc++ and Mesa, I thought I'd
 have a look into what parts of mesa depended on libc++, and I spotted
 this file.

 In this case, it was rather trivial to port the code to plain C, making
 it dead obvious that it doesn't depend on libc++. I'm not proposing all
 C++ gets this treatment, but in this case it seems like a pretty
 straight-forward way to make it obvious that this code does not depend
 on libc++.

 Fwiw this file/code should not cause any linkage to the C++ runtime,
 although it's a nice cleanup imho.

 There is a small catch though - _mesa_strtof can be used by the
 standalone glsl_compiler and perhaps glcpp.

Good point, so perhaps this on top?

---8---
diff --git a/src/glsl/glcpp/glcpp.c b/src/glsl/glcpp/glcpp.c
index 5144516..c62f4ef 100644
--- a/src/glsl/glcpp/glcpp.c
+++ b/src/glsl/glcpp/glcpp.c
@@ -29,6 +29,7 @@
 #include glcpp.h
 #include main/mtypes.h
 #include main/shaderobj.h
+#include util/strtod.h

 extern int glcpp_parser_debug;

@@ -168,6 +169,8 @@ main (int argc, char *argv[])
  if (shader == NULL)
return 1;

+ _mesa_locale_init();
+
  ret = glcpp_preprocess(ctx, shader, info_log, NULL, gl_ctx);

  printf(%s, shader);
diff --git a/src/glsl/main.cpp b/src/glsl/main.cpp
index ccac839..b23b583 100644
--- a/src/glsl/main.cpp
+++ b/src/glsl/main.cpp
@@ -38,6 +38,7 @@
 #include program/hash_table.h
 #include loop_analysis.h
 #include standalone_scaffolding.h
+#include util/strtod.h

 static int glsl_version = 330;

@@ -52,6 +53,8 @@ initialize_context(struct gl_context *ctx, gl_api api)
 {
initialize_context_to_defaults(ctx, api);

+   _mesa_locale_init();
+
/* The standalone compiler needs to claim support for almost
 * everything in order to compile the built-in functions.
 */
---8---
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] nir: Optimize a + neg(a)

[Erik Faye-Lund]

On Sat, Feb 28, 2015 at 9:19 PM, Matt Turner matts...@gmail.com wrote:
 On Sat, Feb 28, 2015 at 11:47 AM, Thomas Helland
 thomashellan...@gmail.com wrote:
 On Feb 28, 2015 8:39 PM, Jason Ekstrand ja...@jlekstrand.net wrote:

 Both patches are

 Reviewed-by: Jason Ekstrand jason.ekstr...@intel.com

 Could you commit them?
 I don't have commit access.

 I'd like to wait a few days to see if there are any comments about the
 floating-point optimizations. If not, I'll commit them. (The integer
 parts are surely fine)

a + - a should give NaN if a is NaN or INF, not zero. However, the
GLSL spec says NaNs are not required to be generated and Operations
and built-in functions that operate on a NaN are not required to
return a NaN as the result.

So I think the patch is good.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/8] util: Change hash_table to use quadratic probing.

[Erik Faye-Lund]

On Sat, Feb 28, 2015 at 1:53 PM, Thomas Helland
thomashellan...@gmail.com wrote:
 This should give better cache locality, less memory consumption,
 less code, and should also be faster since we avoid a modulo operation.

 This is not the quadratic probing function you see most places.
 They do not accumulate, so you try hash +1, +4, +9, etc.
 My code accumulates; so it becomes hash +1, +5, +14, etc.
 It felt more natural to do it like this.

 However, it causes an assertion failure.
 The search function is apparently returning null when it shouldn't.
 This does not get caught by make-check.
 This gets fixed by the next patch.
 ---
  src/util/hash_table.c | 91 
 ---
  src/util/hash_table.h |  1 -
  2 files changed, 43 insertions(+), 49 deletions(-)

 diff --git a/src/util/hash_table.c b/src/util/hash_table.c
 index 3247593..542f583 100644
 --- a/src/util/hash_table.c
 +++ b/src/util/hash_table.c
 @@ -33,7 +33,7 @@
   */

  /**
 - * Implements an open-addressing, linear-reprobing hash table.
 + * Implements an open-addressing, quadratic probing hash table.
   *
   * For more information, see:
   *
 @@ -51,44 +51,44 @@
  static const uint32_t deleted_key_value;

  /**
 - * From Knuth -- a good choice for hash/rehash values is p, p-2 where
 - * p and p-2 are both prime.  These tables are sized to have an extra 10%
 - * free to avoid exponential performance degradation as the hash table fills
 + * From Knuth -- a good choice for hash values is p, where is prime.
 + * These tables are sized to have an extra 10% free to avoid
 + * exponential performance degradation as the hash table fills

[...] where is prime? Where what is prime?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 02/10] mesa: remove M_PI, M_E, M_LOG2E macro definitions

[Erik Faye-Lund]

On Thu, Feb 26, 2015 at 4:24 AM, Matt Turner matts...@gmail.com wrote:
 On Wed, Feb 25, 2015 at 5:29 PM, Brian Paul bri...@vmware.com wrote:
 Should be defined in math.h.  If not, we can add them to c99_math.h

 And FWIW, the MSDN page [0] says that if you define _USE_MATH_DEFINES
 before including math.h, these will be defined for you. Not sure what
 versions of MSVC that applies to.

 [0] https://msdn.microsoft.com/en-us/library/4hwaceh6.aspx

That link says Visual Studio 2013, but roughly the same message is
also in the Visual Studio .NET 2003 docs (available through the Other
Versions-selector at the top).

IIRC, this has been the case since VC6, but we probably don't care
about that old toolchains.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 02/10] mesa: remove M_PI, M_E, M_LOG2E macro definitions

[Erik Faye-Lund]

On Thu, Feb 26, 2015 at 10:10 AM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Thu, Feb 26, 2015 at 4:24 AM, Matt Turner matts...@gmail.com wrote:
 On Wed, Feb 25, 2015 at 5:29 PM, Brian Paul bri...@vmware.com wrote:
 Should be defined in math.h.  If not, we can add them to c99_math.h

 And FWIW, the MSDN page [0] says that if you define _USE_MATH_DEFINES
 before including math.h, these will be defined for you. Not sure what
 versions of MSVC that applies to.

 [0] https://msdn.microsoft.com/en-us/library/4hwaceh6.aspx

 That link says Visual Studio 2013, but roughly the same message is
 also in the Visual Studio .NET 2003 docs (available through the Other
 Versions-selector at the top).

 IIRC, this has been the case since VC6, but we probably don't care
 about that old toolchains.

By the way, it seems we already define _USE_MATH_DEFINES, at least here:

http://cgit.freedesktop.org/mesa/mesa/tree/scons/gallium.py?id=9db7b12cb283b915865d99d2fd3356af0662a18e#n328

But do we always build for MSVC with SCons, and with gallium? If so,
it seems this patch is OK from a MSVC's point of view. But perhaps
it's worth a mention in the commit-message?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 03/20] SQUASH! whitespace fixes after previous commit

[Erik Faye-Lund]

On Thu, Apr 30, 2015 at 1:25 AM, Ian Romanick i...@freedesktop.org wrote:
 From: Ian Romanick ian.d.roman...@intel.com

 Signed-off-by: Ian Romanick ian.d.roman...@intel.com

The title on this one doesn't seem quite right (after previous
should probably be before next), but that probably doesn't matter
because it will get squashed anyway...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 6/9] mesa/es3.1: Allow Multisampled FrameBufferTextures

[Erik Faye-Lund]

On Mon, May 11, 2015 at 4:57 PM, Ilia Mirkin imir...@alum.mit.edu wrote:
 On Mon, May 11, 2015 at 10:08 AM, Erik Faye-Lund kusmab...@gmail.com wrote:
 Shouldn't this be like this instead (and make sure
 ARB_texture_multisample is enabled for ES3.1)?

 @@ -2756,8 +2756,9 @@ _mesa_FramebufferTexture2D(GLenum target, GLenum
 attachment,
   break;
case GL_TEXTURE_2D_MULTISAMPLE:
case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
 - error = _mesa_is_gles(ctx)
 -|| !ctx-Extensions.ARB_texture_multisample;
 + error = !ctx-Extensions.ARB_texture_multisample;

 error = false when you have a driver that supports texture_ms, but you
 have a gles1/2/3 context, whereas you wanted error = true there...

I would expect ctx-Extensions.ARB_texture_multisample to be false in
any pre-GLES 3.1 context, since ARB_texture_multisample is written
against the OpenGL 3.1 specification, and not any Open GL ES flavor.

Are you saying that we do not mask these booleans against what the
context can support? Are these purely about what the driver can
manage?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 6/9] mesa/es3.1: Allow Multisampled FrameBufferTextures

[Erik Faye-Lund]

On Mon, May 11, 2015 at 5:21 PM, Ilia Mirkin imir...@alum.mit.edu wrote:
 On Mon, May 11, 2015 at 11:07 AM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Mon, May 11, 2015 at 4:57 PM, Ilia Mirkin imir...@alum.mit.edu wrote:
 On Mon, May 11, 2015 at 10:08 AM, Erik Faye-Lund kusmab...@gmail.com 
 wrote:
 Shouldn't this be like this instead (and make sure
 ARB_texture_multisample is enabled for ES3.1)?

 @@ -2756,8 +2756,9 @@ _mesa_FramebufferTexture2D(GLenum target, GLenum
 attachment,
   break;
case GL_TEXTURE_2D_MULTISAMPLE:
case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
 - error = _mesa_is_gles(ctx)
 -|| !ctx-Extensions.ARB_texture_multisample;
 + error = !ctx-Extensions.ARB_texture_multisample;

 error = false when you have a driver that supports texture_ms, but you
 have a gles1/2/3 context, whereas you wanted error = true there...

 I would expect ctx-Extensions.ARB_texture_multisample to be false in
 any pre-GLES 3.1 context, since ARB_texture_multisample is written
 against the OpenGL 3.1 specification, and not any Open GL ES flavor.

 Are you saying that we do not mask these booleans against what the
 context can support? Are these purely about what the driver can
 manage?

 Correct. They are exclusively set by the driver and never modified again.

OK. Then yeah, my comments were obviously wrong. Thanks for setting me straight!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 6/9] mesa/es3.1: Allow Multisampled FrameBufferTextures

[Erik Faye-Lund]

On Mon, May 11, 2015 at 3:03 PM, Marta Lofstedt
marta.lofst...@linux.intel.com wrote:
 From: Marta Lofstedt marta.lofst...@intel.com

 GLES 3.1 must be allowed to use multisampled
 frambuffer textures.

 Signed-off-by: Marta Lofstedt marta.lofst...@intel.com
 ---
  src/mesa/main/fbobject.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

 diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
 index 27cf97f..14a015e 100644
 --- a/src/mesa/main/fbobject.c
 +++ b/src/mesa/main/fbobject.c
 @@ -2756,8 +2756,9 @@ _mesa_FramebufferTexture2D(GLenum target, GLenum 
 attachment,
   break;
case GL_TEXTURE_2D_MULTISAMPLE:
case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
 - error = _mesa_is_gles(ctx)
 -|| !ctx-Extensions.ARB_texture_multisample;
 + error = (_mesa_is_gles(ctx)
 +|| !ctx-Extensions.ARB_texture_multisample) 
 +!_mesa_is_gles31(ctx);
   break;
default:
   error = GL_TRUE;

Shouldn't this be like this instead (and make sure
ARB_texture_multisample is enabled for ES3.1)?

@@ -2756,8 +2756,9 @@ _mesa_FramebufferTexture2D(GLenum target, GLenum
attachment,
  break;
   case GL_TEXTURE_2D_MULTISAMPLE:
   case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
- error = _mesa_is_gles(ctx)
-|| !ctx-Extensions.ARB_texture_multisample;
+ error = !ctx-Extensions.ARB_texture_multisample;
  break;
   default:
  error = GL_TRUE;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/6] mesa/es3.1 : Correct error code for zero samples

[Erik Faye-Lund]

On Tue, Jun 23, 2015 at 2:23 PM, Marta Lofstedt
marta.lofst...@linux.intel.com wrote:
 From: Marta Lofstedt marta.lofst...@intel.com

 According to GLES 3.1 CTS test:
 ES31-CTS.texture_storage_multisample.
 APIGLTexStorage2DMultisample.
 multisample_texture_tex_storage_2d_zero_sample-

 A call to glTexStorageMultisample with target
 GL_TEXTURE_2D_MULTISAMPLE and zero samples,
 should return GL_INVALID_VALUE.


OpenGL ES 3.1, section 8.8:
An INVALID_VALUE error is generated if samples is zero

 However, with above the piglit test:
 gl-3.2-layered-rendering-framebuffertexture fails.
 Hence, only limit this for GLES 3.1 contexts.


OpenGL 4.5 say the following about TexStorage2DMultisample:

Calling TexStorage2DMultisample is equivalent to calling
TexImage2DMultisample with the equivalently named parameters set to
the same values, except that the resulting texture has immutable
format.

and defines the following for TexImage*Multisample():

An INVALID_VALUE error is generated if samples is zero

This should cause the INVALID_VALUE error to happen for non-GLES. So
it seems either the Piglit test is wrong, or there's something more
going on. I'd expect the latter, due to some layered rendering
interaction. But I don't know for sure.

 Signed-off-by: Marta Lofstedt marta.lofst...@linux.intel.com
 ---
  src/mesa/main/teximage.c | 10 ++
  1 file changed, 10 insertions(+)

 diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
 index 14af9cd..98f0223 100644
 --- a/src/mesa/main/teximage.c
 +++ b/src/mesa/main/teximage.c
 @@ -5588,6 +5588,16 @@ _mesa_texture_image_multisample(struct gl_context 
 *ctx, GLuint dims,
 GLenum sample_count_error;
 bool dsa = strstr(func, ture) ? true : false;

 +   /*
 +*  According to OpenGL ES 3.1 CTS zero samples
 +*  should generate GL_INVALID_VALUE
 +*/
 +   if(samples == 0  _mesa_is_gles31(ctx))
 +   {
 +  _mesa_error(ctx, GL_INVALID_VALUE, %s(target), func);
 +  return;
 +   }
 +
 if (!(ctx-Extensions.ARB_texture_multisample
 _mesa_is_desktop_gl(ctx))) {
_mesa_error(ctx, GL_INVALID_OPERATION, %s(unsupported), func);
 --
 1.9.1

 ___
 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/6] mesa/es3.1: Correct error code for illegal internal formats

[Erik Faye-Lund]

On Tue, Jun 23, 2015 at 2:23 PM, Marta Lofstedt
marta.lofst...@linux.intel.com wrote:
 From: Marta Lofstedt marta.lofst...@intel.com

 According to GLES 3.1 CTS test:
 ES31-CTS.texture_storage_multisample.
 APIGLTexStorage2DMultisample.
 multisample_texture_tex_storage_2d_non_color_depth_or_stencil_
 internal_formats_receted.

 An illegal internal format should generate a
 GL_INVALID_ENUM error.


OpenGL ES 3.1, section 8.8 defines this, not the CTS:

An INVALID_ENUM error is generated if sizedinternalformat is not
colorrenderable, depth-renderable, or stencil-renderable (as defined
in section 9.4).
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 6/6] mesa/es3.1: Fix error code for glCreateShaderProgram

[Erik Faye-Lund]

On Tue, Jun 23, 2015 at 2:23 PM, Marta Lofstedt
marta.lofst...@linux.intel.com wrote:
 From: Marta Lofstedt marta.lofst...@intel.com

 According to the OpenGL ES standard, 7.3.
 For a call to glCreateShaderProgram with count  0,
 a GL_INVALID_VALUE error should be generated.


OpenGL 4.5 defines it the same way.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/6] mesa/es3.1: Do not allow zero size multisampled textures

[Erik Faye-Lund]

On Tue, Jun 23, 2015 at 2:23 PM, Marta Lofstedt
marta.lofst...@linux.intel.com wrote:
 From: Marta Lofstedt marta.lofst...@intel.com

 According to GLES 3.1 CTS test:
 ES31-CTS.texture_storage_multisample.
 APIGLTexStorage2DMultisample.
 multisample_texture_tex_storage_2d_
 invalid_and_border_case_texture_sizes.

 Textures of size 0x0 are not allowed for
 GL_TEXTURE_2D_MULTISAMPLE.


Please quote the spec rather than the CTS. The spec does define this,
in section 8.8 Multisample Textures:

An INVALID_VALUE error is generated if width or height is less than 1.

  src/mesa/main/teximage.c | 7 +++
  1 file changed, 7 insertions(+)

 diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
 index 3d85615..c76ad54 100644
 --- a/src/mesa/main/teximage.c
 +++ b/src/mesa/main/teximage.c
 @@ -1483,6 +1483,13 @@ _mesa_legal_texture_dimensions(struct gl_context *ctx, 
 GLenum target,
   if (height  0  !_mesa_is_pow_two(height - 2 * border))
  return GL_FALSE;
}
 +  /*
 +  *  according to GLES 3.1 CTS it is not OK with
 +  *  zero size multisampled textures
 +  */
 +  if (width == 0  height == 0  GL_TEXTURE_2D_MULTISAMPLE == target)
 + return GL_FALSE;
 +

However, this is also the case for TexStorage2D, see section 8.17
Immutable-Format Texture Images:

An INVALID_VALUE error is generated if width, height, depth or levels
are less than 1, for commands with the corresponding parameters.

So it seems this patch is somewhat incomplete, and only papers over a
CTS failure.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/6] mesa/es3.1: Correct error code for illegal internal formats

[Erik Faye-Lund]

On Tue, Jun 23, 2015 at 3:11 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Tue, Jun 23, 2015 at 2:23 PM, Marta Lofstedt
 marta.lofst...@linux.intel.com wrote:
 From: Marta Lofstedt marta.lofst...@intel.com

 According to GLES 3.1 CTS test:
 ES31-CTS.texture_storage_multisample.
 APIGLTexStorage2DMultisample.
 multisample_texture_tex_storage_2d_non_color_depth_or_stencil_
 internal_formats_receted.

 An illegal internal format should generate a
 GL_INVALID_ENUM error.


 OpenGL ES 3.1, section 8.8 defines this, not the CTS:

 An INVALID_ENUM error is generated if sizedinternalformat is not
 colorrenderable, depth-renderable, or stencil-renderable (as defined
 in section 9.4).

By the way, OpenGL 4.5 also defines this, so the patch should probably
skip that condition.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/6] mesa/es3.1 : Correct error code for defect texture target

[Erik Faye-Lund]

On Tue, Jun 23, 2015 at 2:23 PM, Marta Lofstedt
marta.lofst...@linux.intel.com wrote:
 From: Marta Lofstedt marta.lofst...@intel.com

 According to GLES 3.1 CTS test:
 ES31-CTS.texture_storage_multisample.
 APIGLGetTexLevelParameterifv.
 invalid_texture_target_rejected:

 GL_INVALID_ENUM should be generated when
 glGetTexLevelParameteriv is called with a defect
 texture target.


Again, this is defined by the spec, not the CTS, section 8.10.3:

An INVALID_ENUM error is generated if target is not one of the
texture targets described above

But The OpenGL 4.5 spec defines the exact same error, so I don't think
we should check for gles3.1 here.

 Signed-off-by: Marta Lofstedt marta.lofst...@linux.intel.com
 ---
  src/mesa/main/texobj.c | 11 +++
  1 file changed, 11 insertions(+)

 diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
 index c563f1e..c239deb 100644
 --- a/src/mesa/main/texobj.c
 +++ b/src/mesa/main/texobj.c
 @@ -222,6 +222,17 @@ _mesa_get_current_tex_object(struct gl_context *ctx, 
 GLenum target)
   return ctx-Extensions.ARB_texture_multisample
  ? ctx-Texture.ProxyTex[TEXTURE_2D_MULTISAMPLE_ARRAY_INDEX] : 
 NULL;
default:
 + if(_mesa_is_gles31(ctx))
 + {
 +/*
 + * According to OpenGL ES 3.1 CTS:
 + * 
 ES31-CTS.texture_storage_multisample.APIGLGetTexLevelParameterifv.
 + * invalid_value_argument_rejected
 + * 
 es31cTextureStorageMultisampleGetTexLevelParameterifvTests.cpp:1277
 + * INVALID_ENUM should be reported for bad targets.
 + */
 +_mesa_error(ctx, GL_INVALID_ENUM, %s(target), __func__);
 + }
   _mesa_problem(NULL, bad target in _mesa_get_current_tex_object());
   return NULL;
 }
 --
 1.9.1

 ___
 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 5/6] mesa/es31: AtomicBufferBindings should be initialized to zero.

[Erik Faye-Lund]

On Tue, Jun 23, 2015 at 2:23 PM, Marta Lofstedt
marta.lofst...@linux.intel.com wrote:
 From: Marta Lofstedt marta.lofst...@intel.com

 Accoring to GLES 3.1 CTS:
 GLES 3.1 CTS: ES31-CTS.shader_atomic_counters.
 basic-buffer-bind.

 AtomicBufferBindings size and start should be initialized
 to zero.


OpenGL 3.1 says:

Buffer variables in shader storage blocks are represented in memory
in the same way as uniforms stored in uniform blocks, as described in
section 7.6.2.1.

Table 6.2, Buffer object parameters and their values defines what
seems like the initial values for this. And OpenGL 4.5 defines the
same.

But I guess someone who knows atomic buffers better than me should clarify.

 Signed-off-by: Marta Lofstedt marta.lofst...@intel.com
 ---
  src/mesa/main/bufferobj.c | 11 ---
  1 file changed, 8 insertions(+), 3 deletions(-)

 diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
 index 66dee68..94629b3 100644
 --- a/src/mesa/main/bufferobj.c
 +++ b/src/mesa/main/bufferobj.c
 @@ -849,9 +849,14 @@ _mesa_init_buffer_objects( struct gl_context *ctx )
_mesa_reference_buffer_object(ctx,
 
 ctx-AtomicBufferBindings[i].BufferObject,
 ctx-Shared-NullBufferObj);
 -  ctx-AtomicBufferBindings[i].Offset = -1;
 -  ctx-AtomicBufferBindings[i].Size = -1;
 -   }
 +  if (_mesa_is_gles31(ctx)) {
 + ctx-AtomicBufferBindings[i].Offset = 0;
 + ctx-AtomicBufferBindings[i].Size = 0;
 +  }
 +  else {
 + ctx-AtomicBufferBindings[i].Offset = -1;
 + ctx-AtomicBufferBindings[i].Size = -1;
 +  }
  }


 --
 1.9.1

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

[Mesa-dev] [PATCH v2 5/7] util: port _mesa_strto[df] to C

[Erik Faye-Lund]

_mesa_strtod and _mesa_strtof are only used from the GLSL compiler and
the ARB_[vertex|fragment]_program code, meaning that the locale doesn't
need to be initialized before the first OpenGL context gets initialized.

So let's use explicit initialization from the one-time init code instead
of depending on a C++ compiler to initialize at image-load time.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Reviewed-by: Matt Turner matts...@gmail.com
---
 src/glsl/glcpp/glcpp.c|  3 ++
 src/glsl/main.cpp |  3 ++
 src/mesa/main/context.c   |  3 ++
 src/util/Makefile.sources |  2 +-
 src/util/strtod.c | 78 +++
 src/util/strtod.cpp   | 75 -
 src/util/strtod.h |  3 ++
 7 files changed, 91 insertions(+), 76 deletions(-)
 create mode 100644 src/util/strtod.c
 delete mode 100644 src/util/strtod.cpp

diff --git a/src/glsl/glcpp/glcpp.c b/src/glsl/glcpp/glcpp.c
index 5144516..c62f4ef 100644
--- a/src/glsl/glcpp/glcpp.c
+++ b/src/glsl/glcpp/glcpp.c
@@ -29,6 +29,7 @@
 #include glcpp.h
 #include main/mtypes.h
 #include main/shaderobj.h
+#include util/strtod.h
 
 extern int glcpp_parser_debug;
 
@@ -168,6 +169,8 @@ main (int argc, char *argv[])
if (shader == NULL)
   return 1;
 
+   _mesa_locale_init();
+
ret = glcpp_preprocess(ctx, shader, info_log, NULL, gl_ctx);
 
printf(%s, shader);
diff --git a/src/glsl/main.cpp b/src/glsl/main.cpp
index 2341298..58651df 100644
--- a/src/glsl/main.cpp
+++ b/src/glsl/main.cpp
@@ -38,6 +38,7 @@
 #include program/hash_table.h
 #include loop_analysis.h
 #include standalone_scaffolding.h
+#include util/strtod.h
 
 static int glsl_version = 330;
 
@@ -46,6 +47,8 @@ initialize_context(struct gl_context *ctx, gl_api api)
 {
initialize_context_to_defaults(ctx, api);
 
+   _mesa_locale_init();
+
/* The standalone compiler needs to claim support for almost
 * everything in order to compile the built-in functions.
 */
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index c4af8ea..e68de68 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -120,6 +120,7 @@
 #include shaderobj.h
 #include shaderimage.h
 #include util/simple_list.h
+#include util/strtod.h
 #include state.h
 #include stencil.h
 #include texcompress_s3tc.h
@@ -374,6 +375,8 @@ one_time_init( struct gl_context *ctx )
   assert( sizeof(GLint) == 4 );
   assert( sizeof(GLuint) == 4 );
 
+  _mesa_locale_init();
+
   _mesa_one_time_init_extension_overrides();
 
   _mesa_get_cpu_features();
diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
index dc55939..82df3bc 100644
--- a/src/util/Makefile.sources
+++ b/src/util/Makefile.sources
@@ -19,7 +19,7 @@ MESA_UTIL_FILES :=\
set.c \
set.h \
simple_list.h \
-   strtod.cpp \
+   strtod.c \
strtod.h \
texcompress_rgtc_tmp.h \
u_atomic.h
diff --git a/src/util/strtod.c b/src/util/strtod.c
new file mode 100644
index 000..e5e6f76
--- /dev/null
+++ b/src/util/strtod.c
@@ -0,0 +1,78 @@
+/*
+ * Copyright 2010 VMware, Inc.
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * Software), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+ * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
+ * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+
+#include stdlib.h
+
+#ifdef _GNU_SOURCE
+#include locale.h
+#ifdef HAVE_XLOCALE_H
+#include xlocale.h
+static locale_t loc;
+static int initialized;
+#endif
+#endif
+
+#include strtod.h
+
+
+void
+_mesa_locale_init(void)
+{
+#if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
+   loc = newlocale(LC_CTYPE_MASK, C, NULL);
+#endif
+}
+
+/**
+ * Wrapper around strtod which uses the C locale so the decimal
+ * point is always '.'
+ */
+double
+_mesa_strtod(const char *s, char **end)
+{
+#if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
+   return strtod_l(s, end, loc);
+#else
+   return strtod(s, end);
+#endif
+}
+
+
+/**
+ * Wrapper around strtof which uses

[Mesa-dev] [PATCH v2 6/7] mesa/main: free locale at exit

[Erik Faye-Lund]

In order to save a small leak if mesa is continously loaded and
unloaded, let's free the locale when the shared object is unloaded.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
 src/mesa/main/context.c | 12 +++-
 src/util/strtod.c   |  8 
 src/util/strtod.h   |  3 +++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index e68de68..dee1fa8 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -346,6 +346,16 @@ _mesa_destroy_visual( struct gl_config *vis )
 mtx_t OneTimeLock = _MTX_INITIALIZER_NP;
 
 
+/**
+ * Calls all the various one-time-fini functions in Mesa
+ */
+
+static void
+one_time_fini()
+{
+   _mesa_destroy_shader_compiler();
+   _mesa_locale_fini();
+}
 
 /**
  * Calls all the various one-time-init functions in Mesa.
@@ -385,7 +395,7 @@ one_time_init( struct gl_context *ctx )
  _mesa_ubyte_to_float_color_tab[i] = (float) i / 255.0F;
   }
 
-  atexit(_mesa_destroy_shader_compiler);
+  atexit(one_time_fini);
 
 #if defined(DEBUG)  defined(__DATE__)  defined(__TIME__)
   if (MESA_VERBOSE != 0) {
diff --git a/src/util/strtod.c b/src/util/strtod.c
index e5e6f76..5c36b05 100644
--- a/src/util/strtod.c
+++ b/src/util/strtod.c
@@ -46,6 +46,14 @@ _mesa_locale_init(void)
 #endif
 }
 
+void
+_mesa_locale_fini(void)
+{
+#if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
+   freelocale(loc);
+#endif
+}
+
 /**
  * Wrapper around strtod which uses the C locale so the decimal
  * point is always '.'
diff --git a/src/util/strtod.h b/src/util/strtod.h
index b7e2beb..60e15cf 100644
--- a/src/util/strtod.h
+++ b/src/util/strtod.h
@@ -34,6 +34,9 @@ extern C {
 extern void
 _mesa_locale_init(void);
 
+extern void
+_mesa_locale_fini(void);
+
 extern double
 _mesa_strtod(const char *s, char **end);
 
-- 
2.1.4

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

[Mesa-dev] [PATCH v2 7/7] util: assert to verify that locale is initialized

[Erik Faye-Lund]

Add an assert to Verify that the locale has been initialized when
we call strtod. This might help some developers sleep better at
night.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
 src/util/strtod.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/util/strtod.c b/src/util/strtod.c
index 5c36b05..7a73041 100644
--- a/src/util/strtod.c
+++ b/src/util/strtod.c
@@ -25,6 +25,7 @@
 
 
 #include stdlib.h
+#include assert.h
 
 #ifdef _GNU_SOURCE
 #include locale.h
@@ -43,6 +44,7 @@ _mesa_locale_init(void)
 {
 #if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
loc = newlocale(LC_CTYPE_MASK, C, NULL);
+   initialized = 1;
 #endif
 }
 
@@ -51,6 +53,7 @@ _mesa_locale_fini(void)
 {
 #if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
freelocale(loc);
+   initialized = 0;
 #endif
 }
 
@@ -62,6 +65,7 @@ double
 _mesa_strtod(const char *s, char **end)
 {
 #if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
+   assert(initialized);
return strtod_l(s, end, loc);
 #else
return strtod(s, end);
@@ -77,6 +81,7 @@ float
 _mesa_strtof(const char *s, char **end)
 {
 #if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
+   assert(initialized);
return strtof_l(s, end, loc);
 #elif defined(HAVE_STRTOF)
return strtof(s, end);
-- 
2.1.4

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

[Mesa-dev] [PATCH v2 4/7] glsl: No need to lock in _mesa_glsl_release_types

[Erik Faye-Lund]

This function only gets called while mesa is unloading, so there's
no potential of racing or multiple calls at the same time. So let's
just get rid of the locking.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
 src/glsl/glsl_types.cpp | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index f675e90..dbe2382 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -324,8 +324,10 @@ const glsl_type *glsl_type::get_scalar_type() const
 void
 _mesa_glsl_release_types(void)
 {
-   mtx_lock(glsl_type::mutex);
-
+   /* should only be called during atexit (either when unloading shared
+* object, or if process terminates), so no mutex-locking should be
+* nessecary.
+*/
if (glsl_type::array_types != NULL) {
   hash_table_dtor(glsl_type::array_types);
   glsl_type::array_types = NULL;
@@ -335,8 +337,6 @@ _mesa_glsl_release_types(void)
   hash_table_dtor(glsl_type::record_types);
   glsl_type::record_types = NULL;
}
-
-   mtx_unlock(glsl_type::mutex);
 }
 
 
-- 
2.1.4

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

[Mesa-dev] [PATCH v2 1/7] mesa/main: Get rid of outdated GDB-hack

[Erik Faye-Lund]

All of these enums are now in use around in the code, so there's no need
to explicitly use them here any more.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
 src/mesa/main/context.c | 27 ---
 1 file changed, 27 deletions(-)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 79fa018..265f98a 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -338,31 +338,6 @@ _mesa_destroy_visual( struct gl_config *vis )
 
 
 /**
- * This is lame.  gdb only seems to recognize enum types that are
- * actually used somewhere.  We want to be able to print/use enum
- * values such as TEXTURE_2D_INDEX in gdb.  But we don't actually use
- * the gl_texture_index type anywhere.  Thus, this lame function.
- */
-static void
-dummy_enum_func(void)
-{
-   gl_buffer_index bi = BUFFER_FRONT_LEFT;
-   gl_face_index fi = FACE_POS_X;
-   gl_frag_result fr = FRAG_RESULT_DEPTH;
-   gl_texture_index ti = TEXTURE_2D_ARRAY_INDEX;
-   gl_vert_attrib va = VERT_ATTRIB_POS;
-   gl_varying_slot vs = VARYING_SLOT_POS;
-
-   (void) bi;
-   (void) fi;
-   (void) fr;
-   (void) ti;
-   (void) va;
-   (void) vs;
-}
-
-
-/**
  * One-time initialization mutex lock.
  *
  * \sa Used by one_time_init().
@@ -434,8 +409,6 @@ one_time_init( struct gl_context *ctx )
 * #ifdef tests here.
 */
atexit(_mesa_destroy_shader_compiler);
-
-   dummy_enum_func();
 }
 
 
-- 
2.1.4

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

[Mesa-dev] [PATCH v2 2/7] dri: don't touch the shader compiler

[Erik Faye-Lund]

This function is for deleting per-screen resources, and the shader
compiler resources are not of such nature. Besides, dri shouldn't
need to even know about the presence of a shader compiler.

These resources will already be released when mesa gets unloaded,
and that should be sufficient.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
 src/mesa/drivers/dri/common/dri_util.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/mesa/drivers/dri/common/dri_util.c 
b/src/mesa/drivers/dri/common/dri_util.c
index e7ababe..ae4592c 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c
@@ -46,7 +46,6 @@
 #include dri_util.h
 #include utils.h
 #include xmlpool.h
-#include ../glsl/glsl_parser_extras.h
 #include main/mtypes.h
 #include main/version.h
 #include main/errors.h
@@ -238,8 +237,6 @@ static void driDestroyScreen(__DRIscreen *psp)
 * stream open to the X-server anymore.
 */
 
-   _mesa_destroy_shader_compiler();
-
psp-driver-DestroyScreen(psp);
 
driDestroyOptionCache(psp-optionCache);
-- 
2.1.4

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

[Mesa-dev] [PATCH v2 3/7] mesa/main: only call _mesa_destroy_shader_compiler once on exit

[Erik Faye-Lund]

There's no point in calling _mesa_destroy_shader_compiler multiple
times on exit; the resources will only be released once anyway.

So let's move the atexit-call into the part that is only called
once.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
 src/mesa/main/context.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 265f98a..c4af8ea 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -382,6 +382,8 @@ one_time_init( struct gl_context *ctx )
  _mesa_ubyte_to_float_color_tab[i] = (float) i / 255.0F;
   }
 
+  atexit(_mesa_destroy_shader_compiler);
+
 #if defined(DEBUG)  defined(__DATE__)  defined(__TIME__)
   if (MESA_VERBOSE != 0) {
 _mesa_debug(ctx, Mesa %s DEBUG build %s %s\n,
@@ -404,11 +406,6 @@ one_time_init( struct gl_context *ctx )
api_init_mask |= 1  ctx-API;
 
mtx_unlock(OneTimeLock);
-
-   /* Hopefully atexit() is widely available.  If not, we may need some
-* #ifdef tests here.
-*/
-   atexit(_mesa_destroy_shader_compiler);
 }
 
 
-- 
2.1.4

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

[Mesa-dev] [PATCH v2 0/7] port _mesa_strto[df] to C

[Erik Faye-Lund]

Back in March[1], I sent a patch porting _mesa_strto[df] to
C rather than C++. I fixed up the patch according to the
criticism, but unfortunately I dropped the ball before I sent
out the result. So here I am, picking it back up!

This time I've taken a deeper dive into the whole init/deinit
of Mesa, and cleaned up a bunch of stuff in that area. And as
a result, this time we end up freeing the locale also.

No Piglit regressions observed.

[1]: 1426446329-23984-1-git-send-email-kusmab...@gmail.com

Erik Faye-Lund (7):
  mesa/main: Get rid of outdated GDB-hack
  dri: don't touch the shader compiler
  mesa/main: only call _mesa_destroy_shader_compiler once on exit
  glsl: No need to lock in _mesa_glsl_release_types
  util: port _mesa_strto[df] to C
  mesa/main: free locale at exit
  util: assert to verify that locale is initialized

 src/glsl/glcpp/glcpp.c |  3 ++
 src/glsl/glsl_types.cpp|  8 +--
 src/glsl/main.cpp  |  3 ++
 src/mesa/drivers/dri/common/dri_util.c |  3 --
 src/mesa/main/context.c| 47 ++
 src/util/Makefile.sources  |  2 +-
 src/util/strtod.c  | 91 ++
 src/util/strtod.cpp| 75 
 src/util/strtod.h  |  6 +++
 9 files changed, 123 insertions(+), 115 deletions(-)
 create mode 100644 src/util/strtod.c
 delete mode 100644 src/util/strtod.cpp

-- 
2.1.4

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

Re: [Mesa-dev] [PATCH v2 4/7] glsl: No need to lock in _mesa_glsl_release_types

[Erik Faye-Lund]

On Fri, Jun 26, 2015 at 12:29 AM, Matt Turner matts...@gmail.com wrote:
 On Thu, Jun 25, 2015 at 2:05 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 This function only gets called while mesa is unloading, so there's
 no potential of racing or multiple calls at the same time. So let's
 just get rid of the locking.

 Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
 ---
  src/glsl/glsl_types.cpp | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

 diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
 index f675e90..dbe2382 100644
 --- a/src/glsl/glsl_types.cpp
 +++ b/src/glsl/glsl_types.cpp
 @@ -324,8 +324,10 @@ const glsl_type *glsl_type::get_scalar_type() const
  void
  _mesa_glsl_release_types(void)
  {
 -   mtx_lock(glsl_type::mutex);
 -
 +   /* should only be called during atexit (either when unloading shared

 Let's capitalize should

 +* object, or if process terminates), so no mutex-locking should be
 +* nessecary.

 typo: necessary

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

Re: [Mesa-dev] [PATCH v2 5/7] util: port _mesa_strto[df] to C

[Erik Faye-Lund]

On Fri, Jun 26, 2015 at 12:44 AM, Matt Turner matts...@gmail.com wrote:
 On Thu, Jun 25, 2015 at 2:05 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 _mesa_strtod and _mesa_strtof are only used from the GLSL compiler and
 the ARB_[vertex|fragment]_program code, meaning that the locale doesn't
 need to be initialized before the first OpenGL context gets initialized.

 So let's use explicit initialization from the one-time init code instead
 of depending on a C++ compiler to initialize at image-load time.

 Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
 Reviewed-by: Matt Turner matts...@gmail.com
 ---
  src/glsl/glcpp/glcpp.c|  3 ++
  src/glsl/main.cpp |  3 ++
  src/mesa/main/context.c   |  3 ++
  src/util/Makefile.sources |  2 +-
  src/util/strtod.c | 78 
 +++
  src/util/strtod.cpp   | 75 -
  src/util/strtod.h |  3 ++
  7 files changed, 91 insertions(+), 76 deletions(-)
  create mode 100644 src/util/strtod.c
  delete mode 100644 src/util/strtod.cpp

 diff --git a/src/glsl/glcpp/glcpp.c b/src/glsl/glcpp/glcpp.c
 index 5144516..c62f4ef 100644
 --- a/src/glsl/glcpp/glcpp.c
 +++ b/src/glsl/glcpp/glcpp.c
 @@ -29,6 +29,7 @@
  #include glcpp.h
  #include main/mtypes.h
  #include main/shaderobj.h
 +#include util/strtod.h

  extern int glcpp_parser_debug;

 @@ -168,6 +169,8 @@ main (int argc, char *argv[])
 if (shader == NULL)
return 1;

 +   _mesa_locale_init();
 +
 ret = glcpp_preprocess(ctx, shader, info_log, NULL, gl_ctx);

 printf(%s, shader);
 diff --git a/src/glsl/main.cpp b/src/glsl/main.cpp
 index 2341298..58651df 100644
 --- a/src/glsl/main.cpp
 +++ b/src/glsl/main.cpp
 @@ -38,6 +38,7 @@
  #include program/hash_table.h
  #include loop_analysis.h
  #include standalone_scaffolding.h
 +#include util/strtod.h

  static int glsl_version = 330;

 @@ -46,6 +47,8 @@ initialize_context(struct gl_context *ctx, gl_api api)
  {
 initialize_context_to_defaults(ctx, api);

 +   _mesa_locale_init();
 +
 /* The standalone compiler needs to claim support for almost
  * everything in order to compile the built-in functions.
  */
 diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
 index c4af8ea..e68de68 100644
 --- a/src/mesa/main/context.c
 +++ b/src/mesa/main/context.c
 @@ -120,6 +120,7 @@
  #include shaderobj.h
  #include shaderimage.h
  #include util/simple_list.h
 +#include util/strtod.h
  #include state.h
  #include stencil.h
  #include texcompress_s3tc.h
 @@ -374,6 +375,8 @@ one_time_init( struct gl_context *ctx )
assert( sizeof(GLint) == 4 );
assert( sizeof(GLuint) == 4 );

 +  _mesa_locale_init();
 +
_mesa_one_time_init_extension_overrides();

_mesa_get_cpu_features();
 diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
 index dc55939..82df3bc 100644
 --- a/src/util/Makefile.sources
 +++ b/src/util/Makefile.sources
 @@ -19,7 +19,7 @@ MESA_UTIL_FILES :=\
 set.c \
 set.h \
 simple_list.h \
 -   strtod.cpp \
 +   strtod.c \
 strtod.h \
 texcompress_rgtc_tmp.h \
 u_atomic.h
 diff --git a/src/util/strtod.c b/src/util/strtod.c
 new file mode 100644
 index 000..e5e6f76
 --- /dev/null
 +++ b/src/util/strtod.c
 @@ -0,0 +1,78 @@
 +/*
 + * Copyright 2010 VMware, Inc.
 + * All Rights Reserved.
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a
 + * copy of this software and associated documentation files (the
 + * Software), to deal in the Software without restriction, including
 + * without limitation the rights to use, copy, modify, merge, publish,
 + * distribute, sub license, and/or sell copies of the Software, and to
 + * permit persons to whom the Software is furnished to do so, subject to
 + * the following conditions:
 + *
 + * The above copyright notice and this permission notice (including the
 + * next paragraph) shall be included in all copies or substantial portions
 + * of the Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS
 + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
 + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
 + * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
 + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
 + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
 + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 + */
 +
 +
 +#include stdlib.h
 +
 +#ifdef _GNU_SOURCE
 +#include locale.h
 +#ifdef HAVE_XLOCALE_H
 +#include xlocale.h
 +static locale_t loc;
 +static int initialized;

 This is unused until commit 7 (and I don't see a lot of value in patch
 7, but maybe I'm just not seeing it :)

Good catch, thanks. Fixed.

I have no strong feelings about patch 7 either way.
___
mesa

Re: [Mesa-dev] [PATCH v2 6/7] mesa/main: free locale at exit

[Erik Faye-Lund]

On Fri, Jun 26, 2015 at 12:36 AM, Matt Turner matts...@gmail.com wrote:
 On Thu, Jun 25, 2015 at 2:05 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 In order to save a small leak if mesa is continously loaded and
 unloaded, let's free the locale when the shared object is unloaded.

 Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
 ---
  src/mesa/main/context.c | 12 +++-
  src/util/strtod.c   |  8 
  src/util/strtod.h   |  3 +++
  3 files changed, 22 insertions(+), 1 deletion(-)

 diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
 index e68de68..dee1fa8 100644
 --- a/src/mesa/main/context.c
 +++ b/src/mesa/main/context.c
 @@ -346,6 +346,16 @@ _mesa_destroy_visual( struct gl_config *vis )
  mtx_t OneTimeLock = _MTX_INITIALIZER_NP;


 +/**
 + * Calls all the various one-time-fini functions in Mesa
 + */
 +
 +static void
 +one_time_fini()
 +{
 +   _mesa_destroy_shader_compiler();
 +   _mesa_locale_fini();
 +}

  /**
   * Calls all the various one-time-init functions in Mesa.
 @@ -385,7 +395,7 @@ one_time_init( struct gl_context *ctx )
   _mesa_ubyte_to_float_color_tab[i] = (float) i / 255.0F;
}

 -  atexit(_mesa_destroy_shader_compiler);
 +  atexit(one_time_fini);

  #if defined(DEBUG)  defined(__DATE__)  defined(__TIME__)
if (MESA_VERBOSE != 0) {
 diff --git a/src/util/strtod.c b/src/util/strtod.c
 index e5e6f76..5c36b05 100644
 --- a/src/util/strtod.c
 +++ b/src/util/strtod.c
 @@ -46,6 +46,14 @@ _mesa_locale_init(void)
  #endif
  }

 +void
 +_mesa_locale_fini(void)
 +{
 +#if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)

 _GNU_SOURCE isn't a macro that you're supposed to check if it's
 defined -- you're supposed to define it if you want GNU extensions.
 We're misusing it elsewhere, but it would be cool if we could do this
 right.

 The man page of freelocale says

Since glibc 2.10:
   _XOPEN_SOURCE = 700
Before glibc 2.10:
   _GNU_SOURCE

 (same for newlocale() used in

 glibc-2.10 is more than 6 years old. I'd be happy to use only _XOPEN_SOURCE.

 I'd be fine with fixing this in a follow up.

Seems like even RedHat 6 is beyond that (RHEL-6.6 is at GLIBC 2.12), so sure.

I was just following suit, I don't really care. But sure, doing this
as a follow-up seems appropriate.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC shader-db] Add support for shadertoy tests

[Erik Faye-Lund]

On Wed, Jun 24, 2015 at 2:18 AM, Rob Clark robdcl...@gmail.com wrote:
 On Tue, Jun 23, 2015 at 7:27 PM, Dylan Baker baker.dyla...@gmail.com wrote:
 I have a couple of python pointers for you, feel free to take them or
 leave them.

 cool, thanks..

 What do others think about including shadertoy in shader-db?  If it is
 a useful thing, I'll clean up my script and re-submit..

It's a pretty neat resource, but AFAIK it has a few (probably minor)
potential problems:

1) The data-set changes over time. This might not be a real-world
   problem, as measurements are presumably taken with two identical
   snap-shots.
2) It contains a *lot* of basically the same thing; raymarched
   raytracers. So I worry a bit that pulling in all of the
   Shadertoy-shaders ends up punishing normal shaders, by making them
   seem more fringe. The raymarchers on Shadertoy tends to be extremely
   ALU heavy compared to games shaders, and do very long loops. I have no
   doubt it's healthy to have *some* such shaders in shader-db, but
   perhaps all is overkill?
3) Since all use the same vertex shader, won't changes to that get
   artificially high scores?

I'm not saying these are showstoppers, just something that might need
addressing.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 0/7] port _mesa_strto[df] to C

[Erik Faye-Lund]

On Fri, Jun 26, 2015 at 6:14 PM, Ilia Mirkin imir...@alum.mit.edu wrote:
 On Fri, Jun 26, 2015 at 12:03 PM, Emil Velikov emil.l.veli...@gmail.com 
 wrote:
 On 25 June 2015 at 23:10, Matt Turner matts...@gmail.com wrote:
 On Thu, Jun 25, 2015 at 2:05 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 Back in March[1], I sent a patch porting _mesa_strto[df] to
 C rather than C++. I fixed up the patch according to the
 criticism, but unfortunately I dropped the ball before I sent
 out the result. So here I am, picking it back up!

 This time I've taken a deeper dive into the whole init/deinit
 of Mesa, and cleaned up a bunch of stuff in that area. And as
 a result, this time we end up freeing the locale also.

 No Piglit regressions observed.

 [1]: 1426446329-23984-1-git-send-email-kusmab...@gmail.com

 Erik Faye-Lund (7):
   mesa/main: Get rid of outdated GDB-hack
   dri: don't touch the shader compiler
   mesa/main: only call _mesa_destroy_shader_compiler once on exit
   glsl: No need to lock in _mesa_glsl_release_types
   util: port _mesa_strto[df] to C
   mesa/main: free locale at exit
   util: assert to verify that locale is initialized

 Thanks for this! The series looks good to me.

 I'm slightly worried about 2/7, but not for any reasons other than I'm
 not very familiar with that code.

 1-6 (with the caveat that I may not have any idea what I'm saying
 about 2/7 ;) are:

 Reviewed-by: Matt Turner matts...@gmail.com

 7/7 looks like it may have been useful for debugging, but I don't
 think we should necessarily commit it.
 7/7 was suggested when the conversion was hooked up in mesa alone ;-)
 So I've hinted that glsl-compiler (and maybe others) are left out and
 adding an assert will help us catch them. If they are fixed now we can
 drop the patch.

 I'm fairly confident that the offline compilers (i965?, nouveau and
 freedreno) and the i965 tests do not use _mesa_strto{d,f} (either
 directly or not). I'm believe that you've already checked ?

 nouveau (and freedreno) compilers use tgsi_text, which definitely can
 parse floats and doubles. No idea what mechanism it uses for that
 though.

tgsi_text call strtod and strtof directly. This means that it breaks
on some locales.

So that should probably be fixed. But that can happen as a follow-up.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] glsls: Modify exec_list to avoid strict-aliasing violations

[Erik Faye-Lund]

On Fri, Jun 26, 2015 at 3:05 PM, Davin McCall dav...@davmac.org wrote:
 On 26/06/15 12:55, Erik Faye-Lund wrote:

 On Fri, Jun 26, 2015 at 1:23 PM, Davin McCall dav...@davmac.org wrote:

 On 26/06/15 12:03, Davin McCall wrote:

 ... The stored value of 'n' is not accessed by any other type than the
 type of n itself. This value is then cast to a different pointer type. You
 are mistaken if you think that the cast accesses the stored value of n. The
 other stored value access that it occurs in that expression is to the
 object pointed at by the result of the cast. [...]:

 I'm sorry, I think that was phrased somewhat abrasively, which I did not
 intend. Let me try this part again. If we by break up the expression in
 order of evaluation:

 From:
return ((const struct exec_node **)n)[0]

 In order of evaluation:

 n
 - which accesses the stored value of n, i.e. a value of type 'struct exec
 node *', via n, which is obviously of that type.

 (const struct exec_node **)n
  - which casts that value, after it has been retrieved, to another type. If
 this were an aliasing violation, then casting any pointer variable to
 another type would be an aliasing violation; this is clearly not the case.

 ((const struct exec_node **)n)[0]
 - which de-references the result of the above cast, thereby accessing a
 stored value of type 'exec node *' using a glvalue of type 'exec node *'.

 I think breaking this up is a mistake, because the strict-aliasing
 rules is explicitly about the *combination* of these two things.


 It is not a mistake, and the strict aliasing rules are not about the
 combination of these two things.

It is. In fact, it's not even possible to violate strict-aliasing
without doing at least two operations. You cannot validate operations
in a vacuum, because that's not how strict-aliasing is defined.

 As I have pointed out, with your reading,
 pretty much any pointer cast constitutes an aliasing violation.


No, only those violating the strict aliasing rules I posted before.

 The strict aliasing rules specify what kind of reference you can use to
 access an object of a particular type. They say nothing about how that
 reference is obtained.

Which means that it applies regardless of how you obtain it.

If a program attempts to access the stored value of an object through
a glvalue of other than one of the following types the behavior is
undefined

It says if a *program* attempts, not if a *statement* attempts or
if an *opreation* attempts. This is a whole-program deal, not
limited to one operation in isolation.

 You *are* accessing the underlying memory of 'n' through a different
 type, and this is what strict aliasing is all about. But it takes two
 steps, a single step isn't enough to do so.


 I'm sorry, but your understanding is incorrect. Most pointer casts would be
 illegal otherwise. And in fact most casts would be illegal. For instance:

 int a;
 long b = (long) a;

 You reasoning says that the second line is a strict-aliasing violation,
 because it access the object in 'a' which is of type 'int' via a glvalue of
 type 'long'.

No, that is not in violation, because it's accessed through, and *then* casted.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] glsls: Modify exec_list to avoid strict-aliasing violations

[Erik Faye-Lund]

On Fri, Jun 26, 2015 at 4:01 PM, Francisco Jerez curroje...@riseup.net wrote:
 Davin McCall dav...@davmac.org writes:

 On 26/06/15 14:31, Eirik Byrkjeflot Anonsen wrote:
 Erik Faye-Lund kusmab...@gmail.com writes:

 On Fri, Jun 26, 2015 at 1:23 PM, Davin McCall dav...@davmac.org wrote:
 On 26/06/15 12:03, Davin McCall wrote:
 ... The stored value of 'n' is not accessed by any other type than the
 type of n itself. This value is then cast to a different pointer type. 
 You
 are mistaken if you think that the cast accesses the stored value of n. 
 The
 other stored value access that it occurs in that expression is to the
 object pointed at by the result of the cast. [...]:

 I'm sorry, I think that was phrased somewhat abrasively, which I did not
 intend. Let me try this part again. If we by break up the expression in
 order of evaluation:

 From:
 return ((const struct exec_node **)n)[0]

 In order of evaluation:

 n
 - which accesses the stored value of n, i.e. a value of type 'struct exec
 node *', via n, which is obviously of that type.

 (const struct exec_node **)n
   - which casts that value, after it has been retrieved, to another type. 
 If
 this were an aliasing violation, then casting any pointer variable to
 another type would be an aliasing violation; this is clearly not the case.

 ((const struct exec_node **)n)[0]
 - which de-references the result of the above cast, thereby accessing a
 stored value of type 'exec node *' using a glvalue of type 'exec node *'.
 I think breaking this up is a mistake, because the strict-aliasing
 rules is explicitly about the *combination* of these two things.

 You *are* accessing the underlying memory of 'n' through a different
 type, and this is what strict aliasing is all about. But it takes two
 steps, a single step isn't enough to do so.

 Those other spec-quotes doesn't undo the strict-aliasing definitions;
 knowing how things are laid out in memory doesn't mean the compiler
 cannot assume two differently typed variables doesn't overlap.
 So basically, you're saying that e.g.:

 p-next = a;
 q = exec_node_get_next_const(p);

 is equivalent to:

 exec_node * p1 = p;
 exec_node ** p2 = (exec_node**)p;
 p1-next = a;
 q = p2[0];

 It is, once the patch is applied (or if strict aliasing is disabled).

 And at this point p1 and p2 are different types, so the compiler can
 freely assume that p1 and p2 are non-overlapping.

 p1 and p2 are two separate variables and of course they are
 non-overlapping, but *p1 and **p2 are the same type and so may overlap.

 Also note that even *p1 and *p2 are allowed to overlap even though they
 are of different types because of section 6.5 of C99:

 | 7 An object shall have its stored value accessed only by an lvalue
 |   expression that has one of the following types:
 |[...]
 |- an aggregate or union type that includes one of the aforementioned
 |  types among its members (including, recursively, a member of a
 |  subaggregate or contained union)[...]

I don't see how this wording legitimates the code above. There's no
unions involved, so that part is irrelevant. And n isn't an
aggregate type, it's a pointer type that happens to point to an
aggregate type, no? But even if it were, it needs to include one of
the aforementioned types among its members, which I cannot see that
it does either.

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

Re: [Mesa-dev] [PATCH v2] glsls: Modify exec_list to avoid strict-aliasing violations

[Erik Faye-Lund]

On Fri, Jun 26, 2015 at 5:25 PM, Francisco Jerez curroje...@riseup.net wrote:
 Erik Faye-Lund kusmab...@gmail.com writes:

 On Fri, Jun 26, 2015 at 4:53 PM, Francisco Jerez curroje...@riseup.net 
 wrote:
 Erik Faye-Lund kusmab...@gmail.com writes:

 On Fri, Jun 26, 2015 at 4:16 PM, Davin McCall dav...@davmac.org wrote:
 On 26/06/15 14:53, Erik Faye-Lund wrote:

 On Fri, Jun 26, 2015 at 3:05 PM, Davin McCall dav...@davmac.org wrote:

 On 26/06/15 12:55, Erik Faye-Lund wrote:

 On Fri, Jun 26, 2015 at 1:23 PM, Davin McCall dav...@davmac.org wrote:

 On 26/06/15 12:03, Davin McCall wrote:

 ... The stored value of 'n' is not accessed by any other type than the
 type of n itself. This value is then cast to a different pointer type.
 You
 are mistaken if you think that the cast accesses the stored value of n.
 The
 other stored value access that it occurs in that expression is to the
 object pointed at by the result of the cast. [...]:

 I'm sorry, I think that was phrased somewhat abrasively, which I did not
 intend. Let me try this part again. If we by break up the expression in
 order of evaluation:

 From:
 return ((const struct exec_node **)n)[0]

 In order of evaluation:

 n
 - which accesses the stored value of n, i.e. a value of type 'struct 
 exec
 node *', via n, which is obviously of that type.

 (const struct exec_node **)n
   - which casts that value, after it has been retrieved, to another 
 type.
 If
 this were an aliasing violation, then casting any pointer variable to
 another type would be an aliasing violation; this is clearly not the
 case.

 ((const struct exec_node **)n)[0]
 - which de-references the result of the above cast, thereby accessing a
 stored value of type 'exec node *' using a glvalue of type 'exec node 
 *'.

 I think breaking this up is a mistake, because the strict-aliasing
 rules is explicitly about the *combination* of these two things.


 It is not a mistake, and the strict aliasing rules are not about the
 combination of these two things.

 It is. In fact, it's not even possible to violate strict-aliasing
 without doing at least two operations. You cannot validate operations
 in a vacuum, because that's not how strict-aliasing is defined.


 Any pointer dereference can violate strict aliasing - that's one 
 operation.
 If you mean that it's first necessary to construct a pointer value in 
 such a
 way that de-referencing it will be an aliasing violation, then yes, I 
 agree
 with this statement.


 Yes, I mean exactly the latter. You cannot look at one operation in
 isolation, you need to look at the whole program.


 As I have pointed out, with your reading,
 pretty much any pointer cast constitutes an aliasing violation.

 No, only those violating the strict aliasing rules I posted before.


 ... which would only allow changing const/volatile qualifiers, not the
 pointed-to type.


 You can change the pointed to type in terms of signedness, you can
 cast it to a compatible type, you can cast a void-pointer or
 char-pointer to any type. But you need to make sure you don't violate
 the strict-aliasing rules in some other way while doing the latter.

 Aliasing *is* hard. But let's not go shopping for that reason.

 Your reading also disallows casting an 'int' variable to type 'long',
 because that isn't on the list.


 The strict aliasing rules specify what kind of reference you can use to
 access an object of a particular type. They say nothing about how that
 reference is obtained.

 Which means that it applies regardless of how you obtain it.


 Yes.

 If a program attempts to access the stored value of an object through
 a glvalue of other than one of the following types the behavior is
 undefined

 It says if a *program* attempts, not if a *statement* attempts or
 if an *opreation* attempts. This is a whole-program deal, not
 limited to one operation in isolation.


 The key part of the wording is through a glvalue:

 If a program attempts to access the stored value of an object *through
 a glvalue* of other than one of the following types ...

 This is exactly what makes this invalid AFAICT, see below.

 Going back to the original example:

return ((const struct exec_node **)n)[0]

 The glvalue used to access the object in n is n itself. (I do not think 
 that
 '(const struct exec_node **)n' is even a glvalue).

 Bur 'n' *is* an lvalue, which also makes it an glvalue (for reference,
 a glvalue is a generalized lvalue, which means that it's either an
 lvalue or an xvalue). You can write stuff like:


 n is indeed an lvalue (which in no way aliases the storage of any
 exec_node or exec_list object)

 '(const struct exec_node **)n' is an lvalue who alias the storage to n.

 , the result of the cast expression is
 not,

 The result of the cast is also an lvalue. You can assign to a casted pointer.

 No, you can't, you can only assign to the result of dereferencing the
 result of a casted pointer.  See the footnote in section 6.5.4 of the
 C99 spec:

 | 89

[Mesa-dev] [PATCH v3 2/6] dri: don't touch the shader compiler

[Erik Faye-Lund]

This function is for deleting per-screen resources, and the shader
compiler resources are not of such nature. Besides, dri shouldn't
need to even know about the presence of a shader compiler.

These resources will already be released when mesa gets unloaded,
and that should be sufficient.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Reviewed-by: Matt Turner matts...@gmail.com
---
 src/mesa/drivers/dri/common/dri_util.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/mesa/drivers/dri/common/dri_util.c 
b/src/mesa/drivers/dri/common/dri_util.c
index e7ababe..ae4592c 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c
@@ -46,7 +46,6 @@
 #include dri_util.h
 #include utils.h
 #include xmlpool.h
-#include ../glsl/glsl_parser_extras.h
 #include main/mtypes.h
 #include main/version.h
 #include main/errors.h
@@ -238,8 +237,6 @@ static void driDestroyScreen(__DRIscreen *psp)
 * stream open to the X-server anymore.
 */
 
-   _mesa_destroy_shader_compiler();
-
psp-driver-DestroyScreen(psp);
 
driDestroyOptionCache(psp-optionCache);
-- 
2.1.4

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

[Mesa-dev] [PATCH v3 6/6] mesa/main: free locale at exit

[Erik Faye-Lund]

In order to save a small leak if mesa is continously loaded and
unloaded, let's free the locale when the shared object is unloaded.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Reviewed-by: Matt Turner matts...@gmail.com
---
 src/mesa/main/context.c | 12 +++-
 src/util/strtod.c   |  8 
 src/util/strtod.h   |  3 +++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index e68de68..dee1fa8 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -346,6 +346,16 @@ _mesa_destroy_visual( struct gl_config *vis )
 mtx_t OneTimeLock = _MTX_INITIALIZER_NP;
 
 
+/**
+ * Calls all the various one-time-fini functions in Mesa
+ */
+
+static void
+one_time_fini()
+{
+   _mesa_destroy_shader_compiler();
+   _mesa_locale_fini();
+}
 
 /**
  * Calls all the various one-time-init functions in Mesa.
@@ -385,7 +395,7 @@ one_time_init( struct gl_context *ctx )
  _mesa_ubyte_to_float_color_tab[i] = (float) i / 255.0F;
   }
 
-  atexit(_mesa_destroy_shader_compiler);
+  atexit(one_time_fini);
 
 #if defined(DEBUG)  defined(__DATE__)  defined(__TIME__)
   if (MESA_VERBOSE != 0) {
diff --git a/src/util/strtod.c b/src/util/strtod.c
index a4a60e0..ea7d395 100644
--- a/src/util/strtod.c
+++ b/src/util/strtod.c
@@ -45,6 +45,14 @@ _mesa_locale_init(void)
 #endif
 }
 
+void
+_mesa_locale_fini(void)
+{
+#if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
+   freelocale(loc);
+#endif
+}
+
 /**
  * Wrapper around strtod which uses the C locale so the decimal
  * point is always '.'
diff --git a/src/util/strtod.h b/src/util/strtod.h
index b7e2beb..60e15cf 100644
--- a/src/util/strtod.h
+++ b/src/util/strtod.h
@@ -34,6 +34,9 @@ extern C {
 extern void
 _mesa_locale_init(void);
 
+extern void
+_mesa_locale_fini(void);
+
 extern double
 _mesa_strtod(const char *s, char **end);
 
-- 
2.1.4

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

Re: [Mesa-dev] [PATCH v2 0/7] port _mesa_strto[df] to C

[Erik Faye-Lund]

On Fri, Jun 26, 2015 at 6:03 PM, Emil Velikov emil.l.veli...@gmail.com wrote:
 On 25 June 2015 at 23:10, Matt Turner matts...@gmail.com wrote:
 On Thu, Jun 25, 2015 at 2:05 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 Back in March[1], I sent a patch porting _mesa_strto[df] to
 C rather than C++. I fixed up the patch according to the
 criticism, but unfortunately I dropped the ball before I sent
 out the result. So here I am, picking it back up!

 This time I've taken a deeper dive into the whole init/deinit
 of Mesa, and cleaned up a bunch of stuff in that area. And as
 a result, this time we end up freeing the locale also.

 No Piglit regressions observed.

 [1]: 1426446329-23984-1-git-send-email-kusmab...@gmail.com

 Erik Faye-Lund (7):
   mesa/main: Get rid of outdated GDB-hack
   dri: don't touch the shader compiler
   mesa/main: only call _mesa_destroy_shader_compiler once on exit
   glsl: No need to lock in _mesa_glsl_release_types
   util: port _mesa_strto[df] to C
   mesa/main: free locale at exit
   util: assert to verify that locale is initialized

 Thanks for this! The series looks good to me.

 I'm slightly worried about 2/7, but not for any reasons other than I'm
 not very familiar with that code.

 1-6 (with the caveat that I may not have any idea what I'm saying
 about 2/7 ;) are:

 Reviewed-by: Matt Turner matts...@gmail.com

 7/7 looks like it may have been useful for debugging, but I don't
 think we should necessarily commit it.
 7/7 was suggested when the conversion was hooked up in mesa alone ;-)
 So I've hinted that glsl-compiler (and maybe others) are left out and
 adding an assert will help us catch them. If they are fixed now we can
 drop the patch.

 I'm fairly confident that the offline compilers (i965?, nouveau and
 freedreno) and the i965 tests do not use _mesa_strto{d,f} (either
 directly or not). I'm believe that you've already checked ?

Yeah. _mesa_strto[df] is only used in src/glsl/glsl_lexer.ll,
src/glsl/s_expression.cpp and src/mesa/program/program_lexer.l. None
of the call-sites seems to be exposed outside of their modules.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 6/6] mesa/main: free locale at exit

[Erik Faye-Lund]

On Fri, Jun 26, 2015 at 9:05 PM, Brian Paul bri...@vmware.com wrote:
 On 06/26/2015 12:06 PM, Erik Faye-Lund wrote:

 In order to save a small leak if mesa is continously loaded and
 unloaded, let's free the locale when the shared object is unloaded.

 Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
 Reviewed-by: Matt Turner matts...@gmail.com
 ---
   src/mesa/main/context.c | 12 +++-
   src/util/strtod.c   |  8 
   src/util/strtod.h   |  3 +++
   3 files changed, 22 insertions(+), 1 deletion(-)

 diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
 index e68de68..dee1fa8 100644
 --- a/src/mesa/main/context.c
 +++ b/src/mesa/main/context.c
 @@ -346,6 +346,16 @@ _mesa_destroy_visual( struct gl_config *vis )
   mtx_t OneTimeLock = _MTX_INITIALIZER_NP;


 +/**
 + * Calls all the various one-time-fini functions in Mesa
 + */
 +
 +static void
 +one_time_fini()


 I think that should be one_time_fini(void) to be consistent and to avoid
 warnings with some compilers.

Good eyes, I've fixed that locally. Thanks :)

 Otherwise, the series looks good to me.  Nice to see patch 1.  I remember
 writing that dumb code years ago.

 Reviewed-by: Brian Paul bri...@vmware.com

Thanks. Is that Reviewed-by for this patch, or the whole series?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] glsls: Modify exec_list to avoid strict-aliasing violations

[Erik Faye-Lund]

On Thu, Jun 25, 2015 at 1:48 AM, Davin McCall dav...@davmac.org wrote:
 This is an alternative to my earlier patch [1] (and it is now constructed
 properly using git format-patch).

 Quick background:
 There is a problem in exec_list due to it directly including a trio
 of 'struct exec_node *' members to implement two overlapping sentinel
 nodes. The sentinel nodes do not exist as exec_node objects and so
 should not be accessed as such, according to C99 6.5 paragraph 7.
 When this strict aliasing rule is violated the compiler may re-order
 reads and writes in unexpected ways, such as demonstrated in another
 email [2].

 The problem only manifests if compiling without -fno-strict-aliasing.

 This patch addresses the issue by introducing some new methods for
 setting the 'next' and 'prev' members of the exec_node structure, which
 avoid the aliasing restrictions by way of casting the exec_node pointer
 (to an exec_node-pointer-pointer) whenever it may actual point to a
 sentinel node. Essentially an exec_node can be seen as an array of two
 exec_node pointers, and this view is compatible with the sentinel
 structure in exec_list.

 Compared to the previous patch, this patch is much less intrusive, and
 does not increase the storage requirements of the exec_list structure.

 While I'm not proposing that -fno-strict-aliasing no longer be used for
 Mesa builds, this patch represents a step in that direction. With this
 patch applied, a working Mesa library can be built, although bugs may
 be present (and could be triggered only when using particular compiler
 versions and options). FWIW file sizes with and without strict aliasing:

 (gcc 4.8.4, -O3 -fomit-frame-pointer -march=corei7).

 -fno-strict-aliasing:with strict aliasing:
 libGL.so  699188  699188(no change)
 *_dri.so 9575876 9563104(-2772)

 (dri bundle includes r300, r600, kms_swrast and swrast).

 So, not a huge win, size-wise. Dave Airlie reports a 30K difference in
 his r600_dri.so build however [3].

 In terms of performance:

 (export LIBGL_ALWAYS_SOFTWARE=1; time glmark2)

 -fno-strict-aliasing:

 glmark2 Score: 244
 real5m34.707s
 user11m36.192s
 sys0m29.596s

 with strict aliasing:

 glmark2 Score: 247
 real5m34.438s
 user11m29.904s
 sys0m29.556s

 Again, only a very small improvement when strict aliasing is enabled.

 With the above in mind it is reasonable to question whether this patch
 is worthwhile. However, it's done, and it seems to work, so I offer it
 for review.


 [1] http://lists.freedesktop.org/archives/mesa-dev/2015-June/087179.html
 [2] http://lists.freedesktop.org/archives/mesa-dev/2015-June/087246.html
 [3] http://lists.freedesktop.org/archives/mesa-dev/2015-June/087206.html
 ---
  src/glsl/list.h | 123
 
  1 file changed, 80 insertions(+), 43 deletions(-)

 diff --git a/src/glsl/list.h b/src/glsl/list.h
 index 15fcd4a..cfbe5a9 100644
 --- a/src/glsl/list.h
 +++ b/src/glsl/list.h
 @@ -76,6 +76,12 @@
  #include util/ralloc.h

  struct exec_node {
 +   /**
 +* Accessing these fields directly may be ill-advised; if the
 'exec_node'
 +* is actually a sentinel node embedded in the exec_list structure, it
 may
 +* be a strict-aliasing violation (C99 6.5 paragraph 7). Use the methods
 +* provided instead.
 +*/
 struct exec_node *next;
 struct exec_node *prev;

 @@ -140,35 +146,55 @@ exec_node_init(struct exec_node *n)
 n-prev = NULL;
  }

 +/**
 + * Strict-aliasing safe method for setting the next pointer for any
 + * node, including sentinel nodes.
 + */
 +static inline void
 +exec_node_set_next(struct exec_node *n, struct exec_node *next)
 +{
 +   ((struct exec_node **)n)[0] = next;
 +}
 +
 +/**
 + * Strict-aliasing safe method for setting the next pointer for any
 + * node, including sentinel nodes.
 + */
 +static inline void
 +exec_node_set_prev(struct exec_node *n, struct exec_node *next)
 +{
 +   ((struct exec_node **)n)[1] = next;
 +}
 +
  static inline const struct exec_node *
  exec_node_get_next_const(const struct exec_node *n)
  {
 -   return n-next;
 +   return ((const struct exec_node **)n)[0];
  }

How exactly is this supposed to be strict-aliasing safe?

Here's the wording from the C++14 spec:

If a program attempts to access the stored value of an object through
a glvalue of other than one of the following types the behavior is
undefined:
* the dynamic type of the object,
* a cv-qualified version of the dynamic type of the object,
* a type similar (as defined in 4.4) to the dynamic type of the object,
* a that is the signed or unsigned type corresponype tding to the
dynamic type of the object,
* a type that is the signed or unsigned type corresponding to a
cv-qualified version of the dynamic type of the object,
* an aggregate or union type that includes one of the aforementioned
types among its elements or non-static data members 

Re: [Mesa-dev] [PATCH 6/7] mesa: build xmlconfig to a separate static library

[Erik Faye-Lund]

On Thu, Jun 11, 2015 at 1:19 AM, Matt Turner matts...@gmail.com wrote:
 On Wed, Jun 10, 2015 at 3:54 PM, Emil Velikov emil.l.veli...@gmail.com 
 wrote:
 From: Erik Faye-Lund kusmab...@gmail.com

 As we use the file from both the dri modules and loader, we end up with
 multiple definition of the symbols provided in our gallium dri  modules.
 Additionally we compile the file twice.

 Resolve both issues, effectively enabling the build on toolchains which
 don't support -Wl,--allow-multiple-definition.

 v2: [Emil Velikov]
  - Fix the Scons/Android build.
  - Resolve libgbm build issues (bring back the missing -lm)

 Cc: Julien Isorce j.iso...@samsung.com
 Cc: 10.5 10.6 mesa-sta...@lists.freedesktop.org
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90310
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90905
 Signed-off-by: Emil Velikov emil.l.veli...@gmail.com
 ---
  src/gallium/targets/dri/Makefile.am  |  6 --
  src/loader/Makefile.am   | 10 +++---
  src/mesa/drivers/dri/Makefile.am |  1 +
  src/mesa/drivers/dri/common/Android.mk   |  4 +++-
  src/mesa/drivers/dri/common/Makefile.am  |  6 +-
  src/mesa/drivers/dri/common/Makefile.sources |  4 +++-
  src/mesa/drivers/dri/common/SConscript   |  2 +-
  src/mesa/drivers/dri/i965/Makefile.am|  1 +
  8 files changed, 17 insertions(+), 17 deletions(-)

 diff --git a/src/gallium/targets/dri/Makefile.am 
 b/src/gallium/targets/dri/Makefile.am
 index f9e4ada..9648396 100644
 --- a/src/gallium/targets/dri/Makefile.am
 +++ b/src/gallium/targets/dri/Makefile.am
 @@ -53,12 +53,6 @@ gallium_dri_la_LIBADD = \
 $(LIBDRM_LIBS) \
 $(GALLIUM_COMMON_LIB_DEPS)

 -# XXX: Temporary allow duplicated symbols, as the loader pulls in 
 xmlconfig.c
 -# which already provides driParse* and driQuery* amongst others.
 -# Remove this hack as we come up with a cleaner solution.
 -gallium_dri_la_LDFLAGS += \
 -   -Wl,--allow-multiple-definition
 -
  EXTRA_gallium_dri_la_DEPENDENCIES = \
 dri.sym \
 $(top_srcdir)/src/gallium/targets/dri-vdpau.dyn
 diff --git a/src/loader/Makefile.am b/src/loader/Makefile.am
 index 36ddba8..aef1bd6 100644
 --- a/src/loader/Makefile.am
 +++ b/src/loader/Makefile.am
 @@ -41,15 +41,11 @@ libloader_la_CPPFLAGS += \
 -I$(top_builddir)/src/mesa/drivers/dri/common/ \
 -I$(top_srcdir)/src/mesa/ \
 -I$(top_srcdir)/src/mapi/ \
 -   -DUSE_DRICONF \
 -   $(EXPAT_CFLAGS)
 +   -DUSE_DRICONF

 -libloader_la_SOURCES += \
 -   $(top_srcdir)/src/mesa/drivers/dri/common/xmlconfig.c
 + libloader_la_LIBADD += \

 Looks like we have an extra leading space here.

 Do I understand correctly that after this patch the Gallium drivers
 will get their only copy of xmlconfig via linking against
 libloader.la?

 If that's correct,

 Acked-by: Matt Turner matts...@gmail.com

That was the intention, yeah.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v4 5/6] util: port _mesa_strto[df] to C

[Erik Faye-Lund]

_mesa_strtod and _mesa_strtof are only used from the GLSL compiler and
the ARB_[vertex|fragment]_program code, meaning that the locale doesn't
need to be initialized before the first OpenGL context gets initialized.

So let's use explicit initialization from the one-time init code instead
of depending on a C++ compiler to initialize at image-load time.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Reviewed-by: Matt Turner matts...@gmail.com
Reviewed-by: Brian Paul bri...@vmware.com
---
 src/glsl/glcpp/glcpp.c|  3 ++
 src/glsl/main.cpp |  3 ++
 src/mesa/main/context.c   |  3 ++
 src/util/Makefile.sources |  2 +-
 src/util/strtod.c | 77 +++
 src/util/strtod.cpp   | 75 -
 src/util/strtod.h |  3 ++
 7 files changed, 90 insertions(+), 76 deletions(-)
 create mode 100644 src/util/strtod.c
 delete mode 100644 src/util/strtod.cpp

diff --git a/src/glsl/glcpp/glcpp.c b/src/glsl/glcpp/glcpp.c
index 5144516..c62f4ef 100644
--- a/src/glsl/glcpp/glcpp.c
+++ b/src/glsl/glcpp/glcpp.c
@@ -29,6 +29,7 @@
 #include glcpp.h
 #include main/mtypes.h
 #include main/shaderobj.h
+#include util/strtod.h
 
 extern int glcpp_parser_debug;
 
@@ -168,6 +169,8 @@ main (int argc, char *argv[])
if (shader == NULL)
   return 1;
 
+   _mesa_locale_init();
+
ret = glcpp_preprocess(ctx, shader, info_log, NULL, gl_ctx);
 
printf(%s, shader);
diff --git a/src/glsl/main.cpp b/src/glsl/main.cpp
index 2341298..58651df 100644
--- a/src/glsl/main.cpp
+++ b/src/glsl/main.cpp
@@ -38,6 +38,7 @@
 #include program/hash_table.h
 #include loop_analysis.h
 #include standalone_scaffolding.h
+#include util/strtod.h
 
 static int glsl_version = 330;
 
@@ -46,6 +47,8 @@ initialize_context(struct gl_context *ctx, gl_api api)
 {
initialize_context_to_defaults(ctx, api);
 
+   _mesa_locale_init();
+
/* The standalone compiler needs to claim support for almost
 * everything in order to compile the built-in functions.
 */
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index c4af8ea..e68de68 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -120,6 +120,7 @@
 #include shaderobj.h
 #include shaderimage.h
 #include util/simple_list.h
+#include util/strtod.h
 #include state.h
 #include stencil.h
 #include texcompress_s3tc.h
@@ -374,6 +375,8 @@ one_time_init( struct gl_context *ctx )
   assert( sizeof(GLint) == 4 );
   assert( sizeof(GLuint) == 4 );
 
+  _mesa_locale_init();
+
   _mesa_one_time_init_extension_overrides();
 
   _mesa_get_cpu_features();
diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
index dc55939..82df3bc 100644
--- a/src/util/Makefile.sources
+++ b/src/util/Makefile.sources
@@ -19,7 +19,7 @@ MESA_UTIL_FILES :=\
set.c \
set.h \
simple_list.h \
-   strtod.cpp \
+   strtod.c \
strtod.h \
texcompress_rgtc_tmp.h \
u_atomic.h
diff --git a/src/util/strtod.c b/src/util/strtod.c
new file mode 100644
index 000..a4a60e0
--- /dev/null
+++ b/src/util/strtod.c
@@ -0,0 +1,77 @@
+/*
+ * Copyright 2010 VMware, Inc.
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * Software), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+ * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
+ * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+
+#include stdlib.h
+
+#ifdef _GNU_SOURCE
+#include locale.h
+#ifdef HAVE_XLOCALE_H
+#include xlocale.h
+static locale_t loc;
+#endif
+#endif
+
+#include strtod.h
+
+
+void
+_mesa_locale_init(void)
+{
+#if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
+   loc = newlocale(LC_CTYPE_MASK, C, NULL);
+#endif
+}
+
+/**
+ * Wrapper around strtod which uses the C locale so the decimal
+ * point is always '.'
+ */
+double
+_mesa_strtod(const char *s, char **end)
+{
+#if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
+   return strtod_l(s, end, loc);
+#else
+   return strtod(s, end);
+#endif
+}
+
+
+/**
+ * Wrapper around

[Mesa-dev] [PATCH v4 3/6] mesa/main: only call _mesa_destroy_shader_compiler once on exit

[Erik Faye-Lund]

There's no point in calling _mesa_destroy_shader_compiler multiple
times on exit; the resources will only be released once anyway.

So let's move the atexit-call into the part that is only called
once.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Reviewed-by: Matt Turner matts...@gmail.com
Reviewed-by: Brian Paul bri...@vmware.com
---
 src/mesa/main/context.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 265f98a..c4af8ea 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -382,6 +382,8 @@ one_time_init( struct gl_context *ctx )
  _mesa_ubyte_to_float_color_tab[i] = (float) i / 255.0F;
   }
 
+  atexit(_mesa_destroy_shader_compiler);
+
 #if defined(DEBUG)  defined(__DATE__)  defined(__TIME__)
   if (MESA_VERBOSE != 0) {
 _mesa_debug(ctx, Mesa %s DEBUG build %s %s\n,
@@ -404,11 +406,6 @@ one_time_init( struct gl_context *ctx )
api_init_mask |= 1  ctx-API;
 
mtx_unlock(OneTimeLock);
-
-   /* Hopefully atexit() is widely available.  If not, we may need some
-* #ifdef tests here.
-*/
-   atexit(_mesa_destroy_shader_compiler);
 }
 
 
-- 
2.1.4

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

[Mesa-dev] [PATCH v4 4/6] glsl: No need to lock in _mesa_glsl_release_types

[Erik Faye-Lund]

This function only gets called while mesa is unloading, so there's
no potential of racing or multiple calls at the same time. So let's
just get rid of the locking.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Reviewed-by: Matt Turner matts...@gmail.com
Reviewed-by: Brian Paul bri...@vmware.com
---
 src/glsl/glsl_types.cpp | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index f675e90..c622380 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -324,8 +324,10 @@ const glsl_type *glsl_type::get_scalar_type() const
 void
 _mesa_glsl_release_types(void)
 {
-   mtx_lock(glsl_type::mutex);
-
+   /* Should only be called during atexit (either when unloading shared
+* object, or if process terminates), so no mutex-locking should be
+* necessary.
+*/
if (glsl_type::array_types != NULL) {
   hash_table_dtor(glsl_type::array_types);
   glsl_type::array_types = NULL;
@@ -335,8 +337,6 @@ _mesa_glsl_release_types(void)
   hash_table_dtor(glsl_type::record_types);
   glsl_type::record_types = NULL;
}
-
-   mtx_unlock(glsl_type::mutex);
 }
 
 
-- 
2.1.4

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

[Mesa-dev] [PATCH v4 6/6] mesa/main: free locale at exit

[Erik Faye-Lund]

In order to save a small leak if mesa is continously loaded and
unloaded, let's free the locale when the shared object is unloaded.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Reviewed-by: Matt Turner matts...@gmail.com
Reviewed-by: Brian Paul bri...@vmware.com
---
 src/mesa/main/context.c | 12 +++-
 src/util/strtod.c   |  8 
 src/util/strtod.h   |  3 +++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index e68de68..fdef412 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -346,6 +346,16 @@ _mesa_destroy_visual( struct gl_config *vis )
 mtx_t OneTimeLock = _MTX_INITIALIZER_NP;
 
 
+/**
+ * Calls all the various one-time-fini functions in Mesa
+ */
+
+static void
+one_time_fini(void)
+{
+   _mesa_destroy_shader_compiler();
+   _mesa_locale_fini();
+}
 
 /**
  * Calls all the various one-time-init functions in Mesa.
@@ -385,7 +395,7 @@ one_time_init( struct gl_context *ctx )
  _mesa_ubyte_to_float_color_tab[i] = (float) i / 255.0F;
   }
 
-  atexit(_mesa_destroy_shader_compiler);
+  atexit(one_time_fini);
 
 #if defined(DEBUG)  defined(__DATE__)  defined(__TIME__)
   if (MESA_VERBOSE != 0) {
diff --git a/src/util/strtod.c b/src/util/strtod.c
index a4a60e0..ea7d395 100644
--- a/src/util/strtod.c
+++ b/src/util/strtod.c
@@ -45,6 +45,14 @@ _mesa_locale_init(void)
 #endif
 }
 
+void
+_mesa_locale_fini(void)
+{
+#if defined(_GNU_SOURCE)  defined(HAVE_XLOCALE_H)
+   freelocale(loc);
+#endif
+}
+
 /**
  * Wrapper around strtod which uses the C locale so the decimal
  * point is always '.'
diff --git a/src/util/strtod.h b/src/util/strtod.h
index b7e2beb..60e15cf 100644
--- a/src/util/strtod.h
+++ b/src/util/strtod.h
@@ -34,6 +34,9 @@ extern C {
 extern void
 _mesa_locale_init(void);
 
+extern void
+_mesa_locale_fini(void);
+
 extern double
 _mesa_strtod(const char *s, char **end);
 
-- 
2.1.4

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

[Mesa-dev] [PATCH v4 2/6] dri: don't touch the shader compiler

[Erik Faye-Lund]

This function is for deleting per-screen resources, and the shader
compiler resources are not of such nature. Besides, dri shouldn't
need to even know about the presence of a shader compiler.

These resources will already be released when mesa gets unloaded,
and that should be sufficient.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Reviewed-by: Matt Turner matts...@gmail.com
Reviewed-by: Brian Paul bri...@vmware.com
---
 src/mesa/drivers/dri/common/dri_util.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/mesa/drivers/dri/common/dri_util.c 
b/src/mesa/drivers/dri/common/dri_util.c
index e7ababe..ae4592c 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c
@@ -46,7 +46,6 @@
 #include dri_util.h
 #include utils.h
 #include xmlpool.h
-#include ../glsl/glsl_parser_extras.h
 #include main/mtypes.h
 #include main/version.h
 #include main/errors.h
@@ -238,8 +237,6 @@ static void driDestroyScreen(__DRIscreen *psp)
 * stream open to the X-server anymore.
 */
 
-   _mesa_destroy_shader_compiler();
-
psp-driver-DestroyScreen(psp);
 
driDestroyOptionCache(psp-optionCache);
-- 
2.1.4

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

[Mesa-dev] [PATCH v4 1/6] mesa/main: Get rid of outdated GDB-hack

[Erik Faye-Lund]

All of these enums are now in use around in the code, so there's no need
to explicitly use them here any more.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Reviewed-by: Matt Turner matts...@gmail.com
Reviewed-by: Brian Paul bri...@vmware.com
---
 src/mesa/main/context.c | 27 ---
 1 file changed, 27 deletions(-)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 79fa018..265f98a 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -338,31 +338,6 @@ _mesa_destroy_visual( struct gl_config *vis )
 
 
 /**
- * This is lame.  gdb only seems to recognize enum types that are
- * actually used somewhere.  We want to be able to print/use enum
- * values such as TEXTURE_2D_INDEX in gdb.  But we don't actually use
- * the gl_texture_index type anywhere.  Thus, this lame function.
- */
-static void
-dummy_enum_func(void)
-{
-   gl_buffer_index bi = BUFFER_FRONT_LEFT;
-   gl_face_index fi = FACE_POS_X;
-   gl_frag_result fr = FRAG_RESULT_DEPTH;
-   gl_texture_index ti = TEXTURE_2D_ARRAY_INDEX;
-   gl_vert_attrib va = VERT_ATTRIB_POS;
-   gl_varying_slot vs = VARYING_SLOT_POS;
-
-   (void) bi;
-   (void) fi;
-   (void) fr;
-   (void) ti;
-   (void) va;
-   (void) vs;
-}
-
-
-/**
  * One-time initialization mutex lock.
  *
  * \sa Used by one_time_init().
@@ -434,8 +409,6 @@ one_time_init( struct gl_context *ctx )
 * #ifdef tests here.
 */
atexit(_mesa_destroy_shader_compiler);
-
-   dummy_enum_func();
 }
 
 
-- 
2.1.4

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

[Mesa-dev] [PATCH v4 0/6] port _mesa_strto[df] to C

[Erik Faye-Lund]

Here's the fourth, and hopefully final version of this series. The
only code-change this time is fixing up the definition of one_time_fini
to have an explict void argument-list.

Erik Faye-Lund (6):
  mesa/main: Get rid of outdated GDB-hack
  dri: don't touch the shader compiler
  mesa/main: only call _mesa_destroy_shader_compiler once on exit
  glsl: No need to lock in _mesa_glsl_release_types
  util: port _mesa_strto[df] to C
  mesa/main: free locale at exit

 src/glsl/glcpp/glcpp.c |  3 ++
 src/glsl/glsl_types.cpp|  8 ++--
 src/glsl/main.cpp  |  3 ++
 src/mesa/drivers/dri/common/dri_util.c |  3 --
 src/mesa/main/context.c| 47 ++-
 src/util/Makefile.sources  |  2 +-
 src/util/strtod.c  | 85 ++
 src/util/strtod.cpp| 75 --
 src/util/strtod.h  |  6 +++
 9 files changed, 117 insertions(+), 115 deletions(-)
 create mode 100644 src/util/strtod.c
 delete mode 100644 src/util/strtod.cpp

-- 
2.1.4

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

Re: [Mesa-dev] [PATCH v4 0/6] port _mesa_strto[df] to C

[Erik Faye-Lund]

On Mon, Jun 29, 2015 at 6:40 PM, Matt Turner matts...@gmail.com wrote:
 Thanks!

 Pushed, and sent a note describing how to fix the problems caused by
 renaming strtod.cpp - strtod.c.

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

[Mesa-dev] [PATCH v2] glsl: add a missing call to _mesa_locale_init

[Erik Faye-Lund]

After c61bc6e (util: port _mesa_strto[df] to C), make check
fails due to a missing _mesa_locale_init. Fixup this oversight,
by moving the stand-alone compiler initializer inside
initialize_context_to_defaults().

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
Here's what the latter suggestion would look like. Personally,
I have a slight preference for this version.

 src/glsl/main.cpp   | 3 ---
 src/glsl/standalone_scaffolding.cpp | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/glsl/main.cpp b/src/glsl/main.cpp
index 58651df..2341298 100644
--- a/src/glsl/main.cpp
+++ b/src/glsl/main.cpp
@@ -38,7 +38,6 @@
 #include program/hash_table.h
 #include loop_analysis.h
 #include standalone_scaffolding.h
-#include util/strtod.h
 
 static int glsl_version = 330;
 
@@ -47,8 +46,6 @@ initialize_context(struct gl_context *ctx, gl_api api)
 {
initialize_context_to_defaults(ctx, api);
 
-   _mesa_locale_init();
-
/* The standalone compiler needs to claim support for almost
 * everything in order to compile the built-in functions.
 */
diff --git a/src/glsl/standalone_scaffolding.cpp 
b/src/glsl/standalone_scaffolding.cpp
index 00db61e..172c6f4 100644
--- a/src/glsl/standalone_scaffolding.cpp
+++ b/src/glsl/standalone_scaffolding.cpp
@@ -33,6 +33,7 @@
 #include stdio.h
 #include string.h
 #include util/ralloc.h
+#include util/strtod.h
 
 void
 _mesa_warning(struct gl_context *ctx, const char *fmt, ...)
@@ -191,4 +192,6 @@ void initialize_context_to_defaults(struct gl_context *ctx, 
gl_api api)
 
for (int sh = 0; sh  MESA_SHADER_STAGES; ++sh)
   memcpy(ctx-Const.ShaderCompilerOptions[sh], options, sizeof(options));
+
+   _mesa_locale_init();
 }
-- 
2.1.4

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

[Mesa-dev] [PATCH] glsl: add a missing call to _mesa_locale_init

[Erik Faye-Lund]

After c61bc6e (util: port _mesa_strto[df] to C), make check
fails due to a missing _mesa_locale_init. Fixup this oversight.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Tested-by: Vinson Lee v...@freedesktop.org
---
 src/glsl/test.cpp | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/glsl/test.cpp b/src/glsl/test.cpp
index b1ff92e..9dc7c47 100644
--- a/src/glsl/test.cpp
+++ b/src/glsl/test.cpp
@@ -37,6 +37,7 @@
 #include stdio.h
 #include stdlib.h
 #include string.h
+#include util/strtod.h
 
 #include test_optpass.h
 
@@ -67,6 +68,9 @@ static const char *extract_command_from_argv(int *argc, char 
**argv)
 int main(int argc, char **argv)
 {
const char *command = extract_command_from_argv(argc, argv);
+
+   _mesa_locale_init();
+
if (strcmp(command, optpass) == 0) {
   return test_optpass(argc, argv);
} else {
-- 
2.1.4

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

Re: [Mesa-dev] [PATCH] glsl: add a missing call to _mesa_locale_init

[Erik Faye-Lund]

On Thu, Jul 2, 2015 at 2:56 PM, Ilia Mirkin imir...@alum.mit.edu wrote:
 On Thu, Jul 2, 2015 at 5:54 PM, Matt Turner matts...@gmail.com wrote:
 On Thu, Jul 2, 2015 at 2:22 PM, Ilia Mirkin imir...@alum.mit.edu wrote:
 Can this be done at dlopen/init time? For example what happens if you do

 static int foo = _mesa_locale_init()

 IIRC things like that are possible in C++, not sure about C.

 gcc has __attribute__((constructor)).

 But I don't think we really care... Erik's series converted the strtod
 code from C++ to C (including moving locale init from being a static
 constructor to being called in one_time_init) and fixing the memory
 leak.

 Well, this is just going to happen over and over again, I was hoping
 there was an easy way to do static initializers in C. If not, then I
 guess we're stuck with this.

The good news is that the breakage was noticed real quick. I agree
that it'd be awesome to have this happen automatically, but AFAIK
there's no perfect solution for this:

* C++ static object initializer leads to libc++ dependencies.
* __attribute__((constructor)) is compiler specific.
* Naively mutex-protecting initialization leads to overhead in the common case.
* Lighter-weight double checked locks are tricky to implement, and
  also have some overhead
* pthread_once() is not available on Windows, and have some overhead

Generally, it leaves a bad taste in the mouth to have to know about
compiler-internals for being able to use the compiler. This could be
slightly mitigated by introducing something like _mesa_glsl_init(),
which means that call-sites only need to know about initializing the
compiler.

However, such a function would only call _mesa_locale_init() at this
point, so we need to know the same amount of things. I'd introduce
_mesa_glsl_init() only if it turns out we need to do more. Besides,
new call-sites for _mesa_strto{d,f} will crash visible and hard if
introduced without _mesa_locale_init.

So, while not being awesome, I think this is the lesser evil for now.

But maybe it would be more palatable to move this into
initialize_context_to_defaults() instead, which is a shared
initializer for stand-alone compilers? Perhaps it's more likely that
this function gets called by other non-driver call-sites... It would
also reduce the number of call-sites by one.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] mesa: AtomicBufferBindings should be initialized to zero.

[Erik Faye-Lund]

On Thu, Aug 13, 2015 at 12:59 PM, Marta Lofstedt
marta.lofst...@linux.intel.com wrote:
 From: Marta Lofstedt marta.lofst...@intel.com

 According to OpenGL specification version 4.5 table 23.46
 and OpenGL ES specification version 3.1 table 20.31:
 ATOMIC_COUNTER_BUFFER_START and ATOMIC_COUNTER_BUFFER_SIZE
 should have the initial value of zero.

 Signed-off-by: Marta Lofstedt marta.lofst...@intel.com
 ---
  src/mesa/main/bufferobj.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
 index 1cdea93..e17b41c 100644
 --- a/src/mesa/main/bufferobj.c
 +++ b/src/mesa/main/bufferobj.c
 @@ -866,8 +866,8 @@ _mesa_init_buffer_objects( struct gl_context *ctx )
_mesa_reference_buffer_object(ctx,
 
 ctx-AtomicBufferBindings[i].BufferObject,
 ctx-Shared-NullBufferObj);
 -  ctx-AtomicBufferBindings[i].Offset = -1;
 -  ctx-AtomicBufferBindings[i].Size = -1;
 +  ctx-AtomicBufferBindings[i].Offset = 0;
 +  ctx-AtomicBufferBindings[i].Size = 0;
 }
  }


Looks good to me!

Reviewed-by: Erik Faye-Lund kusmab...@gmail.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] glsls: Modify exec_list to avoid strict-aliasing violations

[Erik Faye-Lund]

On Fri, Jun 26, 2015 at 1:23 PM, Davin McCall dav...@davmac.org wrote:
 On 26/06/15 12:03, Davin McCall wrote:

 ... The stored value of 'n' is not accessed by any other type than the
 type of n itself. This value is then cast to a different pointer type. You
 are mistaken if you think that the cast accesses the stored value of n. The
 other stored value access that it occurs in that expression is to the
 object pointed at by the result of the cast. [...]:


 I'm sorry, I think that was phrased somewhat abrasively, which I did not
 intend. Let me try this part again. If we by break up the expression in
 order of evaluation:

 From:
return ((const struct exec_node **)n)[0]

 In order of evaluation:

 n
 - which accesses the stored value of n, i.e. a value of type 'struct exec
 node *', via n, which is obviously of that type.

 (const struct exec_node **)n
  - which casts that value, after it has been retrieved, to another type. If
 this were an aliasing violation, then casting any pointer variable to
 another type would be an aliasing violation; this is clearly not the case.

 ((const struct exec_node **)n)[0]
 - which de-references the result of the above cast, thereby accessing a
 stored value of type 'exec node *' using a glvalue of type 'exec node *'.

I think breaking this up is a mistake, because the strict-aliasing
rules is explicitly about the *combination* of these two things.

You *are* accessing the underlying memory of 'n' through a different
type, and this is what strict aliasing is all about. But it takes two
steps, a single step isn't enough to do so.

Those other spec-quotes doesn't undo the strict-aliasing definitions;
knowing how things are laid out in memory doesn't mean the compiler
cannot assume two differently typed variables doesn't overlap.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH V2] glsl: fix atomic buffer index for bindings other than 0

[Erik Faye-Lund]

On Sat, Jul 25, 2015 at 4:24 PM, Timothy Arceri t_arc...@yahoo.com.au wrote:
 Since commit c0cd5b var-data.binding was being used as a replacement
 for atomic buffer index, but they don't have to be the same value they
 just happen to end up the same when binding is 0.

 Now we store atomic buffer index in the unused var-data.index
 to avoid the extra memory of putting back the atmoic buffer index field.

 V2: store buffer index in var-data.index and uniform slot in
 var-data.location to avoid issues when linking more than 2 shaders.
 Also some small tidy ups.

 Cc: Alejandro Piñeiro apinhe...@igalia.com
 Cc: Ian Romanick i...@freedesktop.org
 Cc: 10.4, 10.5 mesa-sta...@lists.freedesktop.org
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90175
 ---
  src/glsl/ir.h  |  3 +++
  src/glsl/link_atomics.cpp  | 18 +++---
  src/glsl/link_uniforms.cpp |  4 
  src/glsl/nir/glsl_to_nir.cpp   |  2 --
  src/glsl/nir/nir.h |  6 --
  src/glsl/nir/nir_lower_atomics.c   |  2 +-
  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |  2 +-
  7 files changed, 20 insertions(+), 17 deletions(-)

 diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
 index 62cdbd4..d97db68 100644
 --- a/src/glsl/nir/nir.h
 +++ b/src/glsl/nir/nir.h
 @@ -307,7 +310,6 @@ typedef struct {
 * Location an atomic counter is stored at.
 */
struct {
 - unsigned buffer_index;
   unsigned offset;
} atomic;

This smells a bit like the struct should be nuked all together...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] mesa: limit scope of GL 1.0 compatibility shim

[Erik Faye-Lund]

OpenGL doesn't expect internalformat={1,2,3,4} to work
elsewhere than glTexImage{1,2,3}D, as clearly stated in the
spec of glCopyTexImage2D (taken from the OpenGL 1.1 spec,
similar wording is present up to OpenGL 3.1 core, where
OpenGL 1.0 compatibility was scrapped entirely):

Parameters level, internalformat, and border are specified using
the same values, with the same meanings, as the equivalent
arguments of TexImage2D, except that internalformat may not be
specified as 1, 2, 3, or 4.

Fixes piglit tests:
 * spec@!opengl 1.1@copyteximage 1d
 * spec@!opengl 1.1@copyteximage 2d
 * spec@arb_texture_cube_map@copyteximage cube
 * spec@arb_texture_rectangle@copyteximage rect

No piglit regressions.
---

These piglit tests were just submitted (by me) on the piglit
mailing list, not yet a part of piglit.

 src/mesa/drivers/dri/nouveau/nouveau_texture.c |  4 ---
 src/mesa/drivers/dri/radeon/radeon_texture.c   |  4 ---
 src/mesa/main/glformats.c  | 12 -
 src/mesa/main/texformat.c  |  4 ---
 src/mesa/main/teximage.c   | 35 +-
 src/mesa/state_tracker/st_format.c |  3 +--
 6 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/src/mesa/drivers/dri/nouveau/nouveau_texture.c 
b/src/mesa/drivers/dri/nouveau/nouveau_texture.c
index dc5699c..ea0e1aa 100644
--- a/src/mesa/drivers/dri/nouveau/nouveau_texture.c
+++ b/src/mesa/drivers/dri/nouveau/nouveau_texture.c
@@ -166,7 +166,6 @@ nouveau_choose_tex_format(struct gl_context *ctx, GLenum 
target,
  GLenum srcFormat, GLenum srcType)
 {
switch (internalFormat) {
-   case 4:
case GL_RGBA:
case GL_RGBA2:
case GL_RGBA4:
@@ -186,13 +185,11 @@ nouveau_choose_tex_format(struct gl_context *ctx, GLenum 
target,
case GL_RGB16:
case GL_COMPRESSED_RGB:
return MESA_FORMAT_B8G8R8X8_UNORM;
-   case 3:
case GL_R3_G3_B2:
case GL_RGB4:
case GL_RGB5:
return MESA_FORMAT_B5G6R5_UNORM;
 
-   case 2:
case GL_LUMINANCE_ALPHA:
case GL_LUMINANCE4_ALPHA4:
case GL_LUMINANCE6_ALPHA2:
@@ -203,7 +200,6 @@ nouveau_choose_tex_format(struct gl_context *ctx, GLenum 
target,
case GL_COMPRESSED_LUMINANCE_ALPHA:
return MESA_FORMAT_B8G8R8A8_UNORM;
 
-   case 1:
case GL_LUMINANCE:
case GL_LUMINANCE4:
case GL_LUMINANCE12:
diff --git a/src/mesa/drivers/dri/radeon/radeon_texture.c 
b/src/mesa/drivers/dri/radeon/radeon_texture.c
index 4794dda..6a6aa04 100644
--- a/src/mesa/drivers/dri/radeon/radeon_texture.c
+++ b/src/mesa/drivers/dri/radeon/radeon_texture.c
@@ -286,7 +286,6 @@ mesa_format radeonChooseTextureFormat(struct gl_context * 
ctx,
__func__, do32bpt, force16bpt);
 
switch (internalFormat) {
-   case 4:
case GL_RGBA:
case GL_COMPRESSED_RGBA:
switch (type) {
@@ -305,7 +304,6 @@ mesa_format radeonChooseTextureFormat(struct gl_context * 
ctx,
_radeon_texformat_argb;
}
 
-   case 3:
case GL_RGB:
case GL_COMPRESSED_RGB:
switch (type) {
@@ -363,7 +361,6 @@ mesa_format radeonChooseTextureFormat(struct gl_context * 
ctx,
 #else
return MESA_FORMAT_A_UNORM8;
 #endif
-   case 1:
case GL_LUMINANCE:
case GL_LUMINANCE4:
case GL_LUMINANCE8:
@@ -372,7 +369,6 @@ mesa_format radeonChooseTextureFormat(struct gl_context * 
ctx,
case GL_COMPRESSED_LUMINANCE:
return MESA_FORMAT_L_UNORM8;
 
-   case 2:
case GL_LUMINANCE_ALPHA:
case GL_LUMINANCE4_ALPHA4:
case GL_LUMINANCE6_ALPHA2:
diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
index 3eb66da..1f8bb4b 100644
--- a/src/mesa/main/glformats.c
+++ b/src/mesa/main/glformats.c
@@ -636,14 +636,12 @@ _mesa_is_enum_format_unorm(GLenum format)
   case GL_ALPHA8:
   case GL_ALPHA12:
   case GL_ALPHA16:
-  case 1:
   case GL_LUMINANCE:
   case GL_SLUMINANCE:
   case GL_LUMINANCE4:
   case GL_LUMINANCE8:
   case GL_LUMINANCE12:
   case GL_LUMINANCE16:
-  case 2:
   case GL_LUMINANCE_ALPHA:
   case GL_SLUMINANCE_ALPHA:
   case GL_LUMINANCE4_ALPHA4:
@@ -662,7 +660,6 @@ _mesa_is_enum_format_unorm(GLenum format)
   case GL_RG:
   case GL_RG8:
   case GL_RG16:
-  case 3:
   case GL_RGB:
   case GL_BGR:
   case GL_SRGB:
@@ -674,7 +671,6 @@ _mesa_is_enum_format_unorm(GLenum format)
   case GL_RGB10:
   case GL_RGB12:
   case GL_RGB16:
-  case 4:
   case GL_ABGR_EXT:
   case GL_RGBA:
   case GL_BGRA:
@@ -881,13 +877,11 @@ _mesa_is_color_format(GLenum format)
   case GL_ALPHA8:
   case GL_ALPHA12:
   case GL_ALPHA16:
-  case 1:
   case GL_LUMINANCE:
   case GL_LUMINANCE4:
   case 

[Mesa-dev] [PATCH 1/2] mesa: only look up base-format once

[Erik Faye-Lund]

There's no point in repeatedly looking up the base-format of an
internalformat. So let's cache it in a variable instead.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
 src/mesa/main/teximage.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index 253e881..7605d1b 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -2004,9 +2004,10 @@ _mesa_legal_texture_base_format_for_target(struct 
gl_context *ctx,
unsigned dimensions,
const char *caller)
 {
-   if (_mesa_base_tex_format(ctx, internalFormat) == GL_DEPTH_COMPONENT
-   || _mesa_base_tex_format(ctx, internalFormat) == GL_DEPTH_STENCIL
-   || _mesa_base_tex_format(ctx, internalFormat) == GL_STENCIL_INDEX) {
+   GLint baseFormat = _mesa_base_tex_format(ctx, internalFormat);
+   if (baseFormat == GL_DEPTH_COMPONENT
+   || baseFormat == GL_DEPTH_STENCIL
+   || baseFormat == GL_STENCIL_INDEX) {
   /* Section 3.8.3 (Texture Image Specification) of the OpenGL 3.3 Core
* Profile spec says:
*
-- 
2.1.4

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

Re: [Mesa-dev] [PATCH 7/7] nir: add helper macros for running NIR passes

[Erik Faye-Lund]

On Sat, Oct 24, 2015 at 7:08 PM, Rob Clark  wrote:
> From: Rob Clark 
>
> +#define NIR_PASS_PROGRESS(pass, nir, ...) ({   \
> +  assert(nir_shader_is_mutable(nir));  \
> +  bool __ret = pass(nir, ##__VA_ARGS__);   \
> +  nir_validate_shader(nir);\
> +  if (__nir_test_clone()) {\
> + nir = nir_shader_clone(ralloc_parent(nir), nir);  \
> + nir_validate_shader(nir); \
> +  }\
> +  __ret;   \
> +   })
> +
0
> +  progress |= NIR_PASS_PROGRESS(nir_copy_prop, nir);

This does not generate valid C code, but rather what looks like a GCC
extension. Is that really a good move?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/4] st/dri2: Add shared flag to missing locations

[Erik Faye-Lund]

On Thu, Oct 22, 2015 at 10:54 AM, Marek Olšák <mar...@gmail.com> wrote:
> On Thu, Oct 22, 2015 at 10:22 AM, Erik Faye-Lund <kusmab...@gmail.com> wrote:
>> On Wed, Oct 21, 2015 at 10:34 PM, Marek Olšák <mar...@gmail.com> wrote:
>>> On Wed, Oct 21, 2015 at 12:28 PM, Axel Davy <axel.d...@ens.fr> wrote:
>>>> The PIPE_BIND_SHARED flag should be added whenever
>>>> the resource may be shared with another process.
>>>>
>>>> In particular if the resource is imported, or may
>>>> be exported, the flag should be used.
>>>
>>> This can't be enforced. EGL_MESA_image_dma_buf_export allows exporting
>>> any texture. Mesa can't know in advance if a texture will be exported.
>>
>> Could we not, at least in theory, crate a new texture and blit the old
>> one into it behind the scenes somehow when a texture gets exported?
>
> Sharing means textures are shared. There is no blitting allowed. Other
> users don't have to be notified that a shared texture has been
> rendered to.

Maybe I wasn't clear enough: I meant to migrate the texture data over
to a new shared home, not to create a copy.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC 10/21] mesa: Remove equality check in helper functions

[Erik Faye-Lund]

On Tue, Oct 20, 2015 at 12:44 AM, Nanley Chery  wrote:
> From: Nanley Chery 
>
> Since the version numbers being compared are integral and we don't ever
> expect gl_context::Version to be equal to 0, subtract 1 from the rhs of
> the equation and perform the optimization of removing the equality check.

Is this really an improvement? Changing '>=' to '>' and then making
all versions off-by-one seems like trading less readable and debugable
code for no real upside...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/4] st/dri2: Add shared flag to missing locations

[Erik Faye-Lund]

On Thu, Oct 22, 2015 at 12:18 PM, Marek Olšák <mar...@gmail.com> wrote:
> On Thu, Oct 22, 2015 at 10:56 AM, Erik Faye-Lund <kusmab...@gmail.com> wrote:
>> On Thu, Oct 22, 2015 at 10:54 AM, Marek Olšák <mar...@gmail.com> wrote:
>>> On Thu, Oct 22, 2015 at 10:22 AM, Erik Faye-Lund <kusmab...@gmail.com> 
>>> wrote:
>>>> On Wed, Oct 21, 2015 at 10:34 PM, Marek Olšák <mar...@gmail.com> wrote:
>>>>> On Wed, Oct 21, 2015 at 12:28 PM, Axel Davy <axel.d...@ens.fr> wrote:
>>>>>> The PIPE_BIND_SHARED flag should be added whenever
>>>>>> the resource may be shared with another process.
>>>>>>
>>>>>> In particular if the resource is imported, or may
>>>>>> be exported, the flag should be used.
>>>>>
>>>>> This can't be enforced. EGL_MESA_image_dma_buf_export allows exporting
>>>>> any texture. Mesa can't know in advance if a texture will be exported.
>>>>
>>>> Could we not, at least in theory, crate a new texture and blit the old
>>>> one into it behind the scenes somehow when a texture gets exported?
>>>
>>> Sharing means textures are shared. There is no blitting allowed. Other
>>> users don't have to be notified that a shared texture has been
>>> rendered to.
>>
>> Maybe I wasn't clear enough: I meant to migrate the texture data over
>> to a new shared home, not to create a copy.
>
> Ok, well, in that case it's easier to just share the texture as-is.

Sure. This was more a theoretical question than a practical one, as
your initial statement sounded a bit black-and-white. I just remember
we had to do something along these lines this in the Mali-400 driver
back when EGL_image was introduced, because the image could be
consumed by things that were unable to deal with our tiling etc (like
video encoders/decoders). I might misremember, though.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/4] st/dri2: Add shared flag to missing locations

[Erik Faye-Lund]

On Wed, Oct 21, 2015 at 10:34 PM, Marek Olšák  wrote:
> On Wed, Oct 21, 2015 at 12:28 PM, Axel Davy  wrote:
>> The PIPE_BIND_SHARED flag should be added whenever
>> the resource may be shared with another process.
>>
>> In particular if the resource is imported, or may
>> be exported, the flag should be used.
>
> This can't be enforced. EGL_MESA_image_dma_buf_export allows exporting
> any texture. Mesa can't know in advance if a texture will be exported.

Could we not, at least in theory, crate a new texture and blit the old
one into it behind the scenes somehow when a texture gets exported?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC 03/21] mesa/extensions: Wrap array entries in macros

[Erik Faye-Lund]

On Tue, Oct 20, 2015 at 12:36 AM, Nanley Chery  wrote:
> From: Nanley Chery 
>
> -   { "GL_SUN_multi_draw_arrays",   o(dummy_true),
>   GLL,1999 },
> +#define EXT(name_str, driver_cap, api_flags, ) \
> +{ .name = "GL_" #name_str, .offset = o(driver_cap), .api_set = 
> api_flags, .year = },

I suspect that using designated initializers will make the MSVC build unhappy.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC 2/2] gallium: add tegra support

[Erik Faye-Lund]

On Sun, Oct 11, 2015 at 5:09 PM, Christian Gmeiner
 wrote:
> @@ -2181,6 +2188,13 @@ if test -n "$with_gallium_drivers"; then
>  done
>  fi
>
> +dnl We need to validate some needed dependencies for renderonly drivers.
> +
> +if test "x$HAVE_GALLIUM_NOUVEAU" != xyes -a "x$HAVE_GALLIUM_TEGRA" == xyes  
> ; then
> +AC_ERROR([Building with tegra requires that nouveau])

Requires that nouveau what? :)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH shader-db 1/2] report.py: rework and update for cycle info

[Erik Faye-Lund]

On Fri, Oct 2, 2015 at 11:37 PM, Connor Abbott  wrote:
> Now that we have three separate things we want to measure (instructions,
> cycles, and loops), it's impractical to keep adding special code for
> changes in each thing. Instead, for each program in before and after we
> store a table of measurement -> value, and when reporting we loop over
> each measurement and report helped/hurt before reporting the gained/lost
> programs.
>
> Signed-off-by: Connor Abbott 
> ---
>  report.py | 140 
> ++
>  1 file changed, 67 insertions(+), 73 deletions(-)
>
> diff --git a/report.py b/report.py
> index 4c06714..bc3a640 100755
> --- a/report.py
> +++ b/report.py
> @@ -10,17 +10,22 @@ def get_results(filename):
>
>  results = {}
>
> -re_match = re.compile(r"(\S+) - (.S \S+) shader: (\S*) inst, (\S*) 
> loops")
> +re_match = re.compile(r"(\S+) - (.S \S+) shader: (\S*) inst, (\S*) 
> cycles, (\S*) loops")
>  for line in lines:
>  match = re.search(re_match, line)
>  if match is None:
>  continue
>
>  groups = match.groups()
> -count = int(groups[2])
> -loop = int(groups[3])
> -if count != 0:
> -results[(groups[0], groups[1])] = count, loop
> +inst_count = int(groups[2])
> +   cycle_count = int(groups[3])

Something's up with the indentation here...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa: Match MESA_FORMAT_B5G6R5 for a shallow pixel format of GL_RGB

[Erik Faye-Lund]

On Wed, Sep 9, 2015 at 12:41 PM, Chris Wilson <ch...@chris-wilson.co.uk> wrote:
> On Wed, Sep 09, 2015 at 12:09:40PM +0200, Erik Faye-Lund wrote:
>> On Wed, Sep 9, 2015 at 11:25 AM, Chris Wilson <ch...@chris-wilson.co.uk> 
>> wrote:
>> > On Wed, Sep 09, 2015 at 11:11:59AM +0200, Erik Faye-Lund wrote:
>> >> On Thu, Sep 3, 2015 at 6:05 PM, Chris Wilson <ch...@chris-wilson.co.uk> 
>> >> wrote:
>> >> > If the user supplies a pixel format of GL_RGB + GL_UNSIGNED_SHORT_5_6_5
>> >> > and specifies a generic unsized GL_RGB internal format, match that to a
>> >> > texture format of MESA_FORMAT_B5G6R5 if supported by the hardware.
>> >> >
>> >> > Noticed while playing with mesa-demos/teximage:
>> >> >
>> >> >   TexImage(RGB/565 256 x 256): 79.8 images/sec, 10.0 MB/sec
>> >> >   TexSubImage(RGB/565 256 x 256): 3804.9 images/sec, 475.6 MB/sec
>> >> >   GetTexImage(RGB/565 256 x 256): 99.5 images/sec, 12.4 MB/sec
>> >> >
>> >> > becomes
>> >> >
>> >> >   TexImage(RGB/565 256 x 256): 3439.1 images/sec, 429.9 MB/sec
>> >> >   TexSubImage(RGB/565 256 x 256): 3744.1 images/sec, 468.0 MB/sec
>> >> >   GetTexImage(RGB/565 256 x 256): 4713.5 images/sec, 589.2 MB/sec
>> >> >
>> >> > on a puny Baytrail which is still far from what it is capable of. The
>> >> > reason for the disparity is that the teximage demo uses a busy texture
>> >> > which is performs an accelerated pixel conversion from the user's B5G6R5
>> >> > into the native X8B8G8R8. After the patch, no conversion is required
>> >> > allowing use of the blitter and memcpy fast paths.
>> >> >
>> >> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
>> >> > ---
>> >> >  src/mesa/main/texformat.c | 2 ++
>> >> >  1 file changed, 2 insertions(+)
>> >> >
>> >> > diff --git a/src/mesa/main/texformat.c b/src/mesa/main/texformat.c
>> >> > index fd9f335..866c7b3 100644
>> >> > --- a/src/mesa/main/texformat.c
>> >> > +++ b/src/mesa/main/texformat.c
>> >> > @@ -114,6 +114,8 @@ _mesa_choose_tex_format(struct gl_context *ctx, 
>> >> > GLenum target,
>> >> > case GL_RGB:
>> >> >if (type == GL_UNSIGNED_INT_2_10_10_10_REV) {
>> >> >   RETURN_IF_SUPPORTED(MESA_FORMAT_B10G10R10A2_UNORM);
>> >> > +  } else if (type == GL_UNSIGNED_SHORT_5_6_5) {
>> >> > + RETURN_IF_SUPPORTED(MESA_FORMAT_B5G6R5_UNORM);
>> >>
>> >> Shouldn't this be MESA_FORMAT_R5G6B5_UNORM? AFAICT, the reason for
>> >> using BGR above, is the _REV suffix on the type...
>> >
>> > No, it's the first line that's "wrong" since the B10G10R10A2 matches
>> > GL_BGRA + GL_UNSIGNED_INT_2_10_10_10_REV, GL_RGB implies that the
>> > incoming data only has 3 channels (no alpha at all).
>>
>> Good point about the alpha channel. It sounds like it should be
>> changed to MESA_FORMAT_B10G10R10X2_UNORM instead.
>>
>> But according to src/mesa/main/format_pack.c's
>> pack_ubyte_b10g10r10a2_unorm(), mesa's B10G10R10A2 corresponds to
>> OpenGL's UNSIGNED_INT_2_10_10_10_REV. So I think it matches GL_RGBA,
>> not GL_BGRA. The latter would mean another swizzle, AFAICT.
>
> The mapping is (_mesa_format_from_format_and_type):
>
>case GL_UNSIGNED_INT_2_10_10_10_REV:
>   if (format == GL_RGB)
>  return MESA_FORMAT_R10G10B10X2_UNORM;
>   if (format == GL_RGBA)
>  return MESA_FORMAT_R10G10B10A2_UNORM;
>   else if (format == GL_RGBA_INTEGER)
>  return MESA_FORMAT_R10G10B10A2_UINT;
>   else if (format == GL_BGRA)
>  return MESA_FORMAT_B10G10R10A2_UNORM;
>   else if (format == GL_BGRA_INTEGER)
>  return MESA_FORMAT_B10G10R10A2_UINT;
>   break;
>
> The trick is that the packed formats are written as lsb first (or it may
> just be native and my lsb bias is showing).
>
>> > i965 know how to do B5G6R5 and not R5G6B5, but for completeness we could
>> > also add RETURN_IF_SUPPORTED(MESA_FORMAT_R5G6B5_UNORM);
>>
>> I think intel-specific hacks (like preferring B5G6R5 over R5G6B5)
>> shouldn't leak into _mesa_choose_tex_format(),
>
> Hah, did you look at _mesa_choose_tex_format()? :)
> I sent an another patch to do the hardware agnostic unswizzled conversions.
>
>> So I think it'd be a
>&g

Re: [Mesa-dev] [PATCH] mesa: Match MESA_FORMAT_B5G6R5 for a shallow pixel format of GL_RGB

[Erik Faye-Lund]

On Thu, Sep 3, 2015 at 6:05 PM, Chris Wilson  wrote:
> If the user supplies a pixel format of GL_RGB + GL_UNSIGNED_SHORT_5_6_5
> and specifies a generic unsized GL_RGB internal format, match that to a
> texture format of MESA_FORMAT_B5G6R5 if supported by the hardware.
>
> Noticed while playing with mesa-demos/teximage:
>
>   TexImage(RGB/565 256 x 256): 79.8 images/sec, 10.0 MB/sec
>   TexSubImage(RGB/565 256 x 256): 3804.9 images/sec, 475.6 MB/sec
>   GetTexImage(RGB/565 256 x 256): 99.5 images/sec, 12.4 MB/sec
>
> becomes
>
>   TexImage(RGB/565 256 x 256): 3439.1 images/sec, 429.9 MB/sec
>   TexSubImage(RGB/565 256 x 256): 3744.1 images/sec, 468.0 MB/sec
>   GetTexImage(RGB/565 256 x 256): 4713.5 images/sec, 589.2 MB/sec
>
> on a puny Baytrail which is still far from what it is capable of. The
> reason for the disparity is that the teximage demo uses a busy texture
> which is performs an accelerated pixel conversion from the user's B5G6R5
> into the native X8B8G8R8. After the patch, no conversion is required
> allowing use of the blitter and memcpy fast paths.
>
> Signed-off-by: Chris Wilson 
> ---
>  src/mesa/main/texformat.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/mesa/main/texformat.c b/src/mesa/main/texformat.c
> index fd9f335..866c7b3 100644
> --- a/src/mesa/main/texformat.c
> +++ b/src/mesa/main/texformat.c
> @@ -114,6 +114,8 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum 
> target,
> case GL_RGB:
>if (type == GL_UNSIGNED_INT_2_10_10_10_REV) {
>   RETURN_IF_SUPPORTED(MESA_FORMAT_B10G10R10A2_UNORM);
> +  } else if (type == GL_UNSIGNED_SHORT_5_6_5) {
> + RETURN_IF_SUPPORTED(MESA_FORMAT_B5G6R5_UNORM);

Shouldn't this be MESA_FORMAT_R5G6B5_UNORM? AFAICT, the reason for
using BGR above, is the _REV suffix on the type...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa: Match MESA_FORMAT_B5G6R5 for a shallow pixel format of GL_RGB

[Erik Faye-Lund]

On Wed, Sep 9, 2015 at 11:25 AM, Chris Wilson <ch...@chris-wilson.co.uk> wrote:
> On Wed, Sep 09, 2015 at 11:11:59AM +0200, Erik Faye-Lund wrote:
>> On Thu, Sep 3, 2015 at 6:05 PM, Chris Wilson <ch...@chris-wilson.co.uk> 
>> wrote:
>> > If the user supplies a pixel format of GL_RGB + GL_UNSIGNED_SHORT_5_6_5
>> > and specifies a generic unsized GL_RGB internal format, match that to a
>> > texture format of MESA_FORMAT_B5G6R5 if supported by the hardware.
>> >
>> > Noticed while playing with mesa-demos/teximage:
>> >
>> >   TexImage(RGB/565 256 x 256): 79.8 images/sec, 10.0 MB/sec
>> >   TexSubImage(RGB/565 256 x 256): 3804.9 images/sec, 475.6 MB/sec
>> >   GetTexImage(RGB/565 256 x 256): 99.5 images/sec, 12.4 MB/sec
>> >
>> > becomes
>> >
>> >   TexImage(RGB/565 256 x 256): 3439.1 images/sec, 429.9 MB/sec
>> >   TexSubImage(RGB/565 256 x 256): 3744.1 images/sec, 468.0 MB/sec
>> >   GetTexImage(RGB/565 256 x 256): 4713.5 images/sec, 589.2 MB/sec
>> >
>> > on a puny Baytrail which is still far from what it is capable of. The
>> > reason for the disparity is that the teximage demo uses a busy texture
>> > which is performs an accelerated pixel conversion from the user's B5G6R5
>> > into the native X8B8G8R8. After the patch, no conversion is required
>> > allowing use of the blitter and memcpy fast paths.
>> >
>> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
>> > ---
>> >  src/mesa/main/texformat.c | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/src/mesa/main/texformat.c b/src/mesa/main/texformat.c
>> > index fd9f335..866c7b3 100644
>> > --- a/src/mesa/main/texformat.c
>> > +++ b/src/mesa/main/texformat.c
>> > @@ -114,6 +114,8 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum 
>> > target,
>> > case GL_RGB:
>> >if (type == GL_UNSIGNED_INT_2_10_10_10_REV) {
>> >   RETURN_IF_SUPPORTED(MESA_FORMAT_B10G10R10A2_UNORM);
>> > +  } else if (type == GL_UNSIGNED_SHORT_5_6_5) {
>> > + RETURN_IF_SUPPORTED(MESA_FORMAT_B5G6R5_UNORM);
>>
>> Shouldn't this be MESA_FORMAT_R5G6B5_UNORM? AFAICT, the reason for
>> using BGR above, is the _REV suffix on the type...
>
> No, it's the first line that's "wrong" since the B10G10R10A2 matches
> GL_BGRA + GL_UNSIGNED_INT_2_10_10_10_REV, GL_RGB implies that the
> incoming data only has 3 channels (no alpha at all).

Good point about the alpha channel. It sounds like it should be
changed to MESA_FORMAT_B10G10R10X2_UNORM instead.

But according to src/mesa/main/format_pack.c's
pack_ubyte_b10g10r10a2_unorm(), mesa's B10G10R10A2 corresponds to
OpenGL's UNSIGNED_INT_2_10_10_10_REV. So I think it matches GL_RGBA,
not GL_BGRA. The latter would mean another swizzle, AFAICT.

> i965 know how to do B5G6R5 and not R5G6B5, but for completeness we could
> also add RETURN_IF_SUPPORTED(MESA_FORMAT_R5G6B5_UNORM);

I think intel-specific hacks (like preferring B5G6R5 over R5G6B5)
shouldn't leak into _mesa_choose_tex_format(), so I think it'd be a
good move to add "RETURN_IF_SUPPORTED(MESA_FORMAT_R5G6B5_UNORM);"
before the latter return.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nir: add nir_swizzle

[Erik Faye-Lund]

On Thu, Sep 10, 2015 at 10:08 PM, Rob Clark  wrote:
> From: Rob Clark 
>
> Rather than make yet another copy of channel(), let's move it into nir.
>
> Signed-off-by: Rob Clark 
> ---
>  src/glsl/nir/nir_builder.h  |  6 ++
>  src/glsl/nir/nir_lower_tex_projector.c  | 24 +---
>  src/glsl/nir/nir_normalize_cubemap_coords.c | 20 +++-
>  3 files changed, 22 insertions(+), 28 deletions(-)
>
> diff --git a/src/glsl/nir/nir_builder.h b/src/glsl/nir/nir_builder.h
> index ba988d7..6568493 100644
> --- a/src/glsl/nir/nir_builder.h
> +++ b/src/glsl/nir/nir_builder.h
> @@ -216,6 +216,12 @@ nir_swizzle(nir_builder *build, nir_ssa_def *src, 
> unsigned swiz[4],
>   nir_imov_alu(build, alu_src, num_components);
>  }
>
> +static inline nir_ssa_def *
> +nir_channel(nir_builder *b, nir_ssa_def *def, int c)
> +{
> +   return nir_swizzle(b, def, (unsigned[4]){c, c, c, c}, 1, false);
> +}
> +

The subject is "add nir_swizzle", but you seem to rename channel to
nir_channel instead... Old subject, perhaps?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] ralloc: Use __attribute__((destructor)) instead of atexit(3)

[Erik Faye-Lund]

On Thu, Sep 10, 2015 at 7:58 PM, Ian Romanick  wrote:
> On 09/10/2015 05:29 AM, Jose Fonseca wrote:
>> On 10/09/15 02:54, Ian Romanick wrote:
>>
>> It's not necessary to do it in several places, at least for the
>> destructor -- we could have a single C++ module inside src/util, which
>> provided a "dl_atexit".
>>
>> I don't feel too strongly either way, but my impression is that the
>> hassle of doing this with non-standard vendor-specific C extensions is
>> greater than having a small C++ file somewhere.
>
> Like I said, I don't care how we solve the problem.  The only thing I
> don't want is to do it one way now, then in six months have someone say,
> "Ew" and change to some other way that could have been used in the first
> place.  I really annoys me when we have təˈmeɪtoʊ / təˈmɑːtoʊ churn.
>
> If we're going to pick a way, we should document that way in
> docs/devinfo.html so as to significantly raise the bar for someone
> wanting to flip to a different, roughly equivalent method.  We should
> also document the reason for picking the particular method.

My two cents would be that we should "bootstrap" the init and deinit
from one well-known point into some functions (perhaps exactly
one_time_init / one_time_fini?) that takes care about bringing up/down
the rest of mesa (like suggested above). That'd make it easy to have
different systems do different init without having to riddle the rest
of mesa with details about how we figure out when this happens.

For instance, on Windows you have DLL_PROCESS_ATTACH and
DLL_PROCESS_DETACH calls to DllMain() where this can easily be done.
And on most Unix-systems, you can use __attribute__((destructor)) etc.

I just think it's kinda awful to leak these details directly into
places like ralloc. I don't think these subsystems should need to know
when they are deinitialized, only how they are.

>> For the constructor problem, we could just rely on the standard C11
>> once_flag/call_once.
>
> That's basically what c61bc6ed does.  It calls an initializer function
> from core Mesa's one_time_init function.  It could probably be moved
> into the implementation of _mesa_strtof, and that would eliminate the
> need to call the initializer from the standalone preprocessor and the
> standalone GLSL compiler.
>
> Any idea how much overhead is call_once adds on the second and later
> calls?  I seem to recall the pthread_once still adds noticeable
> overhead... but that recollection is from ages ago.

AFAIK, it involves stuff like memory barriers, which you really don't
want mixed up in your shader-code parsing. Committing caches to memory
just to grok a float is a bit of a steep price to pay. I did consider
this approach, but decided against it for these reasons.

Of course, it might turn out that I'm overly wary, and someone
profiles it only to show no real world problem, dunno.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] ralloc: Use __attribute__((destructor)) instead of atexit(3)

[Erik Faye-Lund]

On Mon, Sep 7, 2015 at 3:54 PM, Jose Fonseca  wrote:
> On 07/09/15 10:17, Jean-Sébastien Pédron wrote:
>>
>> On 04.09.2015 01:37, Matt Turner wrote:
>>>
>>> You need to test for this support in configure.ac. It's as simple as
>>> adding a call to AX_GCC_FUNC_ATTRIBUTE in the existing alphabetized
>>> list and then a little bit of preprocessor in src/util/macros.h.
>>
>>
>> Should the code fallbacks on atexit(3) if the attribute is not
>> supported?
>
>
> At least on Windows, with MSVC,  atexit should be the right thing to do,
> since we statically link MSVC RunTime,
>
>
>> Can I use the HAVE_FUNC_ATTRIBUTE_DESTRUCTOR macro in
>>
>> ralloc.c for this purpose?
>
>
>
>
>
> For the record, another alternative (way more portable), is you have a
> simple .cpp file with a static destructior:
>
>
>   class AtExit
>   {
>   public:
>  ~AtExit() {
>  // do what must be done
>   }
>   };
>
>   AtExit atExit();
>
>
> After all, it seems wrong to use non-standard C to replicate what standard
> C++ can do.

That sounds nice and all, until you stumble over problems like
STB_GNU_UNIQUE, which makes renders C++ static destructors completely
unusable on Linux if you have even a single static variable in a
method in your C++ code.

Read up on it on  Read
http://gcc.gnu.org/ml/gcc-help/2011-05/msg00450.html if you feel like
being depressed.

Unfortunately, C++ and it's many questionable implementations make up
an unpredictable beast that keeps on throwing curve-balls that makes
it no fun at all for library development.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 09/11] nir: add lowering stage for user-clip-planes / clipdist

[Erik Faye-Lund]

On Tue, Sep 15, 2015 at 6:49 PM, Rob Clark <robdcl...@gmail.com> wrote:
> On Tue, Sep 15, 2015 at 12:39 PM, Erik Faye-Lund <kusmab...@gmail.com> wrote:
>> On Mon, Sep 14, 2015 at 11:53 AM, Erik Faye-Lund <kusmab...@gmail.com> wrote:
>>> On Sun, Sep 13, 2015 at 5:51 PM, Rob Clark <robdcl...@gmail.com> wrote:
>>>> From: Rob Clark <robcl...@freedesktop.org>
>>>>
>>>> The vertex shader lowering adds calculation for CLIPDIST, if needed
>>>> (ie. user-clip-planes), and the frag shader lowering adds conditional
>>>> kills based on CLIPDIST value (which should be treated as a normal
>>>> interpolated varying by the driver).
>>>
>>> 
>>>
>>>> +
>>>> +/*
>>>> + * FS lowering
>>>> + */
>>>> +
>>>> +static void
>>>> +lower_clip_fs(nir_function_impl *impl, unsigned ucp_enables,
>>>> +  nir_variable **in)
>>>> +{
>>>> +   nir_ssa_def *clipdist[MAX_CLIP_PLANES];
>>>> +   nir_builder b;
>>>> +
>>>> +   nir_builder_init(, impl);
>>>> +   b.cursor = nir_before_cf_list(>body);
>>>> +
>>>> +   if (ucp_enables & 0x0f)
>>>> +  load_clipdist_input(, in[0], [0]);
>>>> +   if (ucp_enables & 0xf0)
>>>> +  load_clipdist_input(, in[1], [4]);
>>>> +
>>>> +   for (int plane = 0; plane < MAX_CLIP_PLANES; plane++) {
>>>> +  if (ucp_enables & (1 << plane)) {
>>>> + nir_intrinsic_instr *discard;
>>>> + nir_ssa_def *cond;
>>>> +
>>>> + cond = nir_flt(, clipdist[plane], nir_imm_float(, 0.0));
>>>> +
>>>> + discard = nir_intrinsic_instr_create(b.shader,
>>>> +  nir_intrinsic_discard_if);
>>>> + discard->src[0] = nir_src_for_ssa(cond);
>>>> + nir_builder_instr_insert(, >instr);
>>>
>>> I think it's worth noting that this isn't *strictly* correct for
>>> multi-sampling, unless the shader is s run with per-sample shading
>>> (ala GL_ARB_sample_shading). Otherwise, all samples for a pixel will
>>> get the same discard-condition, leading to aliasing along the
>>> resulting edge.
>>>
>>> That being said, per-fragment clipping is better than no clipping, so
>>> it's clearly an improvement.
>>
>> So, in order to do this correctly for MSAA, I guess you'd need to use
>> SYSTEM_VALUE_SAMPLE_POS and FRAG_RESULT_SAMPLE_MASK, to perform
>> alpha-testing for each sample-point.
>
> I think it still involves being able to run the frag shader once per
> sample..  although blob does seem to expose OES_sample_shading so
> maybe that is possible?

No, that's the whole point of the above, to not have to run the entire
shader per sample, only the clipping-bit.

> I guess I'd care about it more once I supported MSAA ;-)
>
> (and tbh, all this effort was mostly just to get neverball to render
> correctly :-P)

Oh, I just meant that it might be neat to add to the commit message. I
wouldn't go ahead and implement this just yet.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 09/11] nir: add lowering stage for user-clip-planes / clipdist

[Erik Faye-Lund]

On Mon, Sep 14, 2015 at 11:53 AM, Erik Faye-Lund <kusmab...@gmail.com> wrote:
> On Sun, Sep 13, 2015 at 5:51 PM, Rob Clark <robdcl...@gmail.com> wrote:
>> From: Rob Clark <robcl...@freedesktop.org>
>>
>> The vertex shader lowering adds calculation for CLIPDIST, if needed
>> (ie. user-clip-planes), and the frag shader lowering adds conditional
>> kills based on CLIPDIST value (which should be treated as a normal
>> interpolated varying by the driver).
>
> 
>
>> +
>> +/*
>> + * FS lowering
>> + */
>> +
>> +static void
>> +lower_clip_fs(nir_function_impl *impl, unsigned ucp_enables,
>> +  nir_variable **in)
>> +{
>> +   nir_ssa_def *clipdist[MAX_CLIP_PLANES];
>> +   nir_builder b;
>> +
>> +   nir_builder_init(, impl);
>> +   b.cursor = nir_before_cf_list(>body);
>> +
>> +   if (ucp_enables & 0x0f)
>> +  load_clipdist_input(, in[0], [0]);
>> +   if (ucp_enables & 0xf0)
>> +  load_clipdist_input(, in[1], [4]);
>> +
>> +   for (int plane = 0; plane < MAX_CLIP_PLANES; plane++) {
>> +  if (ucp_enables & (1 << plane)) {
>> + nir_intrinsic_instr *discard;
>> + nir_ssa_def *cond;
>> +
>> + cond = nir_flt(, clipdist[plane], nir_imm_float(, 0.0));
>> +
>> + discard = nir_intrinsic_instr_create(b.shader,
>> +  nir_intrinsic_discard_if);
>> + discard->src[0] = nir_src_for_ssa(cond);
>> + nir_builder_instr_insert(, >instr);
>
> I think it's worth noting that this isn't *strictly* correct for
> multi-sampling, unless the shader is s run with per-sample shading
> (ala GL_ARB_sample_shading). Otherwise, all samples for a pixel will
> get the same discard-condition, leading to aliasing along the
> resulting edge.
>
> That being said, per-fragment clipping is better than no clipping, so
> it's clearly an improvement.

So, in order to do this correctly for MSAA, I guess you'd need to use
SYSTEM_VALUE_SAMPLE_POS and FRAG_RESULT_SAMPLE_MASK, to perform
alpha-testing for each sample-point.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 09/11] nir: add lowering stage for user-clip-planes / clipdist

[Erik Faye-Lund]

On Tue, Sep 15, 2015 at 6:49 PM, Ilia Mirkin  wrote:
> However having a piglit test that covers this would be neat... I guess
> you could clip a pixel in half and make sure that the resolved result
> is some in-between color? Lots of implementation-dependent stuff going
> on in there though.

I think this would simply be a matter of drawing a white triangle on a
black background both with clipping and pre-clipped, and verify that
the result is the same. Perhaps even using a few differently sub-pixel
jittered triangles.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 09/11] nir: add lowering stage for user-clip-planes / clipdist

[Erik Faye-Lund]

On Tue, Sep 15, 2015 at 6:49 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote:
> On Tue, Sep 15, 2015 at 12:39 PM, Erik Faye-Lund <kusmab...@gmail.com> wrote:
>> On Mon, Sep 14, 2015 at 11:53 AM, Erik Faye-Lund <kusmab...@gmail.com> wrote:
>>> On Sun, Sep 13, 2015 at 5:51 PM, Rob Clark <robdcl...@gmail.com> wrote:
>>>> From: Rob Clark <robcl...@freedesktop.org>
>>>>
>>>> The vertex shader lowering adds calculation for CLIPDIST, if needed
>>>> (ie. user-clip-planes), and the frag shader lowering adds conditional
>>>> kills based on CLIPDIST value (which should be treated as a normal
>>>> interpolated varying by the driver).
>>>
>>> 
>>>
>>>> +
>>>> +/*
>>>> + * FS lowering
>>>> + */
>>>> +
>>>> +static void
>>>> +lower_clip_fs(nir_function_impl *impl, unsigned ucp_enables,
>>>> +  nir_variable **in)
>>>> +{
>>>> +   nir_ssa_def *clipdist[MAX_CLIP_PLANES];
>>>> +   nir_builder b;
>>>> +
>>>> +   nir_builder_init(, impl);
>>>> +   b.cursor = nir_before_cf_list(>body);
>>>> +
>>>> +   if (ucp_enables & 0x0f)
>>>> +  load_clipdist_input(, in[0], [0]);
>>>> +   if (ucp_enables & 0xf0)
>>>> +  load_clipdist_input(, in[1], [4]);
>>>> +
>>>> +   for (int plane = 0; plane < MAX_CLIP_PLANES; plane++) {
>>>> +  if (ucp_enables & (1 << plane)) {
>>>> + nir_intrinsic_instr *discard;
>>>> + nir_ssa_def *cond;
>>>> +
>>>> + cond = nir_flt(, clipdist[plane], nir_imm_float(, 0.0));
>>>> +
>>>> + discard = nir_intrinsic_instr_create(b.shader,
>>>> +  nir_intrinsic_discard_if);
>>>> + discard->src[0] = nir_src_for_ssa(cond);
>>>> + nir_builder_instr_insert(, >instr);
>>>
>>> I think it's worth noting that this isn't *strictly* correct for
>>> multi-sampling, unless the shader is s run with per-sample shading
>>> (ala GL_ARB_sample_shading). Otherwise, all samples for a pixel will
>>> get the same discard-condition, leading to aliasing along the
>>> resulting edge.
>>>
>>> That being said, per-fragment clipping is better than no clipping, so
>>> it's clearly an improvement.
>>
>> So, in order to do this correctly for MSAA, I guess you'd need to use
>> SYSTEM_VALUE_SAMPLE_POS and FRAG_RESULT_SAMPLE_MASK, to perform
>> alpha-testing for each sample-point.
>
> Ultimately all it needs to do is be able to compute a
> sample mask though, which *is* supported, but you'd need to do the
> per-sample interp "by hand" (although afaik the IJ components are
> provided... somewhere).

SYSTEM_VALUE_SAMPLE_POS could be lowered to a uniform array. You just
need to know where the hardware sampling points are.

And you could use screen-space derivatives for the per-sample interp.

So yeah, the *real* hardware-support needed is just being able to
write a sample-mask from the fragment shader.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 09/11] nir: add lowering stage for user-clip-planes / clipdist

[Erik Faye-Lund]

On Tue, Sep 15, 2015 at 7:12 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote:
> On Tue, Sep 15, 2015 at 1:09 PM, Erik Faye-Lund <kusmab...@gmail.com> wrote:
>> On Tue, Sep 15, 2015 at 6:49 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote:
>>> However having a piglit test that covers this would be neat... I guess
>>> you could clip a pixel in half and make sure that the resolved result
>>> is some in-between color? Lots of implementation-dependent stuff going
>>> on in there though.
>>
>> I think this would simply be a matter of drawing a white triangle on a
>> black background both with clipping and pre-clipped, and verify that
>> the result is the same. Perhaps even using a few differently sub-pixel
>> jittered triangles.
>
> Right, but sample positions are implementation-defined. Although I
> guess you could retrieve those positions and pre-clip yourself...

No need to do that, I think. Clipping happens earlier in the OpenGL
pipeline than rasterization, so a user-clipped triangle and a
pre-clipped triangle should produce exactly the same result.

There is of course a chance of some variance in the calculations,
though. So perhaps it'd be better to do a user-clipped triangle vs a
near-plane clipped triangle one. Those are defined to be clipped in
the same pipeline-stage, and should yield the same result.

> However the resolve is also implementation-defined I think, so there's
> no good way to know what the resulting color would be. However you
> could ensure that it was neither white nor black.

Yeah, that's why I'm suggesting comparing two renderings that should
give the same result instead.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] Revert "mesa/extensions: restrict GL_OES_EGL_image to GLES"

[Erik Faye-Lund]

On Wed, Sep 16, 2015 at 11:00 PM, Dave Airlie  wrote:
> This reverts commit 48961fa3ba37999a6f8fd812458b735e39604a95.

> I also don't have a copy of this patch in my mail archive, which
> seems wierd, did it get posted to mesa-dev?

The same applies to 8200793 ("mesa/teximage: restrict GL_ETC1_RGB8_OES
support to GLES"). Nanley, what's going on here?

(Also CC'ing Anju, who is listed as a reviewer)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] Revert "mesa/extensions: restrict GL_OES_EGL_image to GLES"

[Erik Faye-Lund]

On Thu, Sep 17, 2015 at 11:15 AM, Erik Faye-Lund <kusmab...@gmail.com> wrote:
> On Wed, Sep 16, 2015 at 11:00 PM, Dave Airlie <airl...@gmail.com> wrote:
>> This reverts commit 48961fa3ba37999a6f8fd812458b735e39604a95.
> 
>> I also don't have a copy of this patch in my mail archive, which
>> seems wierd, did it get posted to mesa-dev?
>
> The same applies to 8200793 ("mesa/teximage: restrict GL_ETC1_RGB8_OES
> support to GLES"). Nanley, what's going on here?

Sorry, my bad. The subject here was just tweaked, and it is indeed on the list.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Compile error in nir/nir_builder.h in current head

[Erik Faye-Lund]

On Thu, Sep 17, 2015 at 3:27 PM, Emil Velikov  wrote:
> Hi Gottfried,
>
> On 17 September 2015 at 13:42, Gottfried Haider
>  wrote:
>> I am getting this error on 7e286506 - comping mesa used to work fine
>> on this system a couple of weeks ago
>>
>>   CXXnir/nir_lower_samplers.lo
>>   CC nir/nir_lower_system_values.lo
>> In file included from nir/nir_lower_samplers.cpp:27:0:
>> nir/nir_builder.h: In function 'nir_ssa_def*
>> nir_imm_float(nir_builder*, float)':
>> nir/nir_builder.h:79:28: error: expected primary-expression before '.' token
>> nir/nir_builder.h:79:44: warning: extended initializer lists only
>> available with -std=c++0x or -std=gnu++0x [enabled by default]
>> nir/nir_builder.h: In function 'nir_ssa_def*
>> nir_imm_vec4(nir_builder*, float, float, float, float)':
>> nir/nir_builder.h:86:28: error: expected primary-expression before '.' token
>> nir/nir_builder.h:86:44: warning: extended initializer lists only
>> available with -std=c++0x or -std=gnu++0x [enabled by default]
>> nir/nir_builder.h: In function 'nir_ssa_def* nir_imm_int(nir_builder*, int)':
>> nir/nir_builder.h:93:28: error: expected primary-expression before '.' token
>> nir/nir_builder.h:93:44: warning: extended initializer lists only
>> available with -std=c++0x or -std=gnu++0x [enabled by default]
>> Makefile:1525: recipe for target 'nir/nir_lower_samplers.lo' failed
>>
> This is caused with the introduction of
>
> commit ef8eebc6ad5d86e524426f0755c0f7d0b4c0cd3e
> Author: Timothy Arceri 
> Date:   Wed Aug 26 22:18:36 2015 +1000
>
>nir: support indirect indexing samplers in struct arrays
>
> The commit adds nir_builder.h to nir_lower_samplers.cpp, with the
> former containing designated initializer(s). As this is not allowed by
> the the standard (albeit some compilers allow it) you see the error.

Or maybe this is enough?

diff --git a/src/glsl/nir/nir_builder.h b/src/glsl/nir/nir_builder.h
index cf50f69..51dd86b 100644
--- a/src/glsl/nir/nir_builder.h
+++ b/src/glsl/nir/nir_builder.h
@@ -76,21 +76,27 @@ nir_build_imm(nir_builder *build, unsigned
num_components, nir_const_value value
 static inline nir_ssa_def *
 nir_imm_float(nir_builder *build, float x)
 {
-   nir_const_value v = { { .f = {x, 0, 0, 0} } };
+   nir_const_value v = { { { 0 } } };
+   v.f[0] = x;
return nir_build_imm(build, 1, v);
 }

 static inline nir_ssa_def *
 nir_imm_vec4(nir_builder *build, float x, float y, float z, float w)
 {
-   nir_const_value v = { { .f = {x, y, z, w} } };
+   nir_const_value v;
+   v.f[0] = x;
+   v.f[1] = y;
+   v.f[2] = z;
+   v.f[3] = w;
return nir_build_imm(build, 4, v);
 }

 static inline nir_ssa_def *
 nir_imm_int(nir_builder *build, int x)
 {
-   nir_const_value v = { { .i = {x, 0, 0, 0} } };
+   nir_const_value v = { { { 0 } } };
+   v.i[0] = x;
return nir_build_imm(build, 1, v);
 }
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 09/11] nir: add lowering stage for user-clip-planes / clipdist

[Erik Faye-Lund]

On Sun, Sep 13, 2015 at 5:51 PM, Rob Clark  wrote:
> From: Rob Clark 
>
> The vertex shader lowering adds calculation for CLIPDIST, if needed
> (ie. user-clip-planes), and the frag shader lowering adds conditional
> kills based on CLIPDIST value (which should be treated as a normal
> interpolated varying by the driver).



> +
> +/*
> + * FS lowering
> + */
> +
> +static void
> +lower_clip_fs(nir_function_impl *impl, unsigned ucp_enables,
> +  nir_variable **in)
> +{
> +   nir_ssa_def *clipdist[MAX_CLIP_PLANES];
> +   nir_builder b;
> +
> +   nir_builder_init(, impl);
> +   b.cursor = nir_before_cf_list(>body);
> +
> +   if (ucp_enables & 0x0f)
> +  load_clipdist_input(, in[0], [0]);
> +   if (ucp_enables & 0xf0)
> +  load_clipdist_input(, in[1], [4]);
> +
> +   for (int plane = 0; plane < MAX_CLIP_PLANES; plane++) {
> +  if (ucp_enables & (1 << plane)) {
> + nir_intrinsic_instr *discard;
> + nir_ssa_def *cond;
> +
> + cond = nir_flt(, clipdist[plane], nir_imm_float(, 0.0));
> +
> + discard = nir_intrinsic_instr_create(b.shader,
> +  nir_intrinsic_discard_if);
> + discard->src[0] = nir_src_for_ssa(cond);
> + nir_builder_instr_insert(, >instr);

I think it's worth noting that this isn't *strictly* correct for
multi-sampling, unless the shader is s run with per-sample shading
(ala GL_ARB_sample_shading). Otherwise, all samples for a pixel will
get the same discard-condition, leading to aliasing along the
resulting edge.

That being said, per-fragment clipping is better than no clipping, so
it's clearly an improvement.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Question: st/mesa and context-shareable shaders

[Erik Faye-Lund]

On Mon, Sep 28, 2015 at 4:39 PM, Roland Scheidegger  wrote:
>
> In short, for simplicity, only things were sharable which were really
> required to be shared (pretty much just actual resources - and yes that
> doesn't work too well for GL neither as you can't share sampler/rt
> views, let's face it GL's resource model there from legacy GL is a
> disaster, not that d3d9 was any better).

OpenGL allows sharing the objects that potentially take up a lot of
sharable memory, like shaders, programs, texture images, renderbuffers
and vertex buffer objects. But not those who does not, like
framebuffer objects and sampler objects. This makes a ton of sense to
me, and calling this model "a disaster" seems quite unfair.

> And IIRC multithreaded GL in general never really used to work all that
> well and noone was really using that much.

That's not quite true. I've been writing multithreaded OpenGL
applications professionally for the last 7 years, and I've experienced
very few problems with the proprietary drivers in this respect. OpenGL
multi-threading works fine, and is in rather heavy use out there. You
might not see those applications, but they exist. And they have been
existing for a long time, without notable problems.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] st/mesa: try PIPE_BIND_RENDER_TARGET when choosing float texture formats

[Erik Faye-Lund]

On Tue, Sep 29, 2015 at 7:50 PM, Brian Paul  wrote:
>
> I was actually thinking of expanding this change to cover _all_ color formats 
> but wanted to take a small step first.  What do you think?
>

I have some patches in this area sitting around here:

https://github.com/kusma/mesa/commits/color_renderable_fix

In particular:

https://github.com/kusma/mesa/commit/4b72b20dc80a79133c037c7c68413034b228bf66
https://github.com/kusma/mesa/commit/ac5f4575bd11c817c03e7dbc5939f5c0d952d3f1

The first one needs fixing up by using _mesa_base_tex_format instead
of adding it's own implementation.

> On 09/29/2015 11:46 AM, Roland Scheidegger wrote:
>>
>> If that was due to some rgb vs. rgbx thing (that is, could texture from
>> rgb but only render to rgbx) I wonder if the same logic shouldn't also
>> apply to the integer formats.
>> But either way (I guess the app should check fbo completeness in any
>> case so strictly speaking for correctness this isn't really required)
>>
>> Reviewed-by: Roland Scheidegger 
>>
>> Am 29.09.2015 um 17:39 schrieb Brian Paul:
>>>
>>> For 8-bit RGB(A) texture formats we set the PIPE_BIND_RENDER_TARGET flag
>>> to try to get a hardware format which also supports rendering (for FBO
>>> textures).  Do the same thing for floating point formats.
>>>
>>> This allows the Redway Flat demo to run.
>>> ---
>>>   src/mesa/state_tracker/st_format.c | 6 +-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/mesa/state_tracker/st_format.c 
>>> b/src/mesa/state_tracker/st_format.c
>>> index 0c94428..144b7d6 100644
>>> --- a/src/mesa/state_tracker/st_format.c
>>> +++ b/src/mesa/state_tracker/st_format.c
>>> @@ -1963,7 +1963,11 @@ st_ChooseTextureFormat(struct gl_context *ctx, 
>>> GLenum target,
>>>  else if (internalFormat == 3 || internalFormat == 4 ||
>>>   internalFormat == GL_RGB || internalFormat == GL_RGBA ||
>>>   internalFormat == GL_RGB8 || internalFormat == GL_RGBA8 ||
>>> -internalFormat == GL_BGRA)
>>> +internalFormat == GL_BGRA ||
>>> +internalFormat == GL_RGB16F ||
>>> +internalFormat == GL_RGBA16F ||
>>> +internalFormat == GL_RGB32F ||
>>> +internalFormat == GL_RGBA32F)
>>>  bindings |= PIPE_BIND_RENDER_TARGET;
>>>
>>>  /* GLES allows the driver to choose any format which matches
>>>
>>
>
> ___
> 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 v2] Add .mailmap

[Erik Faye-Lund]

On Thu, Dec 17, 2015 at 10:09 AM, Giuseppe Bilotta
 wrote:
> +# missing svn authors:

I'm not sure this is useful, but just for kicks, I decided to try to
track down these contributors, and this is what I came up with
(suspected contributors CC'ed, so they can confirm or deny if they
want):

> +pesco 

Probably "Sven M. Hallberg "
(Sources: http://sourceforge.net/p/mesa3d/mailman/message/5416179/ and
https://github.com/jwiegley/lambdabot-1/blob/master/AUTHORS#L17)

> +tanner 

Probably "Thomas Tanner "
(Source: https://www.mail-archive.com/mesa-dev@mesa3d.org/msg00826.html)

> +hmarson 

Probably "Hamish Marson "
(Sources: http://sourceforge.net/p/r300/mailman/message/2172921/ and
http://comments.gmane.org/gmane.comp.video.dri.devel/19600)

> +reist 

Probably "Boris Peterbarg "
(Sources: 
http://sourceforge.net/p/r300/mailman/r300-commit/?style=threaded=200502=16
and 
http://www.bugzilla.icculus.org/projects/beagle-avahi/Filters/entagged-sharp/Tracker/Util/TrackerTagReader.cs)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] Add .mailmap

[Erik Faye-Lund]

On Thu, Dec 17, 2015 at 10:39 AM, Erik Faye-Lund <kusmab...@gmail.com> wrote:
> On Thu, Dec 17, 2015 at 10:09 AM, Giuseppe Bilotta
> <giuseppe.bilo...@gmail.com> wrote:
>> +# missing svn authors:
>
> I'm not sure this is useful, but just for kicks, I decided to try to
> track down these contributors, and this is what I came up with
> (suspected contributors CC'ed, so they can confirm or deny if they
> want):
>
>> +pesco 
>
> Probably "Sven M. Hallberg <pe...@haquebright.de>"
> (Sources: http://sourceforge.net/p/mesa3d/mailman/message/5416179/ and
> https://github.com/jwiegley/lambdabot-1/blob/master/AUTHORS#L17)

OK, this address bounced (or rather, mailer-dae...@kundenserver.de
buonced pe...@gmx.de, so I guess it's forwarded to a dead address)

However, his homepage (http://khjk.org/~pesco/) suggests that "Sven M.
Hallberg <pe...@khjk.org>" might be an up-to-date e-mail address.
CC'ed.

>> +reist 
>
> Probably "Boris Peterbarg <bori...@zahav.net.il>"
> (Sources: 
> http://sourceforge.net/p/r300/mailman/r300-commit/?style=threaded=200502=16
> and 
> http://www.bugzilla.icculus.org/projects/beagle-avahi/Filters/entagged-sharp/Tracker/Util/TrackerTagReader.cs)

This one also bounced, but it seems he committed to Rails
(https://github.com/rails/rails/pull/19351) as "Boris Peterbarg
<bo...@seekingalpha.com>". Again, CC'ed.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] main: get rid of needless conditional

[Erik Faye-Lund]

We already check if the driver changed the completeness, we don't
need to duplicate that check. Let's just early out there instead.

Signed-off-by: Erik Faye-Lund <kusmab...@gmail.com>
---
 src/mesa/main/fbobject.c | 25 -
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index fe6bdc2..3be216d 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -1253,23 +1253,22 @@ _mesa_test_framebuffer_completeness(struct gl_context 
*ctx,
   ctx->Driver.ValidateFramebuffer(ctx, fb);
   if (fb->_Status != GL_FRAMEBUFFER_COMPLETE_EXT) {
  fbo_incomplete(ctx, "driver marked FBO as incomplete", -1);
+ return;
   }
}
 
-   if (fb->_Status == GL_FRAMEBUFFER_COMPLETE_EXT) {
-  /*
-   * Note that if ARB_framebuffer_object is supported and the attached
-   * renderbuffers/textures are different sizes, the framebuffer
-   * width/height will be set to the smallest width/height.
-   */
-  if (numImages != 0) {
- fb->Width = minWidth;
- fb->Height = minHeight;
-  }
-
-  /* finally, update the visual info for the framebuffer */
-  _mesa_update_framebuffer_visual(ctx, fb);
+   /*
+* Note that if ARB_framebuffer_object is supported and the attached
+* renderbuffers/textures are different sizes, the framebuffer
+* width/height will be set to the smallest width/height.
+*/
+   if (numImages != 0) {
+  fb->Width = minWidth;
+  fb->Height = minHeight;
}
+
+   /* finally, update the visual info for the framebuffer */
+   _mesa_update_framebuffer_visual(ctx, fb);
 }
 
 
-- 
2.5.0

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

[Mesa-dev] [PATCH 1/2] gallium/util: removed unused header-file

[Erik Faye-Lund]

This hasn't been in use since c476305 ("gallium/util: pregenerate
half float tables"), where the last bit of run-time init using this
was killed. So let's just get rid of the pointless header.

Signed-off-by: Erik Faye-Lund <kusmab...@gmail.com>
---
 src/gallium/auxiliary/Makefile.sources |  1 -
 src/gallium/auxiliary/util/u_init.h| 52 --
 2 files changed, 53 deletions(-)
 delete mode 100644 src/gallium/auxiliary/util/u_init.h

diff --git a/src/gallium/auxiliary/Makefile.sources 
b/src/gallium/auxiliary/Makefile.sources
index d92da3d..5325f97 100644
--- a/src/gallium/auxiliary/Makefile.sources
+++ b/src/gallium/auxiliary/Makefile.sources
@@ -252,7 +252,6 @@ C_SOURCES := \
util/u_helpers.h \
util/u_index_modify.c \
util/u_index_modify.h \
-   util/u_init.h \
util/u_inlines.h \
util/u_keymap.c \
util/u_keymap.h \
diff --git a/src/gallium/auxiliary/util/u_init.h 
b/src/gallium/auxiliary/util/u_init.h
deleted file mode 100644
index 7bc356a..000
--- a/src/gallium/auxiliary/util/u_init.h
+++ /dev/null
@@ -1,52 +0,0 @@
-/*
- * Copyright 2010 Luca Barbieri
- *
- * Permission is hereby granted, free of charge, to any person obtaining
- * a copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sublicense, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial
- * portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
- * IN NO EVENT SHALL THE COPYRIGHT OWNER(S) AND/OR ITS SUPPLIERS BE
- * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
- * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
- * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- **/
-
-#ifndef U_INIT_H
-#define U_INIT_H
-
-/* Use UTIL_INIT(f) to have f called at program initialization.
-   Note that it is only guaranteed to be called if any symbol in the
-   .c file it is in sis referenced by the program.
-
-   UTIL_INIT functions are called in arbitrary order.
-*/
-
-#ifdef __cplusplus
-/* use a C++ global constructor */
-#define UTIL_INIT(f) struct f##__gctor_t {f##__gctor_t() {x();}} f##__gctor;
-#elif defined(_MSC_VER)
-/* add a pointer to the section where MSVC stores global constructor pointers 
*/
-/* see http://blogs.msdn.com/vcblog/archive/2006/10/20/crt-initialization.aspx 
and
-   
http://stackoverflow.com/questions/1113409/attribute-constructor-equivalent-in-vc
 */
-#pragma section(".CRT$XCU",read)
-#define UTIL_INIT(f) static void __cdecl f##__init(void) {f();}; 
__declspec(allocate(".CRT$XCU")) void (__cdecl* f##__xcu)(void) = f##__init;
-#elif defined(__GNUC__)
-#define UTIL_INIT(f) static void f##__init(void) __attribute__((constructor)); 
static void f##__init(void) {f();}
-#else
-#error Unsupported compiler: please find out how to implement global 
initializers in C on it
-#endif
-
-#endif
-
-- 
2.5.0

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

[Mesa-dev] [PATCH 0/2] trivial cleanups

[Erik Faye-Lund]

Here's some trivial cleanups I found while diving into something else.

Instead of them collecting dust in my tree, perhaps we could apply them to
the central tree?

Erik Faye-Lund (2):
  gallium/util: removed unused header-file
  main: get rid of needless conditional

 src/gallium/auxiliary/Makefile.sources |  1 -
 src/gallium/auxiliary/util/u_init.h| 52 --
 src/mesa/main/fbobject.c   | 25 
 3 files changed, 12 insertions(+), 66 deletions(-)
 delete mode 100644 src/gallium/auxiliary/util/u_init.h

-- 
2.5.0

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

Re: [Mesa-dev] [PATCH v5] Add .mailmap

[Erik Faye-Lund]

On Mon, Dec 28, 2015 at 12:23 PM,   wrote:
> Should I be expecting to see myself on here?
>

Only if you have commits in mesa.git attributed to you using the same
name but different e-mail addresses...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] Add .mailmap

[Erik Faye-Lund]

On Tue, Dec 22, 2015 at 12:47 PM, Thomas Tanner <tan...@gmx.net> wrote:
> Hi,
> my primary email address for open source contributions is tan...@gmx.net
> cheers
>

OK, I think Giuseppe will want this info for his final version of the
patch (CC'ed), thanks :)

> On 17.12.15 10:39, Erik Faye-Lund wrote:
>> On Thu, Dec 17, 2015 at 10:09 AM, Giuseppe Bilotta
>> <giuseppe.bilo...@gmail.com> wrote:
>>> +# missing svn authors:
>>
>> I'm not sure this is useful, but just for kicks, I decided to try to
>> track down these contributors, and this is what I came up with
>> (suspected contributors CC'ed, so they can confirm or deny if they
>> want):
>>
>>> +tanner 
>>
>> Probably "Thomas Tanner <tan...@gnu.org>"
>> (Source: https://www.mail-archive.com/mesa-dev@mesa3d.org/msg00826.html)
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/2] trivial cleanups

[Erik Faye-Lund]

On Mon, Jan 11, 2016 at 11:50 PM, Timothy Arceri <t_arc...@yahoo.com.au> wrote:
> On Mon, 2016-01-11 at 16:35 +0100, Erik Faye-Lund wrote:
>> Ping?
>>
>
> The other patch is also now
> Reviewed-by: Timothy Arceri <timothy.arc...@collabora.com>
>
> I take it you don't have commit access?

Thanks. Yeah, no commit access for me. If you could push it, that'd be awesome.

Perhaps I should apply for commit access soon, though...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 08/18] meta/blit: Don't pollute the sampler object namespace in _mesa_meta_setup_sampler

[Erik Faye-Lund]

On Tue, Jan 12, 2016 at 5:44 PM, Ian Romanick <i...@freedesktop.org> wrote:
> On 01/12/2016 01:36 AM, Erik Faye-Lund wrote:
>> On Sat, Jan 9, 2016 at 3:59 AM, Ian Romanick <i...@freedesktop.org> wrote:
>>> From: Ian Romanick <ian.d.roman...@intel.com>
>>>
>>> tl;dr: For many types of GL object, we can *NEVER* use the Gen function.
>>>
>>> In OpenGL ES (all versions!) and OpenGL compatibility profile,
>>> applications don't have to call Gen functions.  The GL spec is very
>>> clear about how you can mix-and-match generated names and non-generated
>>> names: you can use any name you want for a particular object type until
>>> you call the Gen function for that object type.
>>
>> Not that it affects this patch (except perhaps the commit message),
>> but at least in GLES2 you can even keep on using the old buffer,
>> texture, framebuffer or renderbuffer names even after calling the
>> Gen-function, because Gen{Buffers,Textures,Framebuffers,Renderbuffers}
>> are defined as "returns n previously unused
>> {buffer,texture,framebuffer,renderbuffer} object names in textures".
>> Since the object names needs to be previously unused, the
>> Gen-functions cannot generate names that has been previously bound. I
>> expect that the same goes for other specifications, but I haven't
>> verified that.
>
> Correct.  After you start calling GenFoo you just can't invent your own
> names for foo objects.  There are some cases where it can work, but,
> especially in the case of GLX indirect rendering, it may not work on
> some implementations even when you think it should.
>
> This commit message gets copy-and-pasted a *lot*.  If you think there
> are changes that should be made, I can do that on the future patches in
> this series.

I don't think it's important. This is accurate enough to justify the change.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev