Re: [Piglit] [PATCH] fbo: Fix broken MRT bindings test.

2015-02-18 Thread Ilia Mirkin
On Thu, Feb 19, 2015 at 2:11 AM, Matt Turner  wrote:
> On Wed, Feb 18, 2015 at 11:06 PM, Ilia Mirkin  wrote:
>> On Thu, Feb 19, 2015 at 2:01 AM, Matt Turner  wrote:
>>> The code declared GLuint textures[4] and then proceeded to do things
>>> like glGenTextures(5, textures) and access textures[4]. Presumably the
>>> intention was to declare an array of size 5.
>>> ---
>>> Vinson pointed out that this test was broken a month ago...
>>>
>>>  tests/fbo/fbo-mrt-new-bind.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/fbo/fbo-mrt-new-bind.c b/tests/fbo/fbo-mrt-new-bind.c
>>> index 8e006b5..18c36c5 100644
>>> --- a/tests/fbo/fbo-mrt-new-bind.c
>>> +++ b/tests/fbo/fbo-mrt-new-bind.c
>>> @@ -46,7 +46,7 @@ PIGLIT_GL_TEST_CONFIG_END
>>>  GLenum buffers[] = { GL_COLOR_ATTACHMENT0, GL_COLOR_ATTACHMENT1 };
>>>  GLuint fbos[3];
>>>  GLint prog0, prog1;
>>> -GLuint textures[4];
>>> +GLuint textures[5];
>>
>> Yes.
>>
>>>
>>>  void
>>>  piglit_init(int argc, char **argv)
>>> @@ -56,7 +56,7 @@ piglit_init(int argc, char **argv)
>>> piglit_require_GLSL_version(130);
>>>
>>> glGenTextures(5, textures);
>>> -   for (i=0; i<4; i++) {
>>> +   for (i = 0; i < ARRAY_SIZE(textures); i++) {
>>
>> No. The code below does
>>
>> glBindTexture(GL_TEXTURE_2D, textures[4]);
>> glTexImage2D(GL_TEXTURE_2D, 0, GL_DEPTH24_STENCIL8, 640, 360, 0,
>> GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8, NULL);
>>
>> The loop is just there to initialize the 4 color textures.
>
> Oh, you're right. That's what I get for combining fixes with (what I
> thought was a) superfluous changes! I'll remove that.
>
> R-b on the s/4/5/?

Yes if you drop the second hunk, Reviewed-by: Ilia Mirkin 

>
>>> glBindTexture(GL_TEXTURE_2D, textures[i]);
>>> glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 64, 64, 0, GL_RGBA, 
>>> GL_UNSIGNED_BYTE, NULL);
>>> glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, 
>>> GL_NEAREST);
>>> --
>>> 2.0.5
>>>
>>> ___
>>> Piglit mailing list
>>> Piglit@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] fbo: Fix broken MRT bindings test.

2015-02-18 Thread Matt Turner
On Wed, Feb 18, 2015 at 11:06 PM, Ilia Mirkin  wrote:
> On Thu, Feb 19, 2015 at 2:01 AM, Matt Turner  wrote:
>> The code declared GLuint textures[4] and then proceeded to do things
>> like glGenTextures(5, textures) and access textures[4]. Presumably the
>> intention was to declare an array of size 5.
>> ---
>> Vinson pointed out that this test was broken a month ago...
>>
>>  tests/fbo/fbo-mrt-new-bind.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/fbo/fbo-mrt-new-bind.c b/tests/fbo/fbo-mrt-new-bind.c
>> index 8e006b5..18c36c5 100644
>> --- a/tests/fbo/fbo-mrt-new-bind.c
>> +++ b/tests/fbo/fbo-mrt-new-bind.c
>> @@ -46,7 +46,7 @@ PIGLIT_GL_TEST_CONFIG_END
>>  GLenum buffers[] = { GL_COLOR_ATTACHMENT0, GL_COLOR_ATTACHMENT1 };
>>  GLuint fbos[3];
>>  GLint prog0, prog1;
>> -GLuint textures[4];
>> +GLuint textures[5];
>
> Yes.
>
>>
>>  void
>>  piglit_init(int argc, char **argv)
>> @@ -56,7 +56,7 @@ piglit_init(int argc, char **argv)
>> piglit_require_GLSL_version(130);
>>
>> glGenTextures(5, textures);
>> -   for (i=0; i<4; i++) {
>> +   for (i = 0; i < ARRAY_SIZE(textures); i++) {
>
> No. The code below does
>
> glBindTexture(GL_TEXTURE_2D, textures[4]);
> glTexImage2D(GL_TEXTURE_2D, 0, GL_DEPTH24_STENCIL8, 640, 360, 0,
> GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8, NULL);
>
> The loop is just there to initialize the 4 color textures.

Oh, you're right. That's what I get for combining fixes with (what I
thought was a) superfluous changes! I'll remove that.

R-b on the s/4/5/?

>> glBindTexture(GL_TEXTURE_2D, textures[i]);
>> glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 64, 64, 0, GL_RGBA, 
>> GL_UNSIGNED_BYTE, NULL);
>> glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, 
>> GL_NEAREST);
>> --
>> 2.0.5
>>
>> ___
>> Piglit mailing list
>> Piglit@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] fbo: Fix broken MRT bindings test.

2015-02-18 Thread Ilia Mirkin
On Thu, Feb 19, 2015 at 2:01 AM, Matt Turner  wrote:
> The code declared GLuint textures[4] and then proceeded to do things
> like glGenTextures(5, textures) and access textures[4]. Presumably the
> intention was to declare an array of size 5.
> ---
> Vinson pointed out that this test was broken a month ago...
>
>  tests/fbo/fbo-mrt-new-bind.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/fbo/fbo-mrt-new-bind.c b/tests/fbo/fbo-mrt-new-bind.c
> index 8e006b5..18c36c5 100644
> --- a/tests/fbo/fbo-mrt-new-bind.c
> +++ b/tests/fbo/fbo-mrt-new-bind.c
> @@ -46,7 +46,7 @@ PIGLIT_GL_TEST_CONFIG_END
>  GLenum buffers[] = { GL_COLOR_ATTACHMENT0, GL_COLOR_ATTACHMENT1 };
>  GLuint fbos[3];
>  GLint prog0, prog1;
> -GLuint textures[4];
> +GLuint textures[5];

Yes.

>
>  void
>  piglit_init(int argc, char **argv)
> @@ -56,7 +56,7 @@ piglit_init(int argc, char **argv)
> piglit_require_GLSL_version(130);
>
> glGenTextures(5, textures);
> -   for (i=0; i<4; i++) {
> +   for (i = 0; i < ARRAY_SIZE(textures); i++) {

No. The code below does

glBindTexture(GL_TEXTURE_2D, textures[4]);
glTexImage2D(GL_TEXTURE_2D, 0, GL_DEPTH24_STENCIL8, 640, 360, 0,
GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8, NULL);

The loop is just there to initialize the 4 color textures.

> glBindTexture(GL_TEXTURE_2D, textures[i]);
> glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 64, 64, 0, GL_RGBA, 
> GL_UNSIGNED_BYTE, NULL);
> glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
> --
> 2.0.5
>
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [Mesa-dev] [PATCH] arb_occlusion_query2: expect an error when target mismatch in glBeginQuery()

2015-02-18 Thread Eduardo Lima Mitev
On 02/18/2015 11:15 PM, Mark Janes wrote:
> Hi Eduardo,
> 
> I track piglit regressions for the mesa team at Intel.  It would really
> help me if you put a note in your commit message when a new test is
> known to fail on any platform.
> 

Hi Mark,

Ok, I will have that into account for the future. Sorry if it caused you
trouble finding the regression.

FWIW, the patch in Mesa that fixes this new test cases is at:

http://lists.freedesktop.org/archives/mesa-dev/2015-February/076528.html

It already has R-b but has not been pushed yet (we normally wait to have
a large number of reviewed patches in a series before pushing). Maybe I
should have pushed this one at least to avoid the piglit regression.
Will try to do that ASAP.

BTW, this new test also regresses on NVidia proprietary driver.

Thanks for the heads-up.

cheers,
Eduardo

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH] fbo: Fix broken MRT bindings test.

2015-02-18 Thread Matt Turner
The code declared GLuint textures[4] and then proceeded to do things
like glGenTextures(5, textures) and access textures[4]. Presumably the
intention was to declare an array of size 5.
---
Vinson pointed out that this test was broken a month ago...

 tests/fbo/fbo-mrt-new-bind.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/fbo/fbo-mrt-new-bind.c b/tests/fbo/fbo-mrt-new-bind.c
index 8e006b5..18c36c5 100644
--- a/tests/fbo/fbo-mrt-new-bind.c
+++ b/tests/fbo/fbo-mrt-new-bind.c
@@ -46,7 +46,7 @@ PIGLIT_GL_TEST_CONFIG_END
 GLenum buffers[] = { GL_COLOR_ATTACHMENT0, GL_COLOR_ATTACHMENT1 };
 GLuint fbos[3];
 GLint prog0, prog1;
-GLuint textures[4];
+GLuint textures[5];
 
 void
 piglit_init(int argc, char **argv)
@@ -56,7 +56,7 @@ piglit_init(int argc, char **argv)
piglit_require_GLSL_version(130);
 
glGenTextures(5, textures);
-   for (i=0; i<4; i++) {
+   for (i = 0; i < ARRAY_SIZE(textures); i++) {
glBindTexture(GL_TEXTURE_2D, textures[i]);
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 64, 64, 0, GL_RGBA, 
GL_UNSIGNED_BYTE, NULL);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
-- 
2.0.5

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 0/4] calculation of tolerance for complex GLSL built-ins

2015-02-18 Thread Ilia Mirkin
Do you have a git tree for this? Unfortunately you sent the patches
with Content-type: ... format="flowed", which makes it annoying to
read the patches in a conformant email client.

On Thu, Feb 19, 2015 at 1:06 AM, Micah Fedke
 wrote:
> This series upgrades the ARB_shader_precision test generator with support
> for complex function tolerances.
>
> The ARB_shader_precision spec does not give specific guidance in regard to
> acceptable error ranges for complex functions, supplying only the murky
> "Built-in functions defined in the specification with an equation built from
> the above operations inherit the above errors."  Currently, the
> shader_precision tests allow zero tolerance for complex functions, but this
> is causing many false negatives and needs to be improved.
>
> This series is my best attempt at due diligence - any discussion/input is
> certainly welcomed :)
>
> The theory behind this is probably worth a blog post and I won't belabor the
> point here, but this series basically implements a filter for the test_suite
> dict that will drop any test vectors that would push a function's output
> into regions of greatly multiplied error - the "badlands".  If the output
> does not trespass on the badlands, then the error estimate in ulps is
> calculated and attached to the current test vector for the shader_test to
> use as the allowable error range when analyzing the output generated on the
> GPU during test run-time.
>
> The bigfloat package is a necessary dependency for this filter, as it
> provides both arbitrary-size floating point operations and it allows all
> floating point values and operations in a code block to be forced into a
> particular precision, leaving no question as to what precision was employed
> for an intermediate calculation.  This comes in handy when running the
> reference versions of the complex built-ins.  Performance impact is small
> (~6s for the entire gen_shader_precision_tests.py script on IvyBridge i7 @
> 2.9GHz) and is localized to build-time.
>
> The technique described above actually takes very little python code to
> construct - most of the complexity is in dealing with the polymorphic nature
> of the GLSL built-ins - componentwise vs. non-componentwise parameters, etc.
>
>
> Micah Fedke (4):
> arb_shader_precision: support scalar values in shader_runner_format
> arb_shader_precision: add framework for calculating tolerances for complex
> functions
> arb_shader_precision: add reference implementations of complex functions
> arb_shader_precision: enable calculation of complex function tolerances  -
> update mako templates to support vectors of tolerances
>
>  generated_tests/gen_shader_precision_tests.py| 376
> +-
>  generated_tests/templates/gen_shader_precision_tests/fs.mako |  51
> +++--
>  generated_tests/templates/gen_shader_precision_tests/gs.mako |  51
> +++--
>  generated_tests/templates/gen_shader_precision_tests/vs.mako |  51
> +++--
>  4 files changed, 451 insertions(+), 78 deletions(-)
>
> --
>
> Micah Fedke
> Collabora Ltd.
> +44 1223 362967
> https://www.collabora.com/
> https://twitter.com/collaboraltd
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/4] arb_shader_precision: support scalar values in shader_runner_format

2015-02-18 Thread Dylan Baker
On Thu, Feb 19, 2015 at 12:06:15AM -0600, Micah Fedke wrote:
> Prep work for using arrays of tolerances in the shader_test files.
> 
> ---
>   generated_tests/gen_shader_precision_tests.py | 34 
> +--
>   1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/generated_tests/gen_shader_precision_tests.py 
> b/generated_tests/gen_shader_precision_tests.py
> index 0d24a80..cfa5065 100644
> --- a/generated_tests/gen_shader_precision_tests.py
> +++ b/generated_tests/gen_shader_precision_tests.py
> @@ -68,6 +68,10 @@ trig_builtins = ('sin', 'cos', 'tan',
>'sinh', 'cosh', 'tanh',   'asinh', 
> 'acosh', 'atanh')
>   +def _is_sequence(arg):
> +return (not hasattr(arg, "strip") and
> +hasattr(arg, "__iter__"))
> +

All of the built-in sequence types (list, set, frozenset, tuple) inherit
from one of the ABC's in collections, collections.Sequence is probably
the right one. Would using isinstance(arg, collections.Sequence) work?
This just feels...  abusive, definitely not idiomatic.

>   def make_indexers(signature):
>  """Build a list of strings which index into every possible
>  value of the result.  For example, if the result is a vec2,
> @@ -105,20 +109,26 @@ def shader_runner_format(values):
>   "probe rgba" command.  Bools are converted to 0's and 1's, and
>   values are separated by spaces.
>   """
> -transformed_values = []
> -retval = ''
> -for value in values:
> -if isinstance(value, (bool, np.bool_)):
> -transformed_values.append(int(value))
> -else:
> -transformed_values.append(value)
> -for x in transformed_values:
> -if isinstance(x,np.float32):
> -retval+=' {0}'.format('{0:1.8e}'.format(x))
> -else:
> -retval+=' {0}'.format(repr(x))
> +
> +if _is_sequence(values):
> +transformed_values = []
> +retval = ''
> +for value in values:
> +if isinstance(value, (bool, np.bool_)):
> +transformed_values.append(int(value))
> +else:
> +transformed_values.append(value)
> +for x in transformed_values:
> +if isinstance(x,np.float32):
> +retval+=' {0}'.format('{0:1.8e}'.format(x))
> +else:
> +retval+=' {0}'.format(repr(x))
> +else:
> +retval = '{}'.format(values)

What is values? I assume in this else block it's a string? If it is a
string then just setting retval = values should be sufficient.

> +
>   return retval
>   +
>   def main():
>   """ Main function """
>   -- 2.2.2
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit


signature.asc
Description: Digital signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH 3/4] arb_shader_precision: add reference implementations of complex functions

2015-02-18 Thread Micah Fedke

---
 generated_tests/gen_shader_precision_tests.py | 177 
+-

 1 file changed, 176 insertions(+), 1 deletion(-)

diff --git a/generated_tests/gen_shader_precision_tests.py 
b/generated_tests/gen_shader_precision_tests.py

index 0bda05a..f1ef420 100644
--- a/generated_tests/gen_shader_precision_tests.py
+++ b/generated_tests/gen_shader_precision_tests.py
@@ -89,6 +89,171 @@ def _ulpsize(f):
 return bigfloat.next_up(f)-f if f >= 0.0 \
 else f-bigfloat.next_down(f)
 +def _to_bigfloat_any(arg):
+if type(arg).__name__ == 'BigFloat':
+return arg
+else:
+return bigfloat.BigFloat.exact('{0:1.8e}'.format(arg), 
precision=23) if _len_any(arg) == 1 \

+   else map(bigfloat.BigFloat, map('{0:1.8e}'.format, arg))
+
+def _mod_ref(args):
+x = _to_bigfloat_any(args[0])
+y = _to_bigfloat_any(args[1])
+return x - y*(bigfloat.floor(x/y))
+
+def _smoothstep_ref(args):
+edge0 = _to_bigfloat_any(args[0])
+edge1 = _to_bigfloat_any(args[1])
+x = _to_bigfloat_any(args[2])
+t = (x-edge0)/(edge1-edge0) +return t*t*(3.0-2.0*t)
+
+def _mix_ref(args):
+x = _to_bigfloat_any(args[0])
+y = _to_bigfloat_any(args[1])
+a = _to_bigfloat_any(args[2])
+return x*(1.0-a)+y*a
+
+def _length_ref(args):
+component_sum = bigfloat.BigFloat(0.0)
+for i, arg in enumerate(args[0] if _len_any(args[0]) > 1 else args):
+component_sum += bigfloat.pow(_to_bigfloat_any(arg), 2.0)
+return bigfloat.sqrt(component_sum)
+
+def _distance_ref(args):
+components = _len_any(args[0])
+difference = []
+for i in range(components):
+p0 = _to_bigfloat_any(args[0] if components == 1 else args[0][i])
+p1 = _to_bigfloat_any(args[1] if components == 1 else args[1][i])
+difference.append(p0-p1)
+return _length_ref(difference)
+
+def _dot_ref(args):
+components = _len_any(args[0])
+product = bigfloat.BigFloat(0.0)
+for i in range(components):
+x = _to_bigfloat_any(args[0] if components == 1 else args[0][i])
+y = _to_bigfloat_any(args[1] if components == 1 else args[1][i])
+product += x*y
+return product
+
+def _cross_ref(args):
+x0 = _to_bigfloat_any(args[0][0])
+x1 = _to_bigfloat_any(args[0][1])
+x2 = _to_bigfloat_any(args[0][2])
+y0 = _to_bigfloat_any(args[1][0])
+y1 = _to_bigfloat_any(args[1][1])
+y2 = _to_bigfloat_any(args[1][2])
+ret = [x1*y2-y1*x2, x2*y0-y2*x0, x0*y1-y0*x1]
+return ret
+
+def _normalize_ref(args):
+num_components = _len_any(args[0])
+normalized_components = []
+comp_len = _length_ref(args if num_components == 1 else args[0])
+for i in range(num_components):
+component = _to_bigfloat_any(args[0] if num_components == 1 
else args[0][i])

+normalized_components.append(component/comp_len)
+return normalized_components[0] if num_components == 1 else 
normalized_components

+
+def _faceforward_ref(args):
+N = _to_bigfloat_any(args[0])
+I = _to_bigfloat_any(args[1])
+Nref = _to_bigfloat_any(args[2])
+components = _len_any(args[0])
+if _dot_ref([Nref,I]) < 0.0:
+ret = N
+else:
+if components == 1:
+negN = N*-1.0
+else:
+negN = []
+for i in range(components):
+negN.append(N[i]*-1.0)
+ret = negN
+return ret
+
+def _reflect_ref(args):
+I = _to_bigfloat_any(args[0])
+N = _to_bigfloat_any(args[1])
+components = _len_any(args[0])
+dotNI = _dot_ref([N,I])
+if components == 1:
+ret = 2.0*dotNI*N
+else:
+ret = []
+for i in range(components):
+ret.append(2.0*dotNI*N[i])
+return ret
+
+def _refract_ref(args):
+I = args[0]
+N = args[1]
+eta = _to_bigfloat_any(args[2])
+components = _len_any(args[0])
+k = 1.0-eta*eta*(1.0-_dot_ref([N,I])*_dot_ref([N,I]))
+if k < 0.0:
+if components == 1:
+ret = bigfloat.BigFloat(0.0)
+else:
+ret = []
+for i in range(components):
+ret.append(bigfloat.BigFloat(0.0))
+else:
+if components == 1:
+ret = eta*I-(eta*_dot_ref([N,I])+bigfloat.sqrt(k))*N
+else:
+Ntemp = []
+Itemp = []
+ret = []
+for i in range(components):
+Ntemp = (eta*_dot_ref([N,I])+bigfloat.sqrt(k))*N[i]
+Itemp = eta*I[i]
+ret.append(Itemp - Ntemp)
+return ret
+
+def _vec_times_mat_ref(args):
+v = args[0]
+m = args[1]
+m_type = glsl_type_of(m)
+num_cols = m_type.num_cols
+num_rows = m_type.num_rows
+components = num_cols
+ret = []
+for i in range(components):
+m_col = []
+for j in range(num_rows):
+m_col.append(m[j][i])
+ret.append(_dot_ref([v,m_col]))
+return ret
+
+def _mat_times_vec_ref(args):
+m = args[0]
+v = args[1]
+ 

[Piglit] [PATCH 4/4] arb_shader_precision: enable calculation of complex function tolerances

2015-02-18 Thread Micah Fedke

 - update mako templates to support vectors of tolerances

---
 generated_tests/gen_shader_precision_tests.py  | 23 +++---
 .../templates/gen_shader_precision_tests/fs.mako   | 51 
++
 .../templates/gen_shader_precision_tests/gs.mako   | 51 
++
 .../templates/gen_shader_precision_tests/vs.mako   | 51 
++

 4 files changed, 119 insertions(+), 57 deletions(-)

diff --git a/generated_tests/gen_shader_precision_tests.py 
b/generated_tests/gen_shader_precision_tests.py

index f1ef420..ceebb5e 100644
--- a/generated_tests/gen_shader_precision_tests.py
+++ b/generated_tests/gen_shader_precision_tests.py
@@ -438,15 +438,21 @@ def main():
 arg.base_type == glsl_float for arg in 
signature.argtypes)
 # Filter the test vectors down to only those which deal 
exclusively in float types

 #and are not trig functions or determinant()
-indexers = make_indexers(signature)
-num_elements = 
signature.rettype.num_cols*signature.rettype.num_rows
-invocation = signature.template.format( *['arg{0}'.format(i) - 
   for i in 
xrange(len(signature.argtypes))])

 if (signature.rettype.base_type == glsl_float and
 arg_float_check and
 all(arg_float_check) and
 signature.name not in trig_builtins and
 signature.name != 'determinant'): +# replace 
the tolerances in each test_vector with

+# our own tolerances specified in ulps and
+# reject vectors that produce results with too much error
+refined_test_vectors = []
+complex_tol_type = signature.rettype
+for test_num, test_vector in enumerate(test_vectors):
+tolerance = _gen_tolerance(signature.name, 
signature.rettype, test_vector.arguments)

+if tolerance >= 0.0:
+ 
refined_test_vectors.append(TestVector(test_vector.arguments, 
test_vector.result, tolerance))

+# Then generate the shader_test scripts
 for shader_stage in ('vs', 'fs', 'gs'):
 template = template_file('gen_shader_precision_tests', 
'{0}.mako'.format(shader_stage))
 output_filename = os.path.join( 'spec', 
'arb_shader_precision',

@@ -458,10 +464,15 @@ def main():
 dirname = os.path.dirname(output_filename)
 if not os.path.exists(dirname):
 os.makedirs(dirname)
+indexers = make_indexers(signature)
+num_elements = 
signature.rettype.num_cols*signature.rettype.num_rows
+invocation = signature.template.format( 
*['arg{0}'.format(i) + 
  for i in xrange(len(signature.argtypes))])

 with open(output_filename, 'w') as f:
 f.write(template.render_unicode( 
signature=signature, - 
   test_vectors=test_vectors,

- tolerances=simple_fns,
+ 
is_complex_tolerance=signature.name in complex_fns,
+ 
complex_tol_type=signature.rettype,
+ 
test_vectors=refined_test_vectors,


invocation=invocation,

num_elements=num_elements,
  indexers=indexers,
diff --git 
a/generated_tests/templates/gen_shader_precision_tests/fs.mako 
b/generated_tests/templates/gen_shader_precision_tests/fs.mako

index 4ea5e50..a53e5e7 100644
--- a/generated_tests/templates/gen_shader_precision_tests/fs.mako
+++ b/generated_tests/templates/gen_shader_precision_tests/fs.mako
@@ -16,7 +16,11 @@ void main()
 % for i, arg in enumerate(signature.argtypes):
 uniform ${arg} arg${i};
 % endfor
+% if is_complex_tolerance and complex_tol_type.name != 'float':
+uniform ${complex_tol_type} tolerance;
+% else:
 uniform float tolerance;
+% endif
 uniform ${signature.rettype} expected;
  void main()
@@ -53,29 +57,34 @@ ${' || '.join('(resultbits[{0}]>>31 != 
expectedbits[{0}]>>31)'.format(i) for i i

   ${signature.rettype} ulps = ${signature.rettype}(\
 ${', '.join('abs(resultbits[{0}] - expectedbits[{0}])'.format(i) for i 
in xrange(0, num_elements))}\

 );
-  ##
-  ## find the maximum error in ulps of all the calculations using a 
nested max() sort

-  ##
+% if is_complex_tolerance and complex_tol_type.name != 'float':
+## compare vecs directly, use a boolean version of the rettype
+  b${signature.rettype} calcerr = greaterThan(ulps, tolerance);
+% else:
+##
+## find the maximum error in ulps of all the calculations using a 
nested max() sort

+##
   float max_error = \
-## start with the outermost max() if there are more than 2 elements
-## (two element arrays, eg. vec2, are handled by the final max() 
below, only)

-% if num_elements > 2:
+  ## start with the outermost max() if there are more than 2 elements
+  ## (two element arrays, eg. vec2, are handled by the final max() 
below, only)

+  % if num_ele

[Piglit] [PATCH 1/4] arb_shader_precision: support scalar values in shader_runner_format

2015-02-18 Thread Micah Fedke

Prep work for using arrays of tolerances in the shader_test files.

---
 generated_tests/gen_shader_precision_tests.py | 34 
+--

 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/generated_tests/gen_shader_precision_tests.py 
b/generated_tests/gen_shader_precision_tests.py

index 0d24a80..cfa5065 100644
--- a/generated_tests/gen_shader_precision_tests.py
+++ b/generated_tests/gen_shader_precision_tests.py
@@ -68,6 +68,10 @@ trig_builtins = ('sin', 'cos', 'tan',
  'sinh', 'cosh', 'tanh',   'asinh', 
'acosh', 'atanh')

 +def _is_sequence(arg):
+return (not hasattr(arg, "strip") and
+hasattr(arg, "__iter__"))
+
 def make_indexers(signature):
"""Build a list of strings which index into every possible
value of the result.  For example, if the result is a vec2,
@@ -105,20 +109,26 @@ def shader_runner_format(values):
 "probe rgba" command.  Bools are converted to 0's and 1's, and
 values are separated by spaces.
 """
-transformed_values = []
-retval = ''
-for value in values:
-if isinstance(value, (bool, np.bool_)):
-transformed_values.append(int(value))
-else:
-transformed_values.append(value)
-for x in transformed_values:
-if isinstance(x,np.float32):
-retval+=' {0}'.format('{0:1.8e}'.format(x))
-else:
-retval+=' {0}'.format(repr(x))
+
+if _is_sequence(values):
+transformed_values = []
+retval = ''
+for value in values:
+if isinstance(value, (bool, np.bool_)):
+transformed_values.append(int(value))
+else:
+transformed_values.append(value)
+for x in transformed_values:
+if isinstance(x,np.float32):
+retval+=' {0}'.format('{0:1.8e}'.format(x))
+else:
+retval+=' {0}'.format(repr(x))
+else:
+retval = '{}'.format(values)
+
 return retval
 +
 def main():
 """ Main function """
 -- 2.2.2
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH 2/4] arb_shader_precision: add framework for calculating tolerances for complex functions

2015-02-18 Thread Micah Fedke

---
 generated_tests/gen_shader_precision_tests.py | 148 
--

 1 file changed, 137 insertions(+), 11 deletions(-)

diff --git a/generated_tests/gen_shader_precision_tests.py 
b/generated_tests/gen_shader_precision_tests.py

index cfa5065..0bda05a 100644
--- a/generated_tests/gen_shader_precision_tests.py
+++ b/generated_tests/gen_shader_precision_tests.py
@@ -49,29 +49,155 @@
 from builtin_function import *  import mako.template  import os 
+import struct

+import bigfloat
  from templates import template_file
 -tolerances = {'pow': 16.0, -  'exp': 3.0,
-  'exp2': 3.0,
-  'log': 3.0,
-  'log2': 3.0,
-  'sqrt': 3.0,
-  'inversesqrt': 2.0,
-  'op-div': 2.5,
-  'op-assign-div': 2.5,
-  }
+
+allowed_error_scale = 4.0
  trig_builtins = ('sin', 'cos', 'tan',   'asin', 
'acos', 'atan',   'sinh', 'cosh', 'tanh', 
'asinh', 'acosh', 'atanh')

 +high_precision = bigfloat.precision(113)
+low_precision = bigfloat.precision(23)
+
 def _is_sequence(arg):
 return (not hasattr(arg, "strip") and
 hasattr(arg, "__iter__"))
 +def _len_any(a):
+""" a version of len that returns 1 if passed a non-sequence type
+"""
+return len(a) if _is_sequence(a) else 1
+
+def _floatToBits(f):
+s = struct.pack('>f', f)
+return struct.unpack('>l', s)[0]
+
+def _bitsToFloat(b):
+s = struct.pack('>l', b)
+return struct.unpack('>f', s)[0]
+
+def _ulpsize(f):
+""" determine _ulpsize in the direction of nearest infinity
+which gives the worst case scenario for edge cases
+"""
+return bigfloat.next_up(f)-f if f >= 0.0 \
+else f-bigfloat.next_down(f)
+
+def _capture_error(precise, imprecise):
+"""Perform the legwork of calculating the difference in error of 
the high
+precision and low precision runs.  Decide whether this difference 
in error

+is within allowable tolerances.  The range of allowable tolerances is
+subjective, as ARB_shader_precision (and GLSL spec as of v4.5) gives no
+direct guidance for complex functions.  Toronto, et.  al. use 
quadrupled
+error as a limit in "Practically Accurate Floating-Point Math," 
Computing
+Now, Oct. 2014.  Also use the difference in error and the value of 
one ulp

+at the output to calculate the tolerance range in ulps for use by the
+shader test, should this vector pass the badlands check.
+"""
+
+ers = []
+bls = []
+cts = []
+with high_precision:
+error = bigfloat.abs(precise - imprecise)
+ers.append(error)
+with low_precision:
+ulpsz = _ulpsize(imprecise)
+with high_precision:
+bls.append(error > ulpsz*allowed_error_scale)
+cts.append(bigfloat.round(error/ulpsz))
+return {'errors':ers, 'badlands':bls, 'component_tolerances':cts}
+
+def _analyze_ref_fn(fn, args):
+"""Many functions contain ill-conditioned regions referred to as 
"badlands"

+(see Toronto, et. al., "Practically Accurate Floating-Point Math,"
+Computing Now, Oct. 2014).  Within these regions errors in the 
inputs are
+magnified significantly, making the function impossible to test 
with any
+reasonable accuracy.  A complex function that operates on floating 
point
+numbers has the potential to generate such error propagation even 
if the
+inputs are exact floating point numbers, since intermediate results 
can be
+generated with error.  In order to identify and avoid these areas, 
we run
+the function once at a lower precision and once at a higher 
precision, and
+compare the outputs.  Propagating errors will be greater at lower 
precision
+and less at higher precision for a given set of function inputs, 
allowing

+us to identify the badlands of the function.
+"""
+
+ret = {'errors':[], 'badlands':[], 'component_tolerances':[]}
+with high_precision:
+precise = fn(args)
+with low_precision:
+imprecise = fn(args)
+if _len_any(imprecise) == 1:
+ret = _capture_error(precise, imprecise)
+else:
+for i, arg in enumerate(imprecise):
+rettmp = _capture_error(precise[i], arg)
+ret['errors'].extend(rettmp['errors'])
+ret['badlands'].extend(rettmp['badlands'])
+ 
ret['component_tolerances'].extend(rettmp['component_tolerances'])

+return ret
+
+simple_fns = {'op-mult': 0.0,
+  'op-assign-mult': 0.0,
+  'op-div': 2.5,
+  'op-assign-div': 2.5,
+  'pow': 16.0, +  'exp': 3.0,
+  'exp2': 3.0,
+  'log': 3.0,
+  'log2': 3.0,
+  'sqrt': 3.0,
+  'inversesqrt': 2.0}
+ +complex_fns = {}
+
+componentwise_fns = ('mod', 'mix', 'smoothstep' )
+
+def _gen_tolerance(name, rettype, args):
+"""Return the tolerance that should be allowed for a function for the
+  

[Piglit] [PATCH 0/4] calculation of tolerance for complex GLSL built-ins

2015-02-18 Thread Micah Fedke
This series upgrades the ARB_shader_precision test generator with 
support for complex function tolerances.


The ARB_shader_precision spec does not give specific guidance in regard 
to acceptable error ranges for complex functions, supplying only the 
murky "Built-in functions defined in the specification with an equation 
built from the above operations inherit the above errors."  Currently, 
the shader_precision tests allow zero tolerance for complex functions, 
but this is causing many false negatives and needs to be improved.


This series is my best attempt at due diligence - any discussion/input 
is certainly welcomed :)


The theory behind this is probably worth a blog post and I won't belabor 
the point here, but this series basically implements a filter for the 
test_suite dict that will drop any test vectors that would push a 
function's output into regions of greatly multiplied error - the 
"badlands".  If the output does not trespass on the badlands, then the 
error estimate in ulps is calculated and attached to the current test 
vector for the shader_test to use as the allowable error range when 
analyzing the output generated on the GPU during test run-time.


The bigfloat package is a necessary dependency for this filter, as it 
provides both arbitrary-size floating point operations and it allows all 
floating point values and operations in a code block to be forced into a 
particular precision, leaving no question as to what precision was 
employed for an intermediate calculation.  This comes in handy when 
running the reference versions of the complex built-ins.  Performance 
impact is small (~6s for the entire gen_shader_precision_tests.py script 
on IvyBridge i7 @ 2.9GHz) and is localized to build-time.


The technique described above actually takes very little python code to 
construct - most of the complexity is in dealing with the polymorphic 
nature of the GLSL built-ins - componentwise vs. non-componentwise 
parameters, etc.



Micah Fedke (4):
arb_shader_precision: support scalar values in shader_runner_format
arb_shader_precision: add framework for calculating tolerances for 
complex functions

arb_shader_precision: add reference implementations of complex functions
arb_shader_precision: enable calculation of complex function tolerances 
 - update mako templates to support vectors of tolerances


 generated_tests/gen_shader_precision_tests.py| 376 
+-
 generated_tests/templates/gen_shader_precision_tests/fs.mako |  51 
+++--
 generated_tests/templates/gen_shader_precision_tests/gs.mako |  51 
+++--
 generated_tests/templates/gen_shader_precision_tests/vs.mako |  51 
+++--

 4 files changed, 451 insertions(+), 78 deletions(-)

--

Micah Fedke
Collabora Ltd.
+44 1223 362967
https://www.collabora.com/
https://twitter.com/collaboraltd
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH] gl-3.1: Verify error for drawing with default VAO

2015-02-18 Thread Ian Romanick
From: Ian Romanick 

This was the test case that I wrote for Mesa commit b93dcb0.

Signed-off-by: Ian Romanick 
---
This has been sitting in my piglit tree since early November 2014.  I
thought I already sent it to the list, but patchwork disagrees.

 tests/all.py|  1 +
 tests/spec/gl-3.1/CMakeLists.gl.txt |  1 +
 tests/spec/gl-3.1/default-vao.c | 56 +
 3 files changed, 58 insertions(+)
 create mode 100644 tests/spec/gl-3.1/default-vao.c

diff --git a/tests/all.py b/tests/all.py
index 35b353e..7917fd9 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -1009,6 +1009,7 @@ add_plain_test(gl30, ['sampler-cube-shadow'])
 
 gl31 = {}
 spec['!OpenGL 3.1'] = gl31
+gl31['default-vao'] = PiglitGLTest(['gl-3.1-default-vao'], run_concurrent=True)
 gl31['draw-buffers-errors'] = PiglitGLTest(['gl-3.1-draw-buffers-errors'], 
run_concurrent=True)
 gl31['genned-names'] = PiglitGLTest(['gl-3.1-genned-names'], 
run_concurrent=True)
 gl31['minmax'] = PiglitGLTest(['gl-3.1-minmax'], run_concurrent=True)
diff --git a/tests/spec/gl-3.1/CMakeLists.gl.txt 
b/tests/spec/gl-3.1/CMakeLists.gl.txt
index 2a7882d..a5f28c1 100644
--- a/tests/spec/gl-3.1/CMakeLists.gl.txt
+++ b/tests/spec/gl-3.1/CMakeLists.gl.txt
@@ -9,6 +9,7 @@ link_libraries (
${OPENGL_glu_LIBRARY}
 )
 
+piglit_add_executable (gl-3.1-default-vao default-vao.c)
 piglit_add_executable (gl-3.1-draw-buffers-errors draw-buffers-errors.c)
 piglit_add_executable (gl-3.1-genned-names genned-names.c)
 piglit_add_executable (gl-3.1-minmax minmax.c)
diff --git a/tests/spec/gl-3.1/default-vao.c b/tests/spec/gl-3.1/default-vao.c
new file mode 100644
index 000..95c7322
--- /dev/null
+++ b/tests/spec/gl-3.1/default-vao.c
@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * on 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 NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+/**
+ * @file default-vao.c
+ * Verify that GL_INVALID_OPERATION is generated when the default VAO is used
+ * for rendering.
+ */
+
+#include "piglit-util-gl.h"
+#include "piglit-matrix.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+   config.supports_gl_core_version = 31;
+   config.window_visual = PIGLIT_GL_VISUAL_RGBA | PIGLIT_GL_VISUAL_DOUBLE;
+PIGLIT_GL_TEST_CONFIG_END
+
+void
+piglit_init(int argc, char **argv)
+{
+   GLuint prog = piglit_build_simple_program(
+   "#version 130\n void main() { gl_Position = vec4(0); }",
+   "#version 130\n void main() { gl_FragColor = vec4(0); 
}");
+   glUseProgram(prog);
+
+   glDrawArrays(GL_TRIANGLES, 0, 3);
+   piglit_report_result(piglit_check_gl_error(GL_INVALID_OPERATION)
+? PIGLIT_PASS : PIGLIT_FAIL);
+}
+
+enum piglit_result
+piglit_display(void)
+{
+   /* unreached */
+   return PIGLIT_FAIL;
+}
-- 
2.1.0

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH] registry/gl.py: remove redundant if trees

2015-02-18 Thread Dylan Baker
Essentially this replace checks of
if thing_that_returns_bool:
return True
return False

with:

return thing_that_returns_bool

And throws away about 15 lines of code in doing so.

I have verified that this produces the same output as before this patch
by running
diff -NaurBw  

Signed-off-by: Dylan Baker 
---

I can't believe I didn't see what I was doing the first time
around...jeez.

 registry/gl.py | 30 +++---
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/registry/gl.py b/registry/gl.py
index 2f8cda0..075935a 100644
--- a/registry/gl.py
+++ b/registry/gl.py
@@ -562,16 +562,10 @@ class Extension(object):
 return False
 elif self.is_ratified != other.is_ratified:
 # sort ratified before unratified
-if self.is_ratified:
-return True
-else:
-return False
+return self.is_ratified
 elif (other.vendor_namespace == 'EXT') != (self.vendor_namespace == 
'EXT'):
 # Sort EXT before others
-if self.vendor_namespace == 'EXT':
-return True
-else:
-return False
+return self.vendor_namespace == 'EXT'
 return self.name < other.name
 
 def __repr__(self):
@@ -1152,35 +1146,25 @@ class Enum(object):
 
 """
 if self.num_value != other.num_value:
-if self.num_value < other.num_value:
-return True
-return False
+return self.num_value < other.num_value
 
 x = self.vendor_namespace is None
 y = other.vendor_namespace is None
 if x != y:
-if x and not y:
-return True
-return False
+return x and not y
 
 x = self.vendor_namespace in Extension.RATIFIED_NAMESPACES
 y = other.vendor_namespace in Extension.RATIFIED_NAMESPACES
 if x != y:
-if x and not y:
-return True
-return False
+return x and not y
 
 x = self.vendor_namespace == 'EXT'
 y = other.vendor_namespace == 'EXT'
 if x != y:
-if x and not y:
-return True
-return False
+return x and not y
 
 if self.name != other.name:
-if self.name < other.name:
-return True
-return False
+return self.name < other.name
 
 return self.api < other.api
 
-- 
2.3.0

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/3] piglit-util-gl: Add piglit_get_gl_enum_from_name

2015-02-18 Thread Dylan Baker
On Wed, Feb 18, 2015 at 11:34:55PM +, Emil Velikov wrote:
> On 18 February 2015 at 23:26, Ilia Mirkin  wrote:
> > On Wed, Feb 18, 2015 at 6:20 PM, Jordan Justen
> >  wrote:
> >> On 2015-02-18 14:32:29, Jordan Justen wrote:
> >>> diff --git a/tests/util/piglit-util-gl.h b/tests/util/piglit-util-gl.h
> >>> index 0f8eb81..fa4a6e4 100644
> >>> --- a/tests/util/piglit-util-gl.h
> >>> +++ b/tests/util/piglit-util-gl.h
> >>> @@ -87,6 +87,15 @@ const char* piglit_get_gl_error_name(GLenum error);
> >>>  const char *piglit_get_gl_enum_name(GLenum param);
> >>>
> >>>  /**
> >>> + * \brief Convert a string to a GL enum.
> >>> + *
> >>> + * For example, given "GL_INVALID_ENUM", return GL_INVALID_ENUM.
> >>> + *
> >>> + * abort() if the string is not recognized.
> >>
> >> Any suggestions for a better action if the string is not recognized?
> >
> > I think abort is fine here. The test is completely broken if the
> > string's not recognized.
> Fair enough. I'm not sure how our python runner interprets the abort's
> return code - pass/fail/crash. But I would assume that a warning
> message will be useful (alongside the abort()).

Does it set a non-zero returncode? If it does that will be fail (99%
sure).

We can always override that for ShaderTest (shader runner) to consider
certain return codes crash too.

> 
> Feel free to ignore that idea.
> 
> -Emil
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit


signature.asc
Description: Digital signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/3] piglit-util-gl: Add piglit_get_gl_enum_from_name

2015-02-18 Thread Dylan Baker
On Wed, Feb 18, 2015 at 02:32:29PM -0800, Jordan Justen wrote:
> Signed-off-by: Jordan Justen 
> Reviewed-by: Chad Versace 
> ---
>  tests/util/gen_dispatch.py| 21 ++-
>  tests/util/piglit-util-gl-enum-gen.c.mako | 35 
> +++
>  tests/util/piglit-util-gl.h   |  9 
>  3 files changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/util/gen_dispatch.py b/tests/util/gen_dispatch.py
> index 4d7d756..d4715e8 100644
> --- a/tests/util/gen_dispatch.py
> +++ b/tests/util/gen_dispatch.py
> @@ -132,11 +132,13 @@ class EnumCode(object):
>  def emit(cls, out_dir, gl_registry):
>  assert isinstance(gl_registry, registry.gl.Registry)
>  enums = cls.get_unique_enums_in_default_namespace(gl_registry)
> +enums_by_name = 
> cls.get_enums_by_name_in_default_namespace(gl_registry)
>  render_template(
>  cls.C_TEMPLATE,
>  out_dir,
>  gl_registry=gl_registry,
> -sorted_unique_enums_in_default_namespace=enums)
> +sorted_unique_enums_in_default_namespace=enums,
> +sorted_enums_by_name=enums_by_name)
>  
>  @classmethod
>  def get_unique_enums_in_default_namespace(cls, gl_registry):
> @@ -155,6 +157,23 @@ class EnumCode(object):
>  enums = reduce(append_enum_if_new_value, enums[1:], [enums[0]])
>  return enums
>  
> +@classmethod
> +def get_enums_by_name_in_default_namespace(cls, gl_registry):
> +def append_enum_if_new_value(enum_list, enum):
> +if enum_list[-1].name != enum.name:
> +enum_list.append(enum)
> +return enum_list
> +
> +enums = (
> +enum
> +for enum_group in gl_registry.enum_groups
> +if enum_group.type == 'default_namespace'
> +for enum in enum_group.enums
> +)

I know you just copied Chad's code, but I think that comprehensions like
this are impossible to read, and since you're calling sorted you lose
all the performance advantages of the generator anyway.

I'd prefer to see this reimplemented as a for loop, but that's just a
suggestion.

> +enums = sorted(enums, lambda a, b: cmp(a.name, b.name))


I just finished taking cmp out of this file. Do NOT use cmp it's been
deprecated since python 2.2


I *think* (I haven't tested) you should re-implement this as:
enums = sorted(enums, key=lambda x: x.name)

> +enums = reduce(append_enum_if_new_value, enums[1:], [enums[0]])
> +return enums
> +
>  
>  if __name__ == '__main__':
>  main()
> diff --git a/tests/util/piglit-util-gl-enum-gen.c.mako 
> b/tests/util/piglit-util-gl-enum-gen.c.mako
> index e22a75d..729abef 100644
> --- a/tests/util/piglit-util-gl-enum-gen.c.mako
> +++ b/tests/util/piglit-util-gl-enum-gen.c.mako
> @@ -50,4 +50,39 @@ piglit_get_prim_name(GLenum prim)
>  >---default: return "(unrecognized enum)";
>  >---}
>  }
> +
> +struct gl_name_to_enum {
> +>---const char *name;
> +>---GLenum _enum;
> +};
> +
> +static int
> +compare_enum_name(const void *a, const void *b)
> +{
> +>---return strcmp(((struct gl_name_to_enum*)a)->name,
> +>---  ((struct gl_name_to_enum*)b)->name);
> +}
> +
> +GLenum
> +piglit_get_gl_enum_from_name(const char *name)
> +{
> +>---static struct gl_name_to_enum names[] = {
> +% for enum in sorted_enums_by_name:
> +>--->---{ "${enum.name}", ${enum.c_num_literal} },
> +% endfor
> +>---};
> +>---struct gl_name_to_enum key = { name, 0 };
> +>---struct gl_name_to_enum *result;
> +
> +>---result = (struct gl_name_to_enum*)
> +>--->---bsearch(&key,
> +>--->---names, ARRAY_SIZE(names), sizeof names[0],
> +>--->---compare_enum_name);
> +
> +>---if (result == NULL)
> +>--->---abort();
> +
> +>---return result->_enum;
> +}
> +
>  \
> diff --git a/tests/util/piglit-util-gl.h b/tests/util/piglit-util-gl.h
> index 0f8eb81..fa4a6e4 100644
> --- a/tests/util/piglit-util-gl.h
> +++ b/tests/util/piglit-util-gl.h
> @@ -87,6 +87,15 @@ const char* piglit_get_gl_error_name(GLenum error);
>  const char *piglit_get_gl_enum_name(GLenum param);
>  
>  /**
> + * \brief Convert a string to a GL enum.
> + *
> + * For example, given "GL_INVALID_ENUM", return GL_INVALID_ENUM.
> + *
> + * abort() if the string is not recognized.
> + */
> +GLenum piglit_get_gl_enum_from_name(const char *name);
> +
> +/**
>   * \brief Convert a GL primitive type enum value to a string.
>   *
>   * For example, given GL_POLYGON, return "GL_POLYGON".
> -- 
> 2.1.4
> 
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit


signature.asc
Description: Digital signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pi

Re: [Piglit] [Mesa-dev] DSA for core profile only? (was Re: [PATCH 2/2] arb_direct_state_access: New test for GetCompressedTextureImage.)

2015-02-18 Thread Kenneth Graunke
On Tuesday, February 17, 2015 02:25:05 PM Ian Romanick wrote:
> (Cross posting to mesa-dev.)
> 
> On 02/17/2015 01:54 PM, Ilia Mirkin wrote:
> > On Tue, Feb 17, 2015 at 4:45 PM, Ilia Mirkin  wrote:
> >> On Tue, Feb 17, 2015 at 4:34 PM, Ian Romanick  wrote:
> >>> "Dependencies
> >>>
> >>> OpenGL 2.0 is required."
> >>>
> >>> This means Mesa will need a flag for the extension because there are
> >>> pre-2.0 drivers.  Every DSA function will need to check the flag and
> >>> possibly generate GL_INVALID_OPERATION.
> >>>
> >>> If we only support core profile (and always enable it in core profile),
> >>> the functions won't even be put in the dispatch table for non-core.  No
> >>> flag, and no checks in the functions.
> >>>
> >>> So, the $64,000.00 question is: Do we care to support DSA on the few
> >>> cards that can do 2.0 but not 3.1?
> >>
> >> Here's a view of some of the hardware that's stuck in GL2-land... not
> >> exhaustive:
> >>
> >> http://people.freedesktop.org/~imirkin/glxinfo/glxinfo.html#p=compat
> >>
> >> So that's basically pre-gen6 intel, NV4x (GeForce 6000 / 7000 series),
> >> AMD/ATI r300-r500, and the Adreno gpu's. (And presumably VC4.) Click
> >> around the diff mesa versions, I don't have every hw for every mesa
> >> version.
> >>
> >> What's the downside of enabling this in GL1.x drivers BTW? e.g. nv3x
> >> won't report GL2 (in Mesa) due to NPOT, and all the GeForce <= 4
> >> series due to lack of... features. Not that I really care either way,
> >> but just want to make sure there's a reason for it.
> 
> There are a large number of functions added for features that only exist
> in 2.0 and later (all the shader related functions, all the occlusion
> query related functions, etc.).
> 
> In addition, a bunch of the functions have slightly different behavior
> between core and compatibility profile.  For example:
> 
> void VertexArrayElementBuffer(uint vaobj, uint buffer);
> 
>  is
>   [compatibility profile:
>zero, indicating the default vertex array object, or]
> the name of the vertex array object, and  is zero or the name of
> the buffer object. If  is zero, any existing element array
> buffer binding to  is removed.
> Errors
> 
> An INVALID_OPERATION error is generated by VertexArrayElementBuffer if
>  is not
>   [compatibility profile: zero or]
> the name of an existing vertex array object.
> 
> Not only would we have to implement the different compatibility
> behavior, but we'd also have to implement tests for it.
> 
> > Oh, and an additional thought, if we make it core profile-only, then
> > it will not be available in compat profile on any hardware, even the
> > latest. I think people are far from done using compat contexts, and
> > this would unnecessarily restrict the software that can make use of
> > this extension.
> 
> Generally "newer" applications already require some 3.1+ feature, so
> they're either running on core profile or not running on Mesa already.
> The last time I surveyed applications on Steam, which I realize is very
> specific subset of applications, there were basically two kinds of
> applications.
> 
> 1. Games that were basically OpenGL 1.x or 2.x with a tiny set of extra
> features that were absolutely necessary for some effect.  These games
> use no "convenience" features... even ones that would improve performance.
> 
> 2. Games by vendors that are really mad that we don't already support
> 4.5.  They use everything modern, and they never look back.
> 
> The folks in group 1 aren't going to use DSA, and the folks in group 2
> mourned the loss of compatibility profile ages ago.
> 
> So... is there are 3rd group, and do we care enough to do the work
> necessary to support them?

I'm inclined to simply support it for core profile, at least until an
application developer requests compatibility support.  It's part of
OpenGL 4.5 - I suspect people using legacy GL aren't going to want to
use features that are so new they've barely even shipped on the major
proprietary drivers.

--Ken


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


Re: [Piglit] [PATCH] util: migrate the SUBTEST* macros to piglit-util.sh

2015-02-18 Thread Dylan Baker
There's a typo in your subject, it should be .h not .sh

On Wed, Feb 18, 2015 at 04:15:27PM +0200, Martin Peres wrote:
> Laura requested me to move these macros to piglit-util. However, this is
> not the first time such a macro has been written. For example, curro
> introduced subtest() in /tests/spec/arb_shader_image_load_store/common.h.
> He said he also used another macro for another test.
> 
> Not sure if my macros are generic-enough though. They however can make a
> good base for implementing most tests. They were definitely sufficient to
> implement all my DSA-related tests.
> 
> Signed-off-by: Martin Peres 
> ---
>  tests/spec/arb_direct_state_access/dsa-utils.h | 16 -
>  tests/util/piglit-util.h   | 32 
> ++
>  2 files changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/spec/arb_direct_state_access/dsa-utils.h 
> b/tests/spec/arb_direct_state_access/dsa-utils.h
> index 25b5524..a2f70b2 100644
> --- a/tests/spec/arb_direct_state_access/dsa-utils.h
> +++ b/tests/spec/arb_direct_state_access/dsa-utils.h
> @@ -39,22 +39,6 @@ extern "C" {
>  
>  #include "piglit-util-gl.h"
>  
> -#define SUBTEST(error, global, format, args...) \
> -do { \
> - bool local = piglit_check_gl_error((error)); \
> - global = global && local; \
> - piglit_report_subtest_result(local ? PIGLIT_PASS : PIGLIT_FAIL, \
> -  (format), ##args); \
> -} while (0)
> -
> -#define SUBTESTCONDITION(condition, global, format, args...) \
> -do { \
> - bool cond = (condition); \
> - global = global && cond; \
> - piglit_report_subtest_result(cond ? PIGLIT_PASS : PIGLIT_FAIL, \
> -  (format), ##args); \
> -} while (0)
> -
>  void dsa_init_program(void);
>  
>  void dsa_texture_with_unit(GLuint);
> diff --git a/tests/util/piglit-util.h b/tests/util/piglit-util.h
> index 1c522a1..9a53964 100755
> --- a/tests/util/piglit-util.h
> +++ b/tests/util/piglit-util.h
> @@ -343,6 +343,38 @@ piglit_parse_subtest_args(int *argc, char *argv[],
>  uint64_t
>  piglit_gettid(void);
>  
> +/**
> + * \brief Run a subtest checking the error code of OpenGL
> + *
> + * This macro checks if the current gl error is \a error and reports the 
> result
> + * using  piglit_report_subtest_result, using the test name \a format. 
> Finally,
> + * it updates the \a global variable that makes sure that the reported state 
> of
> + * the test as a whole is false if any of the subtest failed.
> + */
> +#define SUBTEST(error, global, format, args...) \
> +do { \
> + bool local = piglit_check_gl_error((error)); \
> + global = global && local; \
> + piglit_report_subtest_result(local ? PIGLIT_PASS : PIGLIT_FAIL, \
> +  (format), ##args); \
> +} while (0)
> +
> +/**
> + * \brief Run a subtest checking a condition
> + *
> + * This macro checks if the \a condition is true and reports the result using
> + * piglit_report_subtest_result, using the test name \a format. Finally, it
> + * updates the \a global variable that makes sure that the reported state of
> + * the test as a whole is false if any of the subtest failed.
> + */
> +#define SUBTESTCONDITION(condition, global, format, args...) \
> +do { \
> + bool cond = (condition); \
> + global = global && cond; \
> + piglit_report_subtest_result(cond ? PIGLIT_PASS : PIGLIT_FAIL, \
> +  (format), ##args); \
> +} while (0)
> +
>  #ifdef __cplusplus
>  } /* end extern "C" */
>  #endif
> -- 
> 2.3.0
> 
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit


signature.asc
Description: Digital signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/3] piglit-util-gl: Add piglit_get_gl_enum_from_name

2015-02-18 Thread Ilia Mirkin
On Wed, Feb 18, 2015 at 6:34 PM, Emil Velikov  wrote:
> On 18 February 2015 at 23:26, Ilia Mirkin  wrote:
>> On Wed, Feb 18, 2015 at 6:20 PM, Jordan Justen
>>  wrote:
>>> On 2015-02-18 14:32:29, Jordan Justen wrote:
 diff --git a/tests/util/piglit-util-gl.h b/tests/util/piglit-util-gl.h
 index 0f8eb81..fa4a6e4 100644
 --- a/tests/util/piglit-util-gl.h
 +++ b/tests/util/piglit-util-gl.h
 @@ -87,6 +87,15 @@ const char* piglit_get_gl_error_name(GLenum error);
  const char *piglit_get_gl_enum_name(GLenum param);

  /**
 + * \brief Convert a string to a GL enum.
 + *
 + * For example, given "GL_INVALID_ENUM", return GL_INVALID_ENUM.
 + *
 + * abort() if the string is not recognized.
>>>
>>> Any suggestions for a better action if the string is not recognized?
>>
>> I think abort is fine here. The test is completely broken if the
>> string's not recognized.
> Fair enough. I'm not sure how our python runner interprets the abort's
> return code - pass/fail/crash. But I would assume that a warning
> message will be useful (alongside the abort()).

crash, I assume. Message sounds good.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/3] piglit-util-gl: Add piglit_get_gl_enum_from_name

2015-02-18 Thread Emil Velikov
On 18 February 2015 at 23:26, Ilia Mirkin  wrote:
> On Wed, Feb 18, 2015 at 6:20 PM, Jordan Justen
>  wrote:
>> On 2015-02-18 14:32:29, Jordan Justen wrote:
>>> diff --git a/tests/util/piglit-util-gl.h b/tests/util/piglit-util-gl.h
>>> index 0f8eb81..fa4a6e4 100644
>>> --- a/tests/util/piglit-util-gl.h
>>> +++ b/tests/util/piglit-util-gl.h
>>> @@ -87,6 +87,15 @@ const char* piglit_get_gl_error_name(GLenum error);
>>>  const char *piglit_get_gl_enum_name(GLenum param);
>>>
>>>  /**
>>> + * \brief Convert a string to a GL enum.
>>> + *
>>> + * For example, given "GL_INVALID_ENUM", return GL_INVALID_ENUM.
>>> + *
>>> + * abort() if the string is not recognized.
>>
>> Any suggestions for a better action if the string is not recognized?
>
> I think abort is fine here. The test is completely broken if the
> string's not recognized.
Fair enough. I'm not sure how our python runner interprets the abort's
return code - pass/fail/crash. But I would assume that a warning
message will be useful (alongside the abort()).

Feel free to ignore that idea.

-Emil
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 3/3] ARB_shader_atomic_counters: Add simple VS test with inc/dec/read

2015-02-18 Thread Ilia Mirkin
Reviewed-by: Ilia Mirkin 

On Wed, Feb 18, 2015 at 5:32 PM, Jordan Justen
 wrote:
> Simple test of atomicCounterIncrement, atomicCounterDecrement and
> atomicCounter being used in the VS.
>
> v2:
>  * Drop CORE requirement
>  * Add GL_ARB_shader_atomic_counters requirement
>  * Require INT GL_MAX_VERTEX_ATOMIC_COUNTER_BUFFERS >= 2
>
> Signed-off-by: Jordan Justen 
> Cc: Ilia Mirkin 
> ---
>  tests/all.py   |  3 +
>  .../execution/vs-simple-inc-dec-read.shader_test   | 71 
> ++
>  2 files changed, 74 insertions(+)
>  create mode 100644 
> tests/spec/arb_shader_atomic_counters/execution/vs-simple-inc-dec-read.shader_test
>
> diff --git a/tests/all.py b/tests/all.py
> index 949b023..b2499d1 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -4392,6 +4392,9 @@ spec['ARB_shader_atomic_counters'] = 
> arb_shader_atomic_counters
>  import_glsl_parser_tests(spec['ARB_shader_atomic_counters'],
>   os.path.join(TESTS_DIR, 'spec', 
> 'arb_shader_atomic_counters'),
>   [''])
> +add_shader_test_dir(spec['ARB_shader_atomic_counters'],
> +os.path.join(TESTS_DIR, 'spec', 
> 'arb_shader_atomic_counters'),
> +recursive=True)
>  arb_shader_atomic_counters['active-counters'] = 
> PiglitGLTest(['arb_shader_atomic_counters-active-counters'], 
> run_concurrent=True)
>  arb_shader_atomic_counters['array-indexing'] = 
> PiglitGLTest(['arb_shader_atomic_counters-array-indexing'], 
> run_concurrent=True)
>  arb_shader_atomic_counters['buffer-binding'] = 
> PiglitGLTest(['arb_shader_atomic_counters-buffer-binding'], 
> run_concurrent=True)
> diff --git 
> a/tests/spec/arb_shader_atomic_counters/execution/vs-simple-inc-dec-read.shader_test
>  
> b/tests/spec/arb_shader_atomic_counters/execution/vs-simple-inc-dec-read.shader_test
> new file mode 100644
> index 000..16ea9db
> --- /dev/null
> +++ 
> b/tests/spec/arb_shader_atomic_counters/execution/vs-simple-inc-dec-read.shader_test
> @@ -0,0 +1,71 @@
> +# Simple test of atomicCounterIncrement, atomicCounterDecrement and
> +# atomicCounter being used in the VS.
> +
> +[require]
> +GLSL >= 1.40
> +GL_ARB_shader_atomic_counters
> +INT GL_MAX_VERTEX_ATOMIC_COUNTER_BUFFERS >= 2
> +
> +[vertex shader]
> +#version 140
> +#extension GL_ARB_shader_atomic_counters: require
> +
> +layout(binding = 0) uniform atomic_uint a0;
> +layout(binding = 0) uniform atomic_uint a1;
> +
> +in vec4 piglit_vertex;
> +out vec4 vcolor;
> +
> +void main()
> +{
> +   bool passed = true;
> +   uint v0, v1;
> +
> +   /* Test that incrementing, followed by a read of an atomic
> +* counter results in a larger value.
> +*
> +* Note: atomicCounterIncrement return the old value
> +*/
> +   v0 = atomicCounterIncrement(a0);
> +   v1 = atomicCounter(a0);
> +   if (v1 <= v0)
> +   passed = false;
> +
> +   /* Skip one decrement since it may be the 0 => 0x
> +* transition.
> +*/
> +   atomicCounterDecrement(a1);
> +
> +   /* Test that a read, followed by a decrement of an atomic
> +* counter results in a smaller value.
> +*
> +* Note: atomicCounterDecrement return the new value
> +*/
> +   v0 = atomicCounter(a1);
> +   v1 = atomicCounterDecrement(a1);
> +   if (v1 >= v0)
> +   passed = false;
> +
> +   if (passed)
> +   vcolor = vec4(0.0, 1.0, 0.0, 1.0);
> +   else
> +   vcolor = vec4(1.0, 0.0, 0.0, 1.0);
> +
> +   gl_Position = piglit_vertex;
> +}
> +
> +[fragment shader]
> +#version 140
> +in vec4 vcolor;
> +out vec4 fcolor;
> +
> +void main()
> +{
> +   fcolor = vcolor;
> +}
> +
> +[test]
> +atomic counters 2
> +
> +draw rect -1 -1 2 2
> +probe all rgba 0.0 1.0 0.0 1.0
> --
> 2.1.4
>
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/3] piglit-util-gl: Add piglit_get_gl_enum_from_name

2015-02-18 Thread Ilia Mirkin
On Wed, Feb 18, 2015 at 6:20 PM, Jordan Justen
 wrote:
> On 2015-02-18 14:32:29, Jordan Justen wrote:
>> diff --git a/tests/util/piglit-util-gl.h b/tests/util/piglit-util-gl.h
>> index 0f8eb81..fa4a6e4 100644
>> --- a/tests/util/piglit-util-gl.h
>> +++ b/tests/util/piglit-util-gl.h
>> @@ -87,6 +87,15 @@ const char* piglit_get_gl_error_name(GLenum error);
>>  const char *piglit_get_gl_enum_name(GLenum param);
>>
>>  /**
>> + * \brief Convert a string to a GL enum.
>> + *
>> + * For example, given "GL_INVALID_ENUM", return GL_INVALID_ENUM.
>> + *
>> + * abort() if the string is not recognized.
>
> Any suggestions for a better action if the string is not recognized?

I think abort is fine here. The test is completely broken if the
string's not recognized.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/3] piglit-util-gl: Add piglit_get_gl_enum_from_name

2015-02-18 Thread Jordan Justen
On 2015-02-18 14:32:29, Jordan Justen wrote:
> diff --git a/tests/util/piglit-util-gl.h b/tests/util/piglit-util-gl.h
> index 0f8eb81..fa4a6e4 100644
> --- a/tests/util/piglit-util-gl.h
> +++ b/tests/util/piglit-util-gl.h
> @@ -87,6 +87,15 @@ const char* piglit_get_gl_error_name(GLenum error);
>  const char *piglit_get_gl_enum_name(GLenum param);
>  
>  /**
> + * \brief Convert a string to a GL enum.
> + *
> + * For example, given "GL_INVALID_ENUM", return GL_INVALID_ENUM.
> + *
> + * abort() if the string is not recognized.

Any suggestions for a better action if the string is not recognized?

I just saw that Emil had the same question. :)

-Jordan

> + */
> +GLenum piglit_get_gl_enum_from_name(const char *name);
> +
> +/**
>   * \brief Convert a GL primitive type enum value to a string.
>   *
>   * For example, given GL_POLYGON, return "GL_POLYGON".
> -- 
> 2.1.4
> 
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/3] piglit-util-gl: Add piglit_get_gl_enum_from_name

2015-02-18 Thread Emil Velikov
On 18 February 2015 at 22:32, Jordan Justen  wrote:
> Signed-off-by: Jordan Justen 
> Reviewed-by: Chad Versace 
> ---
>  tests/util/gen_dispatch.py| 21 ++-
>  tests/util/piglit-util-gl-enum-gen.c.mako | 35 
> +++
>  tests/util/piglit-util-gl.h   |  9 
>  3 files changed, 64 insertions(+), 1 deletion(-)
...
> diff --git a/tests/util/piglit-util-gl-enum-gen.c.mako 
> b/tests/util/piglit-util-gl-enum-gen.c.mako
> index e22a75d..729abef 100644
> --- a/tests/util/piglit-util-gl-enum-gen.c.mako
> +++ b/tests/util/piglit-util-gl-enum-gen.c.mako
> @@ -50,4 +50,39 @@ piglit_get_prim_name(GLenum prim)
>  >---default: return "(unrecognized enum)";
>  >---}
>  }
> +
> +struct gl_name_to_enum {
> +>---const char *name;
> +>---GLenum _enum;
> +};
> +
> +static int
> +compare_enum_name(const void *a, const void *b)
> +{
> +>---return strcmp(((struct gl_name_to_enum*)a)->name,
> +>---  ((struct gl_name_to_enum*)b)->name);
> +}
> +
> +GLenum
> +piglit_get_gl_enum_from_name(const char *name)
> +{
> +>---static struct gl_name_to_enum names[] = {
Depending on how friendly the compiler is one might want to make this
"static const" and move the table outside of the function.

> +% for enum in sorted_enums_by_name:
> +>--->---{ "${enum.name}", ${enum.c_num_literal} },
> +% endfor
> +>---};
> +>---struct gl_name_to_enum key = { name, 0 };
> +>---struct gl_name_to_enum *result;
> +
> +>---result = (struct gl_name_to_enum*)
> +>--->---bsearch(&key,
> +>--->---names, ARRAY_SIZE(names), sizeof names[0],
> +>--->---compare_enum_name);
> +
> +>---if (result == NULL)
> +>--->---abort();
Is it me or does this look a bit excessive - none of the other util_gl
functions go that far. How about just printing a catchy error message
:-P

Cheers,
Emil
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 2/3] shader_runner: Add INT support in require section

2015-02-18 Thread Ilia Mirkin
On Wed, Feb 18, 2015 at 5:32 PM, Jordan Justen
 wrote:
> This allows a shader_test to require something like:
> INT GL_MAX_VERTEX_ATOMIC_COUNTER_BUFFERS >= 2
>
> Signed-off-by: Jordan Justen 
> ---
>  tests/shaders/shader_runner.c | 31 ++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> index c193de9..e9a56ff 100644
> --- a/tests/shaders/shader_runner.c
> +++ b/tests/shaders/shader_runner.c
> @@ -566,7 +566,36 @@ process_requirement(const char *line)
>  * can also require that a particular extension not be supported by
>  * prepending ! to the extension name.
>  */
> -   if (string_match("GL_MAX_FRAGMENT_UNIFORM_COMPONENTS", line)) {
> +   if (string_match("INT ", line)) {
> +   enum comparison cmp;
> +   const char *enum_name = eat_whitespace(line+3);
> +   const char *int_string;
> +   int comparison_value, gl_int_value;
> +   GLenum int_enum;
> +
> +   strcpy_to_space(buffer, enum_name);
> +
> +   int_enum = piglit_get_gl_enum_from_name(buffer);
> +
> +   int_string = process_comparison(eat_whitespace(enum_name + 
> strlen(buffer)), &cmp);
> +   comparison_value = atoi(int_string);
> +
> +   glGetIntegerv(int_enum, &gl_int_value);
> +   if (!piglit_check_gl_error(GL_NO_ERROR)) {
> +   fprintf(stderr, "Error reading %s\n", buffer);
> +   piglit_report_result(PIGLIT_FAIL);

Hmmm... the theory being that you should write the shader test s.t.
the extension that enables the enum is written beforehand, and so if
the enum's not there at all, that's a fail? I _guess_... Perhaps
others have thoughts on this?

> +   }
> +
> +   if (!compare(comparison_value, gl_int_value, cmp)) {
> +   printf("Test requires %s %s %i.  "
> +  "The driver supports %i.\n",
> +  buffer,
> +  comparison_string(cmp),
> +  comparison_value,
> +  gl_int_value);
> +   piglit_report_result(PIGLIT_SKIP);
> +   }
> +   } else if (string_match("GL_MAX_FRAGMENT_UNIFORM_COMPONENTS", line)) {
> enum comparison cmp;
> int maxcomp;
>
> --
> 2.1.4
>
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH 3/3] ARB_shader_atomic_counters: Add simple VS test with inc/dec/read

2015-02-18 Thread Jordan Justen
Simple test of atomicCounterIncrement, atomicCounterDecrement and
atomicCounter being used in the VS.

v2:
 * Drop CORE requirement
 * Add GL_ARB_shader_atomic_counters requirement
 * Require INT GL_MAX_VERTEX_ATOMIC_COUNTER_BUFFERS >= 2

Signed-off-by: Jordan Justen 
Cc: Ilia Mirkin 
---
 tests/all.py   |  3 +
 .../execution/vs-simple-inc-dec-read.shader_test   | 71 ++
 2 files changed, 74 insertions(+)
 create mode 100644 
tests/spec/arb_shader_atomic_counters/execution/vs-simple-inc-dec-read.shader_test

diff --git a/tests/all.py b/tests/all.py
index 949b023..b2499d1 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -4392,6 +4392,9 @@ spec['ARB_shader_atomic_counters'] = 
arb_shader_atomic_counters
 import_glsl_parser_tests(spec['ARB_shader_atomic_counters'],
  os.path.join(TESTS_DIR, 'spec', 
'arb_shader_atomic_counters'),
  [''])
+add_shader_test_dir(spec['ARB_shader_atomic_counters'],
+os.path.join(TESTS_DIR, 'spec', 
'arb_shader_atomic_counters'),
+recursive=True)
 arb_shader_atomic_counters['active-counters'] = 
PiglitGLTest(['arb_shader_atomic_counters-active-counters'], 
run_concurrent=True)
 arb_shader_atomic_counters['array-indexing'] = 
PiglitGLTest(['arb_shader_atomic_counters-array-indexing'], run_concurrent=True)
 arb_shader_atomic_counters['buffer-binding'] = 
PiglitGLTest(['arb_shader_atomic_counters-buffer-binding'], run_concurrent=True)
diff --git 
a/tests/spec/arb_shader_atomic_counters/execution/vs-simple-inc-dec-read.shader_test
 
b/tests/spec/arb_shader_atomic_counters/execution/vs-simple-inc-dec-read.shader_test
new file mode 100644
index 000..16ea9db
--- /dev/null
+++ 
b/tests/spec/arb_shader_atomic_counters/execution/vs-simple-inc-dec-read.shader_test
@@ -0,0 +1,71 @@
+# Simple test of atomicCounterIncrement, atomicCounterDecrement and
+# atomicCounter being used in the VS.
+
+[require]
+GLSL >= 1.40
+GL_ARB_shader_atomic_counters
+INT GL_MAX_VERTEX_ATOMIC_COUNTER_BUFFERS >= 2
+
+[vertex shader]
+#version 140
+#extension GL_ARB_shader_atomic_counters: require
+
+layout(binding = 0) uniform atomic_uint a0;
+layout(binding = 0) uniform atomic_uint a1;
+
+in vec4 piglit_vertex;
+out vec4 vcolor;
+
+void main()
+{
+   bool passed = true;
+   uint v0, v1;
+
+   /* Test that incrementing, followed by a read of an atomic
+* counter results in a larger value.
+*
+* Note: atomicCounterIncrement return the old value
+*/
+   v0 = atomicCounterIncrement(a0);
+   v1 = atomicCounter(a0);
+   if (v1 <= v0)
+   passed = false;
+
+   /* Skip one decrement since it may be the 0 => 0x
+* transition.
+*/
+   atomicCounterDecrement(a1);
+
+   /* Test that a read, followed by a decrement of an atomic
+* counter results in a smaller value.
+*
+* Note: atomicCounterDecrement return the new value
+*/
+   v0 = atomicCounter(a1);
+   v1 = atomicCounterDecrement(a1);
+   if (v1 >= v0)
+   passed = false;
+
+   if (passed)
+   vcolor = vec4(0.0, 1.0, 0.0, 1.0);
+   else
+   vcolor = vec4(1.0, 0.0, 0.0, 1.0);
+
+   gl_Position = piglit_vertex;
+}
+
+[fragment shader]
+#version 140
+in vec4 vcolor;
+out vec4 fcolor;
+
+void main()
+{
+   fcolor = vcolor;
+}
+
+[test]
+atomic counters 2
+
+draw rect -1 -1 2 2
+probe all rgba 0.0 1.0 0.0 1.0
-- 
2.1.4

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH 1/3] piglit-util-gl: Add piglit_get_gl_enum_from_name

2015-02-18 Thread Jordan Justen
Signed-off-by: Jordan Justen 
Reviewed-by: Chad Versace 
---
 tests/util/gen_dispatch.py| 21 ++-
 tests/util/piglit-util-gl-enum-gen.c.mako | 35 +++
 tests/util/piglit-util-gl.h   |  9 
 3 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/tests/util/gen_dispatch.py b/tests/util/gen_dispatch.py
index 4d7d756..d4715e8 100644
--- a/tests/util/gen_dispatch.py
+++ b/tests/util/gen_dispatch.py
@@ -132,11 +132,13 @@ class EnumCode(object):
 def emit(cls, out_dir, gl_registry):
 assert isinstance(gl_registry, registry.gl.Registry)
 enums = cls.get_unique_enums_in_default_namespace(gl_registry)
+enums_by_name = cls.get_enums_by_name_in_default_namespace(gl_registry)
 render_template(
 cls.C_TEMPLATE,
 out_dir,
 gl_registry=gl_registry,
-sorted_unique_enums_in_default_namespace=enums)
+sorted_unique_enums_in_default_namespace=enums,
+sorted_enums_by_name=enums_by_name)
 
 @classmethod
 def get_unique_enums_in_default_namespace(cls, gl_registry):
@@ -155,6 +157,23 @@ class EnumCode(object):
 enums = reduce(append_enum_if_new_value, enums[1:], [enums[0]])
 return enums
 
+@classmethod
+def get_enums_by_name_in_default_namespace(cls, gl_registry):
+def append_enum_if_new_value(enum_list, enum):
+if enum_list[-1].name != enum.name:
+enum_list.append(enum)
+return enum_list
+
+enums = (
+enum
+for enum_group in gl_registry.enum_groups
+if enum_group.type == 'default_namespace'
+for enum in enum_group.enums
+)
+enums = sorted(enums, lambda a, b: cmp(a.name, b.name))
+enums = reduce(append_enum_if_new_value, enums[1:], [enums[0]])
+return enums
+
 
 if __name__ == '__main__':
 main()
diff --git a/tests/util/piglit-util-gl-enum-gen.c.mako 
b/tests/util/piglit-util-gl-enum-gen.c.mako
index e22a75d..729abef 100644
--- a/tests/util/piglit-util-gl-enum-gen.c.mako
+++ b/tests/util/piglit-util-gl-enum-gen.c.mako
@@ -50,4 +50,39 @@ piglit_get_prim_name(GLenum prim)
 >---default: return "(unrecognized enum)";
 >---}
 }
+
+struct gl_name_to_enum {
+>---const char *name;
+>---GLenum _enum;
+};
+
+static int
+compare_enum_name(const void *a, const void *b)
+{
+>---return strcmp(((struct gl_name_to_enum*)a)->name,
+>---  ((struct gl_name_to_enum*)b)->name);
+}
+
+GLenum
+piglit_get_gl_enum_from_name(const char *name)
+{
+>---static struct gl_name_to_enum names[] = {
+% for enum in sorted_enums_by_name:
+>--->---{ "${enum.name}", ${enum.c_num_literal} },
+% endfor
+>---};
+>---struct gl_name_to_enum key = { name, 0 };
+>---struct gl_name_to_enum *result;
+
+>---result = (struct gl_name_to_enum*)
+>--->---bsearch(&key,
+>--->---names, ARRAY_SIZE(names), sizeof names[0],
+>--->---compare_enum_name);
+
+>---if (result == NULL)
+>--->---abort();
+
+>---return result->_enum;
+}
+
 \
diff --git a/tests/util/piglit-util-gl.h b/tests/util/piglit-util-gl.h
index 0f8eb81..fa4a6e4 100644
--- a/tests/util/piglit-util-gl.h
+++ b/tests/util/piglit-util-gl.h
@@ -87,6 +87,15 @@ const char* piglit_get_gl_error_name(GLenum error);
 const char *piglit_get_gl_enum_name(GLenum param);
 
 /**
+ * \brief Convert a string to a GL enum.
+ *
+ * For example, given "GL_INVALID_ENUM", return GL_INVALID_ENUM.
+ *
+ * abort() if the string is not recognized.
+ */
+GLenum piglit_get_gl_enum_from_name(const char *name);
+
+/**
  * \brief Convert a GL primitive type enum value to a string.
  *
  * For example, given GL_POLYGON, return "GL_POLYGON".
-- 
2.1.4

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH 2/3] shader_runner: Add INT support in require section

2015-02-18 Thread Jordan Justen
This allows a shader_test to require something like:
INT GL_MAX_VERTEX_ATOMIC_COUNTER_BUFFERS >= 2

Signed-off-by: Jordan Justen 
---
 tests/shaders/shader_runner.c | 31 ++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
index c193de9..e9a56ff 100644
--- a/tests/shaders/shader_runner.c
+++ b/tests/shaders/shader_runner.c
@@ -566,7 +566,36 @@ process_requirement(const char *line)
 * can also require that a particular extension not be supported by
 * prepending ! to the extension name.
 */
-   if (string_match("GL_MAX_FRAGMENT_UNIFORM_COMPONENTS", line)) {
+   if (string_match("INT ", line)) {
+   enum comparison cmp;
+   const char *enum_name = eat_whitespace(line+3);
+   const char *int_string;
+   int comparison_value, gl_int_value;
+   GLenum int_enum;
+
+   strcpy_to_space(buffer, enum_name);
+
+   int_enum = piglit_get_gl_enum_from_name(buffer);
+
+   int_string = process_comparison(eat_whitespace(enum_name + 
strlen(buffer)), &cmp);
+   comparison_value = atoi(int_string);
+
+   glGetIntegerv(int_enum, &gl_int_value);
+   if (!piglit_check_gl_error(GL_NO_ERROR)) {
+   fprintf(stderr, "Error reading %s\n", buffer);
+   piglit_report_result(PIGLIT_FAIL);
+   }
+
+   if (!compare(comparison_value, gl_int_value, cmp)) {
+   printf("Test requires %s %s %i.  "
+  "The driver supports %i.\n",
+  buffer,
+  comparison_string(cmp),
+  comparison_value,
+  gl_int_value);
+   piglit_report_result(PIGLIT_SKIP);
+   }
+   } else if (string_match("GL_MAX_FRAGMENT_UNIFORM_COMPONENTS", line)) {
enum comparison cmp;
int maxcomp;
 
-- 
2.1.4

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [Mesa-dev] [PATCH] arb_occlusion_query2: expect an error when target mismatch in glBeginQuery()

2015-02-18 Thread Mark Janes
Hi Eduardo,

I track piglit regressions for the mesa team at Intel.  It would really
help me if you put a note in your commit message when a new test is
known to fail on any platform.

thanks,

Mark

Eduardo Lima Mitev  writes:

> As a heads-up, with this patch piglit fails the test for current Mesa.
> But I'm about to send another series of patches for dEQP tests which
> include a fix in Mesa for this issue.
>
> The piglit test in question is:
>
> bin/arb_occlusion_query2-api -auto -fbo
>
> Eduardo
>
> On 02/10/2015 08:48 AM, Eduardo Lima Mitev wrote:
>> From the OpenGL ES 3.0.0 spec, section "2.13. ASYNCHRONOUS QUERIES",
>> page 82:
>>
>> "BeginQuery generates an INVALID_OPERATION error if any of the
>>  following conditions hold: [...]; id is the name of an existing
>>  query object whose type does not match target; [...]
>>
>> OpenGL 3.3 spec has similar wording at section "2.14. ASYNCHRONOUS
>> QUERIES", page 94.
>>
>> Hence, trying to call BeginQuery on a query object which has already
>> been bound to a different target should return GL_INVALID_OPERATION.
>> ---
>>  tests/spec/arb_occlusion_query2/api.c | 49
> +++
>>  1 file changed, 49 insertions(+)
>>
>
> ___
> mesa-dev mailing list
> mesa-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 04/15] arb_direct_state_access: Testing NamedBufferData.

2015-02-18 Thread Ilia Mirkin
On Wed, Feb 18, 2015 at 9:59 AM, Martin Peres
 wrote:
[...]
> and you only test the differences you introduced in mesa but is this really
> how
> we are supposed to write the tests?
>
> I really don't know, hence why I am asking.

In case there's any doubt, no, you're supposed to write self-contained
tests that don't in any way rely on mesa impl details, or what other
tests are out there for unrelated features. [And I say this without
making any sort of judgement on the actual test being discussed.]
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] util: migrate the SUBTEST* macros to piglit-util.sh

2015-02-18 Thread Jose Fonseca

On 18/02/15 16:21, Martin Peres wrote:

Hey Jose,

On 18/02/15 17:10, Jose Fonseca wrote:

FYI, you'll need to rebase on top of
https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_piglit_commit_-3Fid-3D83bc6862386b2d465879bcd372d61ec754534970&d=AwICaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=sVUvrnV7t1PNeIQxUMSOqpqrBVurla-YZBTVE6zrfak&s=Ab2Wxn8llpFWmmedbauWhqjyxSffQWMDqv8amVB9WcI&e=
, so that we use the standard C99 syntax for variadic macros.  You
might need to manually update the moved code, as git might just mark
it as a conflict.


Sorry for breaking the windows build...  No probs, I'll rebase it.


No prob. Thanks.






Concerning the actual change, just a general remark:  these macros
save typing but they make the test code harder to understand


I think there are other approaches that achieve the same amount of
type-saving, without compromising readbility.

For example, we could have a


  // global contaning the current substest result
  // TODO: Make it a enum piglit_result
  bool current_subtest_result;
  char current_subtest_message[4096];

  piglit_subtest_begin() {
current_subtest_result = true;
current_subtest_message[0] = 0;
  }

  void
  piglit_subtest_check(bool condition, format, ...) {
bool pass = piglit_check_gl_error((error));
if (!pass) {
// FIXME: append format+va_args to current_subtest_message.
}
current_subtest_result = current_subtest_result && pass;
  }

  void
  piglit_subtest_check_gl_error(error) {
 ...
  }

 piglit_subtest_end()
   piglit_report_subtest_result(current_subtest_result ? PIGLIT_PASS :
PIGLIT_FAIL, current_subtest_message);
  }


And the test would do

piglit_subtest_begin();
piglit_subtest_check_gl_error(error, etc);
piglit_subtest_check_gl_error(error, etc);
piglit_subtest_check(cond, etc);
piglit_subtest_end();


Not sure this form saves anything since one sub-test would be one line
in my case instead of 3 in this case.
Have a look at
https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_-7Emperes_piglit_commit_-3Fh-3Ddsa-26id-3Dc08558659f7967410ab68f39c3d768e41e35bc6d&d=AwICaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=sVUvrnV7t1PNeIQxUMSOqpqrBVurla-YZBTVE6zrfak&s=eN7ni-8KESSon5ovNFgLvaqSxs_IHB9Wp20IEwtz5YU&e=


Right. I did misunderstood how this was supposed to be used.  I saw 
piglit_report_subtest_result callers that had many partial sub-conditions.


But I now see that there's the test pass/fail, and then the subtest 
pass/fail, and each can be aggregated of multiple sub-conditions, and 
test writers usually need to do "foo = foo && condition" at both levels. 
 You macros target the case where each subtest is one condition/error 
exactly.




What you proposed is a good idea for the whole test though.
piglit_test_begin() would set current_test_result
to keep track of the state and the other functions would follow the
state.


Right.


> I'm not a fan of the global variable

too as what will happen if we have interleaved tests?


True.  But maybe the benefit of not having to keep track of overall 
pass/fail would trump over the inability to write interleaved tests.



In any case, I'm sure some tests will not work with this model. So,
instead of sharing those function and making them
very confusing, I would argue that outside of trivial cases (like the
macros I proposed), you should re-write your own
way of checking. As for the "confusing" argument, not sure how much
simpler can this code be (*no side effect*). It is
also less prone to copy/paste errors.

In my opinion, we won't reach an agreement that would work in every case
or, if we do, people will not use the code
added because it will be too complex. Instead, let's just let devs
handle their test code as it pleases them. If there are
some trivial cases common-enough, then we can add them to
piglit-util.sh. I guess the question now is, are those
common-enough outside of DSA?


Devs are certainly free to stick with their style.  And I'm not active 
enough in piglit to oppose anything.



So I just wondered if it wouldn't be worthwhile to devise some scheme 
that would completely eliminate the need for test writers to keep track 
of these aggregated pass/fail variables.


Jose
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH] drawpixels-color-index: new test for glDrawPixels(GL_COLOR_INDEX)

2015-02-18 Thread Brian Paul
To exercise a Mesa failure after the format overhaul.
Note: this test fails on nvidia if the -auto option is given.
---
 tests/all.py   |   1 +
 tests/spec/gl-1.0/CMakeLists.gl.txt|   5 +-
 tests/spec/gl-1.0/drawpixels-color-index.c | 162 +
 3 files changed, 166 insertions(+), 2 deletions(-)
 create mode 100644 tests/spec/gl-1.0/drawpixels-color-index.c

diff --git a/tests/all.py b/tests/all.py
index d2ae5ea..95cd09a 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -848,6 +848,7 @@ spec['!OpenGL 1.0'] = gl10
 add_concurrent_test(gl10, ['gl-1.0-beginend-coverage'])
 add_concurrent_test(gl10, ['gl-1.0-dlist-beginend'])
 add_concurrent_test(gl10, ['gl-1.0-dlist-shademodel'])
+add_concurrent_test(gl10, ['gl-1.0-drawpixels-color-index'])
 add_concurrent_test(gl10, ['gl-1.0-edgeflag'])
 add_concurrent_test(gl10, ['gl-1.0-edgeflag-const'])
 add_concurrent_test(gl10, ['gl-1.0-edgeflag-quads'])
diff --git a/tests/spec/gl-1.0/CMakeLists.gl.txt 
b/tests/spec/gl-1.0/CMakeLists.gl.txt
index e2e6642..733f3ff 100644
--- a/tests/spec/gl-1.0/CMakeLists.gl.txt
+++ b/tests/spec/gl-1.0/CMakeLists.gl.txt
@@ -10,11 +10,12 @@ link_libraries (
 )
 
 piglit_add_executable (gl-1.0-beginend-coverage beginend-coverage.c)
+piglit_add_executable (gl-1.0-dlist-beginend dlist-beginend.c)
+piglit_add_executable (gl-1.0-dlist-shademodel dlist-shademodel.c)
+piglit_add_executable (gl-1.0-drawpixels-color-index drawpixels-color-index.c)
 piglit_add_executable (gl-1.0-edgeflag edgeflag.c)
 piglit_add_executable (gl-1.0-edgeflag-const edgeflag-const.c)
 piglit_add_executable (gl-1.0-edgeflag-quads edgeflag-quads.c)
-piglit_add_executable (gl-1.0-dlist-beginend dlist-beginend.c)
-piglit_add_executable (gl-1.0-dlist-shademodel dlist-shademodel.c)
 piglit_add_executable (gl-1.0-front-invalidate-back front-invalidate-back.c)
 piglit_add_executable (gl-1.0-long-dlist long-dlist.c)
 piglit_add_executable (gl-1.0-rendermode-feedback rendermode-feedback.c)
diff --git a/tests/spec/gl-1.0/drawpixels-color-index.c 
b/tests/spec/gl-1.0/drawpixels-color-index.c
new file mode 100644
index 000..b9572a5
--- /dev/null
+++ b/tests/spec/gl-1.0/drawpixels-color-index.c
@@ -0,0 +1,162 @@
+/*
+ * Copyright 2015, VMware, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+
+/** @file drawpixels-color-index.c
+ *
+ * Test glDrawPixels(format=GL_COLOR_INDEX, type=GL_UNSIGNED_BYTE) and
+ * glDrawPixels(format=GL_COLOR_INDEX, type=GL_BITMAP).
+ */
+
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+   config.supports_gl_compat_version = 10;
+   config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;
+PIGLIT_GL_TEST_CONFIG_END
+
+
+static bool
+test_ci(int x, int y)
+{
+   bool pass = true;
+   static const float red_map[4] = {1.0, 0.0, 0.0, 1.0};
+   static const float green_map[4] = {0.0, 1.0, 0.0, 1.0};
+   static const float blue_map[4] = {0.0, 0.0, 1.0, 1.0};
+   static const float alpha_map[4] = {1.0, 1.0, 1.0, 1.0};
+   int x1, y1, x2, y2;
+   int i, j;
+   int width = 28, height = 18;
+   GLubyte *image = malloc(width * height);
+
+   /* Setup CI image with each quadrant an index in [0,3] */
+   for (i = 0; i < height; i++) {
+   for (j = 0; j < width; j++) {
+   int index = ((i < height / 2) ? 0 : 2)
+   + (j > width / 2);
+   image[i * width + j] = index;
+   }
+   }
+
+   glPixelMapfv(GL_PIXEL_MAP_I_TO_R, 4, red_map);
+   glPixelMapfv(GL_PIXEL_MAP_I_TO_G, 4, green_map);
+   glPixelMapfv(GL_PIXEL_MAP_I_TO_B, 4, blue_map);
+   glPixelMapfv(GL_PIXEL_MAP_I_TO_A, 4, alpha_map);
+   glPixelTransferi(GL_MAP_COLOR, GL_TRUE);
+
+   glWindowPos2i(x, y);
+   glDrawPixels(width, height, GL_COLOR_INDEX, GL_UNSIGNED_BYTE, image);
+
+   

Re: [Piglit] [PATCH] util: migrate the SUBTEST* macros to piglit-util.sh

2015-02-18 Thread Martin Peres

Hey Jose,

On 18/02/15 17:10, Jose Fonseca wrote:
FYI, you'll need to rebase on top of 
http://cgit.freedesktop.org/piglit/commit/?id=83bc6862386b2d465879bcd372d61ec754534970 
, so that we use the standard C99 syntax for variadic macros.  You 
might need to manually update the moved code, as git might just mark 
it as a conflict.


Sorry for breaking the windows build...  No probs, I'll rebase it.




Concerning the actual change, just a general remark:  these macros 
save typing but they make the test code harder to understand



I think there are other approaches that achieve the same amount of 
type-saving, without compromising readbility.


For example, we could have a


  // global contaning the current substest result
  // TODO: Make it a enum piglit_result
  bool current_subtest_result;
  char current_subtest_message[4096];

  piglit_subtest_begin() {
current_subtest_result = true;
current_subtest_message[0] = 0;
  }

  void
  piglit_subtest_check(bool condition, format, ...) {
bool pass = piglit_check_gl_error((error));
if (!pass) {
// FIXME: append format+va_args to current_subtest_message.
}
current_subtest_result = current_subtest_result && pass;
  }

  void
  piglit_subtest_check_gl_error(error) {
 ...
  }

 piglit_subtest_end()
   piglit_report_subtest_result(current_subtest_result ? PIGLIT_PASS : 
PIGLIT_FAIL, current_subtest_message);

  }


And the test would do

piglit_subtest_begin();
piglit_subtest_check_gl_error(error, etc);
piglit_subtest_check_gl_error(error, etc);
piglit_subtest_check(cond, etc);
piglit_subtest_end();


Not sure this form saves anything since one sub-test would be one line 
in my case instead of 3 in this case.
Have a look at 
http://cgit.freedesktop.org/~mperes/piglit/commit/?h=dsa&id=c08558659f7967410ab68f39c3d768e41e35bc6d


What you proposed is a good idea for the whole test though. 
piglit_test_begin() would set current_test_result
to keep track of the state and the other functions would follow the 
state. I'm not a fan of the global variable

too as what will happen if we have interleaved tests?

In any case, I'm sure some tests will not work with this model. So, 
instead of sharing those function and making them
very confusing, I would argue that outside of trivial cases (like the 
macros I proposed), you should re-write your own
way of checking. As for the "confusing" argument, not sure how much 
simpler can this code be (*no side effect*). It is

also less prone to copy/paste errors.

In my opinion, we won't reach an agreement that would work in every case 
or, if we do, people will not use the code
added because it will be too complex. Instead, let's just let devs 
handle their test code as it pleases them. If there are
some trivial cases common-enough, then we can add them to 
piglit-util.sh. I guess the question now is, are those

common-enough outside of DSA?

Martin
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 05/15] arb_direct_state_access: Testing NamedBufferSubData.

2015-02-18 Thread Martin Peres


On 23/01/15 21:03, Laura Ekstrand wrote:

---
  tests/all.py   |   1 +
  .../spec/arb_direct_state_access/CMakeLists.gl.txt |   1 +
  .../namedbuffersubdata-vbo-sync.c  | 119 +
  3 files changed, 121 insertions(+)
  create mode 100644 
tests/spec/arb_direct_state_access/namedbuffersubdata-vbo-sync.c

diff --git a/tests/all.py b/tests/all.py
index 7ec9d64..69a7cd7 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -4423,6 +4423,7 @@ 
spec['ARB_direct_state_access']['texture-storage-multisample'] = PiglitGLTest(['
  spec['ARB_direct_state_access']['texture-buffer'] = 
PiglitGLTest(['arb_direct_state_access-texture-buffer'], run_concurrent=True)
  spec['ARB_direct_state_access']['texture-buffer-range'] = 
PiglitGLTest(['arb_direct_state_access-texture-buffer-range'], 
run_concurrent=True)
  spec['ARB_direct_state_access']['namedbufferstorage-persistent'] = 
PiglitGLTest(['arb_direct_state_access-namedbufferstorage-persistent'], 
run_concurrent=True)
+spec['ARB_direct_state_access']['namedbuffersubdata-vbo-sync'] = 
PiglitGLTest(['arb_direct_state_access-namedbuffersubdata-vbo-sync'], 
run_concurrent=True)
  
  profile.tests['hiz'] = hiz

  profile.tests['fast_color_clear'] = fast_color_clear
diff --git a/tests/spec/arb_direct_state_access/CMakeLists.gl.txt 
b/tests/spec/arb_direct_state_access/CMakeLists.gl.txt
index 9228917..eedd60d 100644
--- a/tests/spec/arb_direct_state_access/CMakeLists.gl.txt
+++ b/tests/spec/arb_direct_state_access/CMakeLists.gl.txt
@@ -10,6 +10,7 @@ link_libraries (
  )
  
  piglit_add_executable (arb_direct_state_access-namedbufferstorage-persistent namedbufferstorage.c)

+piglit_add_executable (arb_direct_state_access-namedbuffersubdata-vbo-sync 
namedbuffersubdata-vbo-sync.c)
  piglit_add_executable (arb_direct_state_access-dsa-textures dsa-textures.c 
dsa-utils.c)
  piglit_add_executable (arb_direct_state_access-texturesubimage 
texturesubimage.c)
  piglit_add_executable (arb_direct_state_access-bind-texture-unit 
bind-texture-unit.c)
diff --git a/tests/spec/arb_direct_state_access/namedbuffersubdata-vbo-sync.c 
b/tests/spec/arb_direct_state_access/namedbuffersubdata-vbo-sync.c
new file mode 100644
index 000..60ebcda
--- /dev/null
+++ b/tests/spec/arb_direct_state_access/namedbuffersubdata-vbo-sync.c
@@ -0,0 +1,119 @@
+/*
+ * Copyright © 2009, 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *Ben Holmes 
+ *Eric Anholt 
+ *
+ * Adapted by Laura Ekstrand  to test
+ * NamedBufferSubData, January 2015.
+ */
+
+/** @file namedbuffersubdata-vbo-sync.c
+ *
+ * Test for synchronizing behavior of glNamedBufferSubData.
+ */
+
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+
+   config.supports_gl_compat_version = 10;
+
+   config.window_width = 400;
+   config.window_height = 300;
+   config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+static GLuint vbo;
+#define RECT_WIDTH 200
+#define RECT_HEIGHT 150
+
+void
+piglit_init(int argc, char **argv)
+{
+   piglit_require_extension("GL_ARB_vertex_buffer_object");
+   piglit_require_extension("GL_ARB_direct_state_access");
+
+   piglit_ortho_projection(piglit_width, piglit_height, GL_FALSE);
+
+   glCreateBuffers(1, &vbo);
+}
+
+static void
+verify_rect(GLboolean *pass, int hpos, int vpos, const float *expected)
+{
+   *pass = piglit_probe_rect_rgb(hpos * RECT_WIDTH, vpos * RECT_HEIGHT,
+ RECT_WIDTH, RECT_HEIGHT,
+ expected) && *pass;
+}
+
+enum piglit_result
+piglit_display(void)
+{
+   GLfloat white[4] = {1.0, 1.0, 1.0, 0.0};
+   GLfloat black[4] = {0.0, 0.0, 0.0, 0.0};
+   GLboolean pass = GL_TRUE;
+   GLfloat varray1[12] = {1 * RECT_WIDTH, 0 * RECT_HEIGHT, 0,
+   

Re: [Piglit] [PATCH] util: migrate the SUBTEST* macros to piglit-util.sh

2015-02-18 Thread Jose Fonseca
FYI, you'll need to rebase on top of 
http://cgit.freedesktop.org/piglit/commit/?id=83bc6862386b2d465879bcd372d61ec754534970 
, so that we use the standard C99 syntax for variadic macros.  You might 
need to manually update the moved code, as git might just mark it as a 
conflict.



Concerning the actual change, just a general remark:  these macros save 
typing but they make the test code harder to understand



I think there are other approaches that achieve the same amount of 
type-saving, without compromising readbility.


For example, we could have a


  // global contaning the current substest result
  // TODO: Make it a enum piglit_result
  bool current_subtest_result;
  char current_subtest_message[4096];

  piglit_subtest_begin() {
current_subtest_result = true;
current_subtest_message[0] = 0;
  }

  void
  piglit_subtest_check(bool condition, format, ...) {
bool pass = piglit_check_gl_error((error));
if (!pass) {
// FIXME: append format+va_args to current_subtest_message.
}
current_subtest_result = current_subtest_result && pass;
  }

  void
  piglit_subtest_check_gl_error(error) {
 ...
  }

 piglit_subtest_end()
   piglit_report_subtest_result(current_subtest_result ? PIGLIT_PASS : 
PIGLIT_FAIL, current_subtest_message);

  }


And the test would do

piglit_subtest_begin();
piglit_subtest_check_gl_error(error, etc);
piglit_subtest_check_gl_error(error, etc);
piglit_subtest_check(cond, etc);
piglit_subtest_end();

And this scheme could be extended to all piglit_check_* variants we need.

Jose



On 18/02/15 14:15, Martin Peres wrote:

Laura requested me to move these macros to piglit-util. However, this is
not the first time such a macro has been written. For example, curro
introduced subtest() in /tests/spec/arb_shader_image_load_store/common.h.
He said he also used another macro for another test.

Not sure if my macros are generic-enough though. They however can make a
good base for implementing most tests. They were definitely sufficient to
implement all my DSA-related tests.

Signed-off-by: Martin Peres 
---
  tests/spec/arb_direct_state_access/dsa-utils.h | 16 -
  tests/util/piglit-util.h   | 32 ++
  2 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/tests/spec/arb_direct_state_access/dsa-utils.h 
b/tests/spec/arb_direct_state_access/dsa-utils.h
index 25b5524..a2f70b2 100644
--- a/tests/spec/arb_direct_state_access/dsa-utils.h
+++ b/tests/spec/arb_direct_state_access/dsa-utils.h
@@ -39,22 +39,6 @@ extern "C" {

  #include "piglit-util-gl.h"

-#define SUBTEST(error, global, format, args...) \
-do { \
-   bool local = piglit_check_gl_error((error)); \
-   global = global && local; \
-   piglit_report_subtest_result(local ? PIGLIT_PASS : PIGLIT_FAIL, \
-(format), ##args); \
-} while (0)
-
-#define SUBTESTCONDITION(condition, global, format, args...) \
-do { \
-   bool cond = (condition); \
-   global = global && cond; \
-   piglit_report_subtest_result(cond ? PIGLIT_PASS : PIGLIT_FAIL, \
-(format), ##args); \
-} while (0)
-
  void dsa_init_program(void);

  void dsa_texture_with_unit(GLuint);
diff --git a/tests/util/piglit-util.h b/tests/util/piglit-util.h
index 1c522a1..9a53964 100755
--- a/tests/util/piglit-util.h
+++ b/tests/util/piglit-util.h
@@ -343,6 +343,38 @@ piglit_parse_subtest_args(int *argc, char *argv[],
  uint64_t
  piglit_gettid(void);

+/**
+ * \brief Run a subtest checking the error code of OpenGL
+ *
+ * This macro checks if the current gl error is \a error and reports the result
+ * using  piglit_report_subtest_result, using the test name \a format. Finally,
+ * it updates the \a global variable that makes sure that the reported state of
+ * the test as a whole is false if any of the subtest failed.
+ */
+#define SUBTEST(error, global, format, args...) \
+do { \
+   bool local = piglit_check_gl_error((error)); \
+   global = global && local; \
+   piglit_report_subtest_result(local ? PIGLIT_PASS : PIGLIT_FAIL, \
+(format), ##args); \
+} while (0)
+
+/**
+ * \brief Run a subtest checking a condition
+ *
+ * This macro checks if the \a condition is true and reports the result using
+ * piglit_report_subtest_result, using the test name \a format. Finally, it
+ * updates the \a global variable that makes sure that the reported state of
+ * the test as a whole is false if any of the subtest failed.
+ */
+#define SUBTESTCONDITION(condition, global, format, args...) \
+do { \
+   bool cond = (condition); \
+   global = global && cond; \
+   piglit_report_subtest_result(cond ? PIGLIT_PASS : PIGLIT_FAIL, \
+(format), ##args); \
+} while (0)
+
  #ifdef __cplusplus
  } /* end extern "C" */
  #endif



___
Piglit mailing list
Piglit

Re: [Piglit] DSA for core profile only? (was Re: [PATCH 2/2] arb_direct_state_access: New test for GetCompressedTextureImage.)

2015-02-18 Thread Henri Verbeet
On 18 February 2015 at 00:46, Ilia Mirkin  wrote:
> Wine maybe? (They're compat-only for now, although some work is being
> done to support core, but that might only be for their D3D10+ layer.)
The current plan for Wine is just to add support for core profiles.
There may be a case for hardware that can't do core profiles, but I
somewhat doubt that any performance difference from DSA will be large
enough to justify the effort.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 04/15] arb_direct_state_access: Testing NamedBufferData.

2015-02-18 Thread Martin Peres

On 23/01/15 21:03, Laura Ekstrand wrote:

---
  tests/spec/arb_direct_state_access/namedbufferstorage.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/spec/arb_direct_state_access/namedbufferstorage.c 
b/tests/spec/arb_direct_state_access/namedbufferstorage.c
index 2ddfab6..6630467 100644
--- a/tests/spec/arb_direct_state_access/namedbufferstorage.c
+++ b/tests/spec/arb_direct_state_access/namedbufferstorage.c
@@ -217,8 +217,8 @@ read_subtest(GLboolean coherent, GLboolean client_storage)
  
  	glClear(GL_COLOR_BUFFER_BIT);

glCreateBuffers(1, &srcbuf);
+   glNamedBufferData(srcbuf, BUF_SIZE, array, GL_STATIC_DRAW);
glBindBuffer(GL_COPY_READ_BUFFER, srcbuf);
-   glBufferData(GL_COPY_READ_BUFFER, BUF_SIZE, array, GL_STATIC_DRAW);
  
  	/* Copy some data to the mapped buffer and check if the CPU can see it. */

glBindBuffer(GL_COPY_WRITE_BUFFER, buffer);


This is a good functional test but you do not test for conformance with the 
spec. It also
misses entirely all the other usage which could be ill-implemented. I 
understand that you
may be implementing those tests with the assumption that the previous function 
worked
properly and you only test the differences you introduced in mesa but is this 
really how
we are supposed to write the tests?

I really don't know, hence why I am asking.

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] arb_uniform_buffer_object: Exercise coherent fragment shader UBO.

2015-02-18 Thread Jose Fonseca

On 18/02/15 14:43, Roland Scheidegger wrote:

Am 05.02.2015 um 15:37 schrieb Jose Fonseca:

At least for software renderers (like llvmpipe and softpipe), the vertex
and fragment shader stage are very different.

Achieve this by moving the color uniform to the fragment shader stage.
---
  tests/spec/arb_uniform_buffer_object/bufferstorage.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/spec/arb_uniform_buffer_object/bufferstorage.c 
b/tests/spec/arb_uniform_buffer_object/bufferstorage.c
index 334fd4e..52e20e0 100644
--- a/tests/spec/arb_uniform_buffer_object/bufferstorage.c
+++ b/tests/spec/arb_uniform_buffer_object/bufferstorage.c
@@ -43,7 +43,6 @@ static const char vert_shader_text[] =
"\n"
"layout(std140) uniform;\n"
"uniform ub_pos_size { vec2 pos; float size; };\n"
-   "uniform ub_color { vec4 color; float color_scale; };\n"
"uniform ub_rot {float rotation; };\n"
"\n"
"void main()\n"
@@ -54,15 +53,17 @@ static const char vert_shader_text[] =
"   m[1][0] = -m[0][1]; \n"
"   gl_Position.xy = m * gl_Vertex.xy * vec2(size) + pos;\n"
"   gl_Position.zw = vec2(0, 1);\n"
-   "   gl_FrontColor = color * color_scale;\n"
"}\n";

  static const char frag_shader_text[] =
"#extension GL_ARB_uniform_buffer_object : require\n"
"\n"
+   "layout(std140) uniform;\n"
+   "uniform ub_color { vec4 color; float color_scale; };\n"
+   "\n"
"void main()\n"
"{\n"
-   "  gl_FragColor = gl_Color;\n"
+   "   gl_FragColor = color * color_scale;\n"
"}\n";

  #define NUM_SQUARES 4



This looks good to me, the test comment says though it's the same as
rendering.c except for the persistent mapped ubos which is no longer
quite true.

Reviewed-by: Roland Scheidegger 


Thanks for the review. I'll update that comment to says it's "based" as 
opposed to the "same" then.  Another option would be to modify rendering 
in a similar fashion.


Jose



___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 01/15] arb_direct_state_access: Testing glCreateBuffers.

2015-02-18 Thread Martin Peres
Please check that a buffer returned by glCreateBuffers will return true 
when passed as a parameter of IsBuffer();

Please also check that negative n return INVALID_VALUE.

For these reasons, I decided to create the create-*.c tests for all the 
create functions. One could argue that they

could all be folded into the same file now that I think of it...

On 23/01/15 21:03, Laura Ekstrand wrote:

---
  tests/spec/arb_direct_state_access/getcompressedtextureimage.c | 2 +-
  tests/spec/arb_direct_state_access/texture-buffer-range.c  | 2 +-
  tests/spec/arb_direct_state_access/texture-buffer.c| 2 +-
  3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/spec/arb_direct_state_access/getcompressedtextureimage.c 
b/tests/spec/arb_direct_state_access/getcompressedtextureimage.c
index 9546af5..0d72df4 100644
--- a/tests/spec/arb_direct_state_access/getcompressedtextureimage.c
+++ b/tests/spec/arb_direct_state_access/getcompressedtextureimage.c
@@ -224,7 +224,7 @@ getTexImage(bool doPBO, GLenum target, GLubyte *data,
/* Setup the PBO or data array to read into from
 * glGetCompressedTextureImage */
if (doPBO) {
-   glGenBuffers(1, &packPBO);
+   glCreateBuffers(1, &packPBO);
glBindBuffer(GL_PIXEL_PACK_BUFFER, packPBO);
/* Make the buffer big enough to hold uncompressed data. */
glBufferData(GL_PIXEL_PACK_BUFFER, layer_size * num_faces *
diff --git a/tests/spec/arb_direct_state_access/texture-buffer-range.c 
b/tests/spec/arb_direct_state_access/texture-buffer-range.c
index 859000e..49c835a 100644
--- a/tests/spec/arb_direct_state_access/texture-buffer-range.c
+++ b/tests/spec/arb_direct_state_access/texture-buffer-range.c
@@ -135,7 +135,7 @@ piglit_init(int argc, char **argv) {
glGenVertexArrays(1, &vao);
glBindVertexArray(vao);
  
-	glGenBuffers(1, &tbo);

+   glCreateBuffers(1, &tbo);
glBindBuffer(GL_ARRAY_BUFFER, tbo);
glBufferData(GL_ARRAY_BUFFER, sizeof(data), data, GL_STATIC_DRAW);
  
diff --git a/tests/spec/arb_direct_state_access/texture-buffer.c b/tests/spec/arb_direct_state_access/texture-buffer.c

index 1246071..47f694f 100644
--- a/tests/spec/arb_direct_state_access/texture-buffer.c
+++ b/tests/spec/arb_direct_state_access/texture-buffer.c
@@ -70,7 +70,7 @@ piglit_display(void)
prog = piglit_build_simple_program(vs_source, fs_source);
glUseProgram(prog);
  
-	glGenBuffers(1, &bo);

+   glCreateBuffers(1, &bo);
glBindBuffer(GL_TEXTURE_BUFFER, bo);
glBufferData(GL_TEXTURE_BUFFER, sizeof(g_rgba8), g_rgba8,
 GL_STREAM_DRAW);


___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] arb_uniform_buffer_object: Exercise coherent fragment shader UBO.

2015-02-18 Thread Roland Scheidegger
Am 05.02.2015 um 15:37 schrieb Jose Fonseca:
> At least for software renderers (like llvmpipe and softpipe), the vertex
> and fragment shader stage are very different.
> 
> Achieve this by moving the color uniform to the fragment shader stage.
> ---
>  tests/spec/arb_uniform_buffer_object/bufferstorage.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/spec/arb_uniform_buffer_object/bufferstorage.c 
> b/tests/spec/arb_uniform_buffer_object/bufferstorage.c
> index 334fd4e..52e20e0 100644
> --- a/tests/spec/arb_uniform_buffer_object/bufferstorage.c
> +++ b/tests/spec/arb_uniform_buffer_object/bufferstorage.c
> @@ -43,7 +43,6 @@ static const char vert_shader_text[] =
>   "\n"
>   "layout(std140) uniform;\n"
>   "uniform ub_pos_size { vec2 pos; float size; };\n"
> - "uniform ub_color { vec4 color; float color_scale; };\n"
>   "uniform ub_rot {float rotation; };\n"
>   "\n"
>   "void main()\n"
> @@ -54,15 +53,17 @@ static const char vert_shader_text[] =
>   "   m[1][0] = -m[0][1]; \n"
>   "   gl_Position.xy = m * gl_Vertex.xy * vec2(size) + pos;\n"
>   "   gl_Position.zw = vec2(0, 1);\n"
> - "   gl_FrontColor = color * color_scale;\n"
>   "}\n";
>  
>  static const char frag_shader_text[] =
>   "#extension GL_ARB_uniform_buffer_object : require\n"
>   "\n"
> + "layout(std140) uniform;\n"
> + "uniform ub_color { vec4 color; float color_scale; };\n"
> + "\n"
>   "void main()\n"
>   "{\n"
> - "   gl_FragColor = gl_Color;\n"
> + "   gl_FragColor = color * color_scale;\n"
>   "}\n";
>  
>  #define NUM_SQUARES 4
> 

This looks good to me, the test comment says though it's the same as
rendering.c except for the persistent mapped ubos which is no longer
quite true.

Reviewed-by: Roland Scheidegger 
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] cmake: Enforce standard C99 syntax for variadic macros.

2015-02-18 Thread Roland Scheidegger
Am 18.02.2015 um 14:36 schrieb Jose Fonseca:
> Although the non-standard GCC syntax has some nice properties, for most
> practical cases the standard C99 syntax is perfectly fine.  Particuarly
> for printf-like macros, which pretty much account for most the uses of
> variadic macros in piglit.
> 
> Unfortunately this will only be effective on newer GCC versions, due a
> bug in GCC.  See comment for more details.
> ---
>  CMakeLists.txt | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 4236c89..420f76f 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -214,6 +214,17 @@ if (NOT MSVC)
>   IF (C_COMPILER_FLAG_WVLA)
>   SET (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wvla")
>   ENDIF ()
> + # MSVC only supports C99 variadic macros.  It doesn't support the
> + # non-standard GNU named variadic macro syntax that's documented in
> + # https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html
> + #
> + # XXX: on older GCC version this option has no effect unless -Wpedantic
> + # is set, but this should be fixed on future GCC versions, per
> + # https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01459.html
> + check_c_compiler_flag ("-Werror=variadic-macros" 
> C_COMPILER_FLAG_WVARIADIC_MACROS)
> + if (C_COMPILER_FLAG_WVARIADIC_MACROS)
> + set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Werror=variadic-macros")
> + endif ()
>  
>   CHECK_CXX_COMPILER_FLAG("-Wno-narrowing" 
> CXX_COMPILER_FLAG_WNO_NARROWING)
>   IF (CXX_COMPILER_FLAG_WNO_NARROWING)
> 

Looks good to me.

Roland

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH] util: migrate the SUBTEST* macros to piglit-util.sh

2015-02-18 Thread Martin Peres
Laura requested me to move these macros to piglit-util. However, this is
not the first time such a macro has been written. For example, curro
introduced subtest() in /tests/spec/arb_shader_image_load_store/common.h.
He said he also used another macro for another test.

Not sure if my macros are generic-enough though. They however can make a
good base for implementing most tests. They were definitely sufficient to
implement all my DSA-related tests.

Signed-off-by: Martin Peres 
---
 tests/spec/arb_direct_state_access/dsa-utils.h | 16 -
 tests/util/piglit-util.h   | 32 ++
 2 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/tests/spec/arb_direct_state_access/dsa-utils.h 
b/tests/spec/arb_direct_state_access/dsa-utils.h
index 25b5524..a2f70b2 100644
--- a/tests/spec/arb_direct_state_access/dsa-utils.h
+++ b/tests/spec/arb_direct_state_access/dsa-utils.h
@@ -39,22 +39,6 @@ extern "C" {
 
 #include "piglit-util-gl.h"
 
-#define SUBTEST(error, global, format, args...) \
-do { \
-   bool local = piglit_check_gl_error((error)); \
-   global = global && local; \
-   piglit_report_subtest_result(local ? PIGLIT_PASS : PIGLIT_FAIL, \
-(format), ##args); \
-} while (0)
-
-#define SUBTESTCONDITION(condition, global, format, args...) \
-do { \
-   bool cond = (condition); \
-   global = global && cond; \
-   piglit_report_subtest_result(cond ? PIGLIT_PASS : PIGLIT_FAIL, \
-(format), ##args); \
-} while (0)
-
 void dsa_init_program(void);
 
 void dsa_texture_with_unit(GLuint);
diff --git a/tests/util/piglit-util.h b/tests/util/piglit-util.h
index 1c522a1..9a53964 100755
--- a/tests/util/piglit-util.h
+++ b/tests/util/piglit-util.h
@@ -343,6 +343,38 @@ piglit_parse_subtest_args(int *argc, char *argv[],
 uint64_t
 piglit_gettid(void);
 
+/**
+ * \brief Run a subtest checking the error code of OpenGL
+ *
+ * This macro checks if the current gl error is \a error and reports the result
+ * using  piglit_report_subtest_result, using the test name \a format. Finally,
+ * it updates the \a global variable that makes sure that the reported state of
+ * the test as a whole is false if any of the subtest failed.
+ */
+#define SUBTEST(error, global, format, args...) \
+do { \
+   bool local = piglit_check_gl_error((error)); \
+   global = global && local; \
+   piglit_report_subtest_result(local ? PIGLIT_PASS : PIGLIT_FAIL, \
+(format), ##args); \
+} while (0)
+
+/**
+ * \brief Run a subtest checking a condition
+ *
+ * This macro checks if the \a condition is true and reports the result using
+ * piglit_report_subtest_result, using the test name \a format. Finally, it
+ * updates the \a global variable that makes sure that the reported state of
+ * the test as a whole is false if any of the subtest failed.
+ */
+#define SUBTESTCONDITION(condition, global, format, args...) \
+do { \
+   bool cond = (condition); \
+   global = global && cond; \
+   piglit_report_subtest_result(cond ? PIGLIT_PASS : PIGLIT_FAIL, \
+(format), ##args); \
+} while (0)
+
 #ifdef __cplusplus
 } /* end extern "C" */
 #endif
-- 
2.3.0

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/9] arb_direct_state_access: Added glCreateTransformFeedbacks test

2015-02-18 Thread Martin Peres

On 18/02/15 01:16, Laura Ekstrand wrote:
In light of Ian's rb, you could just upstream this commit and then 
send another patch to the ML that moves the macros to piglit-util-gl.h.


DONE, let's have the discussion whether it is a good move or not on the 
patch I sent. In any case, you can already rely on the macro now :)

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 2/3] Use alloca instead of variable length arrays

2015-02-18 Thread Jose Fonseca

On 12/02/15 18:51, Jan Vesely wrote:

On Thu, 2015-02-12 at 11:17 +, Emil Velikov wrote:

On 11 February 2015 at 16:12, Jan Vesely  wrote:

On Tue, 2015-02-10 at 10:28 -0800, Dylan Baker wrote:

I just want to be clear I was asking a question, I don't really care one
way or another, I would just rather not see code churn if it doesn't
actually buy us anything.


Not sure what the question is here. The idea is to force msvc like
limitations on other compilers by using -Werror= for unsupported
features. It should result in fewer commits like [0].
We can do it
a) globally even for stuff that is never built using msvc
b) per directory

I think a) is better given that the required changes are minimal (only
the two posted patches), and using alloca instead gives identical
behavior. it makes codestyle consistent across files, and Jose's way of
removing the flags using string function seems a bit hacky to me


Do you have a rough number how many tests warn about VLA currently ?
Must admit that I've very rarely look at the compilation output of
piglit. That combined the fact that new piglits get added
incrementally is a nice indication, imho, about one should handle
this.

Or in other words - it there are only a few of tests that need fixing,
there should be no problem with adding this. Otherwise it's a
different story.


When configured with all PIGLIT_BUILD_*_TESTS set to ON only the two
patches in this series (1/3, 2/3) are needed to make the entire tree
(0060f5998a) build with -Werror=declaration-after-statement -Werror=vla
(gcc 4.9.2, although I don't think these are version dependent)


I don't think we need the declaration-after-statement anymore.  There 
were some bugs in MSVC 2013 prior to Update 4 that gave the impression 
it wasn't supported, but it seems to work reasonably with Update 4. 
I've seen Waffle using it.



Concerning, the VLA, it doesn't look like anybody feels particularly 
strong either way.


Given it's just a handful of cases, and you already made things working, 
while enabling/disabling per-directory is more work, I say let's just go 
with your patches and move on.



Jose

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH] cmake: Enforce standard C99 syntax for variadic macros.

2015-02-18 Thread Jose Fonseca
Although the non-standard GCC syntax has some nice properties, for most
practical cases the standard C99 syntax is perfectly fine.  Particuarly
for printf-like macros, which pretty much account for most the uses of
variadic macros in piglit.

Unfortunately this will only be effective on newer GCC versions, due a
bug in GCC.  See comment for more details.
---
 CMakeLists.txt | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4236c89..420f76f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -214,6 +214,17 @@ if (NOT MSVC)
IF (C_COMPILER_FLAG_WVLA)
SET (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wvla")
ENDIF ()
+   # MSVC only supports C99 variadic macros.  It doesn't support the
+   # non-standard GNU named variadic macro syntax that's documented in
+   # https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html
+   #
+   # XXX: on older GCC version this option has no effect unless -Wpedantic
+   # is set, but this should be fixed on future GCC versions, per
+   # https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01459.html
+   check_c_compiler_flag ("-Werror=variadic-macros" 
C_COMPILER_FLAG_WVARIADIC_MACROS)
+   if (C_COMPILER_FLAG_WVARIADIC_MACROS)
+   set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Werror=variadic-macros")
+   endif ()
 
CHECK_CXX_COMPILER_FLAG("-Wno-narrowing" 
CXX_COMPILER_FLAG_WNO_NARROWING)
IF (CXX_COMPILER_FLAG_WNO_NARROWING)
-- 
2.1.0

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] Running piglit in chroot

2015-02-18 Thread Matěj Cepl
On 2015-02-18, 00:27 GMT, Emil Velikov wrote:
> For example:
> export LD_LIBRARY_PATH="/opt/rhel6/usr/lib/"
> export LIBGL_DRIVERS_PATH="/opt/rhel6/usr/lib/xorg/modules/dri"

Thanks,

in the end (and with great help of Adam Jackson and others) 
I have managed to make it work (both of these variables don't 
have much of documentation).

> That said, you might end up in a "mess" as you'll be using(?) a mix of
> rhel6 and rhel7 libraries as required by mesa - xcb*, expat, gcc_s,
> llvm to name a few. LD_DEBUG=libs is your friend, but if it was me I
> would port the mako package and be done with it :-)

Well, the problem is that I wouldn't. Just after packaging mako, 
I found I have to port also backport-collections, 
total_ordering, generate 700 lines of patch to make it work, and 
I have made it work 
(https://copr.fedoraproject.org/coprs/mcepl/piglit-el6/), just 
to found out (when trying to upgrade on even more recent piglit) 
that I would have to upgrade NumPy as well. This hole does have 
no end.

Best,

Matěj

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit