Re: [Piglit] [PATCH 3/6] glsl-es-3.00: add uniform block type mismatch link test
Jordan Justen writes: > On Thu, Feb 28, 2013 at 4:41 PM, Eric Anholt wrote: >> Jordan Justen writes: >> >>> Signed-off-by: Jordan Justen >>> --- >>> ...terface-blocks-member-type-mismatch.shader_test | 39 >>> >>> 1 file changed, 39 insertions(+) >>> create mode 100644 >>> tests/spec/glsl-es-3.00/linker/interface-blocks-member-type-mismatch.shader_test >>> >>> diff --git >>> a/tests/spec/glsl-es-3.00/linker/interface-blocks-member-type-mismatch.shader_test >>> >>> b/tests/spec/glsl-es-3.00/linker/interface-blocks-member-type-mismatch.shader_test >>> new file mode 100644 >>> index 000..d586867 >>> --- /dev/null >>> +++ >>> b/tests/spec/glsl-es-3.00/linker/interface-blocks-member-type-mismatch.shader_test >>> @@ -0,0 +1,39 @@ >>> +# Tests that a link error occurs when an interface block member's type >>> +# differs between the vertex and fragment shaders. >>> +# >>> +# GLSL_ES_Specification_3.00.3, 4.3.7 Interface Blocks: >>> +# "Matched block names within an interface (as defined above) must match >>> +# in terms of having the same number of declarations with the same >>> +# sequence of types, precisions and the same sequence of member names, >>> +# as well as having the same member-wise layout qualification (see next >>> +# section)." >>> +[require] >>> +GL ES >= 3.0 >>> +GLSL ES >= 3.00 >>> + >>> +[vertex shader] >>> +#version 300 es >> >> This looks like a single case of what's tested in link-mismatch-blocks. >> I don't know whether it's important to have ES-specific version of that >> test, but we might want to test the same things on both sides if we >> decide to. > > This test fails for us currently. The differing instance name seems to > be significant in masking the linker failure. > > I guess ARB_uniform_buffer_object doesn't support instance names, so > this test can't be added to link-mismatch-blocks. Ah, I see. I'm still confused by the spec about what should happen with the same block name and uniforms within differently-named interfaces, and I'll leave that interpretation up to others :) pgpAjPVevwmAf.pgp Description: PGP signature ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [RFC 3/3] tests/spec: add tests for oes image external
On 02/26/2013 05:15 AM, Topi Pohjolainen wrote:
> This consists of tests adapted from Khronos conformance suite and
> Android surface flinger. While the former deals with "getters/setters",
> enumrations and simple sampling of texture based images, the latter
> addresses bilinear sampling of non-GPU written subsampled UV-planes
> and conversion from YUV to RGB.
>
> The original Android test consist of two YV12 formatted textures,
> one of size 64x64 and another of size 64x66. Both represent checker
> board pattern each YUV component having value 63 or 191. Here I have
> only the first but I'm planning to adopt the latter also if the approach
> I have taken is reasonable. Instead of filling in the entire pattern I
> have only written those YUV-components that are actually checked (a
> dozen odd pixels) while the rest are initialised to zero. In addition
> I used calculated floating point values instead of the hardcoded found
> in the original. There, however, I ended up in deviations and I would
> appreciate if people understanding the domain of YUV to RGB conversion
> better could take a good look.
>
> In the implementation that I have written for mesa/i965 I have also
> support for NV12 format. Whereas YV12 (YVU420) has separate U- and
> V-planes, NV12 has them combined into one. Hence I would like to have
> the same tests for NV12 as for YV12 if acceptable.
>
> The tests are written only for ES2 contexts, I haven't looked into
> how I would separate the tests not dealing with external sampler
> (samplerExternalOES is only defined for ES 2.x).
>
> Signed-off-by: Topi Pohjolainen
> ---
> tests/spec/CMakeLists.txt |1 +
> .../oes_egl_image_external/CMakeLists.gles2.txt| 16 +
> tests/spec/oes_egl_image_external/CMakeLists.txt |3 +
> .../oes_egl_image_external.c | 776
>
> 4 files changed, 796 insertions(+)
> create mode 100644 tests/spec/oes_egl_image_external/CMakeLists.gles2.txt
> create mode 100644 tests/spec/oes_egl_image_external/CMakeLists.txt
> create mode 100644 tests/spec/oes_egl_image_external/oes_egl_image_external.c
> +static bool
> +test_64x64_yv12(void)
> +{
> + bool pass;
> + unsigned char y[64 * 64];
> + unsigned char u[32 * 32];
> + unsigned char v[32 * 32];
> + EGLImageKHR img;
> + void *buf;
> +
> + write_64x64_420(y, u, v);
> + /* Reversing the order of UV-planes gives YVU420 a.k.a. YV12 */
> + buf = piglit_create_ext_420_buf(64, 64, true, y, u, v);
> + if (!buf) {
> + printf("failed to create external buffer\n");
> + return false;
As I mentioned in patch 1, this is one of the places where the test
should skip if the platform is incapable of creating a yuv420 buffer.
If the the platform *is* capable of creating a yuv420, but fails to
do so, only then does it make sense to fail here.
> + }
> +/**
> + * INVALID_ENUM should be generated for TexImage2D, TexSubImage2D,
> + * CompressedTexImager2D and CompressedTexSubImage2D with
> + * TEXTURE_EXTERNAL_OES.
> + */
> +static bool
> +test_disallow_image_2d(void)
> +{
> + GLushort pixel_2x2_565[] = { 0, 0, 0, 0 };
> + GLuint tex;
> +
> + glGenTextures(1, &tex);
> + glBindTexture(GL_TEXTURE_EXTERNAL_OES, tex);
> +
> + glTexImage2D(GL_TEXTURE_EXTERNAL_OES, 0, GL_RGB, 2, 2, 0, GL_RGB,
> + GL_UNSIGNED_SHORT_5_6_5, pixel_2x2_565);
> + if (glGetError() != GL_INVALID_ENUM) {
> + printf("TexImage2D() cannot be invoked for EXTERNAL_OES\n");
> + glDeleteTextures(1, &tex);
> + return false;
> + }
Piglit has a useful utility for checking GL errors: piglit_check_gl_error().
It prints good diagnostic information, so please use it here and in similar
places in your test. A quick grep should show you how to use it.
I tried reviewing the whole test, but quickly realized that I don't know
enough about color formats to do so.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [RFC 1/3] framework: introduce interface for external buffers
On 03/01/2013 10:57 AM, Chad Versace wrote:
> On 02/26/2013 05:15 AM, Topi Pohjolainen wrote:
>> In order to test the OES_EGL_image_external with planar formats such
>> as YUV420 or NV12, one needs a way for creating buffers that can be
>> passed to EGL and filling them with YUV-data for the GL-stack to
>> sample.
>> By the nature the extension in question deals with platform specific
>> buffers and hence the idea here is to push the details down into the
>> platform specific logic leaving the tests themselves platform
>> independent.
>>
>> Signed-off-by: Topi Pohjolainen
>> ---
>> tests/util/piglit-framework-gl.c | 29
>>
>> tests/util/piglit-framework-gl.h | 21 ++
>> .../util/piglit-framework-gl/piglit_gl_framework.h | 23
>> 3 files changed, 73 insertions(+)
>> +void *
>> +piglit_create_ext_420_buf(unsigned w, unsigned h, bool swap_vu,
>> +const void *y, const void *u, const void *v)
>> +{
>> +if (!gl_fw->create_external_420_buffer)
>> +return NULL;
>> +
>> +return gl_fw->create_external_420_buffer(gl_fw, w, h, swap_vu, y, u, v);
>> +}
>
> Later, in patch 3, you write the test so that, if piglit_create_ext_420_buf()
> fails then the test reports PIGLIT_FAIL. But the convention in piglit is to
> report
> PIGLIT_SKIP when the platform is incapable of running the test.
>
> There are two choices to handle this:
> 1. Here, in piglit_create_ext_420_buf(), report PIGLIT_SKIP if it fails
> to create the buffer.
>
> 2. In the testfile itself, report PIGLIT_SKIP if piglit_create_ext_420()
> fails.
My opinion here has changed after I thought about it more. I think that the
piglit_gbm_framework_create() should set gl_fw->create_external_420_buffer
only if the platform is capable of creating a yuv420 buffer.
If a test calls piglit_create_ext_420_buf(), but the function is unable to
create
the buffer because the platform doesn't support it (that is, because
gl_fw->create_external_420_buffer is NULL), then the test should report
PIGLIT_SKIP.
That is the convention in piglit; if a test attempts to test something that the
platform is incapable of doing, then it reports PIGLIT_SKIP. In this case,
the best place to report PIGLIT_SKIP is here inside piglit_create_ext_420_buf().
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [RFC 2/3] framework: gbm: support for creating external buffers
On 02/26/2013 05:15 AM, Topi Pohjolainen wrote:
> Getting direct access to the gbm device through the waffle display
> and creating the buffer is rather straightforward thing to do.
> Writing to the buffer using CPU, however, requires priviledges for
> the underlying gem-ioctl.
>
> Signed-off-by: Topi Pohjolainen
> ---
> .../piglit-framework-gl/piglit_gbm_framework.c | 78
>
> 1 file changed, 78 insertions(+)
It's clear here that you're trying to test a difficult thing and there is no
easy way to do it. I have several questions for you for which I don't know the
answer.
> diff --git a/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> b/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> index 4df3861..00e1425 100644
> --- a/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> +++ b/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> @@ -24,10 +24,18 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
>
> #include "piglit-util-gl-common.h"
> #include "piglit_gbm_framework.h"
>
> +#define ALIGN(value, alignment) (((value) + alignment - 1) & ~(alignment -
> 1))
> +
> static void
> enter_event_loop(struct piglit_winsys_framework *winsys_fw)
> {
> @@ -51,6 +59,73 @@ destroy(struct piglit_gl_framework *gl_fw)
> free(winsys_fw);
> }
>
> +static void *
> +map(struct gbm_device *gbm, struct gbm_bo *bo, unsigned n_bytes)
> +{
> +int res;
> +struct drm_i915_gem_mmap mmap_arg;
> +
> +mmap_arg.handle = gbm_bo_get_handle(bo).u32;
> +mmap_arg.offset = 0;
> +mmap_arg.size = n_bytes;
> +
> +res = ioctl(gbm_device_get_fd(gbm), DRM_IOCTL_I915_GEM_MMAP,
> &mmap_arg);
> +
> +if (res)
> +return 0;
> +
> +return (void *)(uintptr_t)mmap_arg.addr_ptr;
> +}
This function is specific to i915, but gbm is specific only to drm. Is it
possible
to rewrite this function using only drm ioctls? If it's not possbile, or if it
that's a bad idea,
we should somehow detect here whether gbm is using i915 and skip the test if
it's not.
> +
> +/* At the time of writing this, GBM did not have any alignment
> + * restrictions on height or width.
> + */
> +static void *
> +create_ext_420_buffer(struct piglit_gl_framework *gl_fw,
> + unsigned w, unsigned h, bool swap_vu,
> + const void *y, const void *u, const void *v)
> +{
> + struct waffle_gbm_display *native = waffle_display_get_native(
> + piglit_wfl_framework(gl_fw)->display)->gbm;
> + struct gbm_bo *bo = gbm_bo_create(native->gbm_device, w, h,
> + swap_vu ? GBM_FORMAT_YVU420 : GBM_FORMAT_YUV420,
> + GBM_BO_USE_RENDERING);
> + unsigned char *p;
> + unsigned y_off = 0;
> + unsigned u_off = ALIGN(w * h, 4096);
> + unsigned v_off = u_off + ALIGN((w / 2) * (h / 2), 4096);
> + unsigned total = v_off + ALIGN((w / 2) * (h / 2), 4096);
Why the 4096? Are you assuming that the buffer layout is aligned to 4096
because that is the size of a tile on Intel gpus?
I expected to see gbm_bo_get_stride() used in these calculations. Using it
may allow you to remove the hardcoded 4096.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [RFC 1/3] framework: introduce interface for external buffers
On 02/26/2013 05:15 AM, Topi Pohjolainen wrote:
> In order to test the OES_EGL_image_external with planar formats such
> as YUV420 or NV12, one needs a way for creating buffers that can be
> passed to EGL and filling them with YUV-data for the GL-stack to
> sample.
> By the nature the extension in question deals with platform specific
> buffers and hence the idea here is to push the details down into the
> platform specific logic leaving the tests themselves platform
> independent.
>
> Signed-off-by: Topi Pohjolainen
> ---
> tests/util/piglit-framework-gl.c | 29
>
> tests/util/piglit-framework-gl.h | 21 ++
> .../util/piglit-framework-gl/piglit_gl_framework.h | 23
> 3 files changed, 73 insertions(+)
> +
> +void *
> +piglit_create_ext_420_buf(unsigned w, unsigned h, bool swap_vu,
> + const void *y, const void *u, const void *v)
> +{
> + if (!gl_fw->create_external_420_buffer)
> + return NULL;
> +
> + return gl_fw->create_external_420_buffer(gl_fw, w, h, swap_vu, y, u, v);
> +}
Later, in patch 3, you write the test so that, if piglit_create_ext_420_buf()
fails then the test reports PIGLIT_FAIL. But the convention in piglit is to
report
PIGLIT_SKIP when the platform is incapable of running the test.
There are two choices to handle this:
1. Here, in piglit_create_ext_420_buf(), report PIGLIT_SKIP if it fails
to create the buffer.
2. In the testfile itself, report PIGLIT_SKIP if piglit_create_ext_420()
fails.
> +
> +void *
> +piglit_create_ext_nv12_buf(unsigned w, unsigned h, const void *y,
> + const void *uv)
> +{
> + if (!gl_fw->create_external_nv12_buffer)
> + return NULL;
> +
> + return gl_fw->create_external_nv12_buffer(gl_fw, w, h, y, uv);
> +}
Same comments for nv12.
> +/**
> + * Create YUV420 or YV12 (YVU420) formatted buffer of the given size and
> + * set it contents according to the given planes. Here there is no
> + * padding in the planes y-plane having stride equaling the given width
> + * and u/v-planes having stride equaling half of the width. By setting
> + * the 'swap_vu' to true, the order of u/v planes is swapped
> + * corresponding to YV12 (YVU420) format.
> + */
> +void *piglit_create_ext_420_buf(unsigned w, unsigned h, bool swap_vu,
> + const void *y, const void *u, const void *v);
> +
> +/**
> + * Create NV12 formatted buffer of the given size and set it contents
> + * according to the given planes. Here there is no padding in the planes
> + * both y-plane and uv-plane having stride equaling the given width.
> + */
> +void *piglit_create_ext_nv12_buf(unsigned w, unsigned h,
> + const void *y, const void *uv);
> +
> +void piglit_destroy_ext_buf(void *buf);
For these three functions, please comment that "ext" is short for "external".
That confused me.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
