On Thu, Jun 18, 2015 at 5:26 AM, Iago Toral <ito...@igalia.com> wrote:
> On Tue, 2015-06-16 at 11:15 -0700, Anuj Phogat wrote:
>> Without this patch, arb_color_buffer_float-readpixels test fails, when
>> forced to use meta pbo path.
>>
>> Signed-off-by: Anuj Phogat <anuj.pho...@gmail.com>
>> Cc: <mesa-sta...@lists.freedesktop.org>
>> ---
>> src/mesa/drivers/common/meta_tex_subimage.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/mesa/drivers/common/meta_tex_subimage.c
>> b/src/mesa/drivers/common/meta_tex_subimage.c
>> index d2474f5..00364f8 100644
>> --- a/src/mesa/drivers/common/meta_tex_subimage.c
>> +++ b/src/mesa/drivers/common/meta_tex_subimage.c
>> @@ -273,12 +273,14 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx,
>> GLuint dims,
>> format == GL_COLOR_INDEX)
>> return false;
>>
>> - if (ctx->_ImageTransferState)
>> - return false;
>> -
>> -
>
> That test uses glReadPixels so it should call this with tex_image set to
> NULL and it should flow through the if you have below. The call to
> _mesa_get_readpixels_transfer_ops that you add below looks like it does
> what we want for a pixel read from a framebuffer instead of simply
> checking ctx->_ImageTransferState directly. I suppose this is what fixes
> the test, right?
>
Right. Using _mesa_get_readpixels_transfer_ops() in place of simple
ctx->_ImageTransferState check takes care of setting IMAGE_CLAMP_BIT
in ctx->_ImageTransferState when GL_CLAMP_READ_COLOR is set.
I'll add some explanation in commit message.
> The patch also removes the ctx->_ImageTransferState check for the case
> where we are reading from a real texture (tex_image != NULL), that seems
> unrelated to fixing arb_color_buffer_float-readpixels... Looking at the
> texture read code from getteximage.c it seems like this should be fine
> since that file does not seem to use that field for anything either, so
> I guess the check might not valid in this case.
>
After another look I realized that glGetTexImage() do need transfer
operations check to fall back to software path. It's the lack of piglit
tests that I couldn't catch it. Thanks for noticing it. I'll send out a v2.
> I think it would be nice if you updated the changelog to explain these
> things.
>
> Iago
>
>> + /* Don't use meta path for readpixels in below conditions. */
>> if (!tex_image) {
>> rb = ctx->ReadBuffer->_ColorReadBuffer;
>> +
>> + if (_mesa_get_readpixels_transfer_ops(ctx, rb->Format, format,
>> + type, GL_FALSE))
>> + return false;
>> +
>> if (_mesa_need_rgb_to_luminance_conversion(rb->Format, format))
>> return false;
>> }
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
