[WINED3D 1/3] Clean up ActiveRender

Ivan Gyurdiev ivg231 at gmail.com
Mon Sep 4 00:25:08 CDT 2006


Clean up ActiveRender in preparation for FBOs.
It may be better to compare before/after to follow this patch.

More significant changes include:
- break out finding a GLXFBConfig in its own function,
  and only use that in pbuffer case

- break out reapplying of the stateblock and use that immediately
  after glXMakeCurrent in pbuffer and onscreen case (note that FBOs will 
not be calling
  this, and they'll be added to this codepath in the future).

- break out updating renderUpsideDown (and related states) in the 
render-to-texture case in its own function.
  Call that with TRUE or FALSE depending on whether we're rendering to a 
texture.

- apply policy of obtaining all references to swapchains at the beginning
  of the function, and releasing them at the end. This is safer, much 
more clear,
  and saves us at least 1 GetContainer call. Also name swapchain 
references consistently,
  add Impl variables, and remove all casts.

-------------- next part --------------
---
 dlls/wined3d/device.c |  325 +++++++++++++++++++++++++++----------------------
 1 files changed, 182 insertions(+), 143 deletions(-)

diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index c6a2af5..463607f 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -7314,51 +7314,73 @@ #endif
 }
 #endif
 
-/** FIXME: This is currently used called whenever SetRenderTarget or SetStencilBuffer are called
-* the functionality needs splitting up so that we don't do more than we should do.
-* this only seems to impact performance a little.
- ******************************/
-static HRESULT WINAPI IWineD3DDeviceImpl_ActiveRender(IWineD3DDevice* iface,
-                                               IWineD3DSurface *RenderSurface) {
-    HRESULT ret =  WINED3DERR_INVALIDCALL;
+/* Reapply the device stateblock */
+static void device_reapply_stateblock(IWineD3DDeviceImpl* This) {
+
+    BOOL oldRecording;  
+    IWineD3DStateBlockImpl *oldUpdateStateBlock;
+
+    /* Disable recording */
+    oldUpdateStateBlock = This->updateStateBlock;
+    oldRecording= This->isRecordingState;
+    This->isRecordingState = FALSE;
+    This->updateStateBlock = This->stateBlock;
+
+    /* Reapply the state block */ 
+    IWineD3DStateBlock_Apply((IWineD3DStateBlock *)This->stateBlock);
+
+    /* Restore recording */
+    This->isRecordingState = oldRecording;
+    This->updateStateBlock = oldUpdateStateBlock;
+}
+
+/* Set the device to render to a texture, or not.
+ * This involves changing renderUpsideDown */
+
+static void device_render_to_texture(IWineD3DDeviceImpl* This, BOOL isTexture) {
+
+    DWORD cullMode;
     BOOL oldRecording;
     IWineD3DStateBlockImpl *oldUpdateStateBlock;
 
-    /**
-    * Currently only active for GLX >= 1.3
-    * for others versions we'll have to use GLXPixmaps
-    *
-    * normally we must test GLX_VERSION_1_3 but nvidia headers are not correct
-    * as they implement GLX 1.3 but only define GLX_VERSION_1_2
-    * so only check OpenGL version
-    * ..........................
-    * I don't believe that it is a problem with NVidia headers,
-    * XFree only supports GLX1.2, nVidia (and ATI to some extent) provide 1.3 functions
-    * in GLX 1.2, there is no mention of the correct way to tell if the extensions are provided.
-    * ATI Note:
-    * Your application will report GLX version 1.2 on glXQueryVersion.
-    * However, it is safe to call the GLX 1.3 functions as described below.
-    */
-#if defined(GL_VERSION_1_3)
+    /* Disable recording */
+    oldUpdateStateBlock = This->updateStateBlock;
+    oldRecording= This->isRecordingState;
+    This->isRecordingState = FALSE;
+    This->updateStateBlock = This->stateBlock;
+
+    /* Set upside-down rendering, and update the cull mode */
+    /* The surface must be rendered upside down to cancel the flip produced by glCopyTexImage */
+    This->renderUpsideDown = isTexture;
+    This->last_was_rhw = FALSE;
+    This->proj_valid = FALSE;
+    IWineD3DDevice_GetRenderState((IWineD3DDevice*) This, WINED3DRS_CULLMODE, &cullMode);
+    IWineD3DDevice_SetRenderState((IWineD3DDevice*) This, WINED3DRS_CULLMODE, cullMode);
+
+    /* Restore recording */
+    This->isRecordingState = oldRecording;
+    This->updateStateBlock = oldUpdateStateBlock;
+}
+
+/* Returns an array of compatible FBconfig(s).
+ * The array must be freed with XFree. Requires ENTER_GL() */
+
+static GLXFBConfig* device_find_fbconfigs(
+    IWineD3DDeviceImpl* This,
+    IWineD3DSwapChainImpl* implicitSwapchainImpl,
+    IWineD3DSurface* RenderSurface) {
 
-    IWineD3DDeviceImpl *This = (IWineD3DDeviceImpl *)iface;
-    IWineD3DSurface *StencilSurface = This->stencilBufferTarget;
-    IWineD3DSurface *tmp;
-    /** TODO: we only need to look up the configuration !IF! we are setting the target to a texture **/
     GLXFBConfig* cfgs = NULL;
     int nCfgs = 0;
     int attribs[256];
     int nAttribs = 0;
-    IWineD3DSwapChain     *currentSwapchain;
-    IWineD3DSwapChainImpl *swapchain;
-    /** TODO: get rid of Impl usage we should always create a zbuffer/stencil with our contexts if possible,
-    * but switch them off if the StencilSurface is set to NULL
-    ** *********************************************************/
+
+    IWineD3DSurface *StencilSurface = This->stencilBufferTarget;
     D3DFORMAT BackBufferFormat = ((IWineD3DSurfaceImpl *) RenderSurface)->resource.format;
     D3DFORMAT StencilBufferFormat = (NULL != StencilSurface) ? ((IWineD3DSurfaceImpl *) StencilSurface)->resource.format : 0;
 
     /**TODO:
-        if StencilSurface == NULL && zBufferTarget != NULL then switch the zbuffer off,
+        if StencilSurface == NULL && zBufferTarget != NULL then switch the zbuffer off, 
         it StencilSurface != NULL && zBufferTarget == NULL switch it on
     */
 
@@ -7367,43 +7389,40 @@ #define PUSH2(att,value)  attribs[nAttri
 
     /* PUSH2(GLX_BIND_TO_TEXTURE_RGBA_ATI, True); examples of this are few and far between (but I've got a nice working one!)*/
 
-    /** TODO: remove the reff to Impl (context manager should fix this!) **/
-    IWineD3DSwapChainImpl *impSwapChain;
-    IWineD3DDevice_GetSwapChain(iface, 0, (IWineD3DSwapChain **)&impSwapChain);
-    if (NULL == impSwapChain) { /* NOTE: This should NEVER fail */
-        ERR("(%p) Failed to get a the implicit swapchain\n", iface);
-    }
-
-    ENTER_GL();
-
     PUSH2(GLX_DRAWABLE_TYPE, GLX_PBUFFER_BIT);
     PUSH2(GLX_X_RENDERABLE,  TRUE);
     PUSH2(GLX_DOUBLEBUFFER,  TRUE);
     TRACE("calling makeglcfg\n");
     D3DFmtMakeGlCfg(BackBufferFormat, StencilBufferFormat, attribs, &nAttribs, FALSE /* alternate */);
     PUSH1(None);
-
     TRACE("calling chooseFGConfig\n");
-    cfgs = glXChooseFBConfig(impSwapChain->display, DefaultScreen(impSwapChain->display),
-                                                     attribs, &nCfgs);
-
-    if (!cfgs) { /* OK we didn't find the exact config, so use any reasonable match */
+    cfgs = glXChooseFBConfig(implicitSwapchainImpl->display,
+                             DefaultScreen(implicitSwapchainImpl->display),
+                             attribs, &nCfgs);
+    if (cfgs == NULL) {
+        /* OK we didn't find the exact config, so use any reasonable match */
         /* TODO: fill in the 'requested' and 'current' depths, also make sure that's
            why we failed and only show this message once! */
-        MESSAGE("Failed to find exact match, finding alternative but you may suffer performance issues, try changing xfree's depth to match the requested depth\n"); /**/
+        MESSAGE("Failed to find exact match, finding alternative but you may "
+            "suffer performance issues, try changing xfree's depth to match the requested depth\n");
         nAttribs = 0;
         PUSH2(GLX_DRAWABLE_TYPE, GLX_PBUFFER_BIT | GLX_WINDOW_BIT);
-       /* PUSH2(GLX_X_RENDERABLE,  TRUE); */
+        /* PUSH2(GLX_X_RENDERABLE,  TRUE); */
         PUSH2(GLX_RENDER_TYPE,   GLX_RGBA_BIT);
         PUSH2(GLX_DOUBLEBUFFER, FALSE);
         TRACE("calling makeglcfg\n");
         D3DFmtMakeGlCfg(BackBufferFormat, StencilBufferFormat, attribs, &nAttribs, TRUE /* alternate */);
         PUSH1(None);
-        cfgs = glXChooseFBConfig(impSwapChain->display, DefaultScreen(impSwapChain->display),
-                                                        attribs, &nCfgs);
-    }
-
-    if (NULL != cfgs) {
+        cfgs = glXChooseFBConfig(implicitSwapchainImpl->display,
+                                 DefaultScreen(implicitSwapchainImpl->display),
+                                 attribs, &nCfgs);
+    }
+                                                                                                                                                                      
+    if (cfgs == NULL) {
+        ERR("Could not get a valid FBConfig for (%u,%s)/(%u,%s)\n",
+            BackBufferFormat, debug_d3dformat(BackBufferFormat),
+            StencilBufferFormat, debug_d3dformat(StencilBufferFormat));
+    } else {
 #ifdef EXTRA_TRACES
         int i;
         for (i = 0; i < nCfgs; ++i) {
@@ -7411,7 +7430,6 @@ #ifdef EXTRA_TRACES
             debug_d3dformat(BackBufferFormat), StencilBufferFormat,
             debug_d3dformat(StencilBufferFormat), i, cfgs[i]);
         }
-
         if (NULL != This->renderTarget) {
             glFlush();
             vcheckGLcall("glFlush");
@@ -7420,24 +7438,75 @@ #ifdef EXTRA_TRACES
             * the current buffer on the screen. This is easy to do in glx1.3 but
             * we need to do copy-write pixels in glx 1.2.
             ************************************************/
-            glXSwapBuffers(impSwapChain->display, impSwapChain->drawable);
-
+            glXSwapBuffers(implicitSwapChainImpl->display,
+                           implicitSwapChainImpl->drawable);
             printf("Hit Enter to get next frame ...\n");
             getchar();
         }
 #endif
     }
+#undef PUSH1
+#undef PUSH2
+
+   return cfgs;
+}
 
-    if (IWineD3DSurface_GetContainer(This->renderTarget, &IID_IWineD3DSwapChain, (void **)&currentSwapchain) != WINED3D_OK) {
-        /* the selected render target doesn't belong to a swapchain, so use the devices implicit swapchain */
+/** FIXME: This is currently used called whenever SetRenderTarget or SetStencilBuffer are called
+* the functionality needs splitting up so that we don't do more than we should do.
+* this only seems to impact performance a little.
+ ******************************/
+static HRESULT WINAPI IWineD3DDeviceImpl_ActiveRender(IWineD3DDevice* iface,
+                                               IWineD3DSurface *RenderSurface) {
+
+    /**
+    * Currently only active for GLX >= 1.3
+    * for others versions we'll have to use GLXPixmaps
+    *
+    * normally we must test GLX_VERSION_1_3 but nvidia headers are not correct
+    * as they implement GLX 1.3 but only define GLX_VERSION_1_2
+    * so only check OpenGL version
+    * ..........................
+    * I don't believe that it is a problem with NVidia headers,
+    * XFree only supports GLX1.2, nVidia (and ATI to some extent) provide 1.3 functions
+    * in GLX 1.2, there is no mention of the correct way to tell if the extensions are provided.
+    * ATI Note:
+    * Your application will report GLX version 1.2 on glXQueryVersion.
+    * However, it is safe to call the GLX 1.3 functions as described below.
+    */
+#if defined(GL_VERSION_1_3)
+
+    IWineD3DDeviceImpl *This = (IWineD3DDeviceImpl *)iface;
+    IWineD3DSurface *tmp;
+    GLXFBConfig* cfgs = NULL;
+    IWineD3DSwapChain     *currentSwapchain;
+    IWineD3DSwapChainImpl *currentSwapchainImpl;
+    IWineD3DSwapChain     *implicitSwapchain;
+    IWineD3DSwapChainImpl *implicitSwapchainImpl;
+    IWineD3DSwapChain     *renderSurfaceSwapchain;
+    IWineD3DSwapChainImpl *renderSurfaceSwapchainImpl;
+
+    /* Obtain a reference to the device implicit swapchain,
+     * the swapchain of the current render target,
+     * and the swapchain of the new render target.
+     * Fallback to device implicit swapchain if the current render target doesn't have one */
+    IWineD3DDevice_GetSwapChain(iface, 0, &implicitSwapchain);
+    IWineD3DSurface_GetContainer(RenderSurface, &IID_IWineD3DSwapChain, (void**) &renderSurfaceSwapchain);
+    IWineD3DSurface_GetContainer(This->renderTarget, &IID_IWineD3DSwapChain, (void **)&currentSwapchain);
+    if (currentSwapchain == NULL)
         IWineD3DDevice_GetSwapChain(iface, 0, &currentSwapchain);
-    }
+
+    currentSwapchainImpl = (IWineD3DSwapChainImpl*) currentSwapchain;
+    implicitSwapchainImpl = (IWineD3DSwapChainImpl*) implicitSwapchain;
+    renderSurfaceSwapchainImpl = (IWineD3DSwapChainImpl*) renderSurfaceSwapchain;
+
+    ENTER_GL();
 
     /**
     * TODO: remove the use of IWineD3DSwapChainImpl, a context manager will help since it will replace the
     *  renderTarget = swapchain->backBuffer[i] bit and anything to do with *glContexts
      **********************************************************************/
-    if (IWineD3DSurface_GetContainer(RenderSurface, &IID_IWineD3DSwapChain, (void **)&swapchain) == WINED3D_OK) {
+    if (renderSurfaceSwapchain != NULL) {
+
         /* We also need to make sure that the lights &co are also in the context of the swapchains */
         /* FIXME: If the render target gets sent to the frontBuffer should be be presenting it raw? */
         TRACE("making swapchain active\n");
@@ -7445,8 +7514,8 @@ #endif
             BOOL backbuf = FALSE;
             int i;
 
-            for(i = 0; i < swapchain->presentParms.BackBufferCount; i++) {
-                if(RenderSurface == swapchain->backBuffer[i]) {
+            for(i = 0; i < renderSurfaceSwapchainImpl->presentParms.BackBufferCount; i++) {
+                if(RenderSurface == renderSurfaceSwapchainImpl->backBuffer[i]) {
                     backbuf = TRUE;
                     break;
                 }
@@ -7457,19 +7526,27 @@ #endif
                 /* This could be flagged so that some operations work directly with the front buffer */
                 FIXME("Attempting to set the  renderTarget to the frontBuffer\n");
             }
-            if (glXMakeCurrent(swapchain->display, swapchain->win, swapchain->glCtx)
-            == False) {
+            if (glXMakeCurrent(renderSurfaceSwapchainImpl->display,
+                               renderSurfaceSwapchainImpl->win,
+                               renderSurfaceSwapchainImpl->glCtx) == False) {
+
                 TRACE("Error in setting current context: context %p drawable %ld !\n",
-                       impSwapChain->glCtx, impSwapChain->win);
+                       implicitSwapchainImpl->glCtx, implicitSwapchainImpl->win);
             }
+            checkGLcall("glXMakeContextCurrent");
+
+            /* Clean up the old context */
+            IWineD3DDeviceImpl_CleanRender(iface, currentSwapchainImpl);
 
-            IWineD3DDeviceImpl_CleanRender(iface, (IWineD3DSwapChainImpl *)currentSwapchain);
+            /* Reapply the stateblock, and set the device not to render to texture */
+            device_reapply_stateblock(This);
+            device_render_to_texture(This, FALSE);
         }
-        checkGLcall("glXMakeContextCurrent");
 
-        IWineD3DSwapChain_Release((IWineD3DSwapChain *)swapchain);
-    }
-    else if (pbuffer_support && cfgs != NULL /* && some test to make sure that opengl supports pbuffers */) {
+    /* Offscreen rendering: PBuffers (currently disabled).
+     * Also note that this path is never reached if FBOs are supported */
+    } else if (pbuffer_support &&
+               (cfgs = device_find_fbconfigs(This, implicitSwapchainImpl, RenderSurface)) != NULL) {
 
         /** ********************************************************************
         * This is a quickly hacked out implementation of offscreen textures.
@@ -7501,13 +7578,17 @@ #endif
         /* TODO: This should really be part of findGlContext */
         if (NULL == newContext->context) {
 
+            int attribs[256];
+            int nAttribs = 0;
+
             TRACE("making new buffer\n");
-            nAttribs = 0;
-            PUSH2(GLX_PBUFFER_WIDTH,  newContext->Width);
-            PUSH2(GLX_PBUFFER_HEIGHT, newContext->Height);
-            PUSH1(None);
+            attribs[nAttribs++] = GLX_PBUFFER_WIDTH; 
+            attribs[nAttribs++] = newContext->Width;
+            attribs[nAttribs++] = GLX_PBUFFER_HEIGHT;
+            attribs[nAttribs++] = newContext->Height;
+            attribs[nAttribs++] = None;
 
-            newContext->drawable  = glXCreatePbuffer(impSwapChain->display, cfgs[0], attribs);
+            newContext->drawable = glXCreatePbuffer(implicitSwapchainImpl->display, cfgs[0], attribs);
 
             /** ****************************************
             *GLX1.3 isn't supported by XFree 'yet' until that point ATI emulates pBuffers
@@ -7517,12 +7598,14 @@ #endif
             *    so until then we have to use glXGetVisualFromFBConfig &co..
             ********************************************/
 
-
-            visinfo = glXGetVisualFromFBConfig(impSwapChain->display, cfgs[0]);
+            visinfo = glXGetVisualFromFBConfig(implicitSwapchainImpl->display, cfgs[0]);
             if (!visinfo) {
                 ERR("Error: couldn't get an RGBA, double-buffered visual\n");
             } else {
-                newContext->context = glXCreateContext(impSwapChain->display, visinfo, impSwapChain->glCtx,  GL_TRUE);
+                newContext->context = glXCreateContext(
+                    implicitSwapchainImpl->display, visinfo,
+                    implicitSwapchainImpl->glCtx, GL_TRUE);
+
                 XFree(visinfo);
             }
         }
@@ -7530,84 +7613,40 @@ #endif
             ERR("(%p) : Failed to find a context for surface %p\n", iface, RenderSurface);
         } else {
             /* Debug logging, (this function leaks), change to a TRACE when the leak is plugged */
-            if (glXMakeCurrent(impSwapChain->display, newContext->drawable, newContext->context) == False) {
-                TRACE("Error in setting current context: context %p drawable %ld\n", newContext->context, newContext->drawable);
+            if (glXMakeCurrent(implicitSwapchainImpl->display,
+                newContext->drawable, newContext->context) == False) {
+
+                TRACE("Error in setting current context: context %p drawable %ld\n",
+                    newContext->context, newContext->drawable);
             }
+            checkGLcall("glXMakeContextCurrent");
 
             /* Clean up the old context */
-            IWineD3DDeviceImpl_CleanRender(iface, (IWineD3DSwapChainImpl *)currentSwapchain);
-            /* Set the current context of the swapchain to the new context */
-            impSwapChain->drawable   = newContext->drawable;
-            impSwapChain->render_ctx = newContext->context;
-        }
-    }
+            IWineD3DDeviceImpl_CleanRender(iface, currentSwapchainImpl);
 
-    /* Disable recording, and apply the stateblock to the new context 
-     * FIXME: This is a bit of a hack, each context should know it's own state,
-     * the directX current directX state should then be applied to the context */
-    oldUpdateStateBlock = This->updateStateBlock;
-    oldRecording= This->isRecordingState;
-    This->isRecordingState = FALSE;
-    This->updateStateBlock = This->stateBlock;
-    IWineD3DStateBlock_Apply((IWineD3DStateBlock *)This->stateBlock);
+            /* Reapply stateblock, and set device to render to a texture */
+            device_reapply_stateblock(This);
+            device_render_to_texture(This, TRUE);
 
-    /* clean up the current rendertargets swapchain (if it belonged to one) */
-    if (currentSwapchain != NULL) {
-        IWineD3DSwapChain_Release((IWineD3DSwapChain *)currentSwapchain);
+            /* Set the current context of the swapchain to the new context */
+            implicitSwapchainImpl->drawable   = newContext->drawable;
+            implicitSwapchainImpl->render_ctx = newContext->context;
+        }
     }
 
-    /* Were done with the opengl context management, setup the rendertargets */
-
+    /* Replace the render target */
     tmp = This->renderTarget;
     This->renderTarget = RenderSurface;
     IWineD3DSurface_AddRef(This->renderTarget);
     IWineD3DSurface_Release(tmp);
 
-    {
-        DWORD value;
-
-        /* The surface must be rendered upside down to cancel the flip produce by glCopyTexImage */
-        /* Check that the container is not a swapchain member */
-
-        IWineD3DSwapChain *tmpSwapChain;
-        if (WINED3D_OK != IWineD3DSurface_GetContainer(This->renderTarget, &IID_IWineD3DSwapChain, (void **)&tmpSwapChain)) {
-            This->renderUpsideDown = TRUE;
-        }else{
-            This->renderUpsideDown = FALSE;
-            IWineD3DSwapChain_Release(tmpSwapChain);
-        }
-        /* Force updating the cull mode */
-        TRACE("setting render state\n");
-        IWineD3DDevice_GetRenderState(iface, WINED3DRS_CULLMODE, &value);
-        IWineD3DDevice_SetRenderState(iface, WINED3DRS_CULLMODE, value);
-
-        /* Force updating projection matrix */
-        This->last_was_rhw = FALSE;
-        This->proj_valid = FALSE;
-    }
-
-    /* Restore recording state */
-    This->isRecordingState = oldRecording;
-    This->updateStateBlock = oldUpdateStateBlock;
-
-    ret = WINED3D_OK;
-
-    if (cfgs != NULL) {
-        XFree(cfgs);
-    } else {
-        ERR("cannot get valides GLXFBConfig for (%u,%s)/(%u,%s)\n", BackBufferFormat,
-            debug_d3dformat(BackBufferFormat), StencilBufferFormat, debug_d3dformat(StencilBufferFormat));
-    }
-
-#undef PUSH1
-#undef PUSH2
-    if ( NULL != impSwapChain) {
-        IWineD3DSwapChain_Release((IWineD3DSwapChain *)impSwapChain);
-    }
+    if (cfgs != NULL)                   XFree(cfgs);
+    if (implicitSwapchain != NULL)       IWineD3DSwapChain_Release(implicitSwapchain);
+    if (currentSwapchain != NULL)       IWineD3DSwapChain_Release(currentSwapchain);
+    if (renderSurfaceSwapchain != NULL) IWineD3DSwapChain_Release(renderSurfaceSwapchain);
     LEAVE_GL();
-
 #endif
-    return ret;
+    return WINED3D_OK;
 }
 
 static HRESULT  WINAPI  IWineD3DDeviceImpl_SetCursorProperties(IWineD3DDevice* iface, UINT XHotSpot,
-- 
1.4.1



More information about the wine-patches mailing list