[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