[sldev] Compare functions (llspatialpartition.h)

[Nicholaz Beresford]

They are called for sorts on DrawInfo structure (if you grep
CompareTexturePtr or CompareBump you'll just see one reference
for a std::sort).

In fact I have no idea why someone would sort an array of objects
by address ... except maybe hoping to optimize for cpu cache prefetch
or something.


Nick


Second Life from the inside out:
http://nicholaz-beresford.blogspot.com/


Matthew Dowd wrote:
> I think you are correct.
> 
> However, where are these functions called? Whilst the current behaviour may not be correct, is there any code relying on the current (faulty) behaviour which would need fixing too?
> 
> Matthew
> 
> 
> 
> 
> ----------------------------------------
>> Date: Thu, 5 Jul 2007 17:59:48 +0200
>> From: nicholaz at blueflash.cc
>> To: sldev at lists.secondlife.com
>> Subject: [sldev] Compare functions (llspatialpartition.h)
>>
>>
>> There are two compare functions in llspatialpartition.h which I believe
>> are wrong.
>>
>> When doing pointer compares, if NULL pointers are involved I think the
>> result should be consistent (i.e. a NULL either being always considered
>> more or less than a pointer value).  With these implementations however,
>> NULL is considered greater if it appears as lhs, but if it appears as
>> rhs the function returns also TRUE, which implicitely means that NULL
>> is lesser (as far as I understand it, these compare functions for std::sort
>> are bound to return if left is greater than right.  Also in case both are
>> NULL, I think false should be returned (neither is greater).
>>
>> Shouldn't that be (assuming that NULL should be greater, to be sorted to
>> the end of the list):
>>
>> return lhs != rhs && (lhs == NULL || (rhs != NULL && lhs->mTexture > rhs->mTexture));
>>
>> Or am I missing something?
>>
>>
>>
>> Original snippet:
>>
>> struct CompareTexturePtr
>> {
>>      bool operator()(const LLDrawInfo* const& lhs, const LLDrawInfo* const& rhs)
>>      {
>>
>>          return lhs == NULL || rhs == NULL || lhs->mTexture > rhs->mTexture;
>>      }
>> };
>>
>> struct CompareBump
>> {
>>      bool operator()(const LLDrawInfo* const& lhs, const LLDrawInfo* const& rhs)
>>      {
>>          return lhs == NULL || rhs == NULL || lhs->mBump > rhs->mBump;
>>      }
>> };
>>
>> -- 
>> Second Life from the inside out:
>> http://nicholaz-beresford.blogspot.com/
>> _______________________________________________
>> Click here to unsubscribe or manage your list subscription:
>> /index.html
> 
> _________________________________________________________________
> 100’s of Music vouchers to be won with MSN Music
> https://www.musicmashup.co.uk/index.html