[aquaria] [PATCH] Fix logic bugs in reordering of RenderObjects within a layer.
Andrew Church
achurch+aquaria at achurch.org
Sat Sep 3 13:18:57 EDT 2011
No, sort() is never called as far as I was able to discern -- which is
probably why I didn't spend too much time cleaning it up. The current
RLT_FIXED code should extend or compress the array as needed, though,
and I can't reproduce the crash even after waiting ~1 minute (albeit with
my aquaria-psp repository -- too busy to rebuild icculus.org code at the
moment, sorry).
--Andrew Church
achurch at achurch.org
http://achurch.org/
>Reproduced it by starting up at the final save point, and then staying
>idle in sun form for ~20 secs. Crash.
>
>The problem is that these 2 calls ...
>
>lightFormGlow->moveToFront();
>lightFormGlowCone->moveToFront();
>
>...do a move front all the time, alternating, leaving a lot of NULL
>vector entries behind, until "firstFreeIdx = curIdx" reaches the end of
>the vector (preallocated with 100), and overflows the index in the while
>loop pointed out earlier.
>As this happens only every 0.5s, it takes a while to actually reach the end.
>
>The following snippet fixes it, but i suppose it is too hackish.
>Is the vector ever compacted? Seems to me that RenderObjectLayer::sort()
>is never actually called, and it does not set firstFreeIdx accordingly
>anyway - it should, imho. If it was called.
>
>
>
>diff -r 613cf0ce4519 BBGE/RenderObjectLayer.cpp
>--- a/BBGE/RenderObjectLayer.cpp Thu Aug 18 20:44:43 2011 +0900
>+++ b/BBGE/RenderObjectLayer.cpp Sat Sep 03 05:54:26 2011 +0200
>@@ -263,7 +263,11 @@
> r->setIdx(lastUsed);
> firstFreeIdx = curIdx;
> while (renderObjects[firstFreeIdx])
>+ {
> firstFreeIdx++;
>+ if(firstFreeIdx >= size)
>+ firstFreeIdx = 0;
>+ }
> }
> #endif // RLT_FIXED
> #ifdef RLT_DYNAMIC
>
>
>
>Am 03.09.2011 04:33, schrieb Andrew Church:
>> Can you find a way to consistently reproduce it? I can't do so here.
>> I'm happy to look into it if so, but otherwise your best bet may be to
>> switch back to RLT_DYNAMIC and linked list management -- RLT_FIXED was
>> mainly to avoid cache demolition on the PSP from all the pointer lookups,
>> and RLT_DYNAMIC shouldn't be significantly slower on PCs.
>>
>> FWIW, I have a more robust solution for the iPad port, but unfortunately
>> it's nontrivial to backport (the iPad port completely replaces BBGE with
>> a new engine).
>>
>> --Andrew Church
>> achurch at achurch.org
>> http://achurch.org/
>>
>>> This patch introduced a crash bug with the sun form (found by Sindhi).
>>> It goes out of index in a while (renderObjects[firstFreeIdx]) loop, when
>>> called from void Avatar::updateFormVisualEffects(), which uses
>>> moveToFront() twice. Seems it expects a NULL in the vector although
>>> there is none.
>>> Andrew, you wrote the code, any clue what is going wrong?
>>>
>>> -- fg
>>>
>>>
>>>
>>> Am 17.08.2011 13:54, schrieb Andrew Church:
>>>> # HG changeset patch
>>>> # User Andrew Church<achurch at achurch.org>
>>>> # Date 1313581717 -32400
>>>> # Node ID 9c350f140a62de19546039b36d88fd8e0e7d3442
>>>> # Parent 5542b94cae02a6333845854bbbd1abe0a259f1a4
>>>> Fix logic bugs in reordering of RenderObjects within a layer.
>>>>
>>>> This patch fixes logic bugs with state management for the array-based
>>>> RenderObjectLayer implementation which could cause crashes when an
>>>> object was moved to the back of the layer.
>>>>
>>>> This is the correct fix for the bug worked around in r199 (b00e51cba5bc).
>>>>
>>>> diff -r 5542b94cae02 -r 9c350f140a62 BBGE/RenderObjectLayer.cpp
>>>> --- a/BBGE/RenderObjectLayer.cpp Mon Aug 01 14:16:11 2011 -0400
>>>> +++ b/BBGE/RenderObjectLayer.cpp Wed Aug 17 20:48:37 2011 +0900
>>>> @@ -229,8 +229,8 @@
>>>> renderObjects[curIdx] = 0;
>>>> renderObjects[newIdx] = r;
>>>> r->setIdx(newIdx);
>>>> - if (firstFreeIdx == newIdx)
>>>> - firstFreeIdx++;
>>>> + if (firstFreeIdx> curIdx)
>>>> + firstFreeIdx = curIdx;
>>>> }
>>>> else if (objectCount == size)
>>>> {
>>>> @@ -242,7 +242,7 @@
>>>> r->setIdx(size);
>>>> for (int i = size+1; i< newSize; i++)
>>>> renderObjects[i] = 0;
>>>> - firstFreeIdx = size+1;
>>>> + firstFreeIdx = curIdx;
>>>> }
>>>> else
>>>> {
>>>> @@ -254,18 +254,16 @@
>>>> if (!renderObjects[lastFree])
>>>> break;
>>>> }
>>>> -
>>>> for (int i = lastFree + 1; i<= lastUsed; i++)
>>>> {
>>>> renderObjects[i-1] = renderObjects[i];
>>>> - if(renderObjects[i-1])
>>>> - renderObjects[i-1]->setIdx(i-1);
>>>> + renderObjects[i-1]->setIdx(i-1); // Known to be non-NULL.
>>>> }
>>>> -
>>>> renderObjects[lastUsed] = r;
>>>> r->setIdx(lastUsed);
>>>> - if (firstFreeIdx == lastFree)
>>>> - firstFreeIdx = lastUsed + 1;
>>>> + firstFreeIdx = curIdx;
>>>> + while (renderObjects[firstFreeIdx])
>>>> + firstFreeIdx++;
>>>> }
>>>> #endif // RLT_FIXED
>>>> #ifdef RLT_DYNAMIC
>>>> @@ -298,7 +296,9 @@
>>>> renderObjects[curIdx] = 0;
>>>> renderObjects[newIdx] = r;
>>>> r->setIdx(newIdx);
>>>> - if (firstFreeIdx == newIdx)
>>>> + // firstFreeIdx must be 0 here; if we filled slot 0, then
>>>> + // scan forward for the next empty element.
>>>> + while (renderObjects[firstFreeIdx])
>>>> firstFreeIdx++;
>>>> }
>>>> else if (objectCount == size)
>>>> @@ -312,8 +312,7 @@
>>>> for (int i = newSize - 1; i>= sizeDiff; i--)
>>>> {
>>>> renderObjects[i] = renderObjects[i - sizeDiff];
>>>> - if(renderObjects[i])
>>>> - renderObjects[i]->setIdx(i);
>>>> + renderObjects[i]->setIdx(i); // Known to be non-NULL.
>>>> }
>>>> for (int i = 0; i< newIdx; i++)
>>>> renderObjects[i] = 0;
>>>> @@ -329,8 +328,7 @@
>>>> for (int i = firstFreeIdx; i> 0; i--)
>>>> {
>>>> renderObjects[i] = renderObjects[i-1];
>>>> - if(renderObjects[i])
>>>> - renderObjects[i]->setIdx(i);
>>>> + renderObjects[i]->setIdx(i); // Known to be non-NULL.
>>>> }
>>>> renderObjects[0] = r;
>>>> r->setIdx(0);
>>>> _______________________________________________
>>>> aquaria mailing list
>>>> aquaria at icculus.org
>>>> http://icculus.org/mailman/listinfo/aquaria
>>>>
>>> _______________________________________________
>>> aquaria mailing list
>>> aquaria at icculus.org
>>> http://icculus.org/mailman/listinfo/aquaria
>> _______________________________________________
>> aquaria mailing list
>> aquaria at icculus.org
>> http://icculus.org/mailman/listinfo/aquaria
>>
>_______________________________________________
>aquaria mailing list
>aquaria at icculus.org
>http://icculus.org/mailman/listinfo/aquaria
More information about the aquaria
mailing list