[Mesa-dev] [PATCH] st/mesa: st_copy_texsubimage: clip source size to rb size

[Brian Paul]

On 07/18/2011 09:29 AM, Vadim Girlin wrote:
> On Mon, 2011-07-18 at 08:34 -0600, Brian Paul wrote:
>> On Mon, Jul 18, 2011 at 8:11 AM, Vadim Girlin<vadimgirlin at gmail.com>  wrote:
>>> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=39286
>>> ---
>>>   src/mesa/state_tracker/st_cb_texture.c |    8 ++++++++
>>>   1 files changed, 8 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
>>> index 6907cfc..63cd142 100644
>>> --- a/src/mesa/state_tracker/st_cb_texture.c
>>> +++ b/src/mesa/state_tracker/st_cb_texture.c
>>> @@ -1490,6 +1490,14 @@ st_copy_texsubimage(struct gl_context *ctx,
>>>        destY = 0;
>>>     }
>>>
>>> +   if (srcX + width>  strb->Base.Width) {
>>> +      width = strb->Base.Width - srcX;
>>> +   }
>>> +
>>> +   if (srcY + height>  strb->Base.Height) {
>>> +      height = strb->Base.Height - srcY;
>>> +   }
>>> +
>>>     if (width<  0 || height<  0)
>>>        return;
>>
>> Clipping for glCopyTexSubImage() should be done by
>> _mesa_clip_copytexsubimage() (in image.c, called from teximage.c).
>> Maybe you could do a bit of debugging to see why that's not doing the
>> job.
>>
>
> Yes, I have some doubts too, especially now when I've seen the comment
> before the st_copy_texsubimage definition, which explicitly states the
> region should be clipped already. AFAICS _mesa_clip_copytexsubimage is
> called only from copytexsubimage, but st_copy_texsubimage is called from
> copyteximage (without sub) too, that's why no clipping occurs. I'm not
> sure now where and how is better to fix it then, because my knowledge of
> this code is still not very good. On the other hand, st_copy_texsubimage
> already contains some clipping code, that's why I've added the checks in
> this function. How should I fix this correctly?

Here's a patch to try.

Basically, it reimplements glCopyTexImage() like a sequence of 
glTexImage() and glCopyTexSubImage() calls (and does the clipping that 
was missing).  I was planning on doing this as a follow-on to the 
map-texture-image-v4 work.

With this change, we can remove the ctx->Driver.CopyTexImage1D/2D() 
hooks and the corresponding code in all the drivers.

-Brian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clip-copyteximage.patch
Type: application/pgp-keys
Size: 3425 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110718/8adb62bc/attachment.key>