[aquaria] [PATCH] Fix logic bugs in reordering of RenderObjects within a layer.
False.Genesis
false.genesis at googlemail.com
Fri Sep 2 22:20:23 EDT 2011
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
>
More information about the aquaria
mailing list