[Mesa-dev] [PATCH] swr: [rasterizer] fix clear with multiple color attachments
Ilia Mirkin
imirkin at alum.mit.edu
Wed Nov 16 17:51:20 UTC 2016
Hi Tim,
I mentioned this on IRC, but let me give a concise view here.
First off - this is a vast improvement over the current state.
Assuming the core bits are up to snuff, this is worth landing.
Secondly - starting GL 3.2, layered attachments (and clears on them)
are a thing. Currently SWR_SURFACE_STATE doesn't /really/ recognize
this. It has a RT lod and arrayIndex. And implicitly the depth
specifies the number of layers, starting at the arrayIndex I believe,
but this is not explicit in the docs, and a bunch of places can end up
bypassing it.
When you have a layered attachment, that means that you're supposed to
clear all the attached layers of every attachment. This is counter to
rendering, which happens on the MIN() of all the numbers of layers. So
if you attach a texture with 2 layers, and another with 4 layers -
you're supposed to render to it as if you overall only had 2 layers.
However when clearing, you're supposed to clear all 4 layers of the
second one.
It appears that hot tiles currently have a single
renderTargetArrayIndex, which is presumably equivalent to the gl_Layer
output from GS. This is added to the SWR_SURFACE_STATE's arrayIndex
(which makes sense). What you'll need to suss out is either at the API
layer or somewhere else, to make sure that the clearing happens on all
attached layers. IMO it makes the most sense to do this in the API,
but perhaps the frontend is the better place. (I mean the swr core
frontend, not the gallium frontend.)
Just some thoughts, based on my grasp of the current architecture.
Cheers,
-ilia
On Tue, Nov 15, 2016 at 9: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)) {
> + mask &= ~(1 << rt);
> +
> + HOTTILE *pHotTile =
> + pContext->pHotTileMgr->GetHotTile(pContext, pDC, macroTile,
> + (SWR_RENDERTARGET_ATTACHMENT)rt,
> + 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;
> + }
> }
>
> - 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)) {
> + 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