[Mesa-dev] [PATCH] swr: perform perspective division on clip distances

[Ilia Mirkin]

On Thu, Dec 8, 2016 at 3:43 PM, Rowley, Timothy O
<timothy.o.rowley at intel.com> wrote:
>
>> On Nov 24, 2016, at 2:29 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>
>> Clip distances need to be perspective-divided. This fixes all the
>> interpolation-*-{distance,vertex} piglits.
>>
>> Also take this opportunity to fix clip distances for points rasterized
>> as triangles - the clip distance is not subject to sprite coord
>> replacement, so there's no interpolation of it. We just take its value
>> and put it in the "z" component of the barycentric-ready plane equation.
>> (We could also just cull it at an earlier point in time, but that would
>> require larger changes.)
>>
>
> Would prefer this second change moved to a separate commit.  I’ve spent the most time looking at that, and still not convinced it’s correct.

OK. I think the current situation is that we feed it 2 points per clip
distance, but the rasterizer expects 3 since they're being fed in as
triangles. I'll split them up though. And try to come up with a
concrete piglit this fixes.

>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>> src/gallium/drivers/swr/rasterizer/core/binner.cpp | 22 +++++++++++++++-------
>> 1 file changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/gallium/drivers/swr/rasterizer/core/binner.cpp b/src/gallium/drivers/swr/rasterizer/core/binner.cpp
>> index 6f9259f..d5f2e97 100644
>> --- a/src/gallium/drivers/swr/rasterizer/core/binner.cpp
>> +++ b/src/gallium/drivers/swr/rasterizer/core/binner.cpp
>> @@ -383,7 +383,7 @@ PFN_PROCESS_ATTRIBUTES GetProcessAttributesFunc(uint32_t NumVerts, bool IsSwizzl
>> /// @param clipDistMask - mask of enabled clip distances
>> /// @param pUserClipBuffer - buffer to store results
>> template<uint32_t NumVerts>
>> -void ProcessUserClipDist(PA_STATE& pa, uint32_t primIndex, uint8_t clipDistMask, float* pUserClipBuffer)
>> +void ProcessUserClipDist(PA_STATE& pa, uint32_t primIndex, uint8_t clipDistMask, float *pRecipW, float* pUserClipBuffer)
>> {
>>     DWORD clipDist;
>>     while (_BitScanForward(&clipDist, clipDistMask))
>> @@ -407,11 +407,12 @@ void ProcessUserClipDist(PA_STATE& pa, uint32_t primIndex, uint8_t clipDistMask,
>>
>>         // setup plane equations for barycentric interpolation in the backend
>>         float baryCoeff[NumVerts];
>> +        float last = vertClipDist[NumVerts - 1] * pRecipW[NumVerts - 1];
>>         for (uint32_t e = 0; e < NumVerts - 1; ++e)
>>         {
>> -            baryCoeff[e] = vertClipDist[e] - vertClipDist[NumVerts - 1];
>> +            baryCoeff[e] = vertClipDist[e] * pRecipW[e] - last;
>>         }
>> -        baryCoeff[NumVerts - 1] = vertClipDist[NumVerts - 1];
>> +        baryCoeff[NumVerts - 1] = last;
>>
>>         for (uint32_t e = 0; e < NumVerts; ++e)
>>         {
>> @@ -834,7 +835,7 @@ endBinTriangles:
>>         {
>>             uint32_t numClipDist = _mm_popcnt_u32(rastState.clipDistanceMask);
>>             desc.pUserClipBuffer = (float*)pArena->Alloc(numClipDist * 3 * sizeof(float));
>> -            ProcessUserClipDist<3>(pa, triIndex, rastState.clipDistanceMask, desc.pUserClipBuffer);
>> +            ProcessUserClipDist<3>(pa, triIndex, rastState.clipDistanceMask, &desc.pTriBuffer[12], desc.pUserClipBuffer);
>>         }
>>
>>         for (uint32_t y = aMTTop[triIndex]; y <= aMTBottom[triIndex]; ++y)
>> @@ -1184,8 +1185,15 @@ void BinPoints(
>>             if (rastState.clipDistanceMask)
>>             {
>>                 uint32_t numClipDist = _mm_popcnt_u32(rastState.clipDistanceMask);
>> -                desc.pUserClipBuffer = (float*)pArena->Alloc(numClipDist * 2 * sizeof(float));
>> -                ProcessUserClipDist<2>(pa, primIndex, rastState.clipDistanceMask, desc.pUserClipBuffer);
>> +                desc.pUserClipBuffer = (float*)pArena->Alloc(numClipDist * 3 * sizeof(float));
>> +                float dists[8];
>> +                float one = 1.0f;
>> +                ProcessUserClipDist<1>(pa, primIndex, rastState.clipDistanceMask, &one, dists);
>> +                for (uint32_t i = 0; i < numClipDist; i++) {
>> +                    desc.pUserClipBuffer[3*i + 0] = 0.0f;
>> +                    desc.pUserClipBuffer[3*i + 1] = 0.0f;
>> +                    desc.pUserClipBuffer[3*i + 2] = dists[i];
>> +                }
>>             }
>>
>>             MacroTileMgr *pTileMgr = pDC->pTileMgr;
>> @@ -1396,7 +1404,7 @@ void BinPostSetupLines(
>>         {
>>             uint32_t numClipDist = _mm_popcnt_u32(rastState.clipDistanceMask);
>>             desc.pUserClipBuffer = (float*)pArena->Alloc(numClipDist * 2 * sizeof(float));
>> -            ProcessUserClipDist<2>(pa, primIndex, rastState.clipDistanceMask, desc.pUserClipBuffer);
>> +            ProcessUserClipDist<2>(pa, primIndex, rastState.clipDistanceMask, &desc.pTriBuffer[12], desc.pUserClipBuffer);
>>         }
>>
>>         MacroTileMgr *pTileMgr = pDC->pTileMgr;
>> --
>> 2.7.3
>>
>