I think I should have noted, I don't have commit access.
Would someone mind pushing this on my behalf? - Anthony On 5/24/19 2:49 PM, Pelloux-prayer, Pierre-eric wrote:
Looks good to me, thanks. Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-pra...@amd.com> ________________________________________ From: Piglit <piglit-boun...@lists.freedesktop.org> on behalf of Anthony Pesch <ape...@nvidia.com> Sent: Friday, May 24, 2019 8:21 PM To: Pierre-Eric Pelloux-Prayer; Anthony Pesch; piglit@lists.freedesktop.org Subject: Re: [Piglit] [PATCH] arb_texture_buffer_range: Fix buffer alignment in ranges-2 test [CAUTION: External Email] Hi Pierre, Thanks! I've modifed the patch to cast data, and added an additional check to ensure unaligned offsets produce INVALID_VALUE. I'm not sure how to use git send-mail to update a review, so I've just inlined the updated patch here and attached it as well. - Anthony commit 770effba655f97be229e13f894884f0e4c3e604a Author: Anthony Pesch <ape...@nvidia.com> Date: Thu Apr 11 12:37:07 2019 -0400 arb_texture_buffer_range: Fix buffer alignment in ranges-2 test The ranges-2 test was failing due to glTexBufferRange returning GL_INVALID_VALUE when the offset parameter wasn't GL_TEXTURE_BUFFER_OFFSET_ALIGNMENT byte aligned. From the OpenGL 4.6 Core spec: An INVALID_VALUE error is generated if offset is not an integer multiple of the value of TEXTURE_BUFFER_OFFSET_ALIGNMENT. diff --git a/tests/spec/arb_texture_buffer_range/ranges-2.c b/tests/spec/arb_texture_buffer_range/ranges-2.c index 3477e4b52..83a009875 100644 --- a/tests/spec/arb_texture_buffer_range/ranges-2.c +++ b/tests/spec/arb_texture_buffer_range/ranges-2.c @@ -91,22 +91,29 @@ float data[] = { 0, 0, 0.5, 0, }; +int aligned_size = 0; +int chunk_size = 24 * sizeof(float); +int num_chunks = 4; + enum piglit_result piglit_display(void) { int i; - int chunk_size = 24 * sizeof(float); bool pass = true; glClearColor(0.2, 0.2, 0.2, 0.2); glClear(GL_COLOR_BUFFER_BIT); - for (i = 0; i < sizeof(data) / chunk_size; i++) { + /* verify unaligned offsets produce an error */ + glTexBufferRange(GL_TEXTURE_BUFFER, GL_RGBA32F, tbo, aligned_size - 1, 1); + pass &= glGetError() == GL_INVALID_VALUE; + + for (i = 0; i < num_chunks; i++) { glTexBufferRange(GL_TEXTURE_BUFFER, GL_RGBA32F, - tbo, i * chunk_size, chunk_size); + tbo, i * aligned_size, chunk_size); glDrawArrays(GL_TRIANGLES, 0, 6); } - for (i = 0; i < sizeof(data) / chunk_size; i++) { + for (i = 0; i < num_chunks; i++) { float c[4] = { data[i * 24 + 2], data[i * 24 + 3], @@ -114,7 +121,7 @@ piglit_display(void) { 1 }; - pass = piglit_probe_rect_rgba( + pass &= piglit_probe_rect_rgba( piglit_width * 0.5 * (1 + data[i * 24 + 0]), piglit_height * 0.5 * (1 + data[i * 24 + 1]), piglit_width/2, @@ -128,8 +135,14 @@ piglit_display(void) { void piglit_init(int argc, char **argv) { + GLint align, i; + uint8_t *chunk; + piglit_require_extension("GL_ARB_texture_buffer_range"); + glGetIntegerv(GL_TEXTURE_BUFFER_OFFSET_ALIGNMENT, &align); + aligned_size = chunk_size % align == 0 ? chunk_size : align; + prog = piglit_build_simple_program(vs_source, fs_source); glUseProgram(prog); @@ -138,7 +151,12 @@ piglit_init(int argc, char **argv) { glGenBuffers(1, &tbo); glBindBuffer(GL_ARRAY_BUFFER, tbo); - glBufferData(GL_ARRAY_BUFFER, sizeof(data), data, GL_STATIC_DRAW); + glBufferData(GL_ARRAY_BUFFER, aligned_size * num_chunks, NULL, GL_STATIC_DRAW); + + for (i = 0, chunk = (uint8_t *)data; i < num_chunks; i++) { + glBufferSubData(GL_ARRAY_BUFFER, aligned_size * i, chunk_size, chunk); + chunk += chunk_size; + } glGenTextures(1, &tex); glBindTexture(GL_TEXTURE_BUFFER, tex); On 5/17/19 6:04 AM, Pierre-Eric Pelloux-Prayer wrote:Hi, The patch looks good, except ranges-2.c now cause a compiler warning: tests/spec/arb_texture_buffer_range/ranges-2.c:152:20: warning: assignment to ‘uint8_t *’ {aka ‘unsigned char *’} from incompatible pointer type ‘float *’ [-Wincompatible-pointer-types] for (i = 0, chunk = data; i < num_chunks; i++) { Another suggestion: it could be interesting to verify that INVALID_VALUE is generated when the alignment constraint is not respected. Thanks, Pierre-Eric On 09/05/2019 20:41, Anthony Pesch wrote:Pinging to see if anyone has the time to review. - Anthony On 4/15/19 4:24 PM, Anthony Pesch wrote:From: Anthony Pesch <ape...@nvidia.com> The ranges-2 test was failing due to glTexBufferRange returning GL_INVALID_VALUE when the offset parameter wasn't GL_TEXTURE_BUFFER_OFFSET_ALIGNMENT byte aligned. From the OpenGL 4.6 Core spec: An INVALID_VALUE error is generated if offset is not an integer multiple of the value of TEXTURE_BUFFER_OFFSET_ALIGNMENT. --- .../spec/arb_texture_buffer_range/ranges-2.c | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/tests/spec/arb_texture_buffer_range/ranges-2.c b/tests/spec/arb_texture_buffer_range/ranges-2.c index 3477e4b52..b9cd543b0 100644 --- a/tests/spec/arb_texture_buffer_range/ranges-2.c +++ b/tests/spec/arb_texture_buffer_range/ranges-2.c @@ -91,22 +91,25 @@ float data[] = { 0, 0, 0.5, 0, }; +int aligned_size = 0; +int chunk_size = 24 * sizeof(float); +int num_chunks = 4; + enum piglit_result piglit_display(void) { int i; - int chunk_size = 24 * sizeof(float); bool pass = true; glClearColor(0.2, 0.2, 0.2, 0.2); glClear(GL_COLOR_BUFFER_BIT); - for (i = 0; i < sizeof(data) / chunk_size; i++) { + for (i = 0; i < num_chunks; i++) { glTexBufferRange(GL_TEXTURE_BUFFER, GL_RGBA32F, - tbo, i * chunk_size, chunk_size); + tbo, i * aligned_size, chunk_size); glDrawArrays(GL_TRIANGLES, 0, 6); } - for (i = 0; i < sizeof(data) / chunk_size; i++) { + for (i = 0; i < num_chunks; i++) { float c[4] = { data[i * 24 + 2], data[i * 24 + 3], @@ -128,8 +131,14 @@ piglit_display(void) { void piglit_init(int argc, char **argv) { + GLint align, i; + uint8_t *chunk; + piglit_require_extension("GL_ARB_texture_buffer_range"); + glGetIntegerv(GL_TEXTURE_BUFFER_OFFSET_ALIGNMENT, &align); + aligned_size = chunk_size % align == 0 ? chunk_size : align; + prog = piglit_build_simple_program(vs_source, fs_source); glUseProgram(prog); @@ -138,7 +147,12 @@ piglit_init(int argc, char **argv) { glGenBuffers(1, &tbo); glBindBuffer(GL_ARRAY_BUFFER, tbo); - glBufferData(GL_ARRAY_BUFFER, sizeof(data), data, GL_STATIC_DRAW); + glBufferData(GL_ARRAY_BUFFER, aligned_size * num_chunks, NULL, GL_STATIC_DRAW); + + for (i = 0, chunk = data; i < num_chunks; i++) { + glBufferSubData(GL_ARRAY_BUFFER, aligned_size * i, chunk_size, chunk); + chunk += chunk_size; + } glGenTextures(1, &tex); glBindTexture(GL_TEXTURE_BUFFER, tex);_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
