Looks good (and concise), with the exception of a few styling issues (suggestions inlined)
Reviewed-by: Bruce Cherniak <[email protected]> after minor styling fixes. > On Nov 15, 2016, at 8:26 PM, Tim Rowley <[email protected]> wrote: > > Fixes fbo-mrt-alphatest > --- > src/gallium/drivers/swr/rasterizer/core/api.cpp | 10 ++--- > src/gallium/drivers/swr/rasterizer/core/api.h | 4 +- > .../drivers/swr/rasterizer/core/backend.cpp | 46 +++++++++++++++------- > src/gallium/drivers/swr/rasterizer/core/context.h | 11 +----- > src/gallium/drivers/swr/rasterizer/core/state.h | 6 --- > src/gallium/drivers/swr/swr_clear.cpp | 16 ++------ > 6 files changed, 41 insertions(+), 52 deletions(-) > > diff --git a/src/gallium/drivers/swr/rasterizer/core/api.cpp > b/src/gallium/drivers/swr/rasterizer/core/api.cpp > index 7e305da..6ade65a 100644 > --- a/src/gallium/drivers/swr/rasterizer/core/api.cpp > +++ b/src/gallium/drivers/swr/rasterizer/core/api.cpp > @@ -1475,14 +1475,14 @@ void SWR_API SwrStoreTiles( > ////////////////////////////////////////////////////////////////////////// > /// @brief SwrClearRenderTarget - Clear attached render targets / depth / > stencil > /// @param hContext - Handle passed back from SwrCreateContext > -/// @param clearMask - combination of SWR_CLEAR_COLOR / SWR_CLEAR_DEPTH / > SWR_CLEAR_STENCIL flags (or SWR_CLEAR_NONE) > +/// @param attachmentMask - combination of SWR_ATTACHMENT_*_BIT attachments > to clear > /// @param clearColor - color use for clearing render targets > /// @param z - depth value use for clearing depth buffer > /// @param stencil - stencil value used for clearing stencil buffer > /// @param clearRect - The pixel-coordinate rectangle to clear in all cleared > buffers > void SWR_API SwrClearRenderTarget( > HANDLE hContext, > - uint32_t clearMask, > + uint32_t attachmentMask, > const float clearColor[4], > float z, > uint8_t stencil, > @@ -1498,15 +1498,11 @@ void SWR_API SwrClearRenderTarget( > > AR_API_BEGIN(APIClearRenderTarget, pDC->drawId); > > - CLEAR_FLAGS flags; > - flags.bits = 0; > - flags.mask = clearMask; > - > pDC->FeWork.type = CLEAR; > pDC->FeWork.pfnWork = ProcessClear; > pDC->FeWork.desc.clear.rect = clearRect; > pDC->FeWork.desc.clear.rect &= g_MaxScissorRect; > - pDC->FeWork.desc.clear.flags = flags; > + pDC->FeWork.desc.clear.attachmentMask = attachmentMask; > pDC->FeWork.desc.clear.clearDepth = z; > pDC->FeWork.desc.clear.clearRTColor[0] = clearColor[0]; > pDC->FeWork.desc.clear.clearRTColor[1] = clearColor[1]; > diff --git a/src/gallium/drivers/swr/rasterizer/core/api.h > b/src/gallium/drivers/swr/rasterizer/core/api.h > index 6bebc39..1a41637 100644 > --- a/src/gallium/drivers/swr/rasterizer/core/api.h > +++ b/src/gallium/drivers/swr/rasterizer/core/api.h > @@ -558,14 +558,14 @@ void SWR_API SwrStoreTiles( > ////////////////////////////////////////////////////////////////////////// > /// @brief SwrClearRenderTarget - Clear attached render targets / depth / > stencil > /// @param hContext - Handle passed back from SwrCreateContext > -/// @param clearMask - combination of SWR_CLEAR_COLOR / SWR_CLEAR_DEPTH / > SWR_CLEAR_STENCIL flags (or SWR_CLEAR_NONE) > +/// @param attachmentMask - combination of SWR_ATTACHMENT_*_BIT attachments > to clear > /// @param clearColor - color use for clearing render targets > /// @param z - depth value use for clearing depth buffer > /// @param stencil - stencil value used for clearing stencil buffer > /// @param clearRect - The pixel-coordinate rectangle to clear in all cleared > buffers > void SWR_API SwrClearRenderTarget( > HANDLE hContext, > - uint32_t clearMask, > + uint32_t attachmentMask, > const float clearColor[4], > float z, > uint8_t stencil, > diff --git a/src/gallium/drivers/swr/rasterizer/core/backend.cpp > b/src/gallium/drivers/swr/rasterizer/core/backend.cpp > index 16c4537..e03f3e9 100644 > --- a/src/gallium/drivers/swr/rasterizer/core/backend.cpp > +++ b/src/gallium/drivers/swr/rasterizer/core/backend.cpp > @@ -237,29 +237,39 @@ void ProcessClearBE(DRAW_CONTEXT *pDC, uint32_t > workerId, uint32_t macroTile, vo > SWR_MULTISAMPLE_COUNT sampleCount = > pDC->pState->state.rastState.sampleCount; > uint32_t numSamples = GetNumSamples(sampleCount); > > - SWR_ASSERT(pClear->flags.bits != 0); // shouldn't be here without a > reason. > + SWR_ASSERT(pClear->attachmentMask != 0); // shouldn't be here > without a reason. > > AR_BEGIN(BEClear, pDC->drawId); > > - if (pClear->flags.mask & SWR_CLEAR_COLOR) > + if (pClear->attachmentMask & SWR_ATTACHMENT_MASK_COLOR) > { > - HOTTILE *pHotTile = pContext->pHotTileMgr->GetHotTile(pContext, > pDC, macroTile, SWR_ATTACHMENT_COLOR0, true, numSamples); > - // All we want to do here is to mark the hot tile as being in a > "needs clear" state. > - pHotTile->clearData[0] = *(DWORD*)&(pClear->clearRTColor[0]); > - pHotTile->clearData[1] = *(DWORD*)&(pClear->clearRTColor[1]); > - pHotTile->clearData[2] = *(DWORD*)&(pClear->clearRTColor[2]); > - pHotTile->clearData[3] = *(DWORD*)&(pClear->clearRTColor[3]); > - pHotTile->state = HOTTILE_CLEAR; > + unsigned long rt = 0; > + uint32_t mask = pClear->attachmentMask & > SWR_ATTACHMENT_MASK_COLOR; > + while (_BitScanForward(&rt, mask)) { { on line of its own > + mask &= ~(1 << rt); > + > + HOTTILE *pHotTile = > + pContext->pHotTileMgr->GetHotTile(pContext, pDC, > macroTile, > + > (SWR_RENDERTARGET_ATTACHMENT)rt, > + true, numSamples); core files not limited to 80 columns, let it fly. > + > + // All we want to do here is to mark the hot tile as being > in a "needs clear" state. > + pHotTile->clearData[0] = *(DWORD*)&(pClear->clearRTColor[0]); > + pHotTile->clearData[1] = *(DWORD*)&(pClear->clearRTColor[1]); > + pHotTile->clearData[2] = *(DWORD*)&(pClear->clearRTColor[2]); > + pHotTile->clearData[3] = *(DWORD*)&(pClear->clearRTColor[3]); > + pHotTile->state = HOTTILE_CLEAR; > + } > } > > - if (pClear->flags.mask & SWR_CLEAR_DEPTH) > + if (pClear->attachmentMask & SWR_ATTACHMENT_DEPTH_BIT) > { > HOTTILE *pHotTile = pContext->pHotTileMgr->GetHotTile(pContext, > pDC, macroTile, SWR_ATTACHMENT_DEPTH, true, numSamples); > pHotTile->clearData[0] = *(DWORD*)&pClear->clearDepth; > pHotTile->state = HOTTILE_CLEAR; > } > > - if (pClear->flags.mask & SWR_CLEAR_STENCIL) > + if (pClear->attachmentMask & SWR_ATTACHMENT_STENCIL_BIT) > { > HOTTILE *pHotTile = pContext->pHotTileMgr->GetHotTile(pContext, > pDC, macroTile, SWR_ATTACHMENT_STENCIL, true, numSamples); > > @@ -275,7 +285,7 @@ void ProcessClearBE(DRAW_CONTEXT *pDC, uint32_t workerId, > uint32_t macroTile, vo > CLEAR_DESC *pClear = (CLEAR_DESC*)pUserData; > AR_BEGIN(BEClear, pDC->drawId); > > - if (pClear->flags.mask & SWR_CLEAR_COLOR) > + if (pClear->attachmentMask & SWR_ATTACHMENT_MASK_COLOR) > { > /// @todo clear data should come in as RGBA32_FLOAT > DWORD clearData[4]; > @@ -292,10 +302,16 @@ void ProcessClearBE(DRAW_CONTEXT *pDC, uint32_t > workerId, uint32_t macroTile, vo > PFN_CLEAR_TILES pfnClearTiles = > sClearTilesTable[KNOB_COLOR_HOT_TILE_FORMAT]; > SWR_ASSERT(pfnClearTiles != nullptr); > > - pfnClearTiles(pDC, SWR_ATTACHMENT_COLOR0, macroTile, clearData, > pClear->rect); > + unsigned long rt = 0; > + uint32_t mask = pClear->attachmentMask & > SWR_ATTACHMENT_MASK_COLOR; > + while (_BitScanForward(&rt, mask)) { { on line of its own > + mask &= ~(1 << rt); > + > + pfnClearTiles(pDC, (SWR_RENDERTARGET_ATTACHMENT)rt, > macroTile, clearData, pClear->rect); > + } > } > > - if (pClear->flags.mask & SWR_CLEAR_DEPTH) > + if (pClear->attachmentMask & SWR_ATTACHMENT_DEPTH_BIT) > { > DWORD clearData[4]; > clearData[0] = *(DWORD*)&pClear->clearDepth; > @@ -305,7 +321,7 @@ void ProcessClearBE(DRAW_CONTEXT *pDC, uint32_t workerId, > uint32_t macroTile, vo > pfnClearTiles(pDC, SWR_ATTACHMENT_DEPTH, macroTile, clearData, > pClear->rect); > } > > - if (pClear->flags.mask & SWR_CLEAR_STENCIL) > + if (pClear->attachmentMask & SWR_ATTACHMENT_STENCIL_BIT) > { > uint32_t value = pClear->clearStencil; > DWORD clearData[4]; > diff --git a/src/gallium/drivers/swr/rasterizer/core/context.h > b/src/gallium/drivers/swr/rasterizer/core/context.h > index 69be280..21ea827 100644 > --- a/src/gallium/drivers/swr/rasterizer/core/context.h > +++ b/src/gallium/drivers/swr/rasterizer/core/context.h > @@ -100,19 +100,10 @@ struct TRIANGLE_WORK_DESC > TRI_FLAGS triFlags; > }; > > -union CLEAR_FLAGS > -{ > - struct > - { > - uint32_t mask : 3; > - }; > - uint32_t bits; > -}; > - > struct CLEAR_DESC > { > SWR_RECT rect; > - CLEAR_FLAGS flags; > + uint32_t attachmentMask; > float clearRTColor[4]; // RGBA_32F > float clearDepth; // [0..1] > uint8_t clearStencil; > diff --git a/src/gallium/drivers/swr/rasterizer/core/state.h > b/src/gallium/drivers/swr/rasterizer/core/state.h > index f6b6ed2..2f3b913 100644 > --- a/src/gallium/drivers/swr/rasterizer/core/state.h > +++ b/src/gallium/drivers/swr/rasterizer/core/state.h > @@ -30,12 +30,6 @@ > #include "common/formats.h" > #include "common/simdintrin.h" > > -// clear flags > -#define SWR_CLEAR_NONE 0 > -#define SWR_CLEAR_COLOR (1 << 0) > -#define SWR_CLEAR_DEPTH (1 << 1) > -#define SWR_CLEAR_STENCIL (1 << 2) > - > ////////////////////////////////////////////////////////////////////////// > /// PRIMITIVE_TOPOLOGY. > ////////////////////////////////////////////////////////////////////////// > diff --git a/src/gallium/drivers/swr/swr_clear.cpp > b/src/gallium/drivers/swr/swr_clear.cpp > index a65f8f4..0101b4b 100644 > --- a/src/gallium/drivers/swr/swr_clear.cpp > +++ b/src/gallium/drivers/swr/swr_clear.cpp > @@ -42,25 +42,17 @@ swr_clear(struct pipe_context *pipe, > if (ctx->dirty) > swr_update_derived(pipe); > > -/* Update clearMask/targetMask */ > -#if 0 /* XXX SWR currently only clears SWR_ATTACHMENT_COLOR0, don't bother > \ > - checking others yet. */ > if (buffers & PIPE_CLEAR_COLOR && fb->nr_cbufs) { > - UINT i; > - for (i = 0; i < fb->nr_cbufs; ++i) > + for (unsigned i = 0; i < fb->nr_cbufs; ++i) > if (fb->cbufs[i]) > - clearMask |= (SWR_CLEAR_COLOR0 << i); > + clearMask |= (SWR_ATTACHMENT_COLOR0_BIT << i); > } > -#else > - if (buffers & PIPE_CLEAR_COLOR && fb->cbufs[0]) > - clearMask |= SWR_CLEAR_COLOR; > -#endif > > if (buffers & PIPE_CLEAR_DEPTH && fb->zsbuf) > - clearMask |= SWR_CLEAR_DEPTH; > + clearMask |= SWR_ATTACHMENT_DEPTH_BIT; > > if (buffers & PIPE_CLEAR_STENCIL && fb->zsbuf) > - clearMask |= SWR_CLEAR_STENCIL; > + clearMask |= SWR_ATTACHMENT_STENCIL_BIT; > > #if 0 // XXX HACK, override clear color alpha. On ubuntu, clears are > // transparent. > -- > 2.7.4 > > _______________________________________________ > mesa-dev mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
