[Mesa-dev] [PATCH 09/11] nir: add lowering stage for user-clip-planes / clipdist

[Ilia Mirkin]

On Tue, Sep 15, 2015 at 12:39 PM, Erik Faye-Lund <kusmabite at gmail.com> wrote:
> On Mon, Sep 14, 2015 at 11:53 AM, Erik Faye-Lund <kusmabite at gmail.com> wrote:
>> On Sun, Sep 13, 2015 at 5:51 PM, Rob Clark <robdclark at gmail.com> wrote:
>>> From: Rob Clark <robclark at freedesktop.org>
>>>
>>> The vertex shader lowering adds calculation for CLIPDIST, if needed
>>> (ie. user-clip-planes), and the frag shader lowering adds conditional
>>> kills based on CLIPDIST value (which should be treated as a normal
>>> interpolated varying by the driver).
>>
>> <snip>
>>
>>> +
>>> +/*
>>> + * FS lowering
>>> + */
>>> +
>>> +static void
>>> +lower_clip_fs(nir_function_impl *impl, unsigned ucp_enables,
>>> +              nir_variable **in)
>>> +{
>>> +   nir_ssa_def *clipdist[MAX_CLIP_PLANES];
>>> +   nir_builder b;
>>> +
>>> +   nir_builder_init(&b, impl);
>>> +   b.cursor = nir_before_cf_list(&impl->body);
>>> +
>>> +   if (ucp_enables & 0x0f)
>>> +      load_clipdist_input(&b, in[0], &clipdist[0]);
>>> +   if (ucp_enables & 0xf0)
>>> +      load_clipdist_input(&b, in[1], &clipdist[4]);
>>> +
>>> +   for (int plane = 0; plane < MAX_CLIP_PLANES; plane++) {
>>> +      if (ucp_enables & (1 << plane)) {
>>> +         nir_intrinsic_instr *discard;
>>> +         nir_ssa_def *cond;
>>> +
>>> +         cond = nir_flt(&b, clipdist[plane], nir_imm_float(&b, 0.0));
>>> +
>>> +         discard = nir_intrinsic_instr_create(b.shader,
>>> +                                              nir_intrinsic_discard_if);
>>> +         discard->src[0] = nir_src_for_ssa(cond);
>>> +         nir_builder_instr_insert(&b, &discard->instr);
>>
>> I think it's worth noting that this isn't *strictly* correct for
>> multi-sampling, unless the shader is s run with per-sample shading
>> (ala GL_ARB_sample_shading). Otherwise, all samples for a pixel will
>> get the same discard-condition, leading to aliasing along the
>> resulting edge.
>>
>> That being said, per-fragment clipping is better than no clipping, so
>> it's clearly an improvement.
>
> So, in order to do this correctly for MSAA, I guess you'd need to use
> SYSTEM_VALUE_SAMPLE_POS and FRAG_RESULT_SAMPLE_MASK, to perform
> alpha-testing for each sample-point.

Perhaps worth mentioning that a3xx doesn't *actually* support sample
shading, although it does appear to have a mode where a single
invocation can supply all the samples (but then you can only have a
single RT). Ultimately all it needs to do is be able to compute a
sample mask though, which *is* supported, but you'd need to do the
per-sample interp "by hand" (although afaik the IJ components are
provided... somewhere). a4xx brings real per-sample shading from what
I can tell though. No idea about vc4's capabilities.

This is all a bit moot of course since none of the relevant drivers
actually has support for even basic MSAA, nevermind sample shading.
However having a piglit test that covers this would be neat... I guess
you could clip a pixel in half and make sure that the resolved result
is some in-between color? Lots of implementation-dependent stuff going
on in there though.

  -ilia