[Mesa-dev] [PATCH] swr: [rasterizer] fix clear with multiple color attachments
Cherniak, Bruce
bruce.cherniak at intel.com
Wed Nov 16 14:29:19 UTC 2016
Looks good (and concise), with the exception of a few styling issues (suggestions inlined)
Reviewed-by: Bruce Cherniak <bruce.cherniak at intel.com> after minor styling fixes.
> On Nov 15, 2016, at 8:26 PM, Tim Rowley <timothy.o.rowley at intel.com> 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
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list