[Piglit] [PATCH] arb_texture_buffer_range: Fix buffer alignment in ranges-2 test

Anthony Pesch apesch at nvidia.com
Fri May 24 18:21:46 UTC 2019


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 <apesch at 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 <apesch at 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ranges-2.patch
Type: text/x-patch
Size: 2869 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20190524/7f5554fb/attachment.bin>


More information about the Piglit mailing list