On 20.07.18 02:26, Francisco Jerez wrote:
I don't know where I'm looking... You are right. Only glBindImageTexture will be leftDanylo Piliaiev <danylo.pilia...@gmail.com> writes:Test for the regression which happened when GL_TEXTURE_BUFFER was allowed to have incompatible format. v2: Removed unnecessary code duplication - use upload_image instead of init_level. (Francisco Jerez) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106465 Signed-off-by: Danylo Piliaiev <danylo.pilia...@globallogic.com> --- .../arb_shader_image_load_store/invalid.c | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/tests/spec/arb_shader_image_load_store/invalid.c b/tests/spec/arb_shader_image_load_store/invalid.c index ed4b6c064..99f6703a4 100644 --- a/tests/spec/arb_shader_image_load_store/invalid.c +++ b/tests/spec/arb_shader_image_load_store/invalid.c @@ -266,13 +266,17 @@ static bool invalidate_incompatible_format(const struct image_info img, GLuint prog) { GLenum base_format = image_base_internal_format(img.format); + uint32_t pixels[4 * N]; + init_pixels(img, pixels, 1, 1, 1, 1); + + /* upload_image instead of init_level to support GL_TEXTURE_BUFFER */ + bool ret = upload_image(img, 0, pixels); +I don't think you actually need to call upload_image() here, it should have been called already by init_image().
and it's enough for the test.
This test-case doesn't crash with that patch. I picked the first format from the array (same as the other tests) which appears to be GL_RGBA32F, but unfortunately it's a format with which we cannot go out of bounds. Picking the format which will result in crash would be better (already tested and crash occurred). I'll do this in my hopefully final version of the patch./* Pick an incompatible texture format with a compatible base * type. */ - bool ret = init_level(img, 0, (base_format == GL_RGBA32F ? - GL_RGBA8 : GL_RG32UI), W, H); - glBindImageTexture(0, get_texture(0), 0, GL_TRUE, 0, - GL_READ_WRITE, img.format->format); + GL_READ_WRITE, (base_format == GL_RGBA32F ? + GL_RGBA8 : GL_RG32UI));return ret && piglit_check_gl_error(GL_NO_ERROR);} @@ -346,6 +350,8 @@ piglit_init(int argc, char **argv) for (op = image_ops; op->name; ++op) { const struct image_info def_img = image_info( GL_TEXTURE_2D, op->formats[0].format, W, H); + const struct image_info def_img_buffer = image_info( + GL_TEXTURE_BUFFER, op->formats[0].format, W, H);/** According to the spec, an access is considered @@ -399,6 +405,15 @@ piglit_init(int argc, char **argv) invalidate_incompatible_format, false), "%s/incompatible format test", op->name);+ /* Test for the regression which happened when+ * GL_TEXTURE_BUFFER was allowed to have incompatible format. + */FTR, did you confirm whether this test-case causes a crash after re-applying the mesa patch that led to the regression? Thanks.
Thank you for reviewing these embarrassing patches.
+ subtest(&status, true, + run_test(op, def_img_buffer, def_img_buffer, + invalidate_incompatible_format, false), + "%s/incompatible format test/image%s", + op->name, def_img_buffer.target->name); + /* * " * the texture bound to the image unit has layers, * and the selected layer or cube map face doesn't -- 2.17.1
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit