d3dx8 [6/7] Implement D3DXMatrixStack_Push

tony.wasserka at freenet.de tony.wasserka at freenet.de
Wed Apr 9 06:27:41 CDT 2008


> > +    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).
> 
> You should also assign the result of HeapReAlloc() to This->matrix
> again. 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.

Just a little addition, I think we should resize the stack differently at different situations, i.e.:
- resize it to (This->current+3) if This->currect<5 (something around that is the minimum number of matrices for which anyone would even bother using a MatrixStack)
- resize it to (This->current*2) if This->current<12 (when it seems like the application's gonna push many matrices on the stack)
- resize it to (This->current+16) if This->current>=12 (when *2 could waste memory)
However, I don't know if these numbers are optimal, it's just a suggestion.

For the Pop function, I agree that you should free some memory when it's unused, but my suggestion from above fits here, too. However, one third is okay, too, as it isn't that relevant here.












More information about the wine-devel mailing list