2D Engine

Some points:

• First, you should really check that the Lock() on your vertex buffer succeeds (With the SUCCEEDED or FAILED macro). If it fails, you assume the buffer is locked, and you'll crash your app.


• Aside from that, how big is your vertex buffer? I.e. how many times are you Lock()ing it per frame? You really want to avoid locking a single vertex buffer multiple times per frame.


• One optimisation you could make is you could create a (static) index buffer, and fill it with indices. That way you can use DrawIndexedPrimitive() and cut your code down to 4 vertices per sprite instead of 6. The index buffer doesn't need to be touched on a per-frame basis, because the indices will always be the same.


• What flags do you use to create the vertex buffer? You should be using D3DUSAGE_WRITEONLY | D3DUSAGE_DYNAMIC.


• This Link may be useful to have a read over, although you seem to be doing things the "right way".


• Keeping a vertex buffer locked for a long time is bad - the GPU can't do anything with the buffer while it's locked. I'd recommend keeping a copy of the vertices (I.e. allocate an array of vertices the size of your vertex buffer), writing into that copy in the BlitSprite() function, and then in the FlushVertexBuffer function you can Lock() the vertex buffer, memcpy() into the pointer, and Unlock() it, which keeps the buffer locked for the minimum amount of time


• Just as a style thing; I'd make the cSpriteGroup class take a LPDIRECT3DDEVICE9 as a constructor parameter and then store it as a member variable (And AddRef() it, then Release() it in the destructor). That way you don't need to pass in the device pointer to BlitSprite()

Hope this helps.

Cheers,
Steve

Quote:

Original post by Evil Steve (And AddRef() it, then Release() it in the destructor).

AddRef and Release is only for people too foolish to use CComPtr, _com_ptr_t, or
boost::shared_ptr. [grin]

Quote:

Original post by MJP AddRef and Release is only for people too foolish to use CComPtr, _com_ptr_t, or boost::instrusive_ptr. [grin]

That's true [smile]. I still need to get around to using them in my engine :P

Quote:

Original post by Evil Steve That's true [smile]. I still need to get around to using them in my engine :P

I have vivid memories of doing lots of Find & Replace when I replaced all the raw interface pointers with CComPtr in my old engine...it took a while but it was worth it to get rid of the memory leak problems and all the nasty AddRef/Release code everywhere.

Hi Evil Steve, thank you for the tips!

answering your points:
* I guess the lock always works 'cos never had a crash there - but I'll check that;
* the buffer is 1200 vertex big, and during these tests I'm doing, I lock it at least 8 times a frame. I am blitting 1664 sprites that times 6 are more or less 10000 vertices;
* to create the buffer I use D3DUSAGE_WRITEONLY | D3DUSAGE_DYNAMIC:

    pDevice->CreateVertexBuffer( m_dwMaxVertices * sizeof( CUSTOMVERTEX ), D3DUSAGE_DYNAMIC | D3DUSAGE_WRITEONLY, 0, D3DPOOL_DEFAULT, &m_pVertexBuffer, NULL );

* I will try the memcopy thing too, that's a good idea!

I have just one more question: What's that "CComPtr, _com_ptr_t, or boost::shared_pt" thing? No idea bout that :(

Thanks again
Fibo

They're all classes you can use that will automatically calling AddRef and Release for you so that you don't have to. shared_ptr is actually meant for automatically calling delete on pointers once they've gone out of scope, but the page I linked to shows you how to use it with COM interfaces.

shared_ptr is very much worth looking into for anything you're creating with new.

Thanks MJP,
gonna take a better look at it! although I don't think passing a pointer to a function is a bad thing to do...
cheers
Fibo

Well I was half-kidding when I made that first post, I just think it's good that people are at least aware that such tools exist. Whether or not you think they should be used is up to you. [smile]

Quote:

Original post by Fibo * I guess the lock always works 'cos never had a crash there - but I'll check that;

I'm pretty sure it's succeeding, but that;s not to say that it works on every card out there. You really need to check the return value for every single D3D call that returns (in some way) a pointer, or fills in some structure whose data you then rely on.

Quote:

Original post by Fibo * the buffer is 1200 vertex big, and during these tests I'm doing, I lock it at least 8 times a frame. I am blitting 1664 sprites that times 6 are more or less 10000 vertices;

Eep! That'll be the problem then [smile]. With that many locks of a single buffer in one frame, the driver almost certainly won't be able to do buffer renaming (A clever performance thing it does internally), meaning you'll be stalling the GPU at some point. The index buffer trick would probably help (Since you'll reduce the number of vertices, and therefore the number of locks), but I'd recommend making the buffer "Pretty big". I seem to recall from a (now really quite old) ATI or NVidia presentation, that 4MB was a good size for a dynamic vertex buffer. Assuming 32-bytes per vertex, that's 131072 vertices. Your 1200 vertex buffer is (Again, at 32 bytes per vertex) 37.5KB, which is pretty tiny. Upping the vertex buffer size should give a significant improvement; Ideally you should only lock the vertex buffer once per frame (The Debug Runtimes will probably scream at you for locking a vertex buffer twice per frame, although that might only be static VBs), and multiple locks should only happen in unusual cases.
In my engine, I allocate a vertex buffer twice the size I expect I need for one frame, and alternately lock with DISCARD and NOOVERWRITE flags, so a buffer is discarded every second frame (Usually).