I can push it but can you first resend your patch using git send-mail because
git am doesn't recognize the format of this one.
(or you can open a merge request on gitlab if you prefer)
Thanks!
Pierre-Eric
________________________________________
From: Anthony Pesch <ape...@nvidia.com>
Sent: Friday, May 24, 2019 8:51 PM
To: Pelloux-prayer, Pierre-eric; 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
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
