Re: [Piglit] [PATCH] appveyor: Update instructions for personal repositories.

[Roland Scheidegger]

Looks good to me.
For the series:
Reviewed-by: Roland Scheidegger 


Am 13.03.19 um 15:16 schrieb Jose Fonseca:
> With GitLab is now much easier to setup.
> ---
>  appveyor.yml | 22 --
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/appveyor.yml b/appveyor.yml
> index 1f8779187..56f0d5a57 100644
> --- a/appveyor.yml
> +++ b/appveyor.yml
> @@ -1,22 +1,16 @@
> -# http://www.appveyor.com/docs/appveyor-yml
> +# https://www.appveyor.com/docs/appveyor-yml
>  #
>  # To setup AppVeyor for your own personal repositories do the following:
> -# - Sign up
> +# - Go to https://gitlab.freedesktop.org/profile/personal_access_tokens and
> +#   create a new token with 'api' and 'read_repository' access.
> +# - Sign up to AppVeyor
>  # - Add a new project
> -# - Select Git and fill in the Git clone URL
> -# - Setup a Git hook as explained in
> -#   https://github.com/appveyor/webhooks#installing-git-hook
> -# - Check 'Settings > General > Skip branches without appveyor.yml'
> -# - Check 'Settings > General > Rolling builds'
> -# - Setup the global or project notifications to your liking
> -#
> -# Note that kicking (or restarting) a build via the web UI will not work, as 
> it
> -# will fail to find appveyor.yml .  The Git hook is the most practical way to
> -# kick a build.
> +# - Select GitLab EE
> +# - Fill GitLab URL as https://gitlab.freedesktop.org/ and paste the token 
> above.
> +# - Enable for your personal repository.
>  #
>  # See also:
> -# - 
> http://help.appveyor.com/discussions/problems/2209-node-grunt-build-specify-a-project-or-solution-file-the-directory-does-not-contain-a-project-or-solution-file
> -# - 
> http://help.appveyor.com/discussions/questions/1184-build-config-vs-appveyoryaml
> +# - 
> https://help.appveyor.com/discussions/questions/1184-build-config-vs-appveyoryaml
>  
>  version: '{build}'
>  
> 

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

Re: [Piglit] [PATCH] Add tests for GL_EXTENSION_STRING vs. old idTech2 / idTech3 games

[Roland Scheidegger]

Am 24.09.2018 um 15:56 schrieb Ian Romanick:
> From: Ian Romanick 
> 
> As people dig up other games, we can (and should) easily add them.
> 
> Signed-off-by: Ian Romanick 
> Cc: Federico Dossena 
> Cc: Roland Scheidegger 
> ---
>  tests/general/CMakeLists.gl.txt  |   1 +
>  tests/general/idtech-extension-strings.c | 149 
> +++
>  2 files changed, 150 insertions(+)
>  create mode 100644 tests/general/idtech-extension-strings.c
> 
> diff --git a/tests/general/CMakeLists.gl.txt b/tests/general/CMakeLists.gl.txt
> index ceec9f0b6..83189fc42 100644
> --- a/tests/general/CMakeLists.gl.txt
> +++ b/tests/general/CMakeLists.gl.txt
> @@ -65,6 +65,7 @@ if (UNIX)
>   target_link_libraries (hiz m)
>  endif (UNIX)
>  piglit_add_executable (early-z early-z.c)
> +piglit_add_executable (idtech-extension-strings idtech-extension-strings.c)
>  piglit_add_executable (infinite-spot-light infinite-spot-light.c)
>  piglit_add_executable (isbufferobj isbufferobj.c)
>  piglit_add_executable (linestipple linestipple.c)
> diff --git a/tests/general/idtech-extension-strings.c 
> b/tests/general/idtech-extension-strings.c
> new file mode 100644
> index 0..cff54416b
> --- /dev/null
> +++ b/tests/general/idtech-extension-strings.c
> @@ -0,0 +1,149 @@
> +/*
> + * Copyright © 2018 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.
> + *
> + */
> +
> +/**
> + * \file idtech-extensoin-strings.c
extension


> + * Verify extensions used by idTech2 and idTech3 games occur in the first 4k
> + *
> + * For a long time idTech2- and idTech3-based games contained a bug in
> + * extension string handling.  The engine would copy the extension string
> + * returned by the driver into a 4k buffer.  The engine would not be able to
> + * detect the existence of any extensions that occured after the 4k boundry.
boundary



> + *
> + * For a variety of known games, this test verifies that extensions known to
> + * be used by each game occurs below the 4k boundry.
> + *
> + * A 2011 Wine bug
> + * 
> (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.winehq.org%2Fpipermail%2Fwine-bugs%2F2011-June%2F280463.htmldata=02%7C01%7Csroland%40vmware.com%7Cc108d7c276d7454160cb08d6222598df%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636733942205163850sdata=niqmepi%2BsBkOooa%2BefO3ZNI9z847n2SB4b88tbeNUsA%3Dreserved=0)
>  suggests
> + * that the limit for at least Return to Castle Wofenstein is 4k.  Some other
> + * evidence suggests that other games may have limits as low as 2k.
> + */
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_compat_version = 10;
> +
> + config.window_visual = PIGLIT_GL_VISUAL_RGB;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + /* UNREACHABLE */
> + return PIGLIT_FAIL;
> +}
> +
> +/* List of extensions scraped from the Quake3 demo found in
> + * linuxq3ademo-1.11-6.x86.gz.sh.  The Return to Castle Wolfenstein demo 
> found
> + * in wolfspdemo-linux-1.1b.x86.run had the same list.
> + */
> +const char *const q3demo_list[] = {
> + "GL_S3_s3tc",
> + "GL_EXT_texture_env_add",
> + "GL_ARB_multitexture",
> + "GL_EXT_compiled_vertex_array",
> +};
> +
> +/* List of extensions used by the game "Star Trek Voyager" provided by
> + * Federico Dossena.
> + */
> +const char *const star_trek_voyager_list[] = {
> + "GL_S3_s3tc",
> +

Re: [Piglit] [PATCH] arb_sample_shading: Add gl_SampleMaskIn subtest with msaa disabled, fix test

[Roland Scheidegger]

Am 05.02.2018 um 17:12 schrieb Brian Paul:
> On 02/04/2018 04:03 PM, srol...@vmware.com wrote:
>> From: Roland Scheidegger <srol...@vmware.com>
>>
>> The spec says with msaa disabled gl_SampleMaskIn is always 1.
>> This is not particularly related to arb_sample_shading, but drivers may
>> do different workarounds depending on the state of sample shading and the
>> presence of bits in the shader which would force per-sample execution,
>> so it
>> seems appropriate to test here.
>> At least r600/eg will fail (the hw will give the coverage mask for the
>> pixel,
>> not the coverage per sample or 1 if msaa is disabled).
>> Since the existing partition test could only be passed when giving the
>> wrong
>> answer for this, I would expect more drivers to fail...
>>
>> This also fixes the partition test with msaa disabled, as it expected
>> a wrong
>> gl_SampleMaskIn value in this case (it could easily verify
>> gl_SampleMaskIn is
>> 1 but omit it from there as it's quite likely multiple things are
>> failing in
>> the test simultaneously and then the failures can't be distinguished -
>> the new
>> test for this also doesn't require atomics).
> 
> Can you line-wrap the comments to be a bit shorter?
Sure (albeit none exceed 80 chars).

> 
>> ---
>>   .../arb_sample_shading/execution/samplemask.cpp    | 109
>> ++---
>>   1 file changed, 96 insertions(+), 13 deletions(-)
>>
>> diff --git a/tests/spec/arb_sample_shading/execution/samplemask.cpp
>> b/tests/spec/arb_sample_shading/execution/samplemask.cpp
>> index 74baddc11..8ed65ff42 100644
>> --- a/tests/spec/arb_sample_shading/execution/samplemask.cpp
>> +++ b/tests/spec/arb_sample_shading/execution/samplemask.cpp
>> @@ -34,7 +34,11 @@
>>    * 2. The bits of gl_SampleMaskIn over all fragment shader
>> invocations form a
>>    *    partition of the set of samples. This subtest requires
>>    *    ARB_shader_atomic_counters to disambiguate between fragment
>> shader
>> - *    invocations.
>> + *    invocations. (Also verifies sampleID is 0 when msaa is disabled.)
>> + * Additionally, there's a test to just verify gl_SampleMaskIn is 1 when
>> + * msaa is disabled (regardless of per-sample frequency shader or sample
>> + * shading). (Omitted from test 2 because it's difficult to track down
>> + * what's going wrong if drivers fail too many parts of the test.)
>>    *
>>    * The sample rate is controlled in one of two ways: Either
>> glMinSampleShading
>>    * or a fragment shader variant that uses gl_SampleID is used.
>> @@ -71,21 +75,24 @@ enum rate_mode {
>>   static int num_samples;
>>   static int actual_num_samples;
>>   static bool partition_check_supported;
>> +static bool mask_in_one_supported;
>>   static const char *procname;
>>   static const char *testname;
>>   static const char *sample_rate;
>>   static unsigned prog_fix_sample_mask[2];
>>   static unsigned prog_fix_check;
>> +static unsigned prog_mask_in_one[2];
>>   static unsigned prog_partition_write[2];
>>   static unsigned prog_partition_check;
>>   static unsigned prog_partition_check_have_sampleid;
>> +static unsigned prog_partition_check_msaa_disabled;
>>   static Fbo ms_fbo;
>>   static Fbo ms_ifbo;
>>     static void
>>   print_usage_and_exit(const char *prog_name)
>>   {
>> -    printf("Usage: %s   {fix|partition|all}\n"
>> +    printf("Usage: %s  
>> {fix|partition|mask_in_one|all}\n"
>>  "where  is either a floating point MinSampleShading
>> value\n"
>>  " or 'sample', 'noms', or 'all'\n",
>>  prog_name);
>> @@ -165,6 +172,16 @@ compile_shaders(void)
>>   "  }\n"
>>   "}\n";
>>   +    static const char frag_mask_in_one[] =
>> +    "#version 130\n"
>> +    "#extension GL_ARB_gpu_shader5 : enable\n"
>> +    "#extension GL_ARB_sample_shading : enable\n"
>> +    "out vec4 out_color;\n"
>> +    "void main()\n"
>> +    "{\n"
>> +    "  out_color = vec4(float(gl_SampleMaskIn[0]) / 10.0, 0.0,
>> %s, 0.0);\n"
>> +    "}\n";
>> +
>>   static const char frag_partition_write[] =
>>   "#version 140\n"
>>   "#extension GL_ARB_gpu_shader5 : enable\n"
>> @@ -183,16 +200,23 @@ compile_shaders(void)
>>   "uniform isampler2DM

Re: [Piglit] [PATCH 1/2] arb_internalformat_query2: avoid bogus error spam about unsupported pnames

[Roland Scheidegger]

Am 27.01.2018 um 02:00 schrieb srol...@vmware.com:
> From: Roland Scheidegger <srol...@vmware.com>
> 
> The test fed the equivalent_pname into the get_unsupported_response()
> function, which led to nonsense logged errors as this only recognizes
> the original pnames.
> Refactor pname/equivalent_pname translation so always the original pnames
> are fed into that function.
> ---
>  tests/spec/arb_internalformat_query2/common.c  | 34 
> +-
>  .../internalformat-size-checks.c   | 23 ++-
>  .../internalformat-type-checks.c   | 21 ++---
>  .../arb_transform_feedback_overflow_query/basic.c  | 22 ++
Err, that last bit obviously doesn't belong in here. Just ignore it...

Roland



>  4 files changed, 59 insertions(+), 41 deletions(-)
> 
> diff --git a/tests/spec/arb_internalformat_query2/common.c 
> b/tests/spec/arb_internalformat_query2/common.c
> index 9fa5fa9d1..5fc4c833b 100644
> --- a/tests/spec/arb_internalformat_query2/common.c
> +++ b/tests/spec/arb_internalformat_query2/common.c
> @@ -496,6 +496,37 @@ create_texture(const GLenum target,
>  }
>  return result;
>  }
> +
> +
> +static GLenum
> +translate_pname(const GLenum pname)
> +{
> +switch (pname) {
> +case GL_INTERNALFORMAT_RED_TYPE:
> +case GL_INTERNALFORMAT_GREEN_TYPE:
> +case GL_INTERNALFORMAT_BLUE_TYPE:
> +case GL_INTERNALFORMAT_ALPHA_TYPE:
> +   return pname - GL_INTERNALFORMAT_RED_TYPE + 
> GL_TEXTURE_RED_TYPE;
> + case GL_INTERNALFORMAT_DEPTH_TYPE:
> +/* case GL_INTERNALFORMAT_STENCIL_TYPE, */
> +return GL_TEXTURE_DEPTH_TYPE;
> +case GL_INTERNALFORMAT_RED_SIZE:
> +case GL_INTERNALFORMAT_GREEN_SIZE:
> +case GL_INTERNALFORMAT_BLUE_SIZE:
> +case GL_INTERNALFORMAT_ALPHA_SIZE:
> +return pname - GL_INTERNALFORMAT_RED_SIZE + 
> GL_TEXTURE_RED_SIZE;
> +case GL_INTERNALFORMAT_DEPTH_SIZE:
> +return GL_TEXTURE_DEPTH_SIZE;
> +case GL_INTERNALFORMAT_STENCIL_SIZE:
> +return GL_TEXTURE_STENCIL_SIZE;
> +case GL_INTERNALFORMAT_SHARED_SIZE:
> +return GL_TEXTURE_SHARED_SIZE;
> +default:
> +assert(!"incorrect pname");
> +return 0;
> +}
> +}
> +
>  /*
>   * Builds a a texture using @target and @internalformat, and compares
>   * the result of calling GetTexLevelParameter using @pname with the
> @@ -520,6 +551,7 @@ test_data_check_against_get_tex_level_parameter(test_data 
> *data,
>  GLuint tex;
>  GLuint buffer;
>  GLenum real_target = target;
> +GLenum pname_equiv = translate_pname(pname);
>  
>  result = create_texture(target, internalformat, , );
>  if (!result)
> @@ -530,7 +562,7 @@ test_data_check_against_get_tex_level_parameter(test_data 
> *data,
>  if (target == GL_TEXTURE_CUBE_MAP) {
>  real_target = GL_TEXTURE_CUBE_MAP_POSITIVE_X;
>  }
> -glGetTexLevelParameteriv(real_target, 0, pname, );
> +glGetTexLevelParameteriv(real_target, 0, pname_equiv, );
>  if (!piglit_check_gl_error(GL_NO_ERROR)) {
>  result = false;
>  fprintf(stderr, "\tError calling glGetTexLevelParameter\n");
> diff --git 
> a/tests/spec/arb_internalformat_query2/internalformat-size-checks.c 
> b/tests/spec/arb_internalformat_query2/internalformat-size-checks.c
> index bbccbd6d1..928133133 100644
> --- a/tests/spec/arb_internalformat_query2/internalformat-size-checks.c
> +++ b/tests/spec/arb_internalformat_query2/internalformat-size-checks.c
> @@ -53,24 +53,6 @@ static const GLenum pnames[] = {
>  GL_INTERNALFORMAT_SHARED_SIZE,
>  };
>  
> -/* From spec:
> - *
> - * "For textures this query will return the same information
> - *  as querying GetTexLevelParameter{if}v for TEXTURE_*_SIZE
> - *  would return."
> - *
> - * The following are the pnames we would need to use when
> - * calling GetTexLevelParameter (so equivalent to pnames)
> - */
> -static const GLenum equivalent_pnames[] = {
> -GL_TEXTURE_RED_SIZE,
> -GL_TEXTURE_GREEN_SIZE,
> -GL_TEXTURE_BLUE_SIZE,
> -GL_TEXTURE_ALPHA_SIZE,
> -GL_TEXTURE_DEPTH_SIZE,
> -GL_TEXTURE_STENCIL_SIZE,
> -GL_TEXTURE_SHARED_SIZE,
> -};
>  
>  enum piglit_result
>  piglit_display(void)
> @@ -102,7 +84,6 @@ static bool
>  try_textures_size(const GLenum *targets, unsigned num_targets,
>   

Re: [Piglit] [PATCH] gl-1.0-blend-func: skip some blend tests when using LLVM 3.8

[Roland Scheidegger]

Am 19.01.2018 um 06:35 schrieb Eric Anholt:
> Brian Paul  writes:
> 
>> On 01/18/2018 01:27 PM, Eric Anholt wrote:
>>> Brian Paul  writes:
>>>
 To avoid an infinite loop.  See code comments for details.
>>>
>>> Skipping a failing test and returning pass is wrong to me.
>>
>> It's not ideal.  But the bug is in LLVM and cannot readily be fixed in 
>> llvmpipe.
>>
>> I could have the test return a WARN result in this situation.  Would 
>> that be better?
> 
> It's still a bug in the driver, even if it's because the driver's using
> a buggy external library.  It should be a fail.
> 

Albeit it's just a guess it will hang. With a fixed llvm 3.8 from the
stable branch, it would not hang and pass. Or IIRC if you don't have a
avx-capable cpu, it also would not hang (and with a non-x86 cpu it won't
hang neither).
But I don't really care either way if it just reports fail in this case.
(Would be nice if we could just determine the hang empirically I
suppose, if the testcase runs for more than a second kill it and it's a
fail, but that doesn't work easily.)

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

Re: [Piglit] [PATCH] gl-1.0-blend-func: skip some blend tests when using LLVM 3.8

[Roland Scheidegger]

Am 18.01.2018 um 21:04 schrieb Brian Paul:
> On 01/18/2018 12:38 PM, Roland Scheidegger wrote:
>> Am 18.01.2018 um 20:07 schrieb Brian Paul:
>>> To avoid an infinite loop.  See code comments for details.
>>> ---
>>>   tests/spec/gl-1.0/blend.c | 39 +++
>>>   1 file changed, 39 insertions(+)
>>>
>>> diff --git a/tests/spec/gl-1.0/blend.c b/tests/spec/gl-1.0/blend.c
>>> index 35e940f..e69ed31 100644
>>> --- a/tests/spec/gl-1.0/blend.c
>>> +++ b/tests/spec/gl-1.0/blend.c
>>> @@ -76,6 +76,9 @@ static int test_stride = 1;
>>>   #define img_width drawing_size
>>>   #define img_height drawing_size
>>>   +static bool using_llvm_3_8 = false;
>>> +
>>> +
>>>   PIGLIT_GL_TEST_CONFIG_BEGIN
>>>     config.supports_gl_compat_version = 10;
>>> @@ -177,6 +180,10 @@ image_init(struct image *image)
>>>    GL_RGBA, GL_FLOAT, image->data);
>>>   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
>>>   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
>>> +
>>> +    if (strstr("LLVM 3.8", (const char *) glGetString(GL_RENDERER))
>>> == 0) {
>>> +    using_llvm_3_8 = true;
>>> +    }
>>>   } /* image_init */
>> FWIW I think this might mistakenly catch other drivers which use llvm
>> (radeonsi).
> 
> I'll fix that by also testing for "llvmpipe".
> 
> 
>>
>> I am not really sure it's worth the trouble. Personally, I couldn't care
>> less about llvm 3.8 causing a hang here (distros which updated llvm 3.8
>> with the fixes from the 3.8 stable branch won't have a problem).
> 
> As I mentioned in my other email, I recently updated to Mint 18.3
> (latest version) and this version of LLVM is the default.  Mint 18 is
> based on Ubuntu 16.04 which has LLVM 3.8 also (but I haven't tested it
> to see if has this bug or is fixed).  Same story with Ubuntu 16.10.  So
> this buggy LLVM may be common.

I thought distros would pick up fixed versions - it should be fixed in
llvm 3.8.1. If they don't, well, that's a problem. (I thought that was
pretty much the reason llvm doing stable branches nowadays, so they can
be more easily upgraded to include important fixes without having to
worry about new breakage much...).


> 
> 
>> So all
>> the trouble just for avoiding a bug with some old-ish llvm version seems
>> a bit overkill to me. I am not sure anymore if it actually was an issue
>> both on AVX(2) and non-AVX path neither.
>> Plus an actual app using the affected blend factor combinations will
>> just hang too, so hiding it here doesn't really seem helpful. (IIRC I
>> thought about trying to work around it in llvmpipe code, but there was
>> no trivial solution (for not just avoiding the hang but still correctly
>> rendering) and I didn't care enough about that particular llvm version,
>> even more so since it was fixed in the stable branch...)
>>
>> But in any case if you really think it's worthwile, I am not opposing it
>> neither, so
>> Reviewed-by: Roland Scheidegger <srol...@vmware.com>
> 
> It'll probably save me some grief so I'd like to push it.
Go ahead then.

Roland


> 
> -Brian
> 
> 
>>
>>
>>>   @@ -532,6 +539,32 @@ apply_blend(GLenum src_factor_rgb, GLenum
>>> src_factor_a,
>>>   } /* apply_blend */
>>>     +/**
>>> + * With unpactched LLVM 3.8, llvmpipe can hit an bug in LLVM which
>>> results
>>> + * in an infinite loop.  See
>>> https://bugs.llvm.org/show_bug.cgi?id=27689
>>> + * This function tries to determine if the test case will hit that
>>> bug so
>>> + * we can skip the test.
>>> + */
>>> +bool
>>> +skip_on_llvmpipe(GLenum src_factor_rgb, GLenum src_factor_a,
>>> + GLenum dst_factor_rgb, GLenum dst_factor_a)
>>> +{
>>> +    if (!using_llvm_3_8)
>>> +    return false;
>>> +
>>> +    /* This could probably be a bit tighter, but this seems to catch
>>> + * the troublesome cases.
>>> + */
>>> +    if (src_factor_rgb == GL_CONSTANT_COLOR ||
>>> +    dst_factor_rgb == GL_CONSTANT_COLOR ||
>>> +    dst_factor_a == GL_CONSTANT_COLOR ||
>>> +    dst_factor_a == GL_CONSTANT_ALPHA)
>>> +    return true;
>>> +
>>> +    return false;
>>> +}
>>> +
>>> +
>>>   /* Test for one set of factor levels */
>>>   bool
>>>   run_factor_set(GLenum src_factor_rgb, GLenum src_factor_a,
>>> @@ -542,6 +575,12 @@ run_factor_set(GLenum src_factor_rgb, GLenum
>>> src_factor_a,
>>>   int i, j;
>>>   bool pass = true, p;
>>>   +    if (skip_on_llvmpipe(src_factor_rgb, src_factor_a,
>>> + dst_factor_rgb, dst_factor_a)) {
>>> +    /*printf("Skipping Blend test to avoid LLVM bug\n");*/
>>> +    return true;
>>> +    }
>>> +
>>>   glDisable(GL_DITHER);
>>>   glClear(GL_COLOR_BUFFER_BIT);
>>>  
> 

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

Re: [Piglit] [PATCH] gl-1.0-blend-func: skip some blend tests when using LLVM 3.8

[Roland Scheidegger]

Am 18.01.2018 um 20:07 schrieb Brian Paul:
> To avoid an infinite loop.  See code comments for details.
> ---
>  tests/spec/gl-1.0/blend.c | 39 +++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/tests/spec/gl-1.0/blend.c b/tests/spec/gl-1.0/blend.c
> index 35e940f..e69ed31 100644
> --- a/tests/spec/gl-1.0/blend.c
> +++ b/tests/spec/gl-1.0/blend.c
> @@ -76,6 +76,9 @@ static int test_stride = 1;
>  #define img_width drawing_size
>  #define img_height drawing_size
>  
> +static bool using_llvm_3_8 = false;
> +
> +
>  PIGLIT_GL_TEST_CONFIG_BEGIN
>  
>   config.supports_gl_compat_version = 10;
> @@ -177,6 +180,10 @@ image_init(struct image *image)
>GL_RGBA, GL_FLOAT, image->data);
>   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
>   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
> +
> + if (strstr("LLVM 3.8", (const char *) glGetString(GL_RENDERER)) == 0) {
> + using_llvm_3_8 = true;
> + }
>  } /* image_init */
FWIW I think this might mistakenly catch other drivers which use llvm
(radeonsi).

I am not really sure it's worth the trouble. Personally, I couldn't care
less about llvm 3.8 causing a hang here (distros which updated llvm 3.8
with the fixes from the 3.8 stable branch won't have a problem). So all
the trouble just for avoiding a bug with some old-ish llvm version seems
a bit overkill to me. I am not sure anymore if it actually was an issue
both on AVX(2) and non-AVX path neither.
Plus an actual app using the affected blend factor combinations will
just hang too, so hiding it here doesn't really seem helpful. (IIRC I
thought about trying to work around it in llvmpipe code, but there was
no trivial solution (for not just avoiding the hang but still correctly
rendering) and I didn't care enough about that particular llvm version,
even more so since it was fixed in the stable branch...)

But in any case if you really think it's worthwile, I am not opposing it
neither, so
Reviewed-by: Roland Scheidegger <srol...@vmware.com>


>  
> @@ -532,6 +539,32 @@ apply_blend(GLenum src_factor_rgb, GLenum src_factor_a,
>  } /* apply_blend */
>  
>  
> +/**
> + * With unpactched LLVM 3.8, llvmpipe can hit an bug in LLVM which results
> + * in an infinite loop.  See https://bugs.llvm.org/show_bug.cgi?id=27689
> + * This function tries to determine if the test case will hit that bug so
> + * we can skip the test.
> + */
> +bool
> +skip_on_llvmpipe(GLenum src_factor_rgb, GLenum src_factor_a,
> +  GLenum dst_factor_rgb, GLenum dst_factor_a)
> +{
> + if (!using_llvm_3_8)
> + return false;
> +
> + /* This could probably be a bit tighter, but this seems to catch
> +  * the troublesome cases.
> +  */
> + if (src_factor_rgb == GL_CONSTANT_COLOR ||
> + dst_factor_rgb == GL_CONSTANT_COLOR ||
> + dst_factor_a == GL_CONSTANT_COLOR ||
> + dst_factor_a == GL_CONSTANT_ALPHA)
> + return true;
> +
> + return false;
> +}
> +
> +
>  /* Test for one set of factor levels */
>  bool
>  run_factor_set(GLenum src_factor_rgb, GLenum src_factor_a,
> @@ -542,6 +575,12 @@ run_factor_set(GLenum src_factor_rgb, GLenum 
> src_factor_a,
>   int i, j;
>   bool pass = true, p;
>  
> + if (skip_on_llvmpipe(src_factor_rgb, src_factor_a,
> +  dst_factor_rgb, dst_factor_a)) 
> {
> + /*printf("Skipping Blend test to avoid LLVM bug\n");*/
> + return true;
> + }
> +
>   glDisable(GL_DITHER);
>   glClear(GL_COLOR_BUFFER_BIT);
>  
> 
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH 2/3] gl-1.0-blend-func: add --quick option

[Roland Scheidegger]

That should help llvmpipe quite a bit...
Albeit 1% doesn't sound like it would give a lot of coverage, maybe a
bit more (5% or so) would still cut down the time significantly while
having less risk of missing failures?
Either way,
for 1-2/3
Reviewed-by: Roland Scheidegger <srol...@vmware.com>

Am 17.01.2018 um 23:54 schrieb Brian Paul:
> The test normally runs about 27,000 tests and takes quite a long time
> with some drivers. With the --quick option, only 1% of the tests are run.
> 
> And update tests/quick.py to run the test with --quick.
> ---
>  tests/quick.py|  7 +++
>  tests/spec/gl-1.0/blend.c | 23 ++-
>  2 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/quick.py b/tests/quick.py
> index 53774e4..73c4678 100644
> --- a/tests/quick.py
> +++ b/tests/quick.py
> @@ -68,6 +68,13 @@ with profile.test_list.group_manager(
>  with profile.test_list.allow_reassignment:
>  g(['ext_texture_env_combine-combine', '--quick'], 
> 'texture-env-combine')
>  
> +# Set the --quick flag on the gl-1.0 blending test
> +with profile.test_list.group_manager(
> +PiglitGLTest,
> +grouptools.join('spec', '!opengl 1.0')) as g:
> +with profile.test_list.allow_reassignment:
> +g(['gl-1.0-blend-func', '--quick'], 'gl-1.0-blend-func')
> +
>  # Limit texture size to 512x512 for some texture_multisample tests.
>  # The default (max supported size) can be pretty slow.
>  with profile.test_list.group_manager(
> diff --git a/tests/spec/gl-1.0/blend.c b/tests/spec/gl-1.0/blend.c
> index 769339f..192b271 100644
> --- a/tests/spec/gl-1.0/blend.c
> +++ b/tests/spec/gl-1.0/blend.c
> @@ -64,6 +64,8 @@
>  
>  #define HUGE_STEP 1000
>  
> +static int test_stride = 1;
> +
>  /*
>   * We will check each pair of blend factors
>   * for each pixel in a square image of this
> @@ -187,6 +189,13 @@ piglit_init(int argc, char **argv)
>   const char* blend_rgb_tol = getenv("PIGLIT_BLEND_RGB_TOLERANCE");
>   const char* blend_alpha_tol = getenv("PIGLIT_BLEND_ALPHA_TOLERANCE");
>  
> + if (argc > 1 && strcmp(argv[1], "--quick") == 0) {
> + /* By default we run 27552 tests which is time consuming.
> +  * With --quick we run only 1% of the tests.
> +  */
> + test_stride = 100;
> + }
> +
>   /* 
>* Hack: Make driver tests on incorrect hardware feasible
>* We want to be able to perform meaningful tests
> @@ -687,6 +696,7 @@ run_all_factor_sets(void)
>   bool pass = true;
>   int gl_version = piglit_get_gl_version();
>   int counter = 0; /* Number of tests we have done. */
> + int test_number = 0;
>   int step;
>   int op, opa;
>   int sf, sfa, df, dfa;
> @@ -784,11 +794,14 @@ run_all_factor_sets(void)
>   for (dfa = 0; dfa < 
>num_dst_factors_sep;
>dfa += step) {
> - pass &= proc_factors(
> - sf, sfa, 
> - df, dfa,
> - op, opa);
> - counter++;
> + if (test_number % 
> test_stride == 0) {
> + pass &= 
> proc_factors(
> + sf, 
> sfa, 
> + df, dfa,
> + op, 
> opa);
> + counter++;
> + }
> + test_number++;
>   }
>   }
>   }
> 

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

Re: [Piglit] [PATCH 3/3] gl-1.0-dlist-materials: new test of materials in display lists

[Roland Scheidegger]

Am 17.01.2018 um 23:54 schrieb Brian Paul:
> This exercises two code paths in Mesa's display list VBO code.
> ---
>  tests/all.py|   1 +
>  tests/spec/gl-1.0/CMakeLists.gl.txt |   1 +
>  tests/spec/gl-1.0/dlist-materials.c | 164 
> 
>  3 files changed, 166 insertions(+)
>  create mode 100644 tests/spec/gl-1.0/dlist-materials.c
> 
> diff --git a/tests/all.py b/tests/all.py
> index eb496a6..ef8c01b 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -717,6 +717,7 @@ with profile.test_list.group_manager(
>  g(['gl-1.0-beginend-coverage'])
>  g(['gl-1.0-dlist-beginend'])
>  g(['gl-1.0-dlist-bitmap'])
> +g(['gl-1.0-dlist-materials'])
>  g(['gl-1.0-dlist-shademodel'])
>  g(['gl-1.0-drawpixels-color-index'])
>  g(['gl-1.0-drawpixels-depth-test'])
> diff --git a/tests/spec/gl-1.0/CMakeLists.gl.txt 
> b/tests/spec/gl-1.0/CMakeLists.gl.txt
> index fcb1d1d..f1d4d93 100644
> --- a/tests/spec/gl-1.0/CMakeLists.gl.txt
> +++ b/tests/spec/gl-1.0/CMakeLists.gl.txt
> @@ -12,6 +12,7 @@ piglit_add_executable (gl-1.0-beginend-coverage 
> beginend-coverage.c)
>  piglit_add_executable (gl-1.0-blend-func blend.c)
>  piglit_add_executable (gl-1.0-dlist-beginend dlist-beginend.c)
>  piglit_add_executable (gl-1.0-dlist-bitmap dlist-bitmap.c)
> +piglit_add_executable (gl-1.0-dlist-materials dlist-materials.c)
>  piglit_add_executable (gl-1.0-dlist-shademodel dlist-shademodel.c)
>  piglit_add_executable (gl-1.0-drawbuffer-modes drawbuffer-modes.c)
>  piglit_add_executable (gl-1.0-drawpixels-color-index 
> drawpixels-color-index.c)
> diff --git a/tests/spec/gl-1.0/dlist-materials.c 
> b/tests/spec/gl-1.0/dlist-materials.c
> new file mode 100644
> index 000..1e61825
> --- /dev/null
> +++ b/tests/spec/gl-1.0/dlist-materials.c
> @@ -0,0 +1,164 @@
> +/*
> + * Copyright (C) 2018 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.
> + */
> +
> +/**
> + * Test glMaterial calls in a display list.
> + */
> +
> +#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;
> + config.khr_no_error_support = PIGLIT_NO_ERRORS;
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +
> +static const GLfloat red[] = {1.0, 0.0, 0.0, 1.0};
> +static const GLfloat green[] = {0.0, 1.0, 0.0, 1.0};
> +static const GLfloat black[] = {0.0, 0.0, 0.0, 1.0};
> +static const GLfloat white[] = {1.0, 1.0, 1.0, 1.0};
> +
> +
> +/**
> + * Build display list to draw two quads with a triangle strip
> + * using glMaterial calls to set vertex colors.
> + * \param set_all  If true, set the material attribs for all vertices.
> + * Otherwise, just set the material attribs for two
> + * provoking vertices.
> + *
> + * Note: the set_all parameter controls whether Mesa hits the "loopback" 
> code.
> + */
> +static GLuint
> +make_list(GLenum mat, bool set_all)
> +{
> + GLuint list;
> +
> + list = glGenLists(1);
> + glNewList(list, GL_COMPILE);
> +
> + /*
> +  * Draw tri strip drawing two quads- left=red, right=green.
whitespace before "-" ?

Ahhh materials. Quite a blast from the past :-).
I faintly remember them causing nothing but problems for some old radeon
chips (r100, maybe r200 IIRC) - or rather the ability to being able to
change them even between begin/end...
I could certainly see tons of things going wrong with them not 

Re: [Piglit] [PATCH] teximage-colors: accept -127 instead of -128 for exact snorm up/download

[Roland Scheidegger]

ping?

Am 08.01.2018 um 01:20 schrieb srol...@vmware.com:
> From: Roland Scheidegger <srol...@vmware.com>
> 
> -128 and -127 represent exactly the same value (-1.0) for snorm8 values,
> as do -32768/-32767 for snorm16. Therefore it seems quite reasonable that an
> implementation would return the other representation (in particular r600 will
> do this with a blit, and the snorm->float->snorm conversion implied by this
> will never return the -128/-32768 values).
> ---
>  tests/texturing/teximage-colors.c | 39 
> +++
>  1 file changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/texturing/teximage-colors.c 
> b/tests/texturing/teximage-colors.c
> index de2024644..61a3c5d15 100644
> --- a/tests/texturing/teximage-colors.c
> +++ b/tests/texturing/teximage-colors.c
> @@ -833,10 +833,41 @@ test_exact()
> observed);
>   pass &= piglit_check_gl_error(GL_NO_ERROR);
>  
> - for (i = 0; i < texture_size; ++i)
> - pass &= memcmp([i * texture_size * Bpp],
> -[i * tex_width * Bpp],
> -texture_size * Bpp) == 0;
> + /*
> +  * For snorm formats, -127/-128 and -32767/-32768 represent the exact
> +  * same value (-1.0). Therefore, it is quite reasonable to expect
> +  * an implementation could return the other representation.
> +  * (We'll assume it will happen only one way the other way seems rather
> +  * unlikely.)
> +  */
> + if (format->data_type == GL_BYTE) {
> + int j;
> + for (j = 0; j < texture_size; ++j) {
> + for (i = 0; i < tex_width * channels; i++) {
> + if (!(data[i] == observed[i] ||
> +   (data[i] == 128 && observed[i] == 129))) {
> + pass = GL_FALSE;
> + }
> + }
> + }
> + } else if (format->data_type == GL_SHORT) {
> + int j;
> + for (j = 0; j < texture_size; ++j) {
> + for (i = 0; i < tex_width * channels; i++) {
> + GLshort datas = ((GLshort *)data)[i];
> + GLshort obss = ((GLshort *)observed)[i];
> + if (!(datas == obss ||
> +   (datas == -32768 && obss == -32767))) {
> + pass = GL_FALSE;
> + }
> + }
> + }
> + } else {
> + for (i = 0; i < texture_size; ++i)
> + pass &= memcmp([i * texture_size * Bpp],
> + [i * tex_width * Bpp],
> + texture_size * Bpp) == 0;
> + }
>  
>   free(observed);
>   free(tmp_float);
> 

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

Re: [Piglit] [PATCH 1/2] textureSize: add ability to test tess eval stage

[Roland Scheidegger]

Thanks for the review, I've added the tests to all.py. And good thing I
did, because from the ~40 tests the 3 with bufferSamplers didn't
actually work on my newest r600 patch version in a test run :-).

Roland

Am 03.01.2018 um 23:08 schrieb Fabian Bieler:
> I think the new tes-texture-size test should be added to all.py (in
> multiple places).
> 
> Other than that, series is
> Reviewed-by: Fabian Bieler <fabianbie...@fastmail.fm>
> 
> On 2018-01-01 06:41, srol...@vmware.com wrote:
>> From: Roland Scheidegger <srol...@vmware.com>
>>
>> This is a problem of all texturing tests, they cannot exercise the 
>> tesselation
>> stages. (But I'm too lazy to fix the others, and too lazy to hack something 
>> up
>> for tcs stage, I only need it to verify a bug/fix with r600 buffer textures.)
>> ---
>>  tests/texturing/shaders/common.c  |  3 ++
>>  tests/texturing/shaders/common.h  |  3 +-
>>  tests/texturing/shaders/textureSize.c | 76 
>> ---
>>  3 files changed, 75 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/texturing/shaders/common.c 
>> b/tests/texturing/shaders/common.c
>> index b377bbcae..bda149971 100644
>> --- a/tests/texturing/shaders/common.c
>> +++ b/tests/texturing/shaders/common.c
>> @@ -378,6 +378,9 @@ require_GL_features(enum shader_target test_stage)
>>  glGetIntegerv(GL_MAX_VERTEX_TEXTURE_IMAGE_UNITS, _units);
>>  if (test_stage == VS && tex_units <= 0)
>>  piglit_report_result(PIGLIT_SKIP);
>> +
>> +if (test_stage == TES)
>> +piglit_require_extension("GL_ARB_tessellation_shader");
>>  }
>>  
>>  /**
>> diff --git a/tests/texturing/shaders/common.h 
>> b/tests/texturing/shaders/common.h
>> index 49f38e8b5..3edc68bff 100644
>> --- a/tests/texturing/shaders/common.h
>> +++ b/tests/texturing/shaders/common.h
>> @@ -91,7 +91,8 @@ enum shader_target {
>>  UNKNOWN,
>>  VS,
>>  FS,
>> -GS
>> +GS,
>> +TES,
>>  };
>>  
>>  float max2(float x, float y);
>> diff --git a/tests/texturing/shaders/textureSize.c 
>> b/tests/texturing/shaders/textureSize.c
>> index 2693633fb..3035e0505 100644
>> --- a/tests/texturing/shaders/textureSize.c
>> +++ b/tests/texturing/shaders/textureSize.c
>> @@ -60,7 +60,7 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
>>  piglit_gl_process_args(, argv, );
>>  
>>  parse_args(argc, argv);
>> -if (test_stage == GS) {
>> +if (test_stage == GS || test_stage == TES) {
>>  config.supports_gl_compat_version = 32;
>>  config.supports_gl_core_version = 32;
>>  } else {
>> @@ -154,7 +154,7 @@ piglit_display()
>>  
>>  glUniform1i(lod_location, l);
>>  glViewport(x, 10, 10, 10);
>> -glDrawArrays(GL_TRIANGLE_FAN, 0, 4);
>> +glDrawArrays(test_stage == TES ? GL_PATCHES : GL_TRIANGLE_FAN, 
>> 0, 4);
>>  
>>  pass &= piglit_probe_rect_rgba(x, 10, 10, 10, expected_color);
>>  }
>> @@ -248,12 +248,14 @@ has_lod(void)
>>  int
>>  generate_GLSL(enum shader_target test_stage)
>>  {
>> -int vs, gs = 0, fs;
>> +int vs, gs = 0, tes = 0, tcs = 0, fs;
>>  int prog;
>>  
>>  static char *vs_code;
>>  static char *gs_code = NULL;
>>  static char *fs_code;
>> +static char *tes_code = NULL;
>> +static char *tcs_code = NULL;
>>  char *lod_arg;
>>  static const char *zeroes[3] = { "", "0, ", "0, 0, " };
>>  
>> @@ -357,6 +359,55 @@ generate_GLSL(enum shader_target test_stage)
>>   shader_version, extension, sampler.name, size, lod_arg,
>>   zeroes[3 - size]);
>>  break;
>> +case TES:
>> +(void)!asprintf(_code,
>> + "#version %d\n"
>> + "in vec4 vertex;\n"
>> + "void main()\n"
>> + "{\n"
>> + "gl_Position = vertex;\n"
>> + "}\n",
>> + shader_version);
>> +(void)!asprintf(_code,
>> + "#version %d\n%s"
>> + "#extension GL_ARB_tessellation_shader: require\n"
>> + "#define ivec1 int\n"
>> + "uniform in

Re: [Piglit] [PATCH] arb_texture_buffer_object/indexed: test indexed samplers with tbo

[Roland Scheidegger]

Am 03.01.2018 um 04:40 schrieb Ilia Mirkin:
> Reviewed-by: Ilia Mirkin 
> 
> On Tue, Jan 2, 2018 at 9:27 PM,   wrote:
>> +   static const char *fs_source =
>> +   "#version 150\n"
>> +   "#extension GL_ARB_gpu_shader5: require\n"
>> +   "uniform samplerBuffer s[2];\n"
>> +   "uniform int offset;\n"
>> +   "uniform int index;\n"
> 
> A little trick I like to use in piglits is "uniform int index = 1\n".
> That way it's still a uniform, but I don't have to bother setting it.
> Your call.
> 

Yes, albeit for some reason it's not done that way in most piglits.
Not sure why really, certainly looks like it would make sense for
uniforms which are not meant to be changed.

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

Re: [Piglit] [PATCH] arb_texture_multisample: stress test of very large textures (v3)

[Roland Scheidegger]

Reviewed-by: Roland Scheidegger <srol...@vmware.com>

Am 24.12.2017 um 23:41 schrieb Brian Paul:
> Create the largest possible 2D GL_RGBA_32F multisampled texture, load it
> with known values the read it back and see if the values match up.
> 
> The --array option runs the test with a 2D texture array instead of an
> MSAA texture.  There are other options to specify texture size, number of
> samples, fp16 and a texel value scale.
> 
> Fails with NVIDIA's driver.  See code comments.
> 
> Note: The entry in all.py limits the texture size to 512x512 so it runs
> in a reasonable amount of time.  Ideally, the texture size should be set
> in quick.py instead but I've been unable to make that work.
> 
> v2:
> * Fix a few code issues raised by Fabian Bieler, fix all.py, quick.py code.
> * Update comments about NVIDIA driver, per Roland.
> v3:
> * remove config.khr_no_error_support line.
> ---
>  tests/all.py   |   8 +
>  tests/quick.py |  17 +
>  .../spec/arb_texture_multisample/CMakeLists.gl.txt |   1 +
>  .../arb_texture_multisample/large-float-texture.c  | 727 
> +
>  4 files changed, 753 insertions(+)
>  create mode 100644 tests/spec/arb_texture_multisample/large-float-texture.c
> 
> diff --git a/tests/all.py b/tests/all.py
> index 5d90e8f..d55cca9 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -1651,6 +1651,14 @@ with profile.test_list.group_manager(
>  # Group ARB_texture_multisample
>  with profile.test_list.group_manager(
>  PiglitGLTest, grouptools.join('spec', 'ARB_texture_multisample')) as 
> g:
> +g(['arb_texture_multisample-large-float-texture'], 'large-float-texture',
> +  run_concurrent=False)
> +g(['arb_texture_multisample-large-float-texture', '--array'],
> +  'large-float-texture-array', run_concurrent=False)
> +g(['arb_texture_multisample-large-float-texture', '--fp16'],
> +  'large-float-texture-fp16', run_concurrent=False)
> +g(['arb_texture_multisample-large-float-texture', '--array', '--fp16'],
> +  'large-float-texture-array-fp16', run_concurrent=False)
>  g(['arb_texture_multisample-minmax'])
>  g(['texelFetch', 'fs', 'sampler2DMS', '4', '1x71-501x71'])
>  g(['texelFetch', 'fs', 'sampler2DMS', '4', '1x130-501x130'])
> diff --git a/tests/quick.py b/tests/quick.py
> index 1a7d674..53774e4 100644
> --- a/tests/quick.py
> +++ b/tests/quick.py
> @@ -68,6 +68,23 @@ with profile.test_list.group_manager(
>  with profile.test_list.allow_reassignment:
>  g(['ext_texture_env_combine-combine', '--quick'], 
> 'texture-env-combine')
>  
> +# Limit texture size to 512x512 for some texture_multisample tests.
> +# The default (max supported size) can be pretty slow.
> +with profile.test_list.group_manager(
> +PiglitGLTest,
> +grouptools.join('spec', 'ARB_texture_multisample')) as g:
> +with profile.test_list.allow_reassignment:
> +size_arg = ['--texsize', '512']
> +g(['arb_texture_multisample-large-float-texture'] + size_arg,
> +  'large-float-texture', run_concurrent=False)
> +g(['arb_texture_multisample-large-float-texture', '--array'] +
> +  size_arg, 'large-float-texture-array', run_concurrent=False)
> +g(['arb_texture_multisample-large-float-texture', '--fp16'] +
> +  size_arg, 'large-float-texture-fp16', run_concurrent=False)
> +g(['arb_texture_multisample-large-float-texture', '--array',
> +   '--fp16'] + size_arg,
> +  'large-float-texture-array-fp16', run_concurrent=False)
> +
>  # These take too long
>  profile.filters.append(lambda n, _: '-explosion' not in n)
>  profile.filters.append(FilterVsIn())
> diff --git a/tests/spec/arb_texture_multisample/CMakeLists.gl.txt 
> b/tests/spec/arb_texture_multisample/CMakeLists.gl.txt
> index a347143..31965c4 100644
> --- a/tests/spec/arb_texture_multisample/CMakeLists.gl.txt
> +++ b/tests/spec/arb_texture_multisample/CMakeLists.gl.txt
> @@ -9,6 +9,7 @@ link_libraries (
>   ${OPENGL_gl_LIBRARY}
>  )
>  
> +piglit_add_executable (arb_texture_multisample-large-float-texture 
> large-float-texture.c)
>  piglit_add_executable (arb_texture_multisample-minmax minmax.c)
>  piglit_add_executable (arb_texture_multisample-errors errors.c)
>  piglit_add_executable (arb_texture_multisample-fb-completeness 
> fb-completeness.c)
> diff --git a/tests/spec/arb_texture_multisample/large-float-texture.c 
> b/tests/spec/arb_texture_multisample/large-float-texture.c
> new file mode 100644
> index 000..2e05a6e
> --- /dev/null
> +++ b/tests/spec/arb_texture_multisample/large-float-texture.c
> @@ -0,0 +1,727 @@
> 

Re: [Piglit] [PATCH] draw-vertices-2101010: Accept either SNORM conversion formula.

[Roland Scheidegger]

I'm pretty sure that helps llvmpipe too.

Reviewed-by: Roland Scheidegger <srol...@vmware.com>


Am 26.12.2017 um 07:55 schrieb Kenneth Graunke:
> OpenGL defines two equations for converting from signed-normalized
> to floating point data.  These are:
> 
> f = (2c + 1)/(2^b - 1)(equation 2.2)
> f = max{c/2^(b-1) - 1), -1.0} (equation 2.3)
> 
> ARB_vertex_type_2_10_10_10_rev specifies equation 2.2 is to be used.
> 
> However, OpenGL 4.2 switched to use equation 2.3 in all scenarios.
> This matched an earlier OpenGL ES 3.0 decision to only have one formula,
> as well as a DirectX decision to change to equation 2.3.  Some hardware
> also only supports equation 2.3.  So, basically no one can rely on
> equation 2.2 happening, and some people do rely on 2.3.
> 
> This patch continues to require equation 2.3 for GL 4.2+, but relaxes
> the test to allow either behavior for earlier GL versions.
> 
> See the following discussion for more details:
> https://lists.freedesktop.org/archives/mesa-dev/2013-August/042680.html
> 
> This makes this test pass on i965 with Haswell and later.
> ---
>  .../draw-vertices-2101010.c| 48 
> ++
>  1 file changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git 
> a/tests/spec/arb_vertex_type_2_10_10_10_rev/draw-vertices-2101010.c 
> b/tests/spec/arb_vertex_type_2_10_10_10_rev/draw-vertices-2101010.c
> index 661fa9ca2..5ce4c0655 100644
> --- a/tests/spec/arb_vertex_type_2_10_10_10_rev/draw-vertices-2101010.c
> +++ b/tests/spec/arb_vertex_type_2_10_10_10_rev/draw-vertices-2101010.c
> @@ -187,20 +187,28 @@ static void test_int_vertices_abi(float x1, float y1, 
> float x2, float y2, int in
>  struct test {
>  void (*test)(float x1, float y1, float x2, float y2, int index);
>  int index;
> -float expected_color[4];
> +float expected_color_equation_22[4];
> +float expected_color_equation_23[4];
>  const char *name;
>  };
>  
>  struct test tests[] = {
> -{test_packed_int_vertices, 0, {1, 1, 1, 1}, "Int vertices - 2/10/10/10"},
> -{test_packed_int_vertices, 1, {1, 1, 1, 1}, "Unsigned Int vertices - 
> 2/10/10/10"},
> -{test_packed_int_color_vertices, 0, {1, 0, 0, 0.333}, "Int Color - 
> 2/10/10/10"},
> -{test_packed_int_color_vertices, 1, {1, 0, 0, 0}, "Unsigned Int Color - 
> 2/10/10/10"},
> -{test_packed_int_color_vertices, 2, {0, 0, 1, 0.333}, "Int BGRA Color - 
> 2/10/10/10"},
> -{test_packed_int_color_vertices, 3, {0, 0, 1, 0}, "Unsigned Int BGRA 
> Color - 2/10/10/10"},
> -
> -{test_int_vertices_abi, 0, {1, 0, 0, 1}, "Int 2/10/10/10 - test ABI" },
> -{test_int_vertices_abi, 1, {1, 0, 0, 1}, "Unsigned 2/10/10/10 - test 
> ABI" },
> +{test_packed_int_vertices, 0, {1, 1, 1, 1}, {1, 1, 1, 1},
> + "Int vertices - 2/10/10/10"},
> +{test_packed_int_vertices, 1, {1, 1, 1, 1}, {1, 1, 1, 1},
> + "Unsigned Int vertices - 2/10/10/10"},
> +{test_packed_int_color_vertices, 0, {1, 0, 0, 0.333}, {1, 0, 0, 0},
> + "Int Color - 2/10/10/10"},
> +{test_packed_int_color_vertices, 1, {1, 0, 0, 0}, {1, 0, 0, 0},
> + "Unsigned Int Color - 2/10/10/10"},
> +{test_packed_int_color_vertices, 2, {0, 0, 1, 0.333}, {0, 0, 1, 0},
> + "Int BGRA Color - 2/10/10/10"},
> +{test_packed_int_color_vertices, 3, {0, 0, 1, 0}, {0, 0, 1, 0},
> + "Unsigned Int BGRA Color - 2/10/10/10"},
> +{test_int_vertices_abi, 0, {1, 0, 0, 1}, {1, 0, 0, 1},
> + "Int 2/10/10/10 - test ABI" },
> +{test_int_vertices_abi, 1, {1, 0, 0, 1}, {1, 0, 0, 1},
> + "Unsigned 2/10/10/10 - test ABI" },
>  {0}
>  };
>  
> @@ -213,7 +221,7 @@ piglit_display(void)
>  /* To know what this is all about, see:
>   * http://lists.freedesktop.org/archives/mesa-dev/2013-August/042680.html
>   */
> -GLboolean snorm_equation_23 = piglit_get_gl_version() >= 42;
> +bool require_snorm_equation_23 = piglit_get_gl_version() >= 42;
>  
>  glClear(GL_COLOR_BUFFER_BIT);
>  glEnableClientState(GL_VERTEX_ARRAY);
> @@ -222,14 +230,22 @@ piglit_display(void)
>  for (i = 0; tests[i].test; i++) {
>  glBindBuffer(GL_ARRAY_BUFFER, 0);
>  
> - if (snorm_equation_23 && fabs(tests[i].expected_color[3] - 0.333) < 
> 0.0001)
> - tests[i].expected_color[3] = 0;
> -
> -printf("%s\n", tests[i].name);
>  tests[i].test(x, y, x+20, y+20, tests[i].index);
>  if (!piglit_check_gl_error(GL_NO_ERROR))
>   pig

Re: [Piglit] [PATCH 1/3] arb_texture_multisample: don't use hard-coded sample counts

[Roland Scheidegger]

Am 05.12.2017 um 17:43 schrieb Brian Paul:
> Instead of using a fixed number of samples, query the
> GL_MAX_COLOR_TEXTURE_SAMPLES or GL_MAX_DEPTH_TEXTURE_SAMPLES value
> and use those.
> 
> Fixes failures with llvmpipe and VMware driver when MSAA is disabled.
> ---
>  tests/spec/arb_texture_multisample/errors.c| 10 +-
>  tests/spec/arb_texture_multisample/sample-depth.c  | 12 +++
>  tests/spec/arb_texture_multisample/stencil-clear.c | 23 
> +-
>  .../teximage-2d-multisample.c  | 13 
>  .../teximage-3d-multisample.c  | 13 
>  5 files changed, 49 insertions(+), 22 deletions(-)
> 
> diff --git a/tests/spec/arb_texture_multisample/errors.c 
> b/tests/spec/arb_texture_multisample/errors.c
> index 42cc2c1..08e1696 100644
> --- a/tests/spec/arb_texture_multisample/errors.c
> +++ b/tests/spec/arb_texture_multisample/errors.c
> @@ -44,6 +44,14 @@ piglit_init(int argc, char **argv)
>  
>  GLuint fbo;
>  GLuint tex[2];
> +GLint max_samples;
> +
> +glGetIntegerv(GL_MAX_COLOR_TEXTURE_SAMPLES, _samples);
> +if (max_samples < 1) {
> +   printf("GL_MAX_COLOR_TEXTURE_SAMPLES must be at least one\n");
> +   piglit_report_result(PIGLIT_FAIL);
> +}
Do you want to use the maximum possible or restrict that to something
smaller (in case the value returned is large)?
(Same below, and in the other patches.)
Either way, for the series:
Reviewed-by: Roland Scheidegger <srol...@vmware.com>


> +
>  glGenFramebuffers(1, );
>  
>  glBindFramebuffer(GL_FRAMEBUFFER, fbo);
> @@ -51,7 +59,7 @@ piglit_init(int argc, char **argv)
>  glGenTextures(2, tex);
>  glBindTexture(GL_TEXTURE_2D_MULTISAMPLE_ARRAY, tex[0]);
>  glTexImage3DMultisample(GL_TEXTURE_2D_MULTISAMPLE_ARRAY,
> -4, GL_RGBA, 64, 64, 2, GL_TRUE);
> +max_samples, GL_RGBA, 64, 64, 2, GL_TRUE);
>  
>  if (!piglit_check_gl_error(GL_NO_ERROR)) {
>  printf("should be no error so far\n");
> diff --git a/tests/spec/arb_texture_multisample/sample-depth.c 
> b/tests/spec/arb_texture_multisample/sample-depth.c
> index ef2be19..94bc63d 100644
> --- a/tests/spec/arb_texture_multisample/sample-depth.c
> +++ b/tests/spec/arb_texture_multisample/sample-depth.c
> @@ -38,7 +38,6 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
>  
>  PIGLIT_GL_TEST_CONFIG_END
>  
> -#define NUM_SAMPLES 4
>  #define TEX_WIDTH 64
>  #define TEX_HEIGHT 64
>  
> @@ -93,16 +92,21 @@ void
>  piglit_init(int argc, char **argv)
>  {
>   GLuint tex;
> + int num_samples;
> +
>   piglit_require_extension("GL_ARB_texture_multisample");
>  
> + /* Use the max number of samples for testing */
> + glGetIntegerv(GL_MAX_COLOR_TEXTURE_SAMPLES, _samples);
> +
>   /* setup an fbo with multisample depth texture */
>  
>   glGenTextures(1, );
>   glBindTexture(GL_TEXTURE_2D_MULTISAMPLE, tex);
>   glTexImage2DMultisample(GL_TEXTURE_2D_MULTISAMPLE,
> - NUM_SAMPLES, GL_DEPTH_COMPONENT24,
> - TEX_WIDTH, TEX_HEIGHT,
> - GL_TRUE);
> + num_samples, GL_DEPTH_COMPONENT24,
> + TEX_WIDTH, TEX_HEIGHT,
> + GL_TRUE);
>  
>   glGenFramebuffers(1, );
>   glBindFramebuffer(GL_DRAW_FRAMEBUFFER, fbo);
> diff --git a/tests/spec/arb_texture_multisample/stencil-clear.c 
> b/tests/spec/arb_texture_multisample/stencil-clear.c
> index ca0fd81..413aa41 100644
> --- a/tests/spec/arb_texture_multisample/stencil-clear.c
> +++ b/tests/spec/arb_texture_multisample/stencil-clear.c
> @@ -108,7 +108,7 @@ piglit_display(void)
>   return pass ? PIGLIT_PASS : PIGLIT_FAIL;
>  }
>  
> -GLuint create_fbo(unsigned num_samples)
> +GLuint create_fbo(unsigned num_color_samples, unsigned num_depth_samples)
>  {
>   GLenum tex_target;
>   GLuint texColor;
> @@ -119,17 +119,17 @@ GLuint create_fbo(unsigned num_samples)
>   glGenTextures(1, );
>   glGenTextures(1, );
>  
> - if (num_samples != 0) {
> + if (num_color_samples != 0) {
>   tex_target = GL_TEXTURE_2D_MULTISAMPLE;
>  
>   glBindTexture(tex_target, texZS);
>   glTexImage2DMultisample(
> - tex_target, num_samples, GL_DEPTH32F_STENCIL8,
> + tex_target, num_depth_samples, GL_DEPTH32F_STENCIL8,
>   TEX_WIDTH, TEX_HEIGHT, GL_TRUE);
>  
>   glBindTexture(tex_target, texColor);
>   glTexImage2DMultisample(
> - tex_target, num_samples

Re: [Piglit] [PATCH] fbo-blending-snorm: new test for testing snorm blend behavior

[Roland Scheidegger]

Am 22.11.2017 um 20:46 schrieb Ilia Mirkin:
> On Wed, Nov 22, 2017 at 2:19 PM, Roland Scheidegger <srol...@vmware.com> 
> wrote:
>> Am 22.11.2017 um 20:17 schrieb Roland Scheidegger:
>>> Am 22.11.2017 um 19:45 schrieb Eric Anholt:
>>>> srol...@vmware.com writes:
>>>>
>>>>> From: Roland Scheidegger <srol...@vmware.com>
>>>>>
>>>>> The existing fbo-blending-formats test is mostly useless for anything but
>>>>> unorm formats, since it does not test any values outside [0,1] (neither 
>>>>> for
>>>>> src, dst, intermeidate calculations, blend result).
>>>>> I tried to enhance it but it got too complex, in particular because I 
>>>>> really
>>>>> want to test snorm, not floats (which aren't really validated much 
>>>>> neither),
>>>>> because if you actually use int math for them it's difficult to handle and
>>>>> llvmpipe had lots of bugs. And it's not even obvious from the GL spec when
>>>>> clamping actually happens (in particular, the inverse blend factors will 
>>>>> be
>>>>> in range [0,2]). snorm blending is sort of semi-supported in GL, the
>>>>> presence of EXT_texture_snorm doesn't guarantee it (and nvidia doesn't 
>>>>> support
>>>>> the extension, presumably because they can't or don't want to deal with 
>>>>> the
>>>>> legacy alpha/luminance/intensity formats), and while GL 3.1 introduces the
>>>>> snorm formats too it isn't guaranteed for rendering neither (for GLES OTOH
>>>>> there's a EXT_render_snorm extension), so the format handling of the
>>>>> fbo-blending-formats test isn't really sufficient and not easily 
>>>>> extensible.
>>>>> So, this test will test actual blend behavior with values which will need
>>>>> clamping, and where the intermediate and final values will be negative 
>>>>> (and,
>>>>> for the inverse blend factors, be even larger than one).
>>>>> This passes (now) on llvmpipe, and nvidia blob. softpipe is a complete
>>>>> failure (if there's clamping it will always clamp to [0,1] for starters),
>>>>> and as a matter of fact, softpipe doesn't get the clamping right even with
>>>>> unorm neither, since values outside [0,1] won't get clamped in the tile
>>>>> cache when there's no clamping, hence they'll enter the blend later when
>>>>> blend is enabled unclamped - but there's no test for this (note this is
>>>>> only an issue if the fragment color clamp is disabled).
>>>>
>>>> I thought the ARB_color_buffer_float/render test was trying to cover
>>>> this.
>>>>
>>>
>>> You are quite right, I missed this one also covers blend functionality
>>> (and also covers snorm formats).
>>> It does not however test the really interesting blend combinations,
>>> because it only really tests one/zero blend factors. The really
>>> interesting ones are those with inverse blend factors.
>>> And it looks quite complex to change too (this test actually just draws
>>> one rectangle and relies on the clear color for the dst values).
>>>
>> Oh and FWIW this one also requires EXT_texture_snorm (which at least
>> nvidia doesn't support) for snorm format testing.
> 
> The (NVIDIA) DX10 series didn't support snorm blending. I think the
> DX11 ones do though -- not completely sure.

Yes, they will (it's a required dx11 feature). They just don't bother
with the legacy formats (alpha/intensity/luminance), even for texturing
- if they can't or they just don't want to bother, I don't know (trying
to create a a8_snorm texture will indeed generate a gl error). So you
can use snorm formats just fine (as it's a gl 3.1 feature for
texturing), even for rendering/blending (which isn't guaranteed by gl in
any version), just not the legacy ones, since EXT_texture_snorm isn't
supported.

Roland

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

Re: [Piglit] [PATCH] fbo-blending-snorm: new test for testing snorm blend behavior

[Roland Scheidegger]

Am 22.11.2017 um 20:17 schrieb Roland Scheidegger:
> Am 22.11.2017 um 19:45 schrieb Eric Anholt:
>> srol...@vmware.com writes:
>>
>>> From: Roland Scheidegger <srol...@vmware.com>
>>>
>>> The existing fbo-blending-formats test is mostly useless for anything but
>>> unorm formats, since it does not test any values outside [0,1] (neither for
>>> src, dst, intermeidate calculations, blend result).
>>> I tried to enhance it but it got too complex, in particular because I really
>>> want to test snorm, not floats (which aren't really validated much neither),
>>> because if you actually use int math for them it's difficult to handle and
>>> llvmpipe had lots of bugs. And it's not even obvious from the GL spec when
>>> clamping actually happens (in particular, the inverse blend factors will be
>>> in range [0,2]). snorm blending is sort of semi-supported in GL, the
>>> presence of EXT_texture_snorm doesn't guarantee it (and nvidia doesn't 
>>> support
>>> the extension, presumably because they can't or don't want to deal with the
>>> legacy alpha/luminance/intensity formats), and while GL 3.1 introduces the
>>> snorm formats too it isn't guaranteed for rendering neither (for GLES OTOH
>>> there's a EXT_render_snorm extension), so the format handling of the
>>> fbo-blending-formats test isn't really sufficient and not easily extensible.
>>> So, this test will test actual blend behavior with values which will need
>>> clamping, and where the intermediate and final values will be negative (and,
>>> for the inverse blend factors, be even larger than one).
>>> This passes (now) on llvmpipe, and nvidia blob. softpipe is a complete
>>> failure (if there's clamping it will always clamp to [0,1] for starters),
>>> and as a matter of fact, softpipe doesn't get the clamping right even with
>>> unorm neither, since values outside [0,1] won't get clamped in the tile
>>> cache when there's no clamping, hence they'll enter the blend later when
>>> blend is enabled unclamped - but there's no test for this (note this is
>>> only an issue if the fragment color clamp is disabled).
>>
>> I thought the ARB_color_buffer_float/render test was trying to cover
>> this.
>>
> 
> You are quite right, I missed this one also covers blend functionality
> (and also covers snorm formats).
> It does not however test the really interesting blend combinations,
> because it only really tests one/zero blend factors. The really
> interesting ones are those with inverse blend factors.
> And it looks quite complex to change too (this test actually just draws
> one rectangle and relies on the clear color for the dst values).
> 
Oh and FWIW this one also requires EXT_texture_snorm (which at least
nvidia doesn't support) for snorm format testing.

Roland

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

Re: [Piglit] [PATCH] fbo-blending-snorm: new test for testing snorm blend behavior

[Roland Scheidegger]

Am 22.11.2017 um 19:45 schrieb Eric Anholt:
> srol...@vmware.com writes:
> 
>> From: Roland Scheidegger <srol...@vmware.com>
>>
>> The existing fbo-blending-formats test is mostly useless for anything but
>> unorm formats, since it does not test any values outside [0,1] (neither for
>> src, dst, intermeidate calculations, blend result).
>> I tried to enhance it but it got too complex, in particular because I really
>> want to test snorm, not floats (which aren't really validated much neither),
>> because if you actually use int math for them it's difficult to handle and
>> llvmpipe had lots of bugs. And it's not even obvious from the GL spec when
>> clamping actually happens (in particular, the inverse blend factors will be
>> in range [0,2]). snorm blending is sort of semi-supported in GL, the
>> presence of EXT_texture_snorm doesn't guarantee it (and nvidia doesn't 
>> support
>> the extension, presumably because they can't or don't want to deal with the
>> legacy alpha/luminance/intensity formats), and while GL 3.1 introduces the
>> snorm formats too it isn't guaranteed for rendering neither (for GLES OTOH
>> there's a EXT_render_snorm extension), so the format handling of the
>> fbo-blending-formats test isn't really sufficient and not easily extensible.
>> So, this test will test actual blend behavior with values which will need
>> clamping, and where the intermediate and final values will be negative (and,
>> for the inverse blend factors, be even larger than one).
>> This passes (now) on llvmpipe, and nvidia blob. softpipe is a complete
>> failure (if there's clamping it will always clamp to [0,1] for starters),
>> and as a matter of fact, softpipe doesn't get the clamping right even with
>> unorm neither, since values outside [0,1] won't get clamped in the tile
>> cache when there's no clamping, hence they'll enter the blend later when
>> blend is enabled unclamped - but there's no test for this (note this is
>> only an issue if the fragment color clamp is disabled).
> 
> I thought the ARB_color_buffer_float/render test was trying to cover
> this.
> 

You are quite right, I missed this one also covers blend functionality
(and also covers snorm formats).
It does not however test the really interesting blend combinations,
because it only really tests one/zero blend factors. The really
interesting ones are those with inverse blend factors.
And it looks quite complex to change too (this test actually just draws
one rectangle and relies on the clear color for the dst values).

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

Re: [Piglit] [PATCH] isinf-and-isnan: add clamp / min / max tests

[Roland Scheidegger]

Ping?

Am 09.11.2017 um 19:11 schrieb srol...@vmware.com:
> From: Roland Scheidegger <srol...@vmware.com>
> 
> We expect non-nan results for these (according to d3d10 rules, and at least
> for min/max, also according to ieee rules). The tests will not actually fail
> with other results (since GL's NaN behavior is all undefined), albeit some
> apps may rely on this.
> (We'll use clamp to 0/1 on purpose, which may get optimized to a saturate
> modifier on some hw, and ideally we'd see a non-nan result there too. The
> expected result there is really zero (d3d10 would require this), so if
> it gets decomposed into min/max combo the order is actually important.)
> On r600, right now all 3 give undesired (NaN) results (pending fixes),
> albeit all legal.
> ---
>  tests/spec/glsl-1.30/execution/isinf-and-isnan.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/tests/spec/glsl-1.30/execution/isinf-and-isnan.c 
> b/tests/spec/glsl-1.30/execution/isinf-and-isnan.c
> index 099b5c2..77a7591 100644
> --- a/tests/spec/glsl-1.30/execution/isinf-and-isnan.c
> +++ b/tests/spec/glsl-1.30/execution/isinf-and-isnan.c
> @@ -340,6 +340,7 @@ enum behavior
>   B_FINITE = 1, /* Expected to evaluate to a finite value */
>   B_POSINF = 2, /* Expected to evaluate to +Infinity */
>   B_NEGINF = 3, /* Expected to evaluate to -Infinity */
> + B_FINITE_NANOK = 4, /* Expected finite value, but NaN ok */
>  };
>  
>  struct expression_table_element
> @@ -369,6 +370,10 @@ static struct expression_table_element expressions[] = {
>   { "log(-1.0+z)", B_NAN },
>   { "sqrt(-1.0)", B_NAN },
>   { "sqrt(-1.0+z)", B_NAN },
> + { "clamp(u_nan, 0.0, 1.0)", B_FINITE_NANOK },
> + { "min(u_two, u_nan)", B_FINITE_NANOK },
> + { "max(u_two, u_nan)", B_FINITE_NANOK },
> +
>  };
>  
>  /**
> @@ -446,6 +451,7 @@ test_expr(char *expression, int expected_behavior)
>   "uniform float u_inf;\n" /* Always == +infinity */
>   "uniform float u_minus_inf;\n" /* Always == -infinity */
>   "uniform float u_nan;\n" /* Always == NaN */
> + "uniform float u_two = 2.0;\n" /* To defeat constant folding */
>   "float compute_value() {\n"
>   "  return %s;\n"
>   "}\n",
> @@ -523,6 +529,9 @@ test_expr(char *expression, int expected_behavior)
>   pass = false;
>   }
>   break;
> + case B_FINITE_NANOK:
> + expected_behavior_string = "finite";
> + break;
>   default:
>   expected_behavior_string = "NaN";
>   break;
> 

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

Re: [Piglit] [PATCH] mingw: use default 2MB stack size instead of 1MB

[Roland Scheidegger]

Am 12.10.2017 um 21:19 schrieb Brian Paul:
> On 10/12/2017 12:11 PM, Jose Fonseca wrote:
>> On 12/10/17 17:51, Brian Paul wrote:
>>> On 10/12/2017 08:04 AM, Jose Fonseca wrote:
 The intent here was not so much to match the piglti MSVC build, but
 apps
 build by MSVC in general.

 After all, nothing ever prevented us from setting a huge stack size on
 both MinGW and MSVC alike, as both toolchains allow to congifure the
 stack size to whatever we want.


 The key issue here is that OpenGL driver don't get to pick the apps
 they
 are loaded, and real OpenGL applications will be likely built with MSVC
 instead of Mingw, and therefore will likely only have the MSVC default
 stack size.  And we should err on the side of caution when testing.


 Regardless of the compiler used, if we bump the stack size in piglit,
 one is just increasing the chance that wasteful stack allocations go
 undetected on piglit and blow up on real applications.


 Therefore I suggest we continue to keep 1MB default, and try to fix
 Mesa
 to be less stack hungry.   If that's not pratical here,
>>>
>>> The ir_expression::constant_expression_value() function's local vars
>>> are pretty minimal.  The two that stand out:
>>>
>>>     ir_constant *op[ARRAY_SIZE(this->operands)] = { NULL, };
>>>     ir_constant_data data;
>>>
>>> are only 32 and 128 bytes, respectively (on a 32-bit build).  I'm not
>>> sure what else accounts for the approx 2KB of the activation record.
>>>
>>> I don't see an obvious way to fix the problem.  Even if we could
>>> reduce per-call stack memory, someone could write an evil shader that
>>> adds a thousand or more terms and we'd overflow the stack again.
>>
>> I'm not following...
>>
>> If the application is malicous, it can overflow the stack without OpenGL
>> driver help.  We have to trust the application.  After all, the opengl
>> driver is in the same process.
>>
>> If the application is a browser, who needs to handle untrusted shaders,
>> then it's the application responsibility to ensure it has enough stack
>> to cope with big shaders.
>>
>> And for the sake of argument, if increasing stack size is the solution
>> for malicious shaders, where would one stop?  If increasing stack size
>> is not a solution to malicous shaders, then why is it relevant to the
>> discussion?
> 
> I'm just saying that if we found a way to "fix Mesa to be less stack
> hungry" someone could generate a new "evil" Piglit test that'd still
> breaks things.  Any construct which generates a really deep IR tree and
> is evaluated recursively could run out of stack space.

I think to be safe there, it would be necessary for the compiler to
detect this and refuse compilation (once some recursive call limit is
reached). (But of course not intentionally malicious shaders should
compile if they are at least semi-reasonable.)
Not sure though if that's practical... (I don't know if that piglit test
is really "sane" albeit it doesn't look like it's really meant to stress
this in particular, so it probably would be nice if it would just work.)

Roland


> 
> I wasn't thinking about it, but now that you mention malicious shaders,
> we should probably consider the case of the GLSL compiler running in a
> driver loaded by the X server for indirect rendering.  But that's a
> whole other topic.
> 
> 
>>
 then we should
 try to bump the stack size of specific piglit tests (like those that
 stress the compiler to the extreme, as Cmake allows to set these
 options
 per executable), and only those tests.  Or just mark the affected tests
 as expected fail/skip.
>>>
>>> The op-selection-bool-bvec4-bvec4.frag test is run with
>>> shader_runner.exe.  I think most of the large/complex shaders in
>>> Piglit are run with shader_runner so I don't think it makes much
>>> difference if only shader_runner or all piglit executables are build
>>> with a 2MB stack.
>>>
>>> In any case, I've got a patch which only sets shader_runner's stack
>>> size which I'll post next.
>>>
>>> I guess another option is to change the
>>> op-selection-bool-bvec4-bvec4.frag test to be simpler, but that's just
>>> sweeping the root problem under the rug.  Someone could always craft a
>>> complex shader which blows out the stack.
>>
>> That's fine.  I'm not worried about uber shaders at all.  Malicous or
>> not.
>>
>> What I'm worried is that once you set the stack to 2MB, some appearently
>> innocuous code somewhere in Mesa source tree (probably completely
>> unrelated to GLSL for example) wastefully uses up 1MB of stack, and
>> piglit does not catch the issue because all tests have 2MB of stack.
>>
>> And this is not hypothetical: we had several functions in Mesa related
>> to writing/read/converting pixels to/from textures, eating up wasteful
>> amounts of stack.  And more than once real apps hit stack overflow
>> because of it.  I'd hate to see us increasing the stack 

Re: [Piglit] [PATCH] arb_texture_query_lod: add tolerance for some comparisons

[Roland Scheidegger]

ping?

Am 09.09.2017 um 07:31 schrieb srol...@vmware.com:
> From: Roland Scheidegger <srol...@vmware.com>
> 
> Tolerance was added for the tests a while ago, but it looks like it was
> forgotten for the nearest_biased test (the nearest one has it).
> Also, for the linear test, add tolerance too when comparing x and y lodq
> results - the values should probably be the same mostly, however it's possible
> (due to interpolation inaccuracies) to get values just below 0 or above 3, in
> which case they will get clamped. (Could just do a clamp instead of allowing
> tolerance I suppose, but some tolerance might be allowed in any case there
> too.)
> 
> This is required for llvmpipe (with a in-progress change) to pass.
> ---
>  .../execution/fs-textureQueryLOD-linear.shader_test | 2 +-
>  .../execution/fs-textureQueryLOD-nearest-biased.shader_test | 6 
> +-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git 
> a/tests/spec/arb_texture_query_lod/execution/fs-textureQueryLOD-linear.shader_test
>  
> b/tests/spec/arb_texture_query_lod/execution/fs-textureQueryLOD-linear.shader_test
> index 6afef71..bb2d8ba 100644
> --- 
> a/tests/spec/arb_texture_query_lod/execution/fs-textureQueryLOD-linear.shader_test
> +++ 
> b/tests/spec/arb_texture_query_lod/execution/fs-textureQueryLOD-linear.shader_test
> @@ -60,7 +60,7 @@ void main()
>  }
>  
>  vec2 queried_lod = textureQueryLOD(tex, gl_TexCoord[0].st);
> -if (queried_lod.x != queried_lod.y) {
> +if (!equal(queried_lod.x, queried_lod.y)) {
>   discard;
>  }
>  if (!equal(queried_lod.x, lod)) {
> diff --git 
> a/tests/spec/arb_texture_query_lod/execution/fs-textureQueryLOD-nearest-biased.shader_test
>  
> b/tests/spec/arb_texture_query_lod/execution/fs-textureQueryLOD-nearest-biased.shader_test
> index 1e0c557..4487930 100644
> --- 
> a/tests/spec/arb_texture_query_lod/execution/fs-textureQueryLOD-nearest-biased.shader_test
> +++ 
> b/tests/spec/arb_texture_query_lod/execution/fs-textureQueryLOD-nearest-biased.shader_test
> @@ -51,6 +51,10 @@ GL_ARB_texture_query_lod
>  #define MAX_MIPMAP_LEVEL 3
>  uniform sampler2D tex;
>  uniform float lod;
> +
> +#define tolerance (1.0/255.0)
> +#define equal(x,y) (abs((x) - (y)) <= tolerance)
> +
>  void main()
>  {
>  /* The ARB_texture_query_lod spec says that if TEXTURE_MIN_FILTER is set
> @@ -69,7 +73,7 @@ void main()
>  }
>  
>  vec2 queried_lod = textureQueryLOD(tex, gl_TexCoord[0].st);
> -if (queried_lod.x != min(queried_lod.y, MAX_MIPMAP_LEVEL)) {
> +if (!equal(queried_lod.x, min(queried_lod.y, MAX_MIPMAP_LEVEL))) {
>   discard;
>  }
>  if (queried_lod.x != min(nearest_lod + 1, MAX_MIPMAP_LEVEL)) {
> 

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

Re: [Piglit] [PATCH] gl-3.2/layered-rendering/framebuffertexture: check for MSAA support

[Roland Scheidegger]

Thanks for fixing this.

Reviewed-by: Roland Scheidegger <srol...@vmware.com>


Am 16.08.2017 um 05:30 schrieb Brian Paul:
> Skip testing MSAA textures if GL_MAX_SAMPLES = 0.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102123
> ---
>  tests/spec/gl-3.2/layered-rendering/framebuffertexture.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/tests/spec/gl-3.2/layered-rendering/framebuffertexture.c 
> b/tests/spec/gl-3.2/layered-rendering/framebuffertexture.c
> index 6d11d7e..70ad350 100644
> --- a/tests/spec/gl-3.2/layered-rendering/framebuffertexture.c
> +++ b/tests/spec/gl-3.2/layered-rendering/framebuffertexture.c
> @@ -205,6 +205,18 @@ test_framebuffertexture(GLenum textureType)
>  
>   float expected[] = { 0, 1, 0 };
>  
> + if (textureType == GL_TEXTURE_2D_MULTISAMPLE ||
> + textureType == GL_TEXTURE_2D_MULTISAMPLE_ARRAY) {
> + GLint maxSamples;
> + glGetIntegerv(GL_MAX_SAMPLES, );
> + if (maxSamples == 0) {
> + /* skip */
> + printf("Skipping %s because GL_MAX_SAMPLES=0\n",
> +piglit_get_gl_enum_name(textureType));
> + return true;
> + }
> + }
> +
>   glGenFramebuffers(1, );
>   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
>  
> 

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

Re: [Piglit] [PATCH] ext_framebuffer_multisample: add --alpha-to-one option to coverage test

[Roland Scheidegger]

Yes, it passes here as well but I believe the test allows a lot of
variance - that was just visual inspection. (I believe if you'd just
implement that in the driver as ignore alpha-to-one if alpha-to-coverage
is also active only then you'd get a perfect match.)

Roland

Am 04.08.2017 um 19:28 schrieb Brian Paul:
> It passes here for me w/ NVIDIA's driver but I'll hold off on this until
> I can take a closer look.
> 
> -Brina
> 
> On 08/04/2017 11:11 AM, Roland Scheidegger wrote:
>> I think should mention that the reference probably isn't quite right (if
>> I try it here, the reference is the same with or without alpha-to-one,
>> but with nvidia with alpha-to-one it's a bit different (darker) than
>> without). I think it's because the reference drawing doesn't really take
>> into account that the samples which get written thanks to
>> coverage-to-alpha also had their alpha values changed to 1.0, but I
>> could be wrong and maybe it is correct after all...
>>
>> In any case,
>> Reviewed-by: Roland Scheidegger <srol...@vmware.com>
>>
>> Am 04.08.2017 um 19:03 schrieb Brian Paul:
>>> Alpha-to-one only makes sense when used with alpha-to-coverage.  This
>>> patch adds a flag to the alpha-to-coverage test to exercise
>>> alpha-to-one.
>>> This will fail with drivers which emulate alpha-to-one in the FS.
>>> ---
>>>   tests/all.py  |  4
>>> 
>>>   .../ext_framebuffer_multisample/sample-alpha-to-coverage.cpp  | 11
>>> ---
>>>   2 files changed, 12 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tests/all.py b/tests/all.py
>>> index ae55425..abe6185 100644
>>> --- a/tests/all.py
>>> +++ b/tests/all.py
>>> @@ -3044,6 +3044,10 @@ with profile.test_list.group_manager(
>>>  sample_count, buffer_type],
>>> 'sample-alpha-to-coverage {} {}'.format(
>>> sample_count, buffer_type))
>>> +g(['ext_framebuffer_multisample-sample-alpha-to-coverage',
>>> +   sample_count, buffer_type, '--alpha-to-one'],
>>> +  'sample-alpha-to-coverage-alpha-to-one {} {}'.format(
>>> +  sample_count, buffer_type))
>>>
>>>   for test in ['line-smooth', 'point-smooth', 'polygon-smooth',
>>>'sample-alpha-to-one',
>>> diff --git
>>> a/tests/spec/ext_framebuffer_multisample/sample-alpha-to-coverage.cpp
>>> b/tests/spec/ext_framebuffer_multisample/sample-alpha-to-coverage.cpp
>>> index 7b756ba..32b4161 100644
>>> ---
>>> a/tests/spec/ext_framebuffer_multisample/sample-alpha-to-coverage.cpp
>>> +++
>>> b/tests/spec/ext_framebuffer_multisample/sample-alpha-to-coverage.cpp
>>> @@ -54,11 +54,12 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
>>>   PIGLIT_GL_TEST_CONFIG_END
>>>
>>>   static GLenum buffer_to_test;
>>> +static bool alpha_to_one = false;
>>>
>>>   void
>>>   print_usage_and_exit(char *prog_name)
>>>   {
>>> -printf("Usage: %s  \n"
>>> +printf("Usage: %s   [--alpha-to-one]\n"
>>>  "  where  is one of:\n"
>>>  "color\n"
>>>  "depth\n",
>>> @@ -93,6 +94,10 @@ piglit_init(int argc, char **argv)
>>>   } else
>>>   print_usage_and_exit(argv[0]);
>>>
>>> +if (argc == 4 && strcmp(argv[3], "--alpha-to-one") == 0) {
>>> +alpha_to_one = true;
>>> +}
>>> +
>>>   int pattern_width = piglit_width / 2;
>>>   int pattern_height = piglit_height / num_attachments;
>>>
>>> @@ -133,10 +138,10 @@ piglit_display()
>>>*/
>>>   if(buffer_to_test == GL_COLOR_BUFFER_BIT)
>>>   draw_reference_image(true /* sample_alpha_to_coverage */,
>>> - false /* sample_alpha_to_one */);
>>> + alpha_to_one);
>>>
>>>   draw_test_image(true /* sample_alpha_to_coverage */,
>>> -false /* sample_alpha_to_one */);
>>> +alpha_to_one);
>>>
>>>   pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
>>>
>>>
>>
> 

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

Re: [Piglit] [PATCH] ext_framebuffer_multisample: add --alpha-to-one option to coverage test

[Roland Scheidegger]

I think should mention that the reference probably isn't quite right (if
I try it here, the reference is the same with or without alpha-to-one,
but with nvidia with alpha-to-one it's a bit different (darker) than
without). I think it's because the reference drawing doesn't really take
into account that the samples which get written thanks to
coverage-to-alpha also had their alpha values changed to 1.0, but I
could be wrong and maybe it is correct after all...

In any case,
Reviewed-by: Roland Scheidegger <srol...@vmware.com>

Am 04.08.2017 um 19:03 schrieb Brian Paul:
> Alpha-to-one only makes sense when used with alpha-to-coverage.  This
> patch adds a flag to the alpha-to-coverage test to exercise alpha-to-one.
> This will fail with drivers which emulate alpha-to-one in the FS.
> ---
>  tests/all.py  |  4 
>  .../ext_framebuffer_multisample/sample-alpha-to-coverage.cpp  | 11 
> ---
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/all.py b/tests/all.py
> index ae55425..abe6185 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -3044,6 +3044,10 @@ with profile.test_list.group_manager(
> sample_count, buffer_type],
>'sample-alpha-to-coverage {} {}'.format(
>sample_count, buffer_type))
> +g(['ext_framebuffer_multisample-sample-alpha-to-coverage',
> +   sample_count, buffer_type, '--alpha-to-one'],
> +  'sample-alpha-to-coverage-alpha-to-one {} {}'.format(
> +  sample_count, buffer_type))
>  
>  for test in ['line-smooth', 'point-smooth', 'polygon-smooth',
>   'sample-alpha-to-one',
> diff --git 
> a/tests/spec/ext_framebuffer_multisample/sample-alpha-to-coverage.cpp 
> b/tests/spec/ext_framebuffer_multisample/sample-alpha-to-coverage.cpp
> index 7b756ba..32b4161 100644
> --- a/tests/spec/ext_framebuffer_multisample/sample-alpha-to-coverage.cpp
> +++ b/tests/spec/ext_framebuffer_multisample/sample-alpha-to-coverage.cpp
> @@ -54,11 +54,12 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
>  PIGLIT_GL_TEST_CONFIG_END
>  
>  static GLenum buffer_to_test;
> +static bool alpha_to_one = false;
>  
>  void
>  print_usage_and_exit(char *prog_name)
>  {
> - printf("Usage: %s  \n"
> + printf("Usage: %s   [--alpha-to-one]\n"
>  "  where  is one of:\n"
>  "color\n"
>  "depth\n",
> @@ -93,6 +94,10 @@ piglit_init(int argc, char **argv)
>   } else
>   print_usage_and_exit(argv[0]);
>  
> + if (argc == 4 && strcmp(argv[3], "--alpha-to-one") == 0) {
> + alpha_to_one = true;
> + }
> +
>   int pattern_width = piglit_width / 2;
>   int pattern_height = piglit_height / num_attachments;
>  
> @@ -133,10 +138,10 @@ piglit_display()
>*/
>   if(buffer_to_test == GL_COLOR_BUFFER_BIT)
>   draw_reference_image(true /* sample_alpha_to_coverage */,
> -  false /* sample_alpha_to_one */);
> +  alpha_to_one);
>  
>   draw_test_image(true /* sample_alpha_to_coverage */,
> - false /* sample_alpha_to_one */);
> + alpha_to_one);
>  
>   pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
>  
> 

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

Re: [Piglit] [PATCH] msaa: add test to verify alpha-to-coverage together with alpha-to-one

[Roland Scheidegger]

Am 04.08.2017 um 16:43 schrieb Brian Paul:
> On 08/04/2017 08:32 AM, srol...@vmware.com wrote:
>> From: Roland Scheidegger <srol...@vmware.com>
>>
>> I believe the sole reason alpha-to-one exists at all is that it can be
>> used together with alpha-to-coverage (otherwise why not just output alpha
>> 1.0 in the fs in the first place...).
>> Therefore, it seems quite appropriate to test this together with
>> alpha-to-coverage.
>>
>> Note: the test is a simple copy of sample-alpha-to-coverage with only
>> trivial adjustments. I don't think the common code used actually handles
>> this case correctly for the reference image (as it's the same as when
>> just using alpha-to-coverage, which can't be right), but I haven't looked
>> at it long enough to be able to fix it.
>> It is, however, good enough to pass on my nvidia card, albeit some mesa
>> drivers trying to emulate this feature are definitely broken (I'm looking
>> at you radeons...).
>> ---
>>   tests/all.py   |   5 +
>>   .../ext_framebuffer_multisample/CMakeLists.gl.txt  |   2 +
>>   .../sample-alpha-to-coverage-one.cpp   | 162
>> +
>>   3 files changed, 169 insertions(+)
>>   create mode 100644
>> tests/spec/ext_framebuffer_multisample/sample-alpha-to-coverage-one.cpp
>>
>> diff --git a/tests/all.py b/tests/all.py
>> index fc00d99..bc5b9e0 100644
>> --- a/tests/all.py
>> +++ b/tests/all.py
>> @@ -3028,6 +3028,11 @@ with profile.test_list.group_manager(
>>  sample_count, buffer_type],
>> 'sample-alpha-to-coverage {} {}'.format(
>> sample_count, buffer_type))
>> +   
>> g(['ext_framebuffer_multisample-sample-alpha-to-coverage-one',
>> +   sample_count, buffer_type],
>> +  'sample-alpha-to-coverage-one {} {}'.format(
>> +  sample_count, buffer_type))
>> +
>>
>>   for test in ['line-smooth', 'point-smooth', 'polygon-smooth',
>>'sample-alpha-to-one',
>> diff --git a/tests/spec/ext_framebuffer_multisample/CMakeLists.gl.txt
>> b/tests/spec/ext_framebuffer_multisample/CMakeLists.gl.txt
>> index 6841267..e93c609 100644
>> --- a/tests/spec/ext_framebuffer_multisample/CMakeLists.gl.txt
>> +++ b/tests/spec/ext_framebuffer_multisample/CMakeLists.gl.txt
>> @@ -62,6 +62,8 @@ piglit_add_executable
>> (ext_framebuffer_multisample-samples samples.c)
>>   piglit_add_executable (ext_framebuffer_multisample-sample-coverage
>> common.cpp sample-coverage.cpp)
>>   piglit_add_executable
>> (ext_framebuffer_multisample-sample-alpha-to-coverage common.cpp
>>  draw-buffers-common.cpp sample-alpha-to-coverage.cpp)
>> +piglit_add_executable
>> (ext_framebuffer_multisample-sample-alpha-to-coverage-one common.cpp
>> +   draw-buffers-common.cpp sample-alpha-to-coverage-one.cpp)
>>   piglit_add_executable
>> (ext_framebuffer_multisample-sample-alpha-to-one common.cpp
>>  draw-buffers-common.cpp sample-alpha-to-one.cpp)
>>   piglit_add_executable (ext_framebuffer_multisample-turn-on-off
>> common.cpp turn-on-off.cpp)
>> diff --git
>> a/tests/spec/ext_framebuffer_multisample/sample-alpha-to-coverage-one.cpp
>> b/tests/spec/ext_framebuffer_multisample/sample-alpha-to-coverage-one.cpp
>> new file mode 100644
>> index 000..887893a
>> --- /dev/null
>> +++
>> b/tests/spec/ext_framebuffer_multisample/sample-alpha-to-coverage-one.cpp
>> @@ -0,0 +1,162 @@
>> +/*
>> + * Copyright © 2012 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,
>> + * F

Re: [Piglit] [Review Request (master branch)] srgb_conformance: fix error computation

[Roland Scheidegger]

Reviewed-by: Roland Scheidegger <srol...@vmware.com>

Am 05.05.2017 um 08:47 schrieb Neha Bhende:
> piglit_linear_to_srgb() returns float values in [0,1].  The test was
> comparing it against integer values in [0,255].  This is why test was
> failing.
> 
> Also, fix the overall test pass/fail logic.
> 
> Reviewed-by: Brian Paul<bri...@vmware.com>
> ---
>  tests/spec/arb_framebuffer_srgb/srgb_conformance.c | 15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/spec/arb_framebuffer_srgb/srgb_conformance.c 
> b/tests/spec/arb_framebuffer_srgb/srgb_conformance.c
> index 99018e2..dcf6d2b 100644
> --- a/tests/spec/arb_framebuffer_srgb/srgb_conformance.c
> +++ b/tests/spec/arb_framebuffer_srgb/srgb_conformance.c
> @@ -55,7 +55,8 @@ PIGLIT_GL_TEST_CONFIG_END
>  
>  static enum piglit_result test_format(void)
>  {
> - GLboolean pass = GL_TRUE;
> + bool pass1 = true;
> + bool pass2 = true;
>   GLuint texsrgb, texfb, fb;
>   GLenum status;
>   int i;
> @@ -131,7 +132,7 @@ static enum piglit_result test_format(void)
>   glReadPixels(0, 0, 16, 16, GL_RGBA, GL_FLOAT, [0][0]);
>  
>   for (i = 0; i < 256; i++) {
> - float err = fabs(piglit_linear_to_srgb(readf[i][0]) - (float)i);
> + float err = fabs(piglit_linear_to_srgb(readf[i][0])*255 - 
> (float)i);
>   if (0)
>   printf("readback: %f observed: %f expected: %f\n", 
> readf[i][0],
>   piglit_linear_to_srgb(readf[i][0]), (float)i);
> @@ -140,7 +141,7 @@ static enum piglit_result test_format(void)
>   }
>   if (err > tolerance) {
>   printf("  failed when testing srgb->float result\n");
> - pass = GL_FALSE;
> + pass1 = false;
>   break;
>   }
>   }
> @@ -148,7 +149,7 @@ static enum piglit_result test_format(void)
>  
>   piglit_present_results();
>  
> - piglit_report_subtest_result(pass ? PIGLIT_PASS : PIGLIT_FAIL,
> + piglit_report_subtest_result(pass1 ? PIGLIT_PASS : PIGLIT_FAIL,
>"srgb->linear");
>  
>   /* draw into srgb framebuffer and verify results */
> @@ -195,21 +196,21 @@ static enum piglit_result test_format(void)
>   printf("observed: %d expected: %d\n", readb[i][0], i);
>   if (readb[i][0] != i) {
>   printf("  failed when testing srgb->float->srgb 
> result\n");
> - pass = GL_FALSE;
> + pass2 = GL_FALSE;
>   break;
>   }
>   }
>  
>   piglit_present_results();
>  
> - piglit_report_subtest_result(pass ? PIGLIT_PASS : PIGLIT_FAIL,
> + piglit_report_subtest_result(pass2 ? PIGLIT_PASS : PIGLIT_FAIL,
>"srgb->linear->srgb");
>  
>   glDeleteTextures(1, );
>   glDeleteTextures(1, );
>   glDeleteFramebuffersEXT(1, );
>  
> - return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> + return (pass1 && pass2) ? PIGLIT_PASS : PIGLIT_FAIL;
>  }
>  
>  enum piglit_result piglit_display(void)
> 

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

Re: [Piglit] [PATCH] gl-1.3-alpha_to_coverage_nop: new test of no-op GL_SAMPLE_ALPHA_TO_COVERAGE

[Roland Scheidegger]

This is proabably a feature not a bug in llvmpipe. The reason is that
d3d10 docs state
https://msdn.microsoft.com/de-de/library/windows/desktop/bb205072(v=vs.85).aspx
"Alpha-to-coverage will work regardless of whether or not multisampling
is also turned on in the rasterizer state (by setting MultisampleEnable
to TRUE or FALSE)."
(Albeit I've seen some other docs which stated otherwise...)
Therefore the gallium interface definition has to be that
alpha_to_coverage always applies regardless if multisampling
rasterization state is enabled or not, and the mesa state tracker would
need to translate the alpha_to_coverage bit away, otherwise it's
impossible for a driver to implement both gl and d3d10 behavior.

Reviewed-by: Roland Scheidegger <srol...@vmware.com>

Am 15.09.2016 um 22:16 schrieb Brian Paul:
> Currently fails with llvmpipe.  Fragments with alpha <= 0.5 are discarded.
> Passes with softpipe, NVIDIA driver.
> ---
>  tests/all.py  |  1 +
>  tests/spec/CMakeLists.txt |  1 +
>  tests/spec/gl-1.3/CMakeLists.gl.txt   | 13 +
>  tests/spec/gl-1.3/CMakeLists.txt  |  1 +
>  tests/spec/gl-1.3/alpha_to_coverage_nop.c | 90 
> +++
>  5 files changed, 106 insertions(+)
>  create mode 100644 tests/spec/gl-1.3/CMakeLists.gl.txt
>  create mode 100644 tests/spec/gl-1.3/CMakeLists.txt
>  create mode 100644 tests/spec/gl-1.3/alpha_to_coverage_nop.c
> 
> diff --git a/tests/all.py b/tests/all.py
> index a448331..8610101 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -840,6 +840,7 @@ with profile.group_manager(
>  g(['gl-1.1-xor'])
>  g(['gl-1.1-xor-copypixels'])
>  g(['gl-1.2-texture-base-level'])
> +g(['gl-1.3-alpha_to_coverage'])
>  g(['hiz'], run_concurrent=False)
>  g(['infinite-spot-light'], run_concurrent=False)
>  g(['line-aa-width'], run_concurrent=False)
> diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
> index 9b0f73e..9972785 100644
> --- a/tests/spec/CMakeLists.txt
> +++ b/tests/spec/CMakeLists.txt
> @@ -106,6 +106,7 @@ add_subdirectory (glsl-es-3.00)
>  add_subdirectory (gl-1.0)
>  add_subdirectory (gl-1.1)
>  add_subdirectory (gl-1.2)
> +add_subdirectory (gl-1.3)
>  add_subdirectory (gl-1.4)
>  add_subdirectory (gl-1.5)
>  add_subdirectory (gl-2.0)
> diff --git a/tests/spec/gl-1.3/CMakeLists.gl.txt 
> b/tests/spec/gl-1.3/CMakeLists.gl.txt
> new file mode 100644
> index 000..607d2c1
> --- /dev/null
> +++ b/tests/spec/gl-1.3/CMakeLists.gl.txt
> @@ -0,0 +1,13 @@
> +include_directories(
> + ${GLEXT_INCLUDE_DIR}
> + ${OPENGL_INCLUDE_PATH}
> +)
> +
> +link_libraries (
> + piglitutil_${piglit_target_api}
> + ${OPENGL_gl_LIBRARY}
> +)
> +
> +piglit_add_executable (gl-1.3-alpha_to_coverage_nop alpha_to_coverage_nop.c)
> +
> +# vim: ft=cmake:
> diff --git a/tests/spec/gl-1.3/CMakeLists.txt 
> b/tests/spec/gl-1.3/CMakeLists.txt
> new file mode 100644
> index 000..4a012b9
> --- /dev/null
> +++ b/tests/spec/gl-1.3/CMakeLists.txt
> @@ -0,0 +1 @@
> +piglit_include_target_api()
> \ No newline at end of file
> diff --git a/tests/spec/gl-1.3/alpha_to_coverage_nop.c 
> b/tests/spec/gl-1.3/alpha_to_coverage_nop.c
> new file mode 100644
> index 000..da8323e
> --- /dev/null
> +++ b/tests/spec/gl-1.3/alpha_to_coverage_nop.c
> @@ -0,0 +1,90 @@
> +/*
> + * Copyright © 2016 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.
> + */
> +
> +
> +/**
> + * Test that enabling GL_ALPHA_TO_COVERAGE has no effect for

Re: [Piglit] [PATCH] tests/llvmpipe: Disable gl-1.0-blend-func

[Roland Scheidegger]

Am 09.09.2016 um 23:32 schrieb Adam Jackson:
> On Fri, 2016-09-09 at 12:09 -0700, Ian Romanick wrote:
> 
>> 16 *gigabytes*?  Why?  That has to indicate a bug somewhere...
>> possibly a leak in the test?
> 
> I suspect more likely a leak in llvmpipe, or in how it's driving llvm.
> You're going to end up jitting a fragment shader for every combination
> of 16 src/dst blend factor and five operators which comes out to around
> 1500 specializations. You'd hope that wouldn't hold on to 10M apiece, I
> admit.
> 

More exactly, the test runs 27552 variations.

That said, I've run it here and didn't see high memory consumption -
stayed below 55M for the entire time (but yes it takes ages to run) (the
lru cache size is limited obviously, and old variations freed).
There's two possibilities for leaks:
1) We're doing something wrong when interfacing with llvm (e.g. not
freeing something we should).
2) There's a leak in llvm itself (we're internally using a very old llvm
version still, and the biggest reason for this is we'd need to verify
there's no new memory leaks in llvm which are difficult to track down as
the memory vanishes in generic pools - maybe it's better now but leaks
which weren't noticed in off-line compilation tended to get unnoticed
for quite a while).

So, if there's a memory leak it's going to be dependent on llvm version
- note this doesn't exclude 1) since we'll have to do some things
differently depending on the version.

Roland

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

Re: [Piglit] [PATCH] tex-miplevel-selection: only require glsl 1.30 for textureOffset 2DArrayShadow

[Roland Scheidegger]

Am 19.04.2016 um 19:27 schrieb Jose Fonseca:
> On 19/04/16 18:21, Roland Scheidegger wrote:
>> Am 19.04.2016 um 18:41 schrieb Jose Fonseca:
>>> On 19/04/16 01:32, srol...@vmware.com wrote:
>>>> From: Roland Scheidegger <srol...@vmware.com>
>>>>
>>>> The spec doesn't really say this should work in older versions. It was
>>>> first
>>>> added in glsl 4.30, mentioning it was forgotten (initially part of
>>>> EXT_gpu_shader4, hence should have been added with 1.30), but with the
>>>> wrong
>>>> syntax. Finally fixed in glsl 4.40.
>>>> It does, however, work with nvidia blob with version 130 directive.
>>>> Also works with llvmpipe (with mesa fix).
>>>> ---
>>>>tests/texturing/tex-miplevel-selection.c | 6 --
>>>>1 file changed, 6 deletions(-)
>>>>
>>>> diff --git a/tests/texturing/tex-miplevel-selection.c
>>>> b/tests/texturing/tex-miplevel-selection.c
>>>> index 959bab2..59030b5 100644
>>>> --- a/tests/texturing/tex-miplevel-selection.c
>>>> +++ b/tests/texturing/tex-miplevel-selection.c
>>>> @@ -322,12 +322,6 @@ piglit_init(int argc, char **argv)
>>>>}
>>>>piglit_require_gl_version(NEED_GL3(test) ? 30 : 14);
>>>>
>>>> -if (target == TEX_2D_ARRAY_SHADOW &&
>>>> -test == GL3_TEXTURE_OFFSET) {
>>>> -piglit_require_GLSL_version(430);
>>>> -version = "430";
>>>> -}
>>>> -
>>>>switch (target) {
>>>>case TEX_1D:
>>>>gltarget = GL_TEXTURE_1D;
>>>>
>>>
>>> One can't blame IHVs for taking the spec to the letter, instead of
>>> guessing intent.
>> You are right, seems iffy (albeit if you take the spec _really_
>> literally,
>> it won't work with 430 neither due to the bogus syntax there).
>>
>>> An addition to the change above, how about marking the test as a SKIP
>>> when the IHV throws a error compiling the GLSL (and maybe put a friendly
>>> warning), but still FAIL if the does not throw a compilation error and
>>> produces the wrong results?
>>
>> I think this change can just be dropped. (The test is only about
>> miplevel selection, so the offsets don't really matter all that much.)
>> I mostly just used this quick change to confirm that nvidia supports it
>> all the way down to 130, and to verify the mesa glsl change works.
>>
>> Roland
>>
> 
> As you think best.
> 
> But given the ambiguity in the spec, I think it would be nice to have
> some test in Piglit for it.  In the hope vendors converge, and also to
> quickly check when they don't.
> 
> If the tex-miplevel-selection is the easiest for doing it that, I'd say
> go for it.
> 
> Eitherway, FWIW, the patch is
> 
> Acked-by: Jose Fonseca <jfons...@vmware.com>

I quickly tried to do what you suggested, but due to the way this code
works and the helpers used, we can't actually easily make the test not
fail when compilation fails.
So, maybe a different test would be best, but I'm not working on any.

Roland

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

Re: [Piglit] [PATCH] tex-miplevel-selection: only require glsl 1.30 for textureOffset 2DArrayShadow

[Roland Scheidegger]

Am 19.04.2016 um 18:41 schrieb Jose Fonseca:
> On 19/04/16 01:32, srol...@vmware.com wrote:
>> From: Roland Scheidegger <srol...@vmware.com>
>>
>> The spec doesn't really say this should work in older versions. It was
>> first
>> added in glsl 4.30, mentioning it was forgotten (initially part of
>> EXT_gpu_shader4, hence should have been added with 1.30), but with the
>> wrong
>> syntax. Finally fixed in glsl 4.40.
>> It does, however, work with nvidia blob with version 130 directive.
>> Also works with llvmpipe (with mesa fix).
>> ---
>>   tests/texturing/tex-miplevel-selection.c | 6 --
>>   1 file changed, 6 deletions(-)
>>
>> diff --git a/tests/texturing/tex-miplevel-selection.c
>> b/tests/texturing/tex-miplevel-selection.c
>> index 959bab2..59030b5 100644
>> --- a/tests/texturing/tex-miplevel-selection.c
>> +++ b/tests/texturing/tex-miplevel-selection.c
>> @@ -322,12 +322,6 @@ piglit_init(int argc, char **argv)
>>   }
>>   piglit_require_gl_version(NEED_GL3(test) ? 30 : 14);
>>
>> -if (target == TEX_2D_ARRAY_SHADOW &&
>> -test == GL3_TEXTURE_OFFSET) {
>> -piglit_require_GLSL_version(430);
>> -version = "430";
>> -}
>> -
>>   switch (target) {
>>   case TEX_1D:
>>   gltarget = GL_TEXTURE_1D;
>>
> 
> One can't blame IHVs for taking the spec to the letter, instead of
> guessing intent.
You are right, seems iffy (albeit if you take the spec _really_ literally,
it won't work with 430 neither due to the bogus syntax there).

> An addition to the change above, how about marking the test as a SKIP
> when the IHV throws a error compiling the GLSL (and maybe put a friendly
> warning), but still FAIL if the does not throw a compilation error and
> produces the wrong results?

I think this change can just be dropped. (The test is only about
miplevel selection, so the offsets don't really matter all that much.)
I mostly just used this quick change to confirm that nvidia supports it
all the way down to 130, and to verify the mesa glsl change works.

Roland

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

Re: [Piglit] [PATCH] gl-1.0-simple-readbuffer: new trivial glReadBuffer() test

[Roland Scheidegger]

Am 08.04.2016 um 01:44 schrieb Brian Paul:
> To test a failing assertion regression in Mesa.
> ---
>  tests/all.py  |  1 +
>  tests/spec/gl-1.0/CMakeLists.gl.txt   |  1 +
>  tests/spec/gl-1.0/simple-readbuffer.c | 55 
> +++
>  3 files changed, 57 insertions(+)
>  create mode 100644 tests/spec/gl-1.0/simple-readbuffer.c
> 
> diff --git a/tests/all.py b/tests/all.py
> index e5a79cc..c7088ca 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -1015,6 +1015,7 @@ with profile.group_manager(
>  g(['gl-1.0-readpixsanity'])
>  g(['gl-1.0-logicop'])
>  g(['gl-1.0-no-op-paths'])
> +g(['gl-1.0-simple-readbuffer'])
>  
>  with profile.group_manager(
>  PiglitGLTest,
> diff --git a/tests/spec/gl-1.0/CMakeLists.gl.txt 
> b/tests/spec/gl-1.0/CMakeLists.gl.txt
> index 80b2fde..219b9b1 100644
> --- a/tests/spec/gl-1.0/CMakeLists.gl.txt
> +++ b/tests/spec/gl-1.0/CMakeLists.gl.txt
> @@ -29,6 +29,7 @@ piglit_add_executable (gl-1.0-rastercolor rastercolor.c)
>  piglit_add_executable (gl-1.0-readpixsanity readpix.c)
>  piglit_add_executable (gl-1.0-readpixels-oob readpixels-oob.c)
>  piglit_add_executable (gl-1.0-rendermode-feedback rendermode-feedback.c)
> +piglit_add_executable (gl-1.0-simple-readbuffer simple-readbuffer.c)
>  piglit_add_executable (gl-1.0-swapbuffers-behavior swapbuffers-behavior.c)
>  
>  # vim: ft=cmake:
> diff --git a/tests/spec/gl-1.0/simple-readbuffer.c 
> b/tests/spec/gl-1.0/simple-readbuffer.c
> new file mode 100644
> index 000..e5b67bf
> --- /dev/null
> +++ b/tests/spec/gl-1.0/simple-readbuffer.c
> @@ -0,0 +1,55 @@
> +/*
> + * Copyright (C) 2016 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.
> + */
> +
> +/**
> + * Test that a single glReadBuffer() call works (doesn't crash)
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> + config.supports_gl_compat_version = 10;
> + config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + /* never get here */
> + return PIGLIT_PASS;
> +}
> +
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> + glReadBuffer(GL_FRONT);
> +
> + if (!piglit_check_gl_error(GL_NO_ERROR)) {
> + piglit_report_result(PIGLIT_FAIL);
> + }
> +
> + piglit_report_result(PIGLIT_PASS);
> +}
> 

Is that a new most simple piglit test ;-).

Reviewed-by: Roland Scheidegger <srol...@vmware.com>

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

Re: [Piglit] [PATCH] Add a new GL_ARB_clip_control viewport test (v2)

[Roland Scheidegger]

rn in the current viewport region, regardless of
> + * the clip control settings:
> + *
> + *   +-+-+
> + *   | | |
> + *   |   blue  |  white  |
> + *   | | |
> + *   +-+-+
> + *   | | |
> + *   |   red   |  green  |
> + *   | | |
> + *   +-+-+
> + *
> + * \param invert_y - if true, NDC_Y=-1=top, else NDC_Y=-1=bottom
> + */
> +static void
> +draw_test_pattern(bool invert_y)
> +{
> + /* Since the modelview and projection matrices are identity matrices,
> +  * we're effectively drawing in Normalized Device Coordinates which
> +  * range from [-1,1] in X and Y.
> +  *
> +  * Note: we're careful with our glRectf coordinates so that the
> +  * rect is drawn front-facing.  If a rect is not drawn it must be
> +  * because it was back-face culled by mistake.
> +  */
> +
> + /* lower-left quadrant = red */
> + glColor3fv(red);
> + if (invert_y)
> + glRectf(-1, 0, 0, 1);
> + else
> + glRectf(-1, -1, 0, 0);
> +
> + /* lower-right quadrant = green */
> + glColor3fv(green);
> + if (invert_y)
> + glRectf(0, 0, 1, 1);
> + else
> + glRectf(0, -1, 1, 0);
> +
> + /* upper-left quadrant = blue */
> + glColor3fv(blue);
> + if (invert_y)
> + glRectf(-1, -1, 0, 0);
> + else
> + glRectf(-1, 0, 0, 1);
> +
> + /* upper-right quadrant = white */
> + glColor3fv(white);
> + if (invert_y)
> + glRectf(0, -1, 1, 0);
> + else
> + glRectf(0, 0, 1, 1);
> +}
> +
> +
> +static bool
> +check_test_pattern(int xpos, int ypos)
> +{
> + bool pass = true;
> + int half_w = piglit_width / 2;
> + int half_h = piglit_height / 2;
> + /* coords in center of the color swatches */
> + int x0 = xpos + half_w / 4;
> + int y0 = ypos + half_h / 4;
> + int x1 = xpos + half_w * 3 / 4;
> + int y1 = ypos + half_h * 3 / 4;
> +
> + if (!piglit_probe_pixel_rgb(x0, y0, red)) {
> + printf("wrong color in lower-left quadrant of test pattern\n");
> + pass = false;
> + }
> +
> + if (!piglit_probe_pixel_rgb(x1, y0, green)) {
> + printf("wrong color in lower-right quadrant of test pattern\n");
> + pass = false;
> + }
> +
> + if (!piglit_probe_pixel_rgb(x0, y1, blue)) {
> + printf("wrong color in upper-left quadrant of test pattern\n");
> + pass = false;
> + }
> +
> + if (!piglit_probe_pixel_rgb(x1, y1, white)) {
> + printf("wrong color in upper-right quadrant of test pattern\n");
> + pass = false;
> + }
> +
> + if (!pass) {
> + GLint origin;
> + glGetIntegerv(GL_CLIP_ORIGIN, );
> + printf("GL_CLIP_ORIGIN = %s\n",
> +piglit_get_gl_enum_name(origin));
> + }
> +
> + return pass;
> +}
> +
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + int half_w = piglit_width / 2;
> + int half_h = piglit_height / 2;
> + bool pass = true;
> +
> + /* Test normal GL coordinates */
> + glClipControl(GL_LOWER_LEFT, GL_NEGATIVE_ONE_TO_ONE);
> + glClear(GL_COLOR_BUFFER_BIT);
> +
> + /* Normal GL origin / Draw in upper-left screen quadrant */
> + glViewport(0, half_h, half_w, half_h);
> + draw_test_pattern(false);
> + if (!check_test_pattern(0, half_h))
> + pass = false;
> +
> + /* Normal GL origin / Draw in lower-right screen quadrant */
> + glViewport(half_w, 0, half_w, half_h);
> + draw_test_pattern(false);
> + if (!check_test_pattern(half_w, 0))
> + pass = false;
> +
> +
> + /* Test inverted GL coordinates */
> + glClipControl(GL_UPPER_LEFT, GL_NEGATIVE_ONE_TO_ONE);
> + glClear(GL_COLOR_BUFFER_BIT);
> +
> + /* Inverted GL origin / Draw in upper-left screen quadrant */
> + glViewport(0, half_h, half_w, half_h);
> + draw_test_pattern(true);
> + if (!check_test_pattern(0, half_h))
> + pass = false;
> +
> + /* Inverted GL origin / Draw in lower-right screen quadrant */
> + glViewport(half_w, 0, half_w, half_h);
> + draw_test_pattern(true);
> + if (!check_test_pattern(half_w, 0))
> + pass = false;
> +
> + piglit_present_results();
> +
> + return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> +}
> 

For the series:
Reviewed-by: Roland Scheidegger <srol...@vmware.com>

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

Re: [Piglit] [Mesa-dev] [PATCH] Add (un)packHalf tests which don't fail on GCN

[Roland Scheidegger]

Am 05.02.2016 um 16:08 schrieb Marek Olšák:
> On Fri, Feb 5, 2016 at 3:56 PM, Roland Scheidegger <srol...@vmware.com> wrote:
>> Am 05.02.2016 um 15:44 schrieb Marek Olšák:
>>> On Fri, Feb 5, 2016 at 10:57 AM, Marek Olšák <mar...@gmail.com> wrote:
>>>> On Fri, Feb 5, 2016 at 1:55 AM, Matt Turner <matts...@gmail.com> wrote:
>>>>> On Thu, Feb 4, 2016 at 10:50 AM, Marek Olšák <mar...@gmail.com> wrote:
>>>>>> From: Marek Olšák <marek.ol...@amd.com>
>>>>>>
>>>>>> This is a subset of the generated tests which are known to fail
>>>>>> on everything except CPU emulation (AFAIK).
>>>>>> ---
>>>>>
>>>>> This is really awful. Committing a generated test, but with unknown
>>>>> bits chopped out is gross.
>>>>>
>>>>> If it were me, I'd want to understand why my hardware behaved
>>>>> differently -- not just hack up *different* tests and claim victory.
>>>>>
>>>>> FWIW, the generated tests pass on all Intel hardware exposing
>>>>> ARB_shading_language_packing. Gen7+ has native half-float support, and
>>>>> Gen6 uses the lowering code in lower_packing_builtins.cpp to turn the
>>>>> built-ins into a pile of instructions.
>>>>>
>>>>> If you can identify how AMD hardware behaves differently and can prove
>>>>> that the generator needs to be relaxed or something, that's cool. But
>>>>> as is, I hate this patch.
>>>>>
>>>>> I can't find anything in the AMD docs (I looked at GCN3) about
>>>>> half-precision support, so I can't check my theory that AMD hardware
>>>>> rounds towards zero instead of to-nearest/even like Intel.
>>>>
>>>> Since the tests only fail with very small numbers, I think the problem
>>>> is that denorms are disabled by radeonsi. I can try to confirm that.
>>>>
>>>> The hardware rounds to nearest even. The hw precision is:
>>>> - unpack functions - 0 ULP
>>>> - pack functions = 0.5 ULP
>>>> - input and output denorms are flushed to 0
>>>
>>> Hey Matt,
>>>
>>> I have just confirmed that I was right. After I enable denormals in
>>> hw, the original tests pass. This means that this patch tests the
>>> packing functions but skips denormals.
>>>
>>> Not so awful now, is it? :)
>>>
>>> Sadly, I can't enable denormals on all chips, because they are slow.
>>>
>>> So if I add "-no-denormals" suffix into the test names, I can push this, 
>>> right?
>>>
>>
>> Can't you hack up the generator instead? By the looks of it
>> (gen_builtin_packing_tests.py) it has a list of values which result in
>> denorm f16 values (make_inputs_for_pack_half_2x16). Presumably you could
>> add a test there which uses a different list, not including them.
>>
>> (That said, I'm a bit surprised for conversion to/from fp16 your hw
>> doesn't do fp16 denorms - they'd be required by d3d(11) as well.)
> 
> The hardware can do denormals, they are just slow on all chips except VI.
> 
> GLSL 4.50 doesn't require denormals, thus piglit shouldn't even contain
> tests for them.
> 

I'm not asking about denormals for ordinary operations, just conversion
to fp16 (any fp16 denorm is a fp32 normal). That would be along similar
lines what d3d10 already required - forbids fp32 denorms, but for
instance sampling fp16 surfaces requires you to handle the fp16 denorms
without flushing to zero (that's at least what the docs say - maybe you
can get away without it...). FWIW x86 half-float conversion instructions
(vcvtph2ps, vcvtps2ph) work that way too - even if you have denorms
disabled, that instruction will still produce fp16 denorms (and convert
fp16 denorms to fp32 normals), you can configure rounding mode but
there's no way to disable fp16 denorms.
Albeit gcn3 supports fp16 natively (that is it has actual operations
using them not just conversion), so I suppose it makes sense it would
flush them on conversion too...
But I think you're right glsl shouldn't require them (albeit it is
pretty silent on that topic as far as I can tell - certainly doesn't
require them for fp32).

Roland

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

Re: [Piglit] [Mesa-dev] [PATCH] Add (un)packHalf tests which don't fail on GCN

[Roland Scheidegger]

Am 05.02.2016 um 15:44 schrieb Marek Olšák:
> On Fri, Feb 5, 2016 at 10:57 AM, Marek Olšák  wrote:
>> On Fri, Feb 5, 2016 at 1:55 AM, Matt Turner  wrote:
>>> On Thu, Feb 4, 2016 at 10:50 AM, Marek Olšák  wrote:
 From: Marek Olšák 

 This is a subset of the generated tests which are known to fail
 on everything except CPU emulation (AFAIK).
 ---
>>>
>>> This is really awful. Committing a generated test, but with unknown
>>> bits chopped out is gross.
>>>
>>> If it were me, I'd want to understand why my hardware behaved
>>> differently -- not just hack up *different* tests and claim victory.
>>>
>>> FWIW, the generated tests pass on all Intel hardware exposing
>>> ARB_shading_language_packing. Gen7+ has native half-float support, and
>>> Gen6 uses the lowering code in lower_packing_builtins.cpp to turn the
>>> built-ins into a pile of instructions.
>>>
>>> If you can identify how AMD hardware behaves differently and can prove
>>> that the generator needs to be relaxed or something, that's cool. But
>>> as is, I hate this patch.
>>>
>>> I can't find anything in the AMD docs (I looked at GCN3) about
>>> half-precision support, so I can't check my theory that AMD hardware
>>> rounds towards zero instead of to-nearest/even like Intel.
>>
>> Since the tests only fail with very small numbers, I think the problem
>> is that denorms are disabled by radeonsi. I can try to confirm that.
>>
>> The hardware rounds to nearest even. The hw precision is:
>> - unpack functions - 0 ULP
>> - pack functions = 0.5 ULP
>> - input and output denorms are flushed to 0
> 
> Hey Matt,
> 
> I have just confirmed that I was right. After I enable denormals in
> hw, the original tests pass. This means that this patch tests the
> packing functions but skips denormals.
> 
> Not so awful now, is it? :)
> 
> Sadly, I can't enable denormals on all chips, because they are slow.
> 
> So if I add "-no-denormals" suffix into the test names, I can push this, 
> right?
> 

Can't you hack up the generator instead? By the looks of it
(gen_builtin_packing_tests.py) it has a list of values which result in
denorm f16 values (make_inputs_for_pack_half_2x16). Presumably you could
add a test there which uses a different list, not including them.

(That said, I'm a bit surprised for conversion to/from fp16 your hw
doesn't do fp16 denorms - they'd be required by d3d(11) as well.)

Roland

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

Re: [Piglit] [Mesa-dev] [PATCH] Add (un)packHalf tests which don't fail on GCN

[Roland Scheidegger]

Am 05.02.2016 um 17:04 schrieb Marek Olšák:
> On Fri, Feb 5, 2016 at 4:48 PM, Roland Scheidegger <srol...@vmware.com> wrote:
>> Am 05.02.2016 um 16:08 schrieb Marek Olšák:
>>> On Fri, Feb 5, 2016 at 3:56 PM, Roland Scheidegger <srol...@vmware.com> 
>>> wrote:
>>>> Am 05.02.2016 um 15:44 schrieb Marek Olšák:
>>>>> On Fri, Feb 5, 2016 at 10:57 AM, Marek Olšák <mar...@gmail.com> wrote:
>>>>>> On Fri, Feb 5, 2016 at 1:55 AM, Matt Turner <matts...@gmail.com> wrote:
>>>>>>> On Thu, Feb 4, 2016 at 10:50 AM, Marek Olšák <mar...@gmail.com> wrote:
>>>>>>>> From: Marek Olšák <marek.ol...@amd.com>
>>>>>>>>
>>>>>>>> This is a subset of the generated tests which are known to fail
>>>>>>>> on everything except CPU emulation (AFAIK).
>>>>>>>> ---
>>>>>>>
>>>>>>> This is really awful. Committing a generated test, but with unknown
>>>>>>> bits chopped out is gross.
>>>>>>>
>>>>>>> If it were me, I'd want to understand why my hardware behaved
>>>>>>> differently -- not just hack up *different* tests and claim victory.
>>>>>>>
>>>>>>> FWIW, the generated tests pass on all Intel hardware exposing
>>>>>>> ARB_shading_language_packing. Gen7+ has native half-float support, and
>>>>>>> Gen6 uses the lowering code in lower_packing_builtins.cpp to turn the
>>>>>>> built-ins into a pile of instructions.
>>>>>>>
>>>>>>> If you can identify how AMD hardware behaves differently and can prove
>>>>>>> that the generator needs to be relaxed or something, that's cool. But
>>>>>>> as is, I hate this patch.
>>>>>>>
>>>>>>> I can't find anything in the AMD docs (I looked at GCN3) about
>>>>>>> half-precision support, so I can't check my theory that AMD hardware
>>>>>>> rounds towards zero instead of to-nearest/even like Intel.
>>>>>>
>>>>>> Since the tests only fail with very small numbers, I think the problem
>>>>>> is that denorms are disabled by radeonsi. I can try to confirm that.
>>>>>>
>>>>>> The hardware rounds to nearest even. The hw precision is:
>>>>>> - unpack functions - 0 ULP
>>>>>> - pack functions = 0.5 ULP
>>>>>> - input and output denorms are flushed to 0
>>>>>
>>>>> Hey Matt,
>>>>>
>>>>> I have just confirmed that I was right. After I enable denormals in
>>>>> hw, the original tests pass. This means that this patch tests the
>>>>> packing functions but skips denormals.
>>>>>
>>>>> Not so awful now, is it? :)
>>>>>
>>>>> Sadly, I can't enable denormals on all chips, because they are slow.
>>>>>
>>>>> So if I add "-no-denormals" suffix into the test names, I can push this, 
>>>>> right?
>>>>>
>>>>
>>>> Can't you hack up the generator instead? By the looks of it
>>>> (gen_builtin_packing_tests.py) it has a list of values which result in
>>>> denorm f16 values (make_inputs_for_pack_half_2x16). Presumably you could
>>>> add a test there which uses a different list, not including them.
>>>>
>>>> (That said, I'm a bit surprised for conversion to/from fp16 your hw
>>>> doesn't do fp16 denorms - they'd be required by d3d(11) as well.)
>>>
>>> The hardware can do denormals, they are just slow on all chips except VI.
>>>
>>> GLSL 4.50 doesn't require denormals, thus piglit shouldn't even contain
>>> tests for them.
>>>
>>
>> I'm not asking about denormals for ordinary operations, just conversion
>> to fp16 (any fp16 denorm is a fp32 normal). That would be along similar
>> lines what d3d10 already required - forbids fp32 denorms, but for
>> instance sampling fp16 surfaces requires you to handle the fp16 denorms
>> without flushing to zero (that's at least what the docs say - maybe you
>> can get away without it...). FWIW x86 half-float conversion instructions
>> (vcvtph2ps, vcvtps2ph) work that way too - even if you have denorms
>> disabled, that instruction will still produce fp16 denorms (and convert
>> fp16 denorms to fp32 normals

Re: [Piglit] [Mesa-dev] [PATCH] Add (un)packHalf tests which don't fail on GCN

[Roland Scheidegger]

Am 05.02.2016 um 01:55 schrieb Matt Turner:
> On Thu, Feb 4, 2016 at 10:50 AM, Marek Olšák  wrote:
>> From: Marek Olšák 
>>
>> This is a subset of the generated tests which are known to fail
>> on everything except CPU emulation (AFAIK).
>> ---
> 
> This is really awful. Committing a generated test, but with unknown
> bits chopped out is gross.
> 
> If it were me, I'd want to understand why my hardware behaved
> differently -- not just hack up *different* tests and claim victory.
> 
> FWIW, the generated tests pass on all Intel hardware exposing
> ARB_shading_language_packing. Gen7+ has native half-float support, and
> Gen6 uses the lowering code in lower_packing_builtins.cpp to turn the
> built-ins into a pile of instructions.
I don't think that's surprising. The glsl lowering code (and the core
mesa half float conversion code) were written to match intel hw as far
as I know. And the tests were later written to match those rounding
semantics :-). But, the test passes on nvidia blob as well, so it's not
quite that unreasonable. It would be nice though if the test could tell
if it's failing because it doesn't work at all or because some rounding
is a bit different (which may or may not be allowed by glsl, maybe
someone can figure that out...).

Roland



> 
> If you can identify how AMD hardware behaves differently and can prove
> that the generator needs to be relaxed or something, that's cool. But
> as is, I hate this patch.
> 
> I can't find anything in the AMD docs (I looked at GCN3) about
> half-precision support, so I can't check my theory that AMD hardware
> rounds towards zero instead of to-nearest/even like Intel.
> ___
> 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] arb_texture_view-mipgen: add test to verify correct format is used for mipgen

[Roland Scheidegger]

Am 13.01.2016 um 03:32 schrieb Ilia Mirkin:
> On Tue, Jan 12, 2016 at 8:07 PM,  <srol...@vmware.com> wrote:
>> From: Roland Scheidegger <srol...@vmware.com>
>>
>> At least mesa/st fails this right now (always uses the initial format for mip
>> gen).
>>
>> v2: don't use piglit_display (not actually drawing anything), suggested
>> by Ilia Mirkin.
>> ---
>>  tests/all.py  |   1 +
>>  tests/spec/arb_texture_view/CMakeLists.gl.txt |   1 +
>>  tests/spec/arb_texture_view/mipgen.c  | 104 
>> ++
>>  3 files changed, 106 insertions(+)
>>  create mode 100644 tests/spec/arb_texture_view/mipgen.c
>>
>> diff --git a/tests/all.py b/tests/all.py
>> index d9f88d6..0a97d26 100644
>> --- a/tests/all.py
>> +++ b/tests/all.py
>> @@ -2480,6 +2480,7 @@ with profile.group_manager(
>>  g(['arb_texture_view-cubemap-view'], 'cubemap-view')
>>  g(['arb_texture_view-texture-immutable-levels'], 'immutable_levels')
>>  g(['arb_texture_view-max-level'], 'max-level')
>> +g(['arb_texture_view-mipgen'], 'mipgen')
>>  g(['arb_texture_view-params'], 'params')
>>  g(['arb_texture_view-formats'], 'formats')
>>  g(['arb_texture_view-targets'], 'targets')
>> diff --git a/tests/spec/arb_texture_view/CMakeLists.gl.txt 
>> b/tests/spec/arb_texture_view/CMakeLists.gl.txt
>> index 772f8b4..47b3320 100644
>> --- a/tests/spec/arb_texture_view/CMakeLists.gl.txt
>> +++ b/tests/spec/arb_texture_view/CMakeLists.gl.txt
>> @@ -17,6 +17,7 @@ piglit_add_executable(arb_texture_view-formats formats.c 
>> common.c)
>>  piglit_add_executable(arb_texture_view-getteximage-srgb getteximage-srgb.c)
>>  piglit_add_executable(arb_texture_view-lifetime-format lifetime_format.c 
>> common.c)
>>  piglit_add_executable(arb_texture_view-max-level max-level.c)
>> +piglit_add_executable(arb_texture_view-mipgen mipgen.c)
>>  piglit_add_executable(arb_texture_view-params params.c)
>>  piglit_add_executable(arb_texture_view-queries queries.c)
>>  piglit_add_executable(arb_texture_view-rendering-formats 
>> rendering-formats.c)
>> diff --git a/tests/spec/arb_texture_view/mipgen.c 
>> b/tests/spec/arb_texture_view/mipgen.c
>> new file mode 100644
>> index 000..f29a95c
>> --- /dev/null
>> +++ b/tests/spec/arb_texture_view/mipgen.c
>> @@ -0,0 +1,104 @@
>> +/*
>> + * Copyright © 2016 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.
>> + */
>> +
>> +/**
>> + * Verifies that mipmap generation uses the right format (from view,
>> + * not what was originally specified).
>> + */
>> +
>> +#include "piglit-util-gl.h"
>> +#include "common.h"
>> +
>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>> +
>> +   config.supports_gl_compat_version = 20;
>> +
>> +   config.window_visual = PIGLIT_GL_VISUAL_RGBA | 
>> PIGLIT_GL_VISUAL_DOUBLE;
>> +
>> +PIGLIT_GL_TEST_CONFIG_END
>> +
>> +static const char *TestName = "arb_texture_view-mipgen";
> 
> I don't think this is used anywhere. With that removed, this is
Ahh right. The wonders of copy & paste :-).

Roland


> 
> Reviewed-by: Ilia Mirkin <imir...@alum.mit.edu>
> 
>> +
>> +/**
>> + * Create view with different view format and generate mipmap.
>> +

Re: [Piglit] [PATCH] glsl-1.50-geometry-end-primitive: ensure position data is reasonably aligned

[Roland Scheidegger]

ping?

Am 09.01.2016 um 05:40 schrieb srol...@vmware.com:
> From: Roland Scheidegger <srol...@vmware.com>
> 
> The rect halves comparison is in fact not valid in general. The reason is that
> the same geometry does not have the same precision even if it it just shifted
> by some fixed amount in one direction.
> As an example, some calculated x value near 7 will be near 263 if drawn with
> a viewport x value of 256. The value near 7 has 5 bits more precision -
> when calculating the fixed-point values for rasterization (with subpixel
> accuracy), the lower value thus may be rounded in a different direction
> than the higher one, even with correct rounding (not even entirely sure what
> "correct" rounding here means, nearest-floor or nearest-even or whatever, the
> problem is independent from that). This can then cause different pixels to be
> covered by the primitive.
> This causes a failure with this test in llvmpipe when the rounding mode is
> changed slightly (from "mostly" nearest-away-from-zero to nearest-even). I was
> not able to reproduce this with this test on nvidia or amd hardware, but they
> are definitely affected in theory by the same issue (proven by some quick and
> dirty test).
> So, do floor(tmp*2048) / 2048 to ensure everything is reasonably aligned. This
> still retains some subpixel bits (but only very few).
> 
> This may be a problem with more tests, albeit most tend to use vertex data
> which is already sufficiently aligned (e.g. drawing simple aligned rects).
> 
> v2: use mul/floor/div sequence suggested by Ian Romanick instead of add/sub -
> trying to beat the compiler may not be reliable (as it could reorder
> operations).
> ---
>  tests/spec/glsl-1.50/execution/geometry/end-primitive.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/spec/glsl-1.50/execution/geometry/end-primitive.c 
> b/tests/spec/glsl-1.50/execution/geometry/end-primitive.c
> index 6df3a89..97d8375 100644
> --- a/tests/spec/glsl-1.50/execution/geometry/end-primitive.c
> +++ b/tests/spec/glsl-1.50/execution/geometry/end-primitive.c
> @@ -105,7 +105,9 @@ static const char *spiral_text =
>   "  if (vertex_id % 2 == 1) r += 1.0;\n"
>   "  float max_r = b*sqrt(a*float(num_vertices)) + 1.0;\n"
>   "  r /= max_r;\n"
> - "  return r*vec2(cos(theta), sin(theta));\n"
> + "  vec2 tmp = r*vec2(cos(theta), sin(theta));\n"
> + "  // ensure reasonably aligned vertices\n"
> + "  return floor(tmp * 2048.0f) / 2048.0f;\n"
>   "}\n";
>  
>  /**
> 

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

Re: [Piglit] [PATCH] glsl-1.50-geometry-end-primitive: ensure position data is reasonably aligned

[Roland Scheidegger]

Oops just forget the changes on the quad-invariance test (this is just
what I used to prove the same issue on "real" hw).

Am 09.01.2016 um 02:12 schrieb srol...@vmware.com:
> From: Roland Scheidegger <srol...@vmware.com>
> 
> The rect halves comparison is in fact not valid in general. The reason is that
> the same geometry does not have the same precision even if it it just shifted
> by some fixed amount in one direction.
> As an example, some calculated x value near 7 will be near 263 if drawn with
> a viewport x value of 256. The value near 7 has 5 bits more precision -
> when calculating the fixed-point values for rasterization (with subpixel
> accuracy), the lower value thus may be rounded in a different direction
> than the higher one, even with correct rounding (not even entirely sure what
> "correct" rounding here means, nearest-floor or nearest-even or whatever, the
> problem is independent from that). This can then cause different pixels to be
> covered by the primitive.
> This causes a failure with this test in llvmpipe when the rounding mode is
> changed slightly (from "mostly" nearest-away-from-zero to nearest-even). I was
> not able to reproduce this with this test on nvidia or amd hardware, but they
> are definitely affected in theory by the same issue (proven by some quick and
> dirty test).
> So, simply add then subtract some fixed amount to the position values, which
> ensures there should be no differences in the position later even after
> viewport translate (we use window size + 1 here, so the values in range
> -1.0 - +1.0 will all have the same exponent (just above the window size)).
> (We must do that with uniforms otherwise mesa will optimize the add/sub away -
> this is definitely unsafe math optimization but may well be allowed.)
> 
> This may be a problem with more tests, albeit most tend to use vertex data
> which is already sufficiently aligned (e.g. drawing simple aligned rects).
> ---
>  tests/general/quad-invariance.c| 55 
> +-
>  .../glsl-1.50/execution/geometry/end-primitive.c   | 15 +-
>  2 files changed, 36 insertions(+), 34 deletions(-)
> 
> diff --git a/tests/general/quad-invariance.c b/tests/general/quad-invariance.c
> index b5741d0..fd51dec 100644
> --- a/tests/general/quad-invariance.c
> +++ b/tests/general/quad-invariance.c
> @@ -38,9 +38,13 @@
>  
>  #include "piglit-util-gl.h"
>  
> +#define PATTERN_SIZE 256
> +
>  PIGLIT_GL_TEST_CONFIG_BEGIN
>  
>   config.supports_gl_compat_version = 10;
> + config.window_width = 2*PATTERN_SIZE;
> + config.window_height = PATTERN_SIZE;
>  
>   config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
>  
> @@ -49,41 +53,24 @@ PIGLIT_GL_TEST_CONFIG_END
>  enum piglit_result
>  piglit_display(void)
>  {
> + int i;
>   GLboolean pass = GL_TRUE;
> - float verts[12][2] = {
> + float verts[3][2] = {
>   /* prim 1: left half of screen. */
> - {-1.0, -1.0},
> - { 0.0, -1.0},
> - { 0.0,  1.0},
> - {-1.0,  1.0},
> - /* prim 2: right half of screen. */
> - { 0.0, -1.0},
> - { 1.0, -1.0},
> - { 1.0,  1.0},
> + {-1.0 + 2.0f / PATTERN_SIZE, -1.0},
> + {-1.0 + 2.0f / PATTERN_SIZE,  1.0},
>   { 0.0,  1.0},
> - /* prim 3: somewhere off the screen. */
> - { 2.0, -1.0},
> - { 3.0, -1.0},
> - { 3.0,  1.0},
> - { 2.0,  1.0},
>   };
> - float colors[12][4] = {
> - {1.0, 0.0, 0.0, 0.0},
> - {0.0, 1.0, 0.0, 0.0},
> - {0.0, 0.0, 1.0, 0.0},
> - {1.0, 1.0, 1.0, 0.0},
> -
> - {1.0, 0.0, 0.0, 0.0},
> - {0.0, 1.0, 0.0, 0.0},
> - {0.0, 0.0, 1.0, 0.0},
> - {1.0, 1.0, 1.0, 0.0},
> -
> - {1.0, 0.0, 0.0, 0.0},
> - {0.0, 1.0, 0.0, 0.0},
> - {0.0, 0.0, 1.0, 0.0},
> - {1.0, 1.0, 1.0, 0.0},
> + float colors[3][4] = {
> + {1.0, 0.0, 0.0, 1.0},
> + {1.0, 0.0, 0.0, 1.0},
> + {1.0, 0.0, 0.0, 1.0},
> +
>   };
> +
>   static GLboolean once = GL_TRUE;
> + for (i = 0; i < (1 << 24) && pass; i++) {
> + verts[1][0] += 5.96046447754E-08f * 2.0f;
>  
>   glClearColor(0.0, 0.0, 0.0, 0.0);
>   glClear(GL_COLOR_BUFFER_BIT);
> @@ -94,19 +81,21 @@ piglit_display(void)
>   glEnableClientState(GL_VERTEX_ARRAY);
>  
>   /* left: 1 prim */
> - glDrawArrays(GL_QUADS, 0, 4);
> +

Re: [Piglit] [PATCH] glsl-1.50-geometry-end-primitive: ensure position data is reasonably aligned

[Roland Scheidegger]

Am 09.01.2016 um 04:09 schrieb Ian Romanick:
> On 01/08/2016 05:12 PM, srol...@vmware.com wrote:
>> From: Roland Scheidegger <srol...@vmware.com>
>>
>> The rect halves comparison is in fact not valid in general. The reason is 
>> that
>> the same geometry does not have the same precision even if it it just shifted
>> by some fixed amount in one direction.
>> As an example, some calculated x value near 7 will be near 263 if drawn with
>> a viewport x value of 256. The value near 7 has 5 bits more precision -
>> when calculating the fixed-point values for rasterization (with subpixel
>> accuracy), the lower value thus may be rounded in a different direction
>> than the higher one, even with correct rounding (not even entirely sure what
>> "correct" rounding here means, nearest-floor or nearest-even or whatever, the
>> problem is independent from that). This can then cause different pixels to be
>> covered by the primitive.
>> This causes a failure with this test in llvmpipe when the rounding mode is
>> changed slightly (from "mostly" nearest-away-from-zero to nearest-even). I 
>> was
>> not able to reproduce this with this test on nvidia or amd hardware, but they
>> are definitely affected in theory by the same issue (proven by some quick and
>> dirty test).
>> So, simply add then subtract some fixed amount to the position values, which
>> ensures there should be no differences in the position later even after
>> viewport translate (we use window size + 1 here, so the values in range
>> -1.0 - +1.0 will all have the same exponent (just above the window size)).
>> (We must do that with uniforms otherwise mesa will optimize the add/sub away 
>> -
>> this is definitely unsafe math optimization but may well be allowed.)
>>
>> This may be a problem with more tests, albeit most tend to use vertex data
>> which is already sufficiently aligned (e.g. drawing simple aligned rects).
>> ---
>>  tests/general/quad-invariance.c| 55 
>> +-
>>  .../glsl-1.50/execution/geometry/end-primitive.c   | 15 +-
>>  2 files changed, 36 insertions(+), 34 deletions(-)
>>
>> diff --git a/tests/general/quad-invariance.c 
>> b/tests/general/quad-invariance.c
>> index b5741d0..fd51dec 100644
>> --- a/tests/general/quad-invariance.c
>> +++ b/tests/general/quad-invariance.c
>> @@ -38,9 +38,13 @@
>>  
>>  #include "piglit-util-gl.h"
>>  
>> +#define PATTERN_SIZE 256
>> +
>>  PIGLIT_GL_TEST_CONFIG_BEGIN
>>  
>>  config.supports_gl_compat_version = 10;
>> +config.window_width = 2*PATTERN_SIZE;
>> +config.window_height = PATTERN_SIZE;
>>  
>>  config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
>>  
>> @@ -49,41 +53,24 @@ PIGLIT_GL_TEST_CONFIG_END
>>  enum piglit_result
>>  piglit_display(void)
>>  {
>> +int i;
>>  GLboolean pass = GL_TRUE;
>> -float verts[12][2] = {
>> +float verts[3][2] = {
>>  /* prim 1: left half of screen. */
>> -{-1.0, -1.0},
>> -{ 0.0, -1.0},
>> -{ 0.0,  1.0},
>> -{-1.0,  1.0},
>> -/* prim 2: right half of screen. */
>> -{ 0.0, -1.0},
>> -{ 1.0, -1.0},
>> -{ 1.0,  1.0},
>> +{-1.0 + 2.0f / PATTERN_SIZE, -1.0},
>> +{-1.0 + 2.0f / PATTERN_SIZE,  1.0},
>>  { 0.0,  1.0},
>> -/* prim 3: somewhere off the screen. */
>> -{ 2.0, -1.0},
>> -{ 3.0, -1.0},
>> -{ 3.0,  1.0},
>> -{ 2.0,  1.0},
>>  };
>> -float colors[12][4] = {
>> -{1.0, 0.0, 0.0, 0.0},
>> -{0.0, 1.0, 0.0, 0.0},
>> -{0.0, 0.0, 1.0, 0.0},
>> -{1.0, 1.0, 1.0, 0.0},
>> -
>> -{1.0, 0.0, 0.0, 0.0},
>> -{0.0, 1.0, 0.0, 0.0},
>> -{0.0, 0.0, 1.0, 0.0},
>> -{1.0, 1.0, 1.0, 0.0},
>> -
>> -{1.0, 0.0, 0.0, 0.0},
>> -{0.0, 1.0, 0.0, 0.0},
>> -{0.0, 0.0, 1.0, 0.0},
>> -{1.0, 1.0, 1.0, 0.0},
>> +float colors[3][4] = {
>> +{1.0, 0.0, 0.0, 1.0},
>> +{1.0, 0.0, 0.0, 1.0},
>> +{1.0, 0.0, 0.0, 1.0},
>> +
>>  };
>> +
>>  static GLboolean once = GL_TRUE;
>> +for (i = 0; i < (1 << 24) && pass; i++) {
>> +

Re: [Piglit] [PATCH] polygon-mode-facing: verify facing information is preserved with unfilled prims

[Roland Scheidegger]

Am 04.01.2016 um 20:45 schrieb Jose Fonseca:
> On 22/12/15 18:05, srol...@vmware.com wrote:
>> From: Roland Scheidegger <srol...@vmware.com>
>>
>> This test is quite mean to mesa's draw pipe. llvmpipe/softpipe fail
>> even if
>> front and back fill mode is the same (for points and aa lines as the
>> latter
>> get rendered as tris finally again). Most everything else fails too,
>> before
>> the driver dies in a fire...
> 
> LGTM. Just a few remarks inline.
> 
>> ---
>>   tests/general/CMakeLists.gl.txt |   1 +
>>   tests/general/polygon-mode-facing.c | 381
>> 
>>   2 files changed, 382 insertions(+)
>>   create mode 100644 tests/general/polygon-mode-facing.c
>>
>> diff --git a/tests/general/CMakeLists.gl.txt
>> b/tests/general/CMakeLists.gl.txt
>> index 298f59c..8f291db 100644
>> --- a/tests/general/CMakeLists.gl.txt
>> +++ b/tests/general/CMakeLists.gl.txt
>> @@ -86,6 +86,7 @@ piglit_add_executable (pbo-teximage-tiling-2
>> pbo-teximage-tiling-2.c)
>>   piglit_add_executable (point-line-no-cull point-line-no-cull.c)
>>   piglit_add_executable (point-vertex-id point-vertex-id.c)
>>   piglit_add_executable (polygon-mode-offset polygon-mode-offset.c)
>> +piglit_add_executable (polygon-mode-facing polygon-mode-facing.c)
>>   piglit_add_executable (polygon-mode polygon-mode.c)
>>   piglit_add_executable (polygon-offset polygon-offset.c)
>>   piglit_add_executable (primitive-restart primitive-restart.c)
>> diff --git a/tests/general/polygon-mode-facing.c
>> b/tests/general/polygon-mode-facing.c
>> new file mode 100644
>> index 000..f107616
>> --- /dev/null
>> +++ b/tests/general/polygon-mode-facing.c
>> @@ -0,0 +1,381 @@
>> +/*
>> + * Copyright (c) 2011 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
>> + * 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
>> + * NON-INFRINGEMENT.  IN NO EVENT SHALL VMWARE AND/OR THEIR SUPPLIERS
>> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>> + * SOFTWARE.
>> + */
>> +
>> +/**
>> + * Tests glPolygonMode wrt facing.
>> + * Roland Scheidegger
>> + * December 2015
>> + */
>> +
>> +#include "piglit-util-gl.h"
>> +
>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>> +
>> +config.supports_gl_compat_version = 20;
>> +
>> +config.window_width = 400;
>> +config.window_height = 100;
>> +config.window_visual = PIGLIT_GL_VISUAL_RGB |
>> PIGLIT_GL_VISUAL_DOUBLE;
>> +
>> +PIGLIT_GL_TEST_CONFIG_END
>> +
>> +static const char *TestName = "polygon-mode-facing";
>> +
>> +static const char *vstext =
>> +"#version 130\n"
>> +"\n"
>> +"void main()\n"
>> +"{\n"
>> +"  gl_Position = gl_ModelViewProjectionMatrix * gl_Vertex;\n"
>> +"}\n";
>> +
>> +static const char *fstext =
>> +"#version 130\n"
>> +"\n"
>> +"void main()\n"
>> +"{\n"
>> +"  vec4 color = gl_FrontFacing ? vec4(1.0, 0.0, 0.0, 1.0)\n"
>> +"  : vec4(0.0, 1.0, 0.0, 1.0);\n"
>> +"  gl_FragColor = color;\n"
>> +"}\n";
>> +
>> +
>> +static const GLfloat Colors[2][4] = {
>> +   /* back color */
>> +   {0, 1, 0, 1},
>> +   /* front color 

Re: [Piglit] [PATCH] polygon-mode-facing: verify facing information is preserved with unfilled prims

[Roland Scheidegger]

Ah yes, I'll add that before commiting.

Roland

Am 23.12.2015 um 02:04 schrieb Dylan Baker:
> You'll need to add this to all.py
> 
> On Tue, Dec 22, 2015 at 10:05 AM, <srol...@vmware.com
> <mailto:srol...@vmware.com>> wrote:
> 
> From: Roland Scheidegger <srol...@vmware.com
> <mailto:srol...@vmware.com>>
> 
> This test is quite mean to mesa's draw pipe. llvmpipe/softpipe fail
> even if
> front and back fill mode is the same (for points and aa lines as the
> latter
> get rendered as tris finally again). Most everything else fails too,
> before
> the driver dies in a fire...
> ---
>  tests/general/CMakeLists.gl.txt |   1 +
>  tests/general/polygon-mode-facing.c | 381
> 
>  2 files changed, 382 insertions(+)
>  create mode 100644 tests/general/polygon-mode-facing.c
> 
> diff --git a/tests/general/CMakeLists.gl.txt
> b/tests/general/CMakeLists.gl.txt
> index 298f59c..8f291db 100644
> --- a/tests/general/CMakeLists.gl.txt
> +++ b/tests/general/CMakeLists.gl.txt
> @@ -86,6 +86,7 @@ piglit_add_executable (pbo-teximage-tiling-2
> pbo-teximage-tiling-2.c)
>  piglit_add_executable (point-line-no-cull point-line-no-cull.c)
>  piglit_add_executable (point-vertex-id point-vertex-id.c)
>  piglit_add_executable (polygon-mode-offset polygon-mode-offset.c)
> +piglit_add_executable (polygon-mode-facing polygon-mode-facing.c)
>  piglit_add_executable (polygon-mode polygon-mode.c)
>  piglit_add_executable (polygon-offset polygon-offset.c)
>  piglit_add_executable (primitive-restart primitive-restart.c)
> diff --git a/tests/general/polygon-mode-facing.c
> b/tests/general/polygon-mode-facing.c
> new file mode 100644
> index 000..f107616
> --- /dev/null
> +++ b/tests/general/polygon-mode-facing.c
> @@ -0,0 +1,381 @@
> +/*
> + * Copyright (c) 2011 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
> + * 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
> + * NON-INFRINGEMENT.  IN NO EVENT SHALL VMWARE AND/OR THEIR SUPPLIERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +/**
> + * Tests glPolygonMode wrt facing.
> + * Roland Scheidegger
> + * December 2015
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> +   config.supports_gl_compat_version = 20;
> +
> +   config.window_width = 400;
> +   config.window_height = 100;
> +   config.window_visual = PIGLIT_GL_VISUAL_RGB |
> PIGLIT_GL_VISUAL_DOUBLE;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static const char *TestName = "polygon-mode-facing";
> +
> +static const char *vstext =
> +"#version 130\n"
> +"\n"
> +"void main()\n"
> +"{\n"
> +"  gl_Position = gl_ModelViewProjectionMatrix * gl_Vertex;\n"
> +"}\n";
> +
> +static const char *fstext =
> +"#version 130\n"
> +"\n"
> +"void main()\n"
> +"{\n"
> +"  vec4 color = gl_FrontFacing ? vec4(1.0, 0.0, 0.0, 1.0)\n"
> +"  : vec4(0.0, 1.0, 0.0, 1.0);\n"
> +"  gl_FragColor = color;\n"
> +"}\n";
&g

Re: [Piglit] [PATCH 4/7] all.py: add arb_framebuffer_srgb-srgb_conformance

[Roland Scheidegger]

Am 24.12.2015 um 01:34 schrieb baker.dyla...@gmail.com:
> From: Dylan Baker <baker.dyla...@gmail.com>
> 
> This wasn't added to all.py.
> 
> cc: Roland Scheidegger <srol...@vmware.com>
> Signed-off-by: Dylan Baker <dylanx.c.ba...@intel.com>
> ---
>  tests/all.py | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/all.py b/tests/all.py
> index dce7456..f2cd6ae 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -2053,6 +2053,7 @@ with profile.group_manager(
> 'single-sample'],
>'fbo-fast-clear')
>  g(['arb_framebuffer_srgb-fast-clear-blend'])
> +g(['arb_framebuffer_srgb-srgb_conformance'])
>  
>  with profile.group_manager(
>  PiglitGLTest,
> 

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

Re: [Piglit] [PATCH] cmake: Target C99.

[Roland Scheidegger]

Am 10.12.2015 um 12:05 schrieb Jose Fonseca:
> GCC 5.x and 4.x have different defaults, so it's better to explicitly
> specify the target C standard to keep builds consistent.
> ---
>  CMakeLists.txt| 6 ++
>  tests/shaders/shader_runner.c | 4 +---
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 4df0202..2da1e6f 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -224,6 +224,12 @@ if (NOT MSVC)
>   SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall")
>   ENDIF (CXX_COMPILER_FLAG_WALL)
>  
> + # Target C99.  GCC's default is gnu11 for 5.0 and newer, gnu89 for
> + # older versions.
> + check_c_compiler_flag ("-std=gnu99" C_COMPILER_FLAG_STD_GNU99)
> + if (C_COMPILER_FLAG_STD_GNU99)
> + set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99")
> + endif ()
>   # MSVC does not support C99 variable length arrays
>   CHECK_C_COMPILER_FLAG("-Werror=vla" C_COMPILER_FLAG_WEVLA)
>   IF (C_COMPILER_FLAG_WEVLA)
> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> index 12a46de..966abe9 100644
> --- a/tests/shaders/shader_runner.c
> +++ b/tests/shaders/shader_runner.c
> @@ -973,8 +973,6 @@ leave_state(enum states state, const char *line)
>  void
>  process_shader(GLenum target, unsigned num_shaders, GLuint *shaders)
>  {
> - unsigned i;
> -
>   if (num_shaders == 0)
>   return;
>  
> @@ -983,7 +981,7 @@ process_shader(GLenum target, unsigned num_shaders, 
> GLuint *shaders)
>   glProgramParameteri(prog, GL_PROGRAM_SEPARABLE, GL_TRUE);
>   }
>  
> - for (i = 0; i < num_shaders; i++) {
> + for (unsigned i = 0; i < num_shaders; i++) {
>   glAttachShader(prog, shaders[i]);
>   }
>  
> 

This is fine by me. Albeit it seems a bit strange it just happened to
compile in c89 mode so far without this being an explicit requirement.
But I don't have anything against updating that (implicit?) policy to
c99 (minus whatever msvc still doesn't support...), albeit it's
certainly not me who should say what the requirements need to be.

Reviewed-by: Roland Scheidegger <srol...@vmware.com>

Roland

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

Re: [Piglit] [PATCH] all.py: Actually add_texwrap_target_tests's target parameter to the test.

[Roland Scheidegger]

Am 18.06.2015 um 15:10 schrieb Jose Fonseca:
 The target was only being used for the test name -- all test instances
 were duplicate of one another.
 
 Noticed this because llvmpipe started failing several tests when offset
 was added, and all test command lines were exactly the same.
 ---
  tests/all.py | 12 +++-
  1 file changed, 7 insertions(+), 5 deletions(-)
 
 diff --git a/tests/all.py b/tests/all.py
 index 2839059..f462972 100755
 --- a/tests/all.py
 +++ b/tests/all.py
 @@ -151,13 +151,15 @@ def add_vpfpgeneric(adder, name):
  
  
  def add_texwrap_target_tests(adder, target):
 -adder(['texwrap', 'GL_RGBA8'], 'texwrap {}'.format(target))
 -adder(['texwrap', 'GL_RGBA8', 'offset'],
 +adder(['texwrap', target, 'GL_RGBA8'],
 +  'texwrap {}'.format(target))
 +adder(['texwrap', target, 'GL_RGBA8', 'offset'],
'texwrap {} offset'.format(target))
 -adder(['texwrap', 'GL_RGBA8', 'bordercolor'],
 +adder(['texwrap', target, 'GL_RGBA8', 'bordercolor'],
'texwrap {} bordercolor'.format(target))
 -adder(['texwrap', 'GL_RGBA8', 'proj'], 'texwrap {} proj'.format(target))
 -adder(['texwrap', 'GL_RGBA8', 'proj', 'bordercolor'],
 +adder(['texwrap', target, 'GL_RGBA8', 'proj'],
 +  'texwrap {} proj'.format(target))
 +adder(['texwrap', target, 'GL_RGBA8', 'proj', 'bordercolor'],
'texwrap {} proj bordercolor'.format(target))
  
  
 

Reviewed-by: Roland Scheidegger srol...@vmware.com
___
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.

[Roland Scheidegger]

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

Looks good to me.

Roland

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

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

[Roland Scheidegger]

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

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

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

Re: [Piglit] [PATCH] arb-provoking-vertex-render: a new test that actually checks all prim types

[Roland Scheidegger]

Looks good to me, just a minor nit below. Does not quite check all prim
types as it misses the adjacency ones but I guess that would require
some more strict gl version check...

Reviewed-by: Roland Scheidegger srol...@vmware.com

Am 13.01.2015 um 17:57 schrieb Brian Paul:
 None of the other provoking vertex tests actually check the rendering
 of all the different primitive types with first/last provoking vertex.
 ---
  tests/all.py  |   1 +
  tests/spec/arb_provoking_vertex/CMakeLists.gl.txt |   1 +
  tests/spec/arb_provoking_vertex/render.c  | 346 
 ++
  3 files changed, 348 insertions(+)
  create mode 100644 tests/spec/arb_provoking_vertex/render.c
 
 diff --git a/tests/all.py b/tests/all.py
 index 27bb619..c42f4a9 100644
 --- a/tests/all.py
 +++ b/tests/all.py
 @@ -2178,6 +2178,7 @@ arb_provoking_vertex = {}
  spec['ARB_provoking_vertex'] = arb_provoking_vertex
  add_plain_test(arb_provoking_vertex, ['arb-provoking-vertex-control'])
  add_plain_test(arb_provoking_vertex, ['arb-provoking-vertex-initial'])
 +add_plain_test(arb_provoking_vertex, ['arb-provoking-vertex-render'])
  add_plain_test(arb_provoking_vertex, ['arb-quads-follow-provoking-vertex'])
  add_plain_test(arb_provoking_vertex, ['arb-xfb-before-flatshading'])
  
 diff --git a/tests/spec/arb_provoking_vertex/CMakeLists.gl.txt 
 b/tests/spec/arb_provoking_vertex/CMakeLists.gl.txt
 index d70637c..01db2d9 100644
 --- a/tests/spec/arb_provoking_vertex/CMakeLists.gl.txt
 +++ b/tests/spec/arb_provoking_vertex/CMakeLists.gl.txt
 @@ -11,6 +11,7 @@ link_libraries (
  
  piglit_add_executable (arb-provoking-vertex-control 
 provoking-vertex-control.c)
  piglit_add_executable (arb-provoking-vertex-initial 
 provoking-vertex-initial.c)
 +piglit_add_executable (arb-provoking-vertex-render render.c)
  piglit_add_executable (arb-quads-follow-provoking-vertex 
 quads-follow-provoking-vertex.c)
  piglit_add_executable (arb-xfb-before-flatshading xfb-before-flatshading.c)
  
 diff --git a/tests/spec/arb_provoking_vertex/render.c 
 b/tests/spec/arb_provoking_vertex/render.c
 new file mode 100644
 index 000..c340b4d
 --- /dev/null
 +++ b/tests/spec/arb_provoking_vertex/render.c
 @@ -0,0 +1,346 @@
 +/**
 + * 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.
 + */
 +
 +/**
 + * Test provoking vertex control with rendering.
 + *
 + */
 +
 +#include piglit-util-gl.h
 +
 +PIGLIT_GL_TEST_CONFIG_BEGIN
 + config.supports_gl_compat_version = 10;
 +PIGLIT_GL_TEST_CONFIG_END
 +
 +
 +static const float red[3] = {1, 0, 0},
 + green[3] = {0, 1, 0},
 + blue[3] = {0, 0, 1},
 + yellow[3] = {1, 1, 0},
 + black[3] = {0, 0, 0};
 +
 +
 +/* Do GL_QUADS, GL_QUAD_STRIP obey the provoking vertex control? */
 +static GLboolean quads_pv;
 +
 +
 +void
 +piglit_init(int argc, char **argv)
 +{
 + piglit_require_extension(GL_ARB_provoking_vertex);
 +}
 +
 +
 +static bool
 +test_mode(GLenum prim, GLenum pv_mode)
 +{
 + bool pass = true;
 + const float *expected1, *expected2;
 + int x1 = piglit_width / 4;
 + int x2 = piglit_width * 3 / 4;
 + int y = piglit_height / 2;
 + int dy;
 + int num_black = 0;
 + float dummy[4];
 +
 + glClear(GL_COLOR_BUFFER_BIT);
 +
 + glProvokingVertex(pv_mode);
 +
 + switch (prim) {
 + case GL_LINES:
 + glBegin(GL_LINES);
 + /* first line */
 + glColor3fv(red);
 + glVertex2f(-1, 0);
 + glColor3fv(green);
 + glVertex2f( -0.1, 0);
 + /* second line */
 + glColor3fv(blue);
 + glVertex2f(0.1, 0);
 + glColor3fv(yellow);
 + glVertex2f(1, 0);
 + glEnd();
 + if (pv_mode == GL_FIRST_VERTEX_CONVENTION) {
 + expected1 = red;
 + expected2 = blue

Re: [Piglit] [PATCH] cubemap-mismatch: new test of mismatched cube map faces

[Roland Scheidegger]

LGTM, though the test could even be more mean by trying different
internal formats too.
This is indeed crazy stuff, just an unfortunate side effect of
specifying all faces individually. (Makes you wonder why GL didn't use
TexImage3D for this just like it does for cube map arrays, but I guess
there were reasons for that, texture borders, convolution filters and
other legacy crap probably wouldn't work plus of course cube map array
spec is only quite small because it builds on top of array textures.)

Roland


Am 18.12.2014 um 17:04 schrieb Brian Paul:
 OpenGL actually allows creating a cube map with different sizes and
 formats per-face. Though, it can't actually be used for texturing.
 This test checks that no unexpected error is raised when such a
 texture is created and that the GL actually keeps track of the per-face
 sizes.
 ---
  tests/all.py   |  1 +
  tests/texturing/CMakeLists.gl.txt  |  1 +
  tests/texturing/cubemap-mismatch.c | 98 
 ++
  3 files changed, 100 insertions(+)
  create mode 100644 tests/texturing/cubemap-mismatch.c
 
 diff --git a/tests/all.py b/tests/all.py
 index ba26ebd..5c9893f 100644
 --- a/tests/all.py
 +++ b/tests/all.py
 @@ -2892,6 +2892,7 @@ spec['ARB_texture_cube_map'] = arb_texture_cube_map
  add_msaa_visual_plain_tests(arb_texture_cube_map, 'copyteximage CUBE')
  add_plain_test(arb_texture_cube_map, 'crash-cubemap-order')
  add_plain_test(arb_texture_cube_map, 'cubemap')
 +add_plain_test(arb_texture_cube_map, 'cubemap-mismatch')
  add_concurrent_test(arb_texture_cube_map, 'cubemap-getteximage-pbo')
  arb_texture_cube_map['cubemap npot'] = PiglitGLTest(['cubemap', 'npot'])
  add_plain_test(arb_texture_cube_map, 'cubemap-shader')
 diff --git a/tests/texturing/CMakeLists.gl.txt 
 b/tests/texturing/CMakeLists.gl.txt
 index ce9921a..b77f71f 100644
 --- a/tests/texturing/CMakeLists.gl.txt
 +++ b/tests/texturing/CMakeLists.gl.txt
 @@ -23,6 +23,7 @@ piglit_add_executable (copyteximage-clipping 
 copyteximage-clipping.c)
  piglit_add_executable (crossbar crossbar.c)
  piglit_add_executable (cubemap cubemap.c)
  piglit_add_executable (cubemap-getteximage-pbo cubemap-getteximage-pbo.c)
 +piglit_add_executable (cubemap-mismatch cubemap-mismatch.c)
  piglit_add_executable (cubemap-shader cubemap-shader.c)
  piglit_add_executable (depth-level-clamp depth-level-clamp.c)
  piglit_add_executable (depthstencil-render-miplevels 
 depthstencil-render-miplevels.cpp)
 diff --git a/tests/texturing/cubemap-mismatch.c 
 b/tests/texturing/cubemap-mismatch.c
 new file mode 100644
 index 000..808ec25
 --- /dev/null
 +++ b/tests/texturing/cubemap-mismatch.c
 @@ -0,0 +1,98 @@
 +/*
 + * Copyright (c) 2014 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.
 + */
 +
 +
 +/**
 + * Test cubemap with mismatched face sizes.
 + * It's kind of crazy that OpenGL allows creating cube map textures with
 + * mismatched face sizes, but it is what it is.  Do some basic checks
 + * that no expected errors are raised and the per-face size queries work.
 + */
 +
 +
 +#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
 +
 +
 +enum piglit_result
 +piglit_display(void)
 +{
 + GLuint tex;
 + const int w = 64, h = 64;
 + GLsizei sizes[6][2] = {
 + { w, h },
 + { w/2, h/2 },
 + { w/3, h/3 },
 + { w/2, h/2 },
 + { w, h },
 + { w/2, h/2 }
 + };
 + int face;
 + bool pass = true;
 +
 + glGenTextures(1, tex);
 +
 + glBindTexture(GL_TEXTURE_CUBE_MAP, tex);
 + glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
 + glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
 +

Re: [Piglit] [PATCH] glsl-1.50/execution: add primitive id tests with tri strip primitives

[Roland Scheidegger]

ping?

Am 06.12.2014 um 05:20 schrieb srol...@vmware.com:
 From: Roland Scheidegger srol...@vmware.com
 
 Just using tri strip instead of tri fan.
 Exposing more bugs in llvmpipe/draw...
 ---
  ...imitive-id-no-gs-strip-first-vertex.shader_test | 62 
 ++
  .../execution/primitive-id-no-gs-strip.shader_test | 60 +
  2 files changed, 122 insertions(+)
  create mode 100644 
 tests/spec/glsl-1.50/execution/primitive-id-no-gs-strip-first-vertex.shader_test
  create mode 100644 
 tests/spec/glsl-1.50/execution/primitive-id-no-gs-strip.shader_test
 
 diff --git 
 a/tests/spec/glsl-1.50/execution/primitive-id-no-gs-strip-first-vertex.shader_test
  
 b/tests/spec/glsl-1.50/execution/primitive-id-no-gs-strip-first-vertex.shader_test
 new file mode 100644
 index 000..36c16e8
 --- /dev/null
 +++ 
 b/tests/spec/glsl-1.50/execution/primitive-id-no-gs-strip-first-vertex.shader_test
 @@ -0,0 +1,62 @@
 +# Check proper functioning of the gl_PrimitiveID fragment shader
 +# input, in the case where there is no geometry shader.
 +
 +[require]
 +GLSL = 1.50
 +GL_EXT_provoking_vertex
 +
 +[vertex shader]
 +#version 150
 +
 +in vec4 piglit_vertex;
 +flat out int vertex_id;
 +
 +void main()
 +{
 +  gl_Position = piglit_vertex;
 +  vertex_id = gl_VertexID;
 +}
 +
 +[fragment shader]
 +#version 150
 +
 +flat in int vertex_id;
 +
 +void main()
 +{
 +  /* We draw a triangle strip containing 6 vertices, so the relationship 
 between
 +   * the primitive ID and the input vertex ID's should be:
 +   *
 +   * Primitive ID  Vertex ID's  Provoking vertex ID
 +   *  0 0 1 20
 +   *  1 1 3 21
 +   *  2 2 3 42
 +   *  3 3 5 43
 +   *
 +   * Since vertex_id uses interpolation qualifier flat, it should
 +   * always receive the value from the provoking vertex.  Therefore,
 +   * by the table above, it should always be the same as the
 +   * expected value of gl_PrimitiveID.
 +   */
 +  int expected_primitive_id = vertex_id;
 +  if (expected_primitive_id == gl_PrimitiveID)
 +gl_FragColor = vec4(0.0, 1.0, 0.0, 1.0);
 +  else
 +gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);
 +}
 +
 +[vertex data]
 +piglit_vertex/float/2
 +-1.0 -1.0
 + 1.0 -1.0
 +-1.0  0.0
 + 1.0  0.0
 +-1.0  1.0
 + 1.0  1.0
 +
 +[test]
 +clear color 0.0 0.0 0.0 0.0
 +clear
 +provoking vertex first
 +draw arrays GL_TRIANGLE_STRIP 0 6
 +probe all rgba 0.0 1.0 0.0 1.0
 diff --git 
 a/tests/spec/glsl-1.50/execution/primitive-id-no-gs-strip.shader_test 
 b/tests/spec/glsl-1.50/execution/primitive-id-no-gs-strip.shader_test
 new file mode 100644
 index 000..50ba6b3
 --- /dev/null
 +++ b/tests/spec/glsl-1.50/execution/primitive-id-no-gs-strip.shader_test
 @@ -0,0 +1,60 @@
 +# Check proper functioning of the gl_PrimitiveID fragment shader
 +# input, in the case where there is no geometry shader.
 +
 +[require]
 +GLSL = 1.50
 +
 +[vertex shader]
 +#version 150
 +
 +in vec4 piglit_vertex;
 +flat out int vertex_id;
 +
 +void main()
 +{
 +  gl_Position = piglit_vertex;
 +  vertex_id = gl_VertexID;
 +}
 +
 +[fragment shader]
 +#version 150
 +
 +flat in int vertex_id;
 +
 +void main()
 +{
 +  /* We draw a triangle strip containing 6 vertices, so the relationship 
 between
 +   * the primitive ID and the input vertex ID's should be:
 +   *
 +   * Primitive ID  Vertex ID's  Provoking vertex ID
 +   *  0 0 1 22
 +   *  1 2 1 33
 +   *  2 2 3 44
 +   *  3 4 3 55
 +   *
 +   * Since vertex_id uses interpolation qualifier flat, it should
 +   * always receive the value from the provoking vertex.  Therefore,
 +   * by the table above, it should always be 2 greater than the
 +   * expected value of gl_PrimitiveID.
 +   */
 +  int expected_primitive_id = vertex_id - 2;
 +  if (expected_primitive_id == gl_PrimitiveID)
 +gl_FragColor = vec4(0.0, 1.0, 0.0, 1.0);
 +  else
 +gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);
 +}
 +
 +[vertex data]
 +piglit_vertex/float/2
 +-1.0 -1.0
 + 1.0 -1.0
 +-1.0  0.0
 + 1.0  0.0
 +-1.0  1.0
 + 1.0  1.0
 +
 +[test]
 +clear color 0.0 0.0 0.0 0.0
 +clear
 +draw arrays GL_TRIANGLE_STRIP 0 6
 +probe all rgba 0.0 1.0 0.0 1.0
 

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

Re: [Piglit] [PATCH] glsl-1.50-execution-primitive-id-no-gs-first-vertex: First provoking vertex variation.

[Roland Scheidegger]

One nitpick otherwise looks good to me.

Reviewed-by: Roland Scheidegger srol...@vmware.com

Am 01.12.2014 um 16:57 schrieb jfons...@vmware.com:
 From: José Fonseca jfons...@vmware.com
 
 Adapt the primitive-id-no-gs-first test case to first provoking vertex
 convention, exposing a bug in llvmpipe.
 ---
  tests/shaders/shader_runner.c  | 15 ++
  .../primitive-id-no-gs-first-vertex.shader_test| 63 
 ++
  2 files changed, 78 insertions(+)
  create mode 100644 
 tests/spec/glsl-1.50/execution/primitive-id-no-gs-first-vertex.shader_test
 
 diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
 index 628ef44..fb498fb 100644
 --- a/tests/shaders/shader_runner.c
 +++ b/tests/shaders/shader_runner.c
 @@ -1924,6 +1924,19 @@ set_patch_parameter(const char *line)
  #endif
  }
  
 +void
 +set_provoking_vertex(const char *line)
 +{
 + if (string_match(first, line)) {
 + glProvokingVertexEXT(GL_FIRST_VERTEX_CONVENTION_EXT);
 + } else if (string_match(last, line)) {
 + glProvokingVertexEXT(GL_LAST_VERTEX_CONVENTION_EXT);
 + } else {
 + fprintf(stderr, Unknown provoking vertex parameter `%s'\n, 
 line);
 + piglit_report_result(PIGLIT_FAIL);
 + }
 +}
 +
  static const struct string_to_enum enable_table[] = {
   { GL_CLIP_PLANE0, GL_CLIP_PLANE0 },
   { GL_CLIP_PLANE1, GL_CLIP_PLANE1 },
 @@ -2601,6 +2614,8 @@ piglit_display(void)
   set_parameter(line + strlen(parameter ));
   } else if (string_match(patch parameter , line)) {
   set_patch_parameter(line + strlen(patch parameter ));
 + } else if (string_match(provoking vertex , line)) {
 + set_provoking_vertex(line + strlen(provoking vertex 
 ));
   } else if (string_match(link error, line)) {
   link_error_expected = true;
   if (link_ok) {
 diff --git 
 a/tests/spec/glsl-1.50/execution/primitive-id-no-gs-first-vertex.shader_test 
 b/tests/spec/glsl-1.50/execution/primitive-id-no-gs-first-vertex.shader_test
 new file mode 100644
 index 000..dd97b0f
 --- /dev/null
 +++ 
 b/tests/spec/glsl-1.50/execution/primitive-id-no-gs-first-vertex.shader_test
 @@ -0,0 +1,63 @@
 +# Check proper functioning of the gl_PrimitiveID fragment shader
 +# input, in the case where there is no geometry shader, and provoking vertex
 +# is set to first vertex.
 +
 +[require]
 +GLSL = 1.50
 +GL_EXT_provoking_vertex
 +
 +[vertex shader]
 +#version 150
 +
 +in vec4 piglit_vertex;
 +flat out int vertex_id;
 +
 +void main()
 +{
 +  gl_Position = piglit_vertex;
 +  vertex_id = gl_VertexID;
 +}
 +
 +[fragment shader]
 +#version 150
 +
 +flat in int vertex_id;
 +
 +void main()
 +{
 +  /* We draw a triangle fan containing 6 vertices, so the relationship 
 between
 +   * the primitive ID and the input vertex ID's should be:
 +   *
 +   * Primitive ID  Vertex ID's  Provoking vertex ID
 +   *  0 0 1 21
 +   *  1 0 2 32
 +   *  2 0 3 43
 +   *  3 0 4 54
 +   *
 +   * Since vertex_id uses interpolation qualifier flat, it should
 +   * always receive the value from the provoking vertex.  Therefore,
 +   * by the table above, it should always be 2 greater than the
should be 1 greater

 +   * expected value of gl_PrimitiveID.
 +   */
 +  int expected_primitive_id = vertex_id - 1;
 +  if (expected_primitive_id == gl_PrimitiveID)
 +gl_FragColor = vec4(0.0, 1.0, 0.0, 1.0);
 +  else
 +gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);
 +}
 +
 +[vertex data]
 +piglit_vertex/float/2
 +-1.0 -1.0
 +-1.0  1.0
 + 0.0  1.0
 + 1.0  1.0
 + 1.0  0.0
 + 1.0 -1.0
 +
 +[test]
 +clear color 0.0 0.0 0.0 0.0
 +clear
 +provoking vertex first
 +draw arrays GL_TRIANGLE_FAN 0 6
 +probe all rgba 0.0 1.0 0.0 1.0
 

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

Re: [Piglit] [PATCH 3/4] fp-indirections2: limit max_samples to 256

[Roland Scheidegger]

Am 19.09.2014 17:12, schrieb Brian Paul:
 So that llvmpipe can run in a reasonable amount of time.
 ---
  tests/shaders/fp-indirections2.c |5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/tests/shaders/fp-indirections2.c 
 b/tests/shaders/fp-indirections2.c
 index 0b1ee28..5cf09eb 100644
 --- a/tests/shaders/fp-indirections2.c
 +++ b/tests/shaders/fp-indirections2.c
 @@ -216,6 +216,7 @@ piglit_display(void)
   for(dim = 1; dim = 3; ++dim) {
   samples = 0;
   for(;;) {
 +   printf(Test dim %d samples %d\n, dim, samples);
   result = test(dim, samples);
   if (result != PIGLIT_PASS)
   return result;
 @@ -265,8 +266,8 @@ void piglit_init(int argc, char ** argv)
   max_samples = max_native_tex_instructions;
   }
  
 - if (max_samples  1024)
 - max_samples = 1024;
 + if (max_samples  256)
 + max_samples = 256;
  
   texture_init();
  }
 

This looks good to me. Though 1024 is not an unreasonably high number,
so it could be classified as a llvmpipe bug.
(llvm spends all its time in DominatorTree::dominates, something which
happens with other complex shaders too - not entirely sure if there's
something we could do to easily fix it, though it probably would help if
we'd use real functions for texture sampling at least if the same
sampler/texture combo is used multiple times.)

Roland

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

Re: [Piglit] [PATCH] fp-indirections: limit to 1024 indirections

[Roland Scheidegger]

Am 18.09.2014 16:16, schrieb Brian Paul:
 Reviewed-by: Brian Paul bri...@vmware.com
 
 Just a minor suggestion below (in 2 places).
 
 
 On 09/18/2014 08:09 AM, srol...@vmware.com wrote:
 From: Roland Scheidegger srol...@vmware.com

 The test takes ages (somewhere in the order of an hour) on llvmpipe,
 because
 the driver announces support for one million indirections (some drivers
 actually say billions even). It never gets compiled even, that hour is
 fully
 spent on string concatenation when constructing the shader in the test
 itself,
 which seems rather silly.
 Thus, follow fp-indirections2, which limits it to 1024 indirections.
 If you've
 got that many indirections the hw is pretty much guaranteed to not
 have a limit
 on that anyway.
 ---
   tests/shaders/fp-indirections.c | 20 
   1 file changed, 12 insertions(+), 8 deletions(-)

 diff --git a/tests/shaders/fp-indirections.c
 b/tests/shaders/fp-indirections.c
 index a25432f..927e3dc 100644
 --- a/tests/shaders/fp-indirections.c
 +++ b/tests/shaders/fp-indirections.c
 @@ -187,12 +187,14 @@ GLboolean test_temporary_dest_indirections(void)
   GLboolean pass = GL_TRUE;
   GLuint progname;
   char *prog;
 -GLint indirections_limit;
 +GLint indirections_limit, use_limit;
   GLint count;

   indirections_limit =
 get_program_i(GL_MAX_PROGRAM_TEX_INDIRECTIONS_ARB);

 -count = indirections_limit - 1;
 +use_limit = indirections_limit  1024 ? 1024 : indirections_limit;
 
 use_limit = MIN2(indirections_limit, 1024);
Oh, wasn't aware that macro was defined in piglit as well. I've fixed
that. Thanks!

Roland


 
 
 +
 +count = use_limit - 1;
   printf(testing program with %d indirections from temporary
 dests\n,
  count);
   prog = gen_temporary_dest_indirections(count, progname);
 @@ -206,11 +208,11 @@ GLboolean test_temporary_dest_indirections(void)
   free(prog);
   }

 -count = indirections_limit + 1;
 +count = use_limit + 1;
   printf(testing program with %d indirections from temporary
 dests\n,
  count);
   prog = gen_temporary_dest_indirections(count, progname);
 -if (prog != NULL) {
 +if (prog != NULL  count  indirections_limit) {
   if (get_program_i(GL_PROGRAM_UNDER_NATIVE_LIMITS_ARB)) {
   printf(Program with %d indirections unexpectedly 
  met native limits.\n, count);
 @@ -231,12 +233,14 @@ GLboolean test_temporary_source_indirections(void)
   GLboolean pass = GL_TRUE;
   GLuint progname;
   char *prog;
 -GLint indirections_limit;
 +GLint indirections_limit, use_limit;
   GLint count;

   indirections_limit =
 get_program_i(GL_MAX_PROGRAM_TEX_INDIRECTIONS_ARB);

 -count = indirections_limit - 1;
 +use_limit = indirections_limit  1024 ? 1024 : indirections_limit;
 
 MIN2()
 
 
 +
 +count = use_limit - 1;
   printf(testing program with %d indirections from temporary
 sources\n,
  count);
   prog = gen_temporary_source_indirections(count, progname);
 @@ -250,11 +254,11 @@ GLboolean test_temporary_source_indirections(void)
   free(prog);
   }

 -count = indirections_limit + 1;
 +count = use_limit + 1;
   printf(testing program with %d indirections from temporary
 sources\n,
  count);
   prog = gen_temporary_source_indirections(count, progname);
 -if (prog != NULL) {
 +if (prog != NULL  count  indirections_limit) {
   if (get_program_i(GL_PROGRAM_UNDER_NATIVE_LIMITS_ARB)) {
   printf(Program with %d indirections unexpectedly 
  met native limits.\n, count);

 
 
 

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

Re: [Piglit] [PATCH 1/3] util: add piglit_array_texture() function

[Roland Scheidegger]

,
   GLboolean alpha, GLenum basetype);
  GLuint piglit_depth_texture(GLenum target, GLenum format, int w, int h, int 
 d, GLboolean mip);
 +GLuint piglit_array_texture(GLenum target, GLenum format, int w, int h, int 
 d, GLboolean mip);
  extern float piglit_tolerance[4];
  void piglit_set_tolerance_for_bits(int rbits, int gbits, int bbits, int 
 abits);
  extern void piglit_require_transform_feedback(void);
 

For the series:
Reviewed-by: Roland Scheidegger srol...@vmware.com

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

Re: [Piglit] [PATCH] triangle-rasterization: increase the precision of the test

[Roland Scheidegger]

Am 31.10.2013 20:34, schrieb Ian Romanick:
 On 10/28/2013 04:53 PM, Zack Rusin wrote:
 Increase the subpixel precision to 8 bits. This requires
 changing some of the code to 64 bits to avoid overflows.
 
 I'm not that familiar with this test.  Does this change make this test
 more strict?  Does it make it more strict than the GL spec requires?
I guess so.
I believe this would correspond to the GL_SUBPIXEL_BITS which is
implementation dependent, and the minimum required by GL is just 4, and
not 8.
So maybe it would be better to query this and act accordingly, though I
don't think mesa drivers actually adjust that value according to the hw
capabilities (looks like it's stored in ctx-Const.SubPixelBits, I guess
for gallium we'd need a new PIPE_CAP enum).
Like Jose said, all d3d10 capable hw sould support 8 (or even more), I
don't know about older hw.

Roland


 
 Signed-off-by: Zack Rusin za...@vmware.com
 ---
  tests/general/triangle-rasterization.cpp | 82 
 
  1 file changed, 41 insertions(+), 41 deletions(-)

 diff --git a/tests/general/triangle-rasterization.cpp 
 b/tests/general/triangle-rasterization.cpp
 index 98fa959..2e4143f 100644
 --- a/tests/general/triangle-rasterization.cpp
 +++ b/tests/general/triangle-rasterization.cpp
 @@ -103,7 +103,7 @@ static enum filling_convention_t {
  } filling_convention;
  
  /* Fixed point format */
 -const int FIXED_SHIFT = 4;
 +const int FIXED_SHIFT = 8;
  const int FIXED_ONE = 1  FIXED_SHIFT;
  
  /* Default test size */
 @@ -144,13 +144,13 @@ namespace std {
  }
  
  /* Proper rounding of float to integer */
 -int iround(float v)
 +int64_t iround(float v)
  {
  if (v  0.0f)
  v += 0.5f;
  if (v  0.0f)
  v -= 0.5f;
 -return (int)v;
 +return (int64_t)v;
  }
  
  /* Calculate log2 for integers */
 @@ -169,56 +169,56 @@ void rast_triangle(uint8_t* buffer, uint32_t stride, 
 const Triangle tri)
  float center_offset = -0.5f;
  
  /* 28.4 fixed point coordinates */
 
 The comment isn't correct any more.
 
 -int x1 = iround(FIXED_ONE * (tri[0].x + center_offset));
 -int x2 = iround(FIXED_ONE * (tri[1].x + center_offset));
 -int x3 = iround(FIXED_ONE * (tri[2].x + center_offset));
 +int64_t x1 = iround(FIXED_ONE * (tri[0].x + center_offset));
 +int64_t x2 = iround(FIXED_ONE * (tri[1].x + center_offset));
 +int64_t x3 = iround(FIXED_ONE * (tri[2].x + center_offset));
  
 -int y1 = iround(FIXED_ONE * (tri[0].y + center_offset));
 -int y2 = iround(FIXED_ONE * (tri[1].y + center_offset));
 -int y3 = iround(FIXED_ONE * (tri[2].y + center_offset));
 +int64_t y1 = iround(FIXED_ONE * (tri[0].y + center_offset));
 +int64_t y2 = iround(FIXED_ONE * (tri[1].y + center_offset));
 +int64_t y3 = iround(FIXED_ONE * (tri[2].y + center_offset));
  
  /* Force correct vertex order */
 -const int cross = (x2 - x1) * (y3 - y2) - (y2 - y1) * (x3 - x2);
 +const int64_t cross = (x2 - x1) * (y3 - y2) - (y2 - y1) * (x3 - x2);
  if (cross  0) {
  std::swap(x1, x3);
  std::swap(y1, y3);
  }
  
  /* Deltas */
 -const int dx12 = x1 - x2;
 -const int dx23 = x2 - x3;
 -const int dx31 = x3 - x1;
 +const int64_t dx12 = x1 - x2;
 +const int64_t dx23 = x2 - x3;
 +const int64_t dx31 = x3 - x1;
  
 -const int dy12 = y1 - y2;
 -const int dy23 = y2 - y3;
 -const int dy31 = y3 - y1;
 +const int64_t dy12 = y1 - y2;
 +const int64_t dy23 = y2 - y3;
 +const int64_t dy31 = y3 - y1;
  
  /* Fixed-point deltas */
 -const int fdx12 = dx12  FIXED_SHIFT;
 -const int fdx23 = dx23  FIXED_SHIFT;
 -const int fdx31 = dx31  FIXED_SHIFT;
 +const int64_t fdx12 = dx12  FIXED_SHIFT;
 +const int64_t fdx23 = dx23  FIXED_SHIFT;
 +const int64_t fdx31 = dx31  FIXED_SHIFT;
  
 -const int fdy12 = dy12  FIXED_SHIFT;
 -const int fdy23 = dy23  FIXED_SHIFT;
 -const int fdy31 = dy31  FIXED_SHIFT;
 +const int64_t fdy12 = dy12  FIXED_SHIFT;
 +const int64_t fdy23 = dy23  FIXED_SHIFT;
 +const int64_t fdy31 = dy31  FIXED_SHIFT;
  
  /* Bounding rectangle */
 -int minx = std::min(x1, x2, x3)  FIXED_SHIFT;
 -int maxx = (std::max(x1, x2, x3))  FIXED_SHIFT;
 +int64_t minx = std::min(x1, x2, x3)  FIXED_SHIFT;
 +int64_t maxx = (std::max(x1, x2, x3))  FIXED_SHIFT;
  
 -int miny = (std::min(y1, y2, y3))  FIXED_SHIFT;
 -int maxy = std::max(y1, y2, y3)  FIXED_SHIFT;
 +int64_t miny = (std::min(y1, y2, y3))  FIXED_SHIFT;
 +int64_t maxy = std::max(y1, y2, y3)  FIXED_SHIFT;
  
 -minx = std::max(minx, 0);
 -maxx = std::min(maxx, fbo_width - 1);
 +minx = std::max(minx, (int64_t)0);
 +maxx = std::min(maxx, (int64_t)fbo_width - 1);
  
 -miny = std::max(miny, 0);
 -maxy = std::min(maxy, fbo_height - 1);
 +miny = std::max(miny, (int64_t)0);
 +maxy = std::min(maxy, (int64_t)fbo_height - 1);
  
  /* Half-edge constants */
 -  

Re: [Piglit] [PATCH] arb_shader_texture_lod-texgradcube: Test explicit derivatives for cube maps

[Roland Scheidegger]

Am 04.04.2013 01:55, schrieb Brian Paul:
 On 04/03/2013 05:14 PM, srol...@vmware.com wrote:
 From: Roland Scheideggersrol...@vmware.com

 Similar to arb_shader_texture_lod-texgrad, but using cube maps instead.
 Given the somewhat undefined behavior of explicit gradients with cube
 maps, the main purpose of the test is really to test that those work at
 all, as there doesn't seem to be any other test covering this.
 That said, drivers which simply drop explicit derivatives on the floor
 (like softpipe) pass this perfectly as it simply compares implicit vs.
 explicit behavior (which, given the fuzzy specification, might not be
 really required to be the same here, though given the chosen values, that
 is the major axis derivatives being zero, it might seem like a reasonable
 assumption).
 I guess something which would also test that the implementation is really
 using explicit derivatives instead of implicit ones would also be
 desirable,
 but I'll leave that for now, I couldn't really come up with something.
 ---
   .../execution/CMakeLists.gl.txt|1 +
   .../arb_shader_texture_lod/execution/texgradcube.c |  195
 
   2 files changed, 196 insertions(+)
   create mode 100644
 tests/spec/arb_shader_texture_lod/execution/texgradcube.c

 diff --git
 a/tests/spec/arb_shader_texture_lod/execution/CMakeLists.gl.txt
 b/tests/spec/arb_shader_texture_lod/execution/CMakeLists.gl.txt
 index c403939..2366fa9 100644
 --- a/tests/spec/arb_shader_texture_lod/execution/CMakeLists.gl.txt
 +++ b/tests/spec/arb_shader_texture_lod/execution/CMakeLists.gl.txt
 @@ -10,5 +10,6 @@ link_libraries (
   )

   piglit_add_executable (arb_shader_texture_lod-texgrad texgrad.c)
 +piglit_add_executable (arb_shader_texture_lod-texgradcube texgradcube.c)

   # vim: ft=cmake:
 diff --git a/tests/spec/arb_shader_texture_lod/execution/texgradcube.c
 b/tests/spec/arb_shader_texture_lod/execution/texgradcube.c
 new file mode 100644
 index 000..90f7ddc
 --- /dev/null
 +++ b/tests/spec/arb_shader_texture_lod/execution/texgradcube.c
 @@ -0,0 +1,195 @@
 +/*
 + * Copyright (c) 2013 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
 + * 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
 + * NON-INFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS AND/OR THEIR
 + * SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
 + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 + * DEALINGS IN THE SOFTWARE.
 + *
 + * Authors:
 + *Roland Scheideggersrol...@vmware.com
 + *
 + * Based on arb_shader_texture_lod-texgrad:
 + *Marek Olšákmar...@gmail.com
 + */
 +
 +#include piglit-util-gl-common.h
 +
 +PIGLIT_GL_TEST_CONFIG_BEGIN
 +
 +config.supports_gl_compat_version = 10;
 +
 +config.window_width = 512;
 +config.window_height = 256;
 +config.window_visual = PIGLIT_GL_VISUAL_RGB |
 PIGLIT_GL_VISUAL_DOUBLE;
 +
 +PIGLIT_GL_TEST_CONFIG_END
 +
 +#define TEX_WIDTH 256
 +#define TEX_HEIGHT 256
 +
 +static const float colors[][3] = {
 +{1.0, 0.0, 0.0},
 +{0.0, 1.0, 0.0},
 +{0.0, 0.0, 1.0},
 +{1.0, 1.0, 0.0},
 +{0.0, 1.0, 1.0},
 +{1.0, 0.0, 1.0},
 +{0.5, 0.0, 0.5},
 +{1.0, 1.0, 1.0},
 +};
 +
 +static const char *sh_tex =
 +uniform samplerCube tex;
 +void main()
 +{
 +   gl_FragColor = textureCube(tex, gl_TexCoord[0].xyz);
 +};
 +
 +static const char *sh_texgrad =
 +#extension GL_ARB_shader_texture_lod : enable\n
 +uniform samplerCube tex;
 +void main()
 +{
 +   gl_FragColor = textureCubeGradARB(tex, gl_TexCoord[0].xyz,
 + dFdx(gl_TexCoord[0].xyz),
 + dFdy(gl_TexCoord[0].xyz));
 +};
 +
 +static GLint prog_tex, prog_texgrad;
 +
 +void piglit_init(int argc, char **argv)
 +{
 +GLuint tex, fb;
 +GLenum status;
 +int i, j, dim;
 +static GLuint fs_tex, fs_texgrad;
 +
 +piglit_require_GLSL();
 +piglit_require_extension(GL_EXT_framebuffer_object);
 +piglit_require_extension(GL_ARB_shader_texture_lod);
 +
 +fs_tex = piglit_compile_shader_text(GL_FRAGMENT_SHADER, sh_tex);
 +fs_texgrad = 

Re: [Piglit] [PATCH] arb_shader_texture_lod-texgradcube: Test explicit derivatives for cube maps

[Roland Scheidegger]

Am 04.04.2013 17:48, schrieb Brian Paul:
 On 04/04/2013 09:33 AM, Roland Scheidegger wrote:
 Am 04.04.2013 01:55, schrieb Brian Paul:
 On 04/03/2013 05:14 PM, srol...@vmware.com wrote:
 From: Roland Scheideggersrol...@vmware.com

 Similar to arb_shader_texture_lod-texgrad, but using cube maps instead.
 Given the somewhat undefined behavior of explicit gradients with cube
 maps, the main purpose of the test is really to test that those work at
 all, as there doesn't seem to be any other test covering this.
 That said, drivers which simply drop explicit derivatives on the floor
 (like softpipe) pass this perfectly as it simply compares implicit vs.
 explicit behavior (which, given the fuzzy specification, might not be
 really required to be the same here, though given the chosen values,
 that
 is the major axis derivatives being zero, it might seem like a
 reasonable
 assumption).
 I guess something which would also test that the implementation is
 really
 using explicit derivatives instead of implicit ones would also be
 desirable,
 but I'll leave that for now, I couldn't really come up with something.
 ---
.../execution/CMakeLists.gl.txt|1 +
.../arb_shader_texture_lod/execution/texgradcube.c |  195
 
2 files changed, 196 insertions(+)
create mode 100644
 tests/spec/arb_shader_texture_lod/execution/texgradcube.c

 diff --git
 a/tests/spec/arb_shader_texture_lod/execution/CMakeLists.gl.txt
 b/tests/spec/arb_shader_texture_lod/execution/CMakeLists.gl.txt
 index c403939..2366fa9 100644
 --- a/tests/spec/arb_shader_texture_lod/execution/CMakeLists.gl.txt
 +++ b/tests/spec/arb_shader_texture_lod/execution/CMakeLists.gl.txt
 @@ -10,5 +10,6 @@ link_libraries (
)

piglit_add_executable (arb_shader_texture_lod-texgrad texgrad.c)
 +piglit_add_executable (arb_shader_texture_lod-texgradcube
 texgradcube.c)

# vim: ft=cmake:
 diff --git a/tests/spec/arb_shader_texture_lod/execution/texgradcube.c
 b/tests/spec/arb_shader_texture_lod/execution/texgradcube.c
 new file mode 100644
 index 000..90f7ddc
 --- /dev/null
 +++ b/tests/spec/arb_shader_texture_lod/execution/texgradcube.c
 @@ -0,0 +1,195 @@
 +/*
 + * Copyright (c) 2013 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
 + * 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
 + * NON-INFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS AND/OR THEIR
 + * SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
 + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 + * DEALINGS IN THE SOFTWARE.
 + *
 + * Authors:
 + *Roland Scheideggersrol...@vmware.com
 + *
 + * Based on arb_shader_texture_lod-texgrad:
 + *Marek Olšákmar...@gmail.com
 + */
 +
 +#include piglit-util-gl-common.h
 +
 +PIGLIT_GL_TEST_CONFIG_BEGIN
 +
 +config.supports_gl_compat_version = 10;
 +
 +config.window_width = 512;
 +config.window_height = 256;
 +config.window_visual = PIGLIT_GL_VISUAL_RGB |
 PIGLIT_GL_VISUAL_DOUBLE;
 +
 +PIGLIT_GL_TEST_CONFIG_END
 +
 +#define TEX_WIDTH 256
 +#define TEX_HEIGHT 256
 +
 +static const float colors[][3] = {
 +{1.0, 0.0, 0.0},
 +{0.0, 1.0, 0.0},
 +{0.0, 0.0, 1.0},
 +{1.0, 1.0, 0.0},
 +{0.0, 1.0, 1.0},
 +{1.0, 0.0, 1.0},
 +{0.5, 0.0, 0.5},
 +{1.0, 1.0, 1.0},
 +};
 +
 +static const char *sh_tex =
 +uniform samplerCube tex;
 +void main()
 +{
 +   gl_FragColor = textureCube(tex, gl_TexCoord[0].xyz);
 +};
 +
 +static const char *sh_texgrad =
 +#extension GL_ARB_shader_texture_lod : enable\n
 +uniform samplerCube tex;
 +void main()
 +{
 +   gl_FragColor = textureCubeGradARB(tex, gl_TexCoord[0].xyz,
 + dFdx(gl_TexCoord[0].xyz),
 + dFdy(gl_TexCoord[0].xyz));
 +};
 +
 +static GLint prog_tex, prog_texgrad;
 +
 +void piglit_init(int argc, char **argv)
 +{
 +GLuint tex, fb;
 +GLenum status;
 +int i, j, dim;
 +static GLuint fs_tex, fs_texgrad;
 +
 +piglit_require_GLSL();
 +piglit_require_extension(GL_EXT_framebuffer_object);
 +piglit_require_extension

Re: [Piglit] [PATCH] glsl-1.30: add basic test for testing textureOffset functionality.

[Roland Scheidegger]

Please disregard this sent the wrong stuff...

Am 22.03.2013 23:21, schrieb srol...@vmware.com:
 From: Roland Scheidegger srol...@vmware.com
 
 This is pretty rough and doesn't really test all that much (just
 textureOffsetLod, but there's other texturing functions with offsets),
 doesn't test different wrap modes, NPOT sizes etc., but there's no
 other test using ordinary texture opcodes and texel offsets, so
 it's a start (and in fact it exposed a bug in the mesa state tracker).
 Tested with llvmpipe (pass) and softpipe (fail) which is as expected -
 hopefully the math is ok...
 Inspired from fs-texelFetchOffset
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH] glsl-fs-main-return-conditional: Test for using return in main

[Roland Scheidegger]

Am 23.03.2013 01:41, schrieb Ian Romanick:
 On 03/22/2013 03:28 PM, srol...@vmware.com wrote:
 From: Roland Scheidegger srol...@vmware.com

 Similar to glsl-fs-main-return (and glsl-vs-main-return), this is testing
 using return in main. Contrary to the these other tests, this hits both
 the cases where the return path is and is NOT taken (the gallivm code
 got it wrong and always did an early exit which got unnoticed by the
 existing tests, see https://bugs.freedesktop.org/show_bug.cgi?id=62357).
 It also needs glsl 1.30 (even though this is not specific to what's
 tested
 here).
 
 Are there any drivers in Mesa that don't expose GLSL 1.30 but would
 benefit from this test?
I don't think so but I'm not sure.

 
 ---
   .../glsl-fs-main-return-conditional.shader_test|   34
 
   1 file changed, 34 insertions(+)
   create mode 100644
 tests/shaders/glsl-fs-main-return-conditional.shader_test

 diff --git a/tests/shaders/glsl-fs-main-return-conditional.shader_test
 b/tests/shaders/glsl-fs-main-return-conditional.shader_test
 new file mode 100644
 index 000..124a4cd
 --- /dev/null
 +++ b/tests/shaders/glsl-fs-main-return-conditional.shader_test
 @@ -0,0 +1,34 @@
 +[require]
 +GLSL = 1.30
 +
 +[vertex shader]
 +#version 130
 +void main()
 +{
 +gl_Position = gl_Vertex;
 +}
 +
 +[fragment shader]
 +#version 130
 +uniform vec4 v;
 
 This could just be
 
 const vec4 v = vec4(0., 1., 0., 1.);
 
 and delete the uniform setting in the [test] section.
Yeah I guess this was a uniform originally so the compiler wouldn't just
get rid of the conditional altogether but since the condition no longer
depends on it this is unnecessary.

 
 +
 +void main()
 +{
 +uint posintx = uint(gl_FragCoord.x);
 +uint one = uint(1);
 +gl_FragColor = v;
 +if ((posintx  one) == one) {
 +return;  // return for every second pixel
 +}
 +gl_FragColor = vec4(1.0) - v;
 +}
 
 It seems like this could be done as:
 
 void main()
 {
 floor x = floor(gl_FragCoord.x - 0.5);
 
 gl_FragCoord = v;
 if (mod(x, 2.0)  epsilon)
 return;
 
 gl_FragColor = vec4(1.0) - v;
 }
Sounds like a good idea, I forgot about the mod() which gets rid of glsl
1.30 requirement. In fact the floor is unnecessary too can simplify this
more. I'll send out another version...

Roland


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

Re: [Piglit] [PATCH] glsl-1.30: add basic test for testing textureOffset functionality.

[Roland Scheidegger]

Am 05.03.2013 19:41, schrieb Brian Paul:
 On 03/05/2013 11:03 AM, Roland Scheidegger wrote:
 Am 05.03.2013 17:05, schrieb Brian Paul:
 On 03/05/2013 08:37 AM, srol...@vmware.com wrote:
 From: Roland Scheideggersrol...@vmware.com

 This is pretty rough and doesn't really test all that much (just
 textureOffsetLod, but there's other texturing functions with offsets),
 doesn't test different wrap modes, NPOT sizes etc., but there's no
 other test using ordinary texture opcodes and texel offsets, so
 it's a start (and in fact it exposed a bug in the mesa state tracker).
 Tested with llvmpipe (pass) and softpipe (fail) which is as expected -
 hopefully the math is ok...
 Inspired from fs-texelFetchOffset.
 ---
tests/spec/glsl-1.30/execution/CMakeLists.gl.txt   |1 +
.../spec/glsl-1.30/execution/fs-textureOffset-2D.c |  158
 
2 files changed, 159 insertions(+)
create mode 100644
 tests/spec/glsl-1.30/execution/fs-textureOffset-2D.c

 diff --git a/tests/spec/glsl-1.30/execution/CMakeLists.gl.txt
 b/tests/spec/glsl-1.30/execution/CMakeLists.gl.txt
 index 6737bb1..3c0b2d5 100644
 --- a/tests/spec/glsl-1.30/execution/CMakeLists.gl.txt
 +++ b/tests/spec/glsl-1.30/execution/CMakeLists.gl.txt
 @@ -13,6 +13,7 @@ link_libraries (
piglit_add_executable (fs-discard-exit-2 fs-discard-exit-2.c)
piglit_add_executable (fs-texelFetch-2D fs-texelFetch-2D.c)
piglit_add_executable (fs-texelFetchOffset-2D
 fs-texelFetchOffset-2D.c)
 +piglit_add_executable (fs-textureOffset-2D fs-textureOffset-2D.c)
IF (NOT MSVC)
piglit_add_executable (isinf-and-isnan isinf-and-isnan.c)
ENDIF (NOT MSVC)
 diff --git a/tests/spec/glsl-1.30/execution/fs-textureOffset-2D.c
 b/tests/spec/glsl-1.30/execution/fs-textureOffset-2D.c
 new file mode 100644
 index 000..4c050fb
 --- /dev/null
 +++ b/tests/spec/glsl-1.30/execution/fs-textureOffset-2D.c
 @@ -0,0 +1,158 @@
 +/*
 + * Copyright 2013 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 fs-textureOffset-2D.c
 + *
 + * Tests the built-in function textureLodOffset() in the fragment
 shader.
 + *
 + * Creates a mipmapped 64x32 2D texture and draws a series of squares
 whose
 + * color contains a texel fetched from each quadrant of the rgbw
 texture.
 + *
 + * Author: Roland Scheidegger
 + */
 +#include piglit-util-gl-common.h
 +
 +PIGLIT_GL_TEST_CONFIG_BEGIN
 +
 +config.supports_gl_compat_version = 10;
 +
 +config.window_width = 90;
 +config.window_height = 150;

 width=90 might cause trouble on Windows.  Windows will bump up the size
 to 120 pixels (IIRC).  This has caused other tests to fail in the past.
 Ok, I'll bump it up to 120.
 
 Or just omit them and use the defaults.
Oh yes sounds even better. I wasn't aware that you can omit it.

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

Re: [Piglit] [PATCH] fs-texelFetchOffset-2D: don't assume undefined values to be solid black

[Roland Scheidegger]

Am 27.11.2012 08:25, schrieb Eric Anholt:
 srol...@vmware.com writes:
 
 From: Roland Scheidegger srol...@vmware.com

 core GL specifies out-of-bound accesses have undefined behavior,
 which includes crashes.
 Crashes are very much undesired, but ARB_robustness still allows undefined
 values to be returned (with a recommendation to return 0).
 Only ARB_robust_buffer_access_behavior would require to return zero, but the
 test doesn't need this extension. In any case even returning zero is not what
 the test expected, since it wanted [0,0,0,1].
 (With this change softpipe passes the test, as it clamps the coords.)
 
 Are you saying texelFetch is supposed to not do wrap modes?
 
Yes. From the OpenGL specification 4.3 compatibility profile, page 387
(in a section you'd never find it..., subsection 11.1.3.2 Texel Fetches).
FWIW the DX10 equivalent (ld opcode) doesn't even have any sampler state
(and hence clamping behavior) associated with it.


Roland

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

Re: [Piglit] [PATCH] tests/spec/arb_robustness/draw-vbo-bounds.c: add clipping

[Roland Scheidegger]

Am 30.10.2012 20:01, schrieb Ian Romanick:
 On 10/29/2012 05:34 PM, Roland Scheidegger wrote:
 Am 30.10.2012 00:22, schrieb Brian Paul:
 On 10/29/2012 05:05 PM, srol...@vmware.com wrote:
 From: Roland Scheideggersrol...@vmware.com

 Make sure clipping is needed sometimes, and more often use small index
 counts,
 to expose issues and excercise more paths in mesa's draw module.
 ---
tests/spec/arb_robustness/draw-vbo-bounds.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/tests/spec/arb_robustness/draw-vbo-bounds.c
 b/tests/spec/arb_robustness/draw-vbo-bounds.c
 index 4351ac9..c780a3a 100644
 --- a/tests/spec/arb_robustness/draw-vbo-bounds.c
 +++ b/tests/spec/arb_robustness/draw-vbo-bounds.c
 @@ -95,7 +95,7 @@ random_vertices(GLsizei offset, GLsizei stride,
 GLsizei count)

for (i = 0; i  count; ++i) {
GLfloat *vertex = (GLfloat *)(vertices + offset + i*stride);
 -vertex[0] = (rand() % 1000) * .001;
 +vertex[0] = (rand() % 1000) * ((rand() % 1000) ? 0.001 : 1.0);
vertex[1] = (rand() % 1000) * .001;
}

 @@ -145,7 +145,7 @@ static void test(void)
vertex_count = 1 + rand() % 0x;

index_offset = (rand() % 0xff) * sizeof(GLushort);
 -index_count = 1 + rand() % 0x;
 +index_count = rand() % 10 ? 1 + rand() % 0x : 1 + rand() %
 0x7ff;
min_index = rand() % vertex_count;
max_index = min_index + rand() % (vertex_count - min_index);


 Randomness in tests can be OK, but in this case wouldn't you want to
 explicitly test some specific coordinates and indexes to make sure the
 corner cases are hit?

 -Brain

 Well I'm not sure it's worth the trouble note the test is run 1000 times
 so probability is very very high that these cases are hit anyway.
 
 What happens when one of us gets a bug report that this test fails, but
 we're unable to reproduce it?  If I had been paying attention, I would
 have already objected to the initial use of rand in this test...
 

Well my guess is José wanted to keep it simple without testing a
gazillion different interesting combinations manually (besides what's
interesting might well depend on the driver).
It looks though there's strong opinion against this, would you prefer a
fully deterministic test? I guess since it runs so many iterations
anyway could just loop through pretty much all cases instead (aside from
the exact indices and vertices, but the indices aren't really all that
interesting except if they are inbound or out-of-bound same is true for
the vertices which shouldn't matter except if they are clipped or not).

Roland


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

Re: [Piglit] [PATCH] tests/spec/arb_robustness/draw-vbo-bounds.c: add clipping

[Roland Scheidegger]

Am 30.10.2012 00:22, schrieb Brian Paul:
 On 10/29/2012 05:05 PM, srol...@vmware.com wrote:
 From: Roland Scheideggersrol...@vmware.com

 Make sure clipping is needed sometimes, and more often use small index
 counts,
 to expose issues and excercise more paths in mesa's draw module.
 ---
   tests/spec/arb_robustness/draw-vbo-bounds.c |4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/tests/spec/arb_robustness/draw-vbo-bounds.c
 b/tests/spec/arb_robustness/draw-vbo-bounds.c
 index 4351ac9..c780a3a 100644
 --- a/tests/spec/arb_robustness/draw-vbo-bounds.c
 +++ b/tests/spec/arb_robustness/draw-vbo-bounds.c
 @@ -95,7 +95,7 @@ random_vertices(GLsizei offset, GLsizei stride,
 GLsizei count)

   for (i = 0; i  count; ++i) {
   GLfloat *vertex = (GLfloat *)(vertices + offset + i*stride);
 -vertex[0] = (rand() % 1000) * .001;
 +vertex[0] = (rand() % 1000) * ((rand() % 1000) ? 0.001 : 1.0);
   vertex[1] = (rand() % 1000) * .001;
   }

 @@ -145,7 +145,7 @@ static void test(void)
   vertex_count = 1 + rand() % 0x;

   index_offset = (rand() % 0xff) * sizeof(GLushort);
 -index_count = 1 + rand() % 0x;
 +index_count = rand() % 10 ? 1 + rand() % 0x : 1 + rand() %
 0x7ff;
   min_index = rand() % vertex_count;
   max_index = min_index + rand() % (vertex_count - min_index);

 
 Randomness in tests can be OK, but in this case wouldn't you want to
 explicitly test some specific coordinates and indexes to make sure the
 corner cases are hit?
 
 -Brain
 
 

Well I'm not sure it's worth the trouble note the test is run 1000 times
so probability is very very high that these cases are hit anyway.

Roland


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