Re: [9/10] WineD3D: Use VBOs for index buffers

[Mirek]

Hi, this patch caused regression in many Direct3D games. TES IV: 
Oblivion can't start game (menu works), Flatout II is completly broken, 
Alpine Sky Racing 2007 can't run, HalfLife 2 EO can't run. I tried 
rewrite GLint vbo; to GLuint vbo; but it doesn't help.

Mirek

Stefan Dösinger napsal(a):
> From 4d52be700666b6b1e853bad296c502b5b0d76189 Mon Sep 17 00:00:00 2001
> From: Stefan Doesinger 
> Date: Mon, 19 Feb 2007 15:12:39 +0100
> Subject: [PATCH] WineD3D: Use VBOs for index buffers
>
> This patch uses (if available) gl vertex buffer objects for static d3d index buffers
> to store indices in video memory. Therefore a new index buffer state is added which
> binds the vbo to GL_ELEMENT_ARRAY_BUFFER. DrawStrided* use the index buffer stride to
> tell if indices are used for drawing because a NULL index data pointer is valid now.
> DrawStridedSlow has to read the indices from the index buffer if it is used, because
> it can't use the data from the vbo(or only very inefficiently).
>
> dlls/wined3d/device.c          |   68 ++++++++++++++++++++++++++++++++++++---
>  dlls/wined3d/drawprim.c        |   14 ++++++--
>  dlls/wined3d/indexbuffer.c     |   49 +++++++++++++++++++++++++++-
>  dlls/wined3d/state.c           |   12 +++++++
>  dlls/wined3d/wined3d_private.h |    8 ++++-
>  5 files changed, 139 insertions(+), 12 deletions(-)
>
> diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
> index 18f3e75..af0e96a 100644
> --- a/dlls/wined3d/device.c
> +++ b/dlls/wined3d/device.c
> @@ -443,21 +443,67 @@ static HRESULT WINAPI IWineD3DDeviceImpl_CreateVertexBuffer(IWineD3DDevice *ifac
>      return WINED3D_OK;
>  }
>
> +static void CreateIndexBufferVBO(IWineD3DDeviceImpl *This, IWineD3DIndexBufferImpl *object) {
> - GLenum error, glUsage;
> - TRACE("Creating VBO for Index Buffer %p\n", object);
> -
> - ENTER_GL();
> - while(glGetError());
> -
> - GL_EXTCALL(glGenBuffersARB(1, &object->vbo));
> - error = glGetError();
> - if(error != GL_NO_ERROR || object->vbo == 0) {
> -
>    ERR("Creating a vbo failed, continueing without vbo for this buffer\n");
> -
>    object->vbo = 0;
> -
>    return;
> - }
> -
> - GL_EXTCALL(glBindBufferARB(GL_ELEMENT_ARRAY_BUFFER_ARB, object->vbo));
> - error = glGetError();
> - if(error != GL_NO_ERROR) {
> -
>    ERR("Failed to bind index buffer, continueing without vbo for this buffer\n");
> -
>    goto out;
> - }
> -
> - /* Use static write only usage for now. Dynamic index buffers stay in sysmem, and due to the sysmem
> -
>    * copy no readback will be needed
> -
>    */
> - glUsage = GL_STATIC_DRAW;
> - GL_EXTCALL(glBufferDataARB(GL_ELEMENT_ARRAY_BUFFER_ARB, object->resource.size, NULL, glUsage));
> - error = glGetError();
> - if(error != GL_NO_ERROR) {
> -
>    ERR("Failed to initialize the index buffer\n");
> -
>    goto out;
> - }
> - LEAVE_GL();
> - TRACE("Successfully created vbo %d for index buffer %p\n", object->vbo, object);
> - return;
> -
>
> +out:
> - GL_EXTCALL(glBindBufferARB(GL_ELEMENT_ARRAY_BUFFER_ARB, 0));
> - GL_EXTCALL(glDeleteBuffersARB(1, &object->vbo));
> - LEAVE_GL();
> - object->vbo = 0;
>
> +}
> -
>
> static HRESULT WINAPI IWineD3DDeviceImpl_CreateIndexBuffer(IWineD3DDevice *iface, UINT Length, DWORD Usage, 
>                                                      WINED3DFORMAT Format, WINED3DPOOL Pool, IWineD3DIndexBuffer** ppIndexBuffer,
>                                                      HANDLE *sharedHandle, IUnknown *parent) {
>      IWineD3DDeviceImpl *This = (IWineD3DDeviceImpl *)iface;
>      IWineD3DIndexBufferImpl *object;
>      TRACE("(%p) Creating index buffer\n", This);
> -
> - /* Allocate the storage for the device */
>    D3DCREATERESOURCEOBJECTINSTANCE(object,IndexBuffer,WINED3DRTYPE_INDEXBUFFER, Length)
> -
> - /*TODO: use VBO's */
> - if (Pool == WINED3DPOOL_DEFAULT ) { /* Allocate some system memory for now */
> -
> -
>
> if (Pool == WINED3DPOOL_DEFAULT ) { /* We need a local copy for drawStridedSlow */
>     object->resource.allocatedMemory = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,object->resource.size);
> }
> -
>
> if(Pool != WINED3DPOOL_SYSTEMMEM && !(Usage & WINED3DUSAGE_DYNAMIC) && GL_SUPPORT(ARB_VERTEX_BUFFER_OBJECT)) {
> -
>    CreateIndexBufferVBO(This, object);
> -
>
> }
> -
>
> TRACE("(%p) : Len=%d, Use=%x, Format=(%u,%s), Pool=%d - Memory@%p, Iface@%p\n", This, Length, Usage, Format, 
>                           debug_d3dformat(Format), Pool, object, object->resource.allocatedMemory);
>    *ppIndexBuffer = (IWineD3DIndexBuffer *) object;
>
> @@ -2557,10 +2603,15 @@ static HRESULT WINAPI IWineD3DDeviceImpl_SetIndices(IWineD3DDevice *iface, IWine
>          return WINED3D_OK;
>      }
> - /* So far only the base vertex index is tracked */
> - /* The base vertex index affects the stream sources, while
> -
> * The index buffer is a seperate index buffer state
> -
> */
> if(BaseVertexIndex != oldBaseIndex) {
>        IWineD3DDeviceImpl_MarkStateDirty(This, STATE_STREAMSRC);
>    }
> - if(oldIdxs != pIndexData) {
> -
>    IWineD3DDeviceImpl_MarkStateDirty(This, STATE_INDEXBUFFER);
> - }
> return WINED3D_OK;
>
> }
>
> @@ -4224,9 +4275,11 @@ static HRESULT  WINAPI  IWineD3DDeviceImpl_DrawIndexedPrimitive(IWineD3DDevice *
>      UINT                 idxStride = 2;
>      IWineD3DIndexBuffer *pIB;
>      WINED3DINDEXBUFFER_DESC  IdxBufDsc;
> -
>
> GLint vbo;
>
> pIB = This->stateBlock->pIndexData;
> This->stateBlock->streamIsUP = FALSE;
> -
>
> vbo = ((IWineD3DIndexBufferImpl *) pIB)->vbo;
>
> TRACE("(%p) : Type=(%d,%s), min=%d, CountV=%d, startIdx=%d, countP=%d\n", This,
>       PrimitiveType, debug_d3dprimitivetype(PrimitiveType),
>
> @@ -4245,7 +4298,7 @@ static HRESULT  WINAPI  IWineD3DDeviceImpl_DrawIndexedPrimitive(IWineD3DDevice *
>      }
>  drawPrimitive(iface, PrimitiveType, primCount, 0, NumVertices, startIndex,
> -
>               idxStride, ((IWineD3DIndexBufferImpl *) pIB)->resource.allocatedMemory, minIndex);
> -
>               idxStride, vbo ? NULL : ((IWineD3DIndexBufferImpl *) pIB)->resource.allocatedMemory, minIndex);
>
> return WINED3D_OK;
>
> }
> @@ -4310,6 +4363,7 @@ static HRESULT WINAPI IWineD3DDeviceImpl_DrawIndexedPrimitiveUP(IWineD3DDevice *
>      This->stateBlock->loadBaseVertexIndex = 0;
>      /* Mark the state dirty until we have nicer tracking of the stream source pointers */
>      IWineD3DDeviceImpl_MarkStateDirty(This, STATE_VDECL);
> -
>
> IWineD3DDeviceImpl_MarkStateDirty(This, STATE_INDEXBUFFER);
>
> drawPrimitive(iface, PrimitiveType, PrimitiveCount, 0 /* vertexStart */, NumVertices, 0 /* indxStart */, idxStride, pIndexData, MinVertexIndex);
>
> @@ -4332,8 +4386,10 @@ static HRESULT WINAPI IWineD3DDeviceImpl_DrawPrimitiveStrided (IWineD3DDevice *i
>       * that value.
>       */
>      IWineD3DDeviceImpl_MarkStateDirty(This, STATE_VDECL);
> - IWineD3DDeviceImpl_MarkStateDirty(This, STATE_INDEXBUFFER);
> This->stateBlock->baseVertexIndex = 0;
> This->up_strided = DrawPrimStrideData;
> - This->stateBlock->streamIsUP = TRUE;
> drawPrimitive(iface, PrimitiveType, PrimitiveCount, 0, 0, 0, 0, NULL, 0);
> This->up_strided = NULL;
> return WINED3D_OK;
>
> diff --git a/dlls/wined3d/drawprim.c b/dlls/wined3d/drawprim.c
> index 2cc3a15..7cf6877 100644
> --- a/dlls/wined3d/drawprim.c
> +++ b/dlls/wined3d/drawprim.c
> @@ -571,7 +571,7 @@ static void drawStridedFast(IWineD3DDevice *iface,UINT numberOfVertices, GLenum
>                       const void *idxData, short idxSize, ULONG minIndex, ULONG startIdx, ULONG startVertex) {
>      IWineD3DDeviceImpl *This = (IWineD3DDeviceImpl *)iface;
> - if (idxData != NULL /* This crashes sometimes!*/) {
> - if (idxSize != 0 /* This crashes sometimes!*/) {
>     TRACE("(%p) : glElements(%x, %d, %d, ...)\n", This, glPrimitiveType, numberOfVertices, minIndex);
>     idxData = idxData == (void *)-1 ? NULL : idxData;
>
> #if 1
> @@ -629,7 +629,15 @@ static void drawStridedSlow(IWineD3DDevice *iface, WineDirect3DVertexStridedData
>      TRACE("Using slow vertex array code\n");
>  /* Variable Initialization */
> - if (idxData != NULL) {
> - if (idxSize != 0) {
> -
>    /* Immediate mode drawing can't make use of indices in a vbo - get the data from the index buffer.
> -
>     * If the index buffer has no vbo(not supported or other reason), or with user pointer drawing
> -
>     * idxData will be != NULL
> -
>     */
> -
>    if(idxData == NULL) {
> -
>        idxData = ((IWineD3DIndexBufferImpl *) This->stateBlock->pIndexData)->resource.allocatedMemory;
> -
>    }
> -
>    if (idxSize == 2) pIdxBufS = (const short *) idxData;
>    else pIdxBufL = (const long *) idxData;
> }
>
> @@ -1165,7 +1173,7 @@ inline void drawStridedInstanced(IWineD3DDevice *iface, WineDirect3DVertexStride
>      IWineD3DDeviceImpl *This = (IWineD3DDeviceImpl *) iface;
>      IWineD3DStateBlockImpl *stateblock = This->stateBlock;
> - if (idxData == NULL) {
> - if (idxSize == 0) {
>     /* This is a nasty thing. MSDN says no hardware supports that and apps have to use software vertex processing.
>      * We don't support this for now
>      *
>
> diff --git a/dlls/wined3d/indexbuffer.c b/dlls/wined3d/indexbuffer.c
> index 39a270d..0f768ac 100644
> --- a/dlls/wined3d/indexbuffer.c
> +++ b/dlls/wined3d/indexbuffer.c
> @@ -58,6 +58,15 @@ static ULONG WINAPI IWineD3DIndexBufferImpl_Release(IWineD3DIndexBuffer *iface)
>      ULONG ref = InterlockedDecrement(&This->resource.ref);
>      TRACE("(%p) : Releasing from %d\n", This, ref + 1);
>      if (ref == 0) {
> -
>    if(This->vbo) {
> -
>        ENTER_GL();
> -
>        GL_EXTCALL(glBindBufferARB(GL_ELEMENT_ARRAY_BUFFER_ARB, 0));
> -
>        checkGLcall("glBindBufferARB");
> -
>        GL_EXTCALL(glDeleteBuffersARB(1, &This->vbo));
> -
>        checkGLcall("glDeleteBuffersARB");
> -
>        LEAVE_GL();
> -
>    }
> -
>    IWineD3DResourceImpl_CleanUp((IWineD3DResource *)iface);
>    HeapFree(GetProcessHeap(), 0, This);
> }
>
> @@ -108,13 +117,49 @@ static HRESULT WINAPI IWineD3DIndexBufferImpl_GetParent(IWineD3DIndexBuffer *ifa
>     ****************************************************** */
>  static HRESULT WINAPI IWineD3DIndexBufferImpl_Lock(IWineD3DIndexBuffer *iface, UINT OffsetToLock, UINT SizeToLock, BYTE** ppbData, DWORD Flags) {
>      IWineD3DIndexBufferImpl *This = (IWineD3DIndexBufferImpl *)iface;
> - TRACE("(%p) : no real locking yet, offset %d, size %d, Flags=%x\n", This, OffsetToLock, SizeToLock, Flags);
> - TRACE("(%p) : offset %d, size %d, Flags=%x\n", This, OffsetToLock, SizeToLock, Flags);
> -
> - InterlockedIncrement(&This->lockcount);
> *ppbData = (BYTE *)This->resource.allocatedMemory + OffsetToLock;
> -
> - if(Flags & (WINED3DLOCK_READONLY | WINED3DLOCK_NO_DIRTY_UPDATE) || This->vbo == 0) {
> -
>    return WINED3D_OK;
> - }
> -
> - if(This->dirtystart != This->dirtyend) {
> -
>    if(This->dirtystart > OffsetToLock) This->dirtystart = OffsetToLock;
> -
>    if(SizeToLock) {
> -
>        if(This->dirtyend < OffsetToLock + SizeToLock) This->dirtyend = OffsetToLock + SizeToLock;
> -
>    } else {
> -
>        This->dirtyend = This->resource.size;
> -
>    }
> - } else {
> -
>    This->dirtystart = OffsetToLock;
> -
>    if(SizeToLock)
> -
>        This->dirtyend = OffsetToLock + SizeToLock;
> -
>    else
> -
>        This->dirtyend = This->resource.size;
> - }
> - return WINED3D_OK;
>
> }
>  static HRESULT WINAPI IWineD3DIndexBufferImpl_Unlock(IWineD3DIndexBuffer *iface) {
>      IWineD3DIndexBufferImpl *This = (IWineD3DIndexBufferImpl *)iface;
> - TRACE("(%p) : stub\n", This);
> - unsigned long locks = InterlockedDecrement(&This->lockcount);;
> - TRACE("(%p)\n", This);
> -
> - /* For now load in unlock */
> - if(locks == 0 && This->vbo) {
> -
>    ENTER_GL();
> -
>    GL_EXTCALL(glBindBufferARB(GL_ELEMENT_ARRAY_BUFFER_ARB, This->vbo));
> -
>    checkGLcall("glBindBufferARB");
> -
>    GL_EXTCALL(glBufferSubDataARB(GL_ELEMENT_ARRAY_BUFFER_ARB,
> -
>            This->dirtystart, This->dirtyend - This->dirtystart, This->resource.allocatedMemory + This->dirtystart));
> -
>    checkGLcall("glBufferSubDataARB");
> -
>    LEAVE_GL();
> -
>    This->dirtystart = 0;
> -
>    This->dirtyend = 0;
> - }
> return WINED3D_OK;
>
> }
>  static HRESULT WINAPI IWineD3DIndexBufferImpl_GetDesc(IWineD3DIndexBuffer *iface, WINED3DINDEXBUFFER_DESC *pDesc) {
> diff --git a/dlls/wined3d/state.c b/dlls/wined3d/state.c
> index 0b96e54..f78720a 100644
> --- a/dlls/wined3d/state.c
> +++ b/dlls/wined3d/state.c
> @@ -3062,6 +3062,17 @@ static void scissorrect(DWORD state, IWineD3DStateBlockImpl *stateblock, WineD3D
>      checkGLcall("glScissor");
>  }
>
> +static void indexbuffer(DWORD state, IWineD3DStateBlockImpl *stateblock, WineD3DContext *context) {
> - if(GL_SUPPORT(ARB_VERTEX_BUFFER_OBJECT)) {
> -
>    if(stateblock->streamIsUP || stateblock->pIndexData == NULL ) {
> -
>        GL_EXTCALL(glBindBufferARB(GL_ELEMENT_ARRAY_BUFFER_ARB, 0));
> -
>    } else {
> -
>        IWineD3DIndexBufferImpl *ib = (IWineD3DIndexBufferImpl *) stateblock->pIndexData;
> -
>        GL_EXTCALL(glBindBufferARB(GL_ELEMENT_ARRAY_BUFFER_ARB, ib->vbo));
> -
>    }
> - }
>
> +}
> -
>
> const struct StateEntry StateTable[] =
>  {
>        /* State name                                         representative,                                     apply function */
> @@ -4077,6 +4088,7 @@ const struct StateEntry StateTable[] =
>      { /*511, WINED3DTS_WORLDMATRIX(255)             */      STATE_TRANSFORM(WINED3DTS_WORLDMATRIX(255)),        transform_worldex   },
>        /* Various Vertex states follow */
>      { /*   , STATE_STREAMSRC                        */      STATE_VDECL,                                        vertexdeclaration   },
> - { /*   , STATE_INDEXBUFFER                      */      STATE_INDEXBUFFER,                                  indexbuffer         },
> { /*   , STATE_VDECL                            */      STATE_VDECL,                                        vertexdeclaration   },
> { /*   , STATE_VSHADER                          */      STATE_VDECL,                                        vertexdeclaration   },
> { /*   , STATE_VIEWPORT                         */      STATE_VIEWPORT,                                     viewport            },
>
> diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h
> index 3bf0c8e..be8cd6e 100644
> --- a/dlls/wined3d/wined3d_private.h
> +++ b/dlls/wined3d/wined3d_private.h
> @@ -426,8 +426,10 @@ typedef void (*APPLYSTATEFUNC)(DWORD state, IWineD3DStateBlockImpl *stateblock,
>
> #define STATE_STREAMSRC (STATE_TRANSFORM(WINED3DTS_WORLDMATRIX(255)) + 1)
>  #define STATE_IS_STREAMSRC(a) ((a) == STATE_STREAMSRC)
> +#define STATE_INDEXBUFFER (STATE_STREAMSRC + 1)
> +#define STATE_IS_INDEXBUFFER(a) ((a) == STATE_INDEXBUFFER)
>
> -#define STATE_VDECL (STATE_STREAMSRC + 1)
> +#define STATE_VDECL (STATE_INDEXBUFFER + 1)
>  #define STATE_IS_VDECL(a) ((a) == STATE_VDECL)
>
> #define STATE_VSHADER (STATE_VDECL + 1)
> @@ -793,6 +795,10 @@ typedef struct IWineD3DIndexBufferImpl
>      const IWineD3DIndexBufferVtbl *lpVtbl;
>      IWineD3DResourceClass     resource;
> - GLuint                    vbo;
> - UINT                      dirtystart, dirtyend;
> - LONG                      lockcount;
> - /* WineD3DVertexBuffer specifics */
>
> } IWineD3DIndexBufferImpl;