Re: several bugs in pack/unpacking of textures

I do apologize for jumping the gun there. At the same time I did that, 
though, I fixed some functions where the packet size was being 
implicitly added to the packed data after calling crPackAlloc, which 
sets that itself, as Greg pointed out.

Looking through what I added to swap_texture, I noticed that most of 
the SWAP functions in pack_swap_texture make calls to crPackFree after 
crHugePacket.  I don't see any significant differences in those 
functions other than swapping the bytes, so perhaps those functions 
should be looked into as well.

  -Chris

On Apr 11, 2005, at 12:07 PM, Greg Humphreys wrote:

> He already checked in the extra calls to crPackFree(), which means
> they are probably being called twice and should be rolled back.  Note
> that this bug will only manifest itself if the texture is larger than
> the pack buffer size, since crPackFree() is a NOP if the packet fits
> in the buffer.
>
> We have to leave the free'ing up to the SPU because some SPU's want to
> keep these pack buffers around for a long time for later analysis or
> playback.  You can't assume that HugePacket() is a fire-and-forget
> model.
>
> On Apr 11, 2005 11:25 AM, Brian Paul <[email protected]> 
> wrote:
>>
>> I just did a quick look into this, you should double-check before
>> chaning anything.
>>
>> Anyway, if you call crPackAlloc() you should use crHugePacket() send
>> the message.  crHugePacket() calls the pc->SendHuge() callback.  In
>> the case of the Pack SPU, this is the packspuHuge() function.  The
>> last thing that function does is to call crPackFree() to deallocate
>> the buffer.
>>
>> You might check that the packer functions which call crPackAlloc()
>> also call crHugePacket().
>>
>> -Brian
>>
>>
>> Christopher Waters wrote:
>>> I went through a little more of the packer/unpacker code and tossed 
>>> in a
>>> few more fixes. I also took out one of the "special case" functions,
>>> which seems to probably be the only one that 'needed' to be done at 
>>> all.
>>>
>>> I'm finding some quirks in that crPackFree is not being called in the
>>> functions that use crPackAlloc... shouldn't it always be called?
>>>
>>> -Chris
>>>
>>> Brian Paul wrote:
>>>
>>>> Christopher Waters wrote:
>>>>
>>>>> I just threw together the other 5.  I'm not checking it in myself
>>>>> because I'm only 99% sure it's right, mainly because I've never
>>>>> touched the unpacker before, but it was easy enough.  I also fixed
>>>>> the function that was just added (GLenum was used in all the
>>>>> READ_DATAs, instead of each variable's type)
>>>>
>>>>
>>>>
>>>> The code looks good to me.  I've checked it in.  Thanks!
>>>>
>>>> -Brian
>>>>
>>>>
>>>
>>>
>>> -------------------------------------------------------
>>> SF email is sponsored by - The IT Product Guide
>>> Read honest & candid reviews on hundreds of IT Products from real 
>>> users.
>>> Discover which products truly live up to the hype. Start reading now.
>>> http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
>>> _______________________________________________
>>> Chromium-dev mailing list
>>> [email protected]
>>> https://lists.sourceforge.net/lists/listinfo/chromium-dev
>>>
>>
>> -------------------------------------------------------
>> SF email is sponsored by - The IT Product Guide
>> Read honest & candid reviews on hundreds of IT Products from real 
>> users.
>> Discover which products truly live up to the hype. Start reading now.
>> http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
>> _______________________________________________
>> Chromium-dev mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/chromium-dev
>>
>>
>
>
> -- 
> Greg Humphreys, Assistant Professor
> Department of Computer Science, University of Virginia
> http://www.cs.virginia.edu/~humper/
>



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click