Re: [Piglit] [PATCH] fbo: Fix broken MRT bindings test.
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.
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.
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()
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.
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
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
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
--- 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
- 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
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
--- 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
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
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
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
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
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.)
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
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
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
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
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
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
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
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
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
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
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
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()
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.
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
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)
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
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.
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
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.)
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.
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.
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.
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.
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.
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
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
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
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.
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
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