[Mesa-dev] [PATCH 2/3] mesa/st: add per sample shading state to fp key and set interpolation

[Marek Olšák]

What you say makes sense. I'm just asking what that sentence in the
spec means if it isn't about interpolation. :)

Marek

On Sun, Jul 6, 2014 at 12:40 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Sat, Jul 5, 2014 at 6:22 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> There is this vague statement in the sample shading spec:
>>
>>     When the sample shading fraction is 1.0, a separate set of colors and
>>     other associated data are evaluated for each sample, each set of values
>>     are evaluated at the sample location.
>>
>> I thought it was about interpolation, meaning that the fraction must
>> 1.0 for the per-sample interpolation to be enabled, right?
>
> I guess so. I'm pretty much just doing what the intel driver is
> doing... see brw_wm.c.
>
> /* Gets the minimum number of shader invocations per fragment.
>  * This function is useful to determine if we need to do per
>  * sample shading or per fragment shading.
>  */
> GLint
> _mesa_get_min_invocations_per_fragment(struct gl_context *ctx,
>                                        const struct gl_fragment_program *prog,
>                                        bool ignore_sample_qualifier)
>
> So I guess instead of > 1 this should be checking for ==
> ctx->DrawBuffer->Visual.samples? (and > 1)
>
> Although in that case, I _really_ don't get the point of having
> non-1.0/0.0 fractions ever existing. Why would you want it to be
> shaded for e.g. 4/8 samples if all the inputs are going to get
> interpolated the same way?
>
>   -ilia
>
>>
>> Marek
>>
>>
>> On Sat, Jul 5, 2014 at 6:07 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>> This enables a gallium driver not to care about the semantics of
>>> ARB_sample_shading vs ARB_gpu_shader5 sample attributes. When
>>> ARB_sample_shading-style sample shading is enabled, all of the fp inputs
>>> are marked for interpolation at the sample location.
>>>
>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> ---
>>>
>>> I _think_ I got this right... at least the tests still pass. But my
>>> understanding of all the various update atoms, etc is really weak... please
>>> read carefully :)
>>>
>>> Now that I understand all this interpolation stuff better, I see why it was
>>> suggested I add proper per-sample interpolation first and the
>>> ARB_sample_shading stuff second. Oh well...
>>>
>>>  src/mesa/state_tracker/st_atom_shader.c | 6 +++++-
>>>  src/mesa/state_tracker/st_program.c     | 3 +++
>>>  src/mesa/state_tracker/st_program.h     | 3 +++
>>>  3 files changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/mesa/state_tracker/st_atom_shader.c b/src/mesa/state_tracker/st_atom_shader.c
>>> index 67c6157..6515a98 100644
>>> --- a/src/mesa/state_tracker/st_atom_shader.c
>>> +++ b/src/mesa/state_tracker/st_atom_shader.c
>>> @@ -89,6 +89,10 @@ update_fp( struct st_context *st )
>>>     key.clamp_color = st->clamp_frag_color_in_shader &&
>>>                       st->ctx->Color._ClampFragmentColor;
>>>
>>> +   /* Ignore sample qualifier while computing this flag. */
>>> +   key.persample_shading =
>>> +      _mesa_get_min_invocations_per_fragment(st->ctx, &stfp->Base, true) > 1;
>>> +
>>>     st->fp_variant = st_get_fp_variant(st, stfp, &key);
>>>
>>>     st_reference_fragprog(st, &st->fp, stfp);
>>> @@ -108,7 +112,7 @@ update_fp( struct st_context *st )
>>>  const struct st_tracked_state st_update_fp = {
>>>     "st_update_fp",                                     /* name */
>>>     {                                                   /* dirty */
>>> -      _NEW_BUFFERS,                                    /* mesa */
>>> +      _NEW_BUFFERS | _NEW_MULTISAMPLE,                 /* mesa */
>>>        ST_NEW_FRAGMENT_PROGRAM                           /* st */
>>>     },
>>>     update_fp                                   /* update */
>>> diff --git a/src/mesa/state_tracker/st_program.c b/src/mesa/state_tracker/st_program.c
>>> index b603759..9d7b7c4 100644
>>> --- a/src/mesa/state_tracker/st_program.c
>>> +++ b/src/mesa/state_tracker/st_program.c
>>> @@ -548,6 +548,9 @@ st_translate_fragment_program(struct st_context *st,
>>>           else
>>>              interpLocation[slot] = TGSI_INTERPOLATE_LOC_CENTER;
>>>
>>> +         if (key->persample_shading)
>>> +            interpLocation[slot] = TGSI_INTERPOLATE_LOC_SAMPLE;
>>> +
>>>           switch (attr) {
>>>           case VARYING_SLOT_POS:
>>>              input_semantic_name[slot] = TGSI_SEMANTIC_POSITION;
>>> diff --git a/src/mesa/state_tracker/st_program.h b/src/mesa/state_tracker/st_program.h
>>> index ce9174f..9a5b6a8 100644
>>> --- a/src/mesa/state_tracker/st_program.h
>>> +++ b/src/mesa/state_tracker/st_program.h
>>> @@ -58,6 +58,9 @@ struct st_fp_variant_key
>>>
>>>     /** for ARB_color_buffer_float */
>>>     GLuint clamp_color:1;
>>> +
>>> +   /** for ARB_sample_shading */
>>> +   GLuint persample_shading:1;
>>>  };
>>>
>>>
>>> --
>>> 1.8.5.5
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev