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

[Brian Paul]

On 07/18/2011 09:19 PM, Brian Paul wrote:
> 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.
>

I've made a branch that does this:  remove-copyteximage-hook

The intel and radeon drivers and the gallium state tracker were 
basically doing what core Mesa is now doing.

I think it's a pretty safe change but it wouldn't hurt if a few people 
tested the branch.  I'll merge it to master in a day or two.

It always feels good to remove a few hundred lines of code.

-Brian