[Mesa-dev] [PATCH 2/3] mesa/st: add per sample shading state to fp key and set interpolation
Marek Olšák
maraeo at gmail.com
Tue Jul 8 17:59:08 PDT 2014
Alright. For the patch:
Reviewed-by: Marek Olšák <marek.olsak at amd.com>
Marek
On Wed, Jul 9, 2014 at 2:48 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> So... I don't think we're going to figure this out here. At least I
> have nothing enlightening to say. FWIW this is doing the same thing as
> what i965 does wrt the persample_shading computation. It should be
> pretty easy to change should we decide on a different interpretation
> of the spec.
>
> The only question is whether this is necessary at all. i.e. what
> happens if you have a ARB_gs5-enabled shader with a "regular" varying
> and sample varying. If the regular varying should just be interpolated
> at the sample, perhaps all this stuff is stupid and I should just drop
> it and keep the "if per-sample shading, just interpolate at the sample
> no matter what" semantics that ARB_sample_shading introduced. I tried
> to start a thread about this but no one bit :)
>
> Hopefully people who have more experience than I will be able to
> suggest how to proceed. As always, happy to do it whichever way.
>
> -ilia
>
> On Sat, Jul 5, 2014 at 6:48 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> 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
More information about the mesa-dev
mailing list