[Mesa-dev] [PATCH 2/2] radeonsi: fix gl_ClipDistance and gl_ClipVertex for points

[Nicolai Hähnle]

On 15.12.2016 17:19, Ilia Mirkin wrote:
> Perhaps this is a point-specific function, but if this is applying to
> all primitives, won't this end up culling primitives that have *any*
> clip distance value < 0?

Setting the cull bit here just enables the normal gl_CullDistance logic 
for whatever the current primitive type happens to be.

Cull distance logic is a subset of clip distance logic: when there's 
some distance value which is < 0 for all vertices, cull the primitive. 
Clipping also does that (+ of course it does more complicated things 
when only a subset of of the vertices have < 0 for a distance value). 
Just that for whatever reason, the hardware folks decided that clipping 
should just do nothing at all for points.

Cheers,
Nicolai

>
> On Thu, Dec 15, 2016 at 10:42 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>
>> The clipper hardware doesn't consider points as primitives that can be
>> clipped. Simply setting the corresponding cull bits works, and should not
>> have an adverse effect on other primitive types according to the hardware
>> team.
>> ---
>>  src/gallium/drivers/radeonsi/si_state.c | 12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c
>> index d86b99f..f6f61a6 100644
>> --- a/src/gallium/drivers/radeonsi/si_state.c
>> +++ b/src/gallium/drivers/radeonsi/si_state.c
>> @@ -671,34 +671,42 @@ static void si_emit_clip_regs(struct si_context *sctx, struct r600_atom *atom)
>>         unsigned total_mask;
>>         bool misc_vec_ena;
>>
>>         if (vs->key.opt.hw_vs.clip_disable) {
>>                 assert(!info->culldist_writemask);
>>                 clipdist_mask = 0;
>>                 culldist_mask = 0;
>>         }
>>         total_mask = clipdist_mask | culldist_mask;
>>
>> +       /* Clip distances on points have no effect, so need to be implemented
>> +        * as cull distances. This applies for the clipvertex case as well.
>> +        *
>> +        * Setting this for primitives other than points should have no adverse
>> +        * effects.
>> +        */
>> +       clipdist_mask &= rs->clip_plane_enable;
>> +       culldist_mask |= clipdist_mask;
>> +
>>         misc_vec_ena = info->writes_psize || info->writes_edgeflag ||
>>                        info->writes_layer || info->writes_viewport_index;
>>
>>         radeon_set_context_reg(cs, R_02881C_PA_CL_VS_OUT_CNTL,
>>                 S_02881C_USE_VTX_POINT_SIZE(info->writes_psize) |
>>                 S_02881C_USE_VTX_EDGE_FLAG(info->writes_edgeflag) |
>>                 S_02881C_USE_VTX_RENDER_TARGET_INDX(info->writes_layer) |
>>                 S_02881C_USE_VTX_VIEWPORT_INDX(info->writes_viewport_index) |
>>                 S_02881C_VS_OUT_CCDIST0_VEC_ENA((total_mask & 0x0F) != 0) |
>>                 S_02881C_VS_OUT_CCDIST1_VEC_ENA((total_mask & 0xF0) != 0) |
>>                 S_02881C_VS_OUT_MISC_VEC_ENA(misc_vec_ena) |
>>                 S_02881C_VS_OUT_MISC_SIDE_BUS_ENA(misc_vec_ena) |
>> -               (rs->clip_plane_enable &
>> -                clipdist_mask) | (culldist_mask << 8));
>> +               clipdist_mask | (culldist_mask << 8));
>>         radeon_set_context_reg(cs, R_028810_PA_CL_CLIP_CNTL,
>>                 rs->pa_cl_clip_cntl |
>>                 ucp_mask |
>>                 S_028810_CLIP_DISABLE(window_space));
>>
>>         /* reuse needs to be set off if we write oViewport */
>>         radeon_set_context_reg(cs, R_028AB4_VGT_REUSE_OFF,
>>                                S_028AB4_REUSE_OFF(info->writes_viewport_index));
>>  }
>>
>> --
>> 2.7.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev