On 08/04/2008, David Adam <david.adam.cnrs at gmail.com <http://www.winehq.org/mailman/listinfo/wine-patches>> wrote: >* + This->current = This->current +1; *>* + HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, This->matrix, (This->current +1) * sizeof(D3DXMATRIX) ); *>* + if ( This->matrix == NULL ) return E_OUTOFMEMORY; * > Aside from being a bit suboptimal (doing a realloc on every push), > this probably doesn't do what you want. Consider what happens to the > size of the array when you do something like push/pop/push/pop/...etc. > > It would be better to keep track of the current stack size, and then > grow it by a factor (eg. 2 or 1.5) if a push would overflow the > current stack. > You could also consider shrinking the array again if a > pop would make it use less than a certain percentage of the current > size (eg. a third).
This would increase the size of the stack exponentially. Would it be better to do this: Take a stack_size_reference (for instance 32 items) When This->current =32, then increase the size of the stack of stack_size_reference (that is 64 items now) Then when This->current is =64, again increase the size of the stack of stack_size_reference (that is 98 items now), and so on.... This would increase the size of the stack linearly instead of exponentially. In the opposite, for MatrixStack_Pop, assume that the size of the stack is 98 and This->current is 64. Then one could shrink the size of the stack from 98 to 98-size_stack_reference that is 64. > You should also assign the result of HeapReAlloc() to This->matrix > again. *I thought that HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, This->matrix, ..........);** * do exactly what you say Although it's quite possible for HeapReAlloc to just grow the > current block without changing its location, there's no guarantee it > will. The NULL check is useless this way. Thanks for your very useful feedback. David
