[Mesa-dev] [PATCH v2 3/3] mesa/st: enable ARB_fragment_layer_viewport

[Roland Scheidegger]

Am 23.06.2014 21:14, schrieb Ilia Mirkin:
> On Mon, Jun 23, 2014 at 12:01 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>> Am 23.06.2014 17:18, schrieb Ilia Mirkin:
>>> On Mon, Jun 23, 2014 at 11:06 AM, Roland Scheidegger <sroland at vmware.com> wrote:
>>>> Am 23.06.2014 16:43, schrieb Ilia Mirkin:
>>>>> On Mon, Jun 23, 2014 at 9:51 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>>>> On Mon, Jun 23, 2014 at 9:39 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>>>>> If multiple viewports are supported, that implies the presence of a GS
>>>>>>> and layered rendering, so we can enable ARB_fragment_layer_viewport as
>>>>>>> well.
>>>>>>>
>>>>>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>>>>>> ---
>>>>>>>
>>>>>>> Untested on r600, but nv50/nvc0/llvmpipe seem to pass basic testing.
>>>>>>
>>>>>> Grr... actually llvmpipe fails for viewport, but works for layer.
>>>>>
>>>>> But... it draws all black instead of red, which means it's probably
>>>>> either my or llvmpipe's understanding of default viewport state that's
>>>>> wrong. I assumed that by default viewports were initialized s.t. they
>>>>> would "Just Work" without having to be touched. Or... it's not
>>>>> clamping the viewport index properly. Will investigate later.
>>>> Hmm I'm not sure. This was tested with d3d10 rules which of course
>>>> doesn't have the commands to set all viewports at once. But this is
>>>> handled by the state tracker.
>>>
>>> Well, the tests don't set viewports at all. I was under the
>>> (potentially incorrect) assumption that the default state was good
>>> enough.
>> Yes it should be - the viewport should be set somewhere (I'm not exactly
>> sure where it gets set) and since in GL the "old" commands just set all
>> viewports this should work.
> 
> This is the first time I'm looking at llvmpipe in detail, so forgive
> the potential stupidity...
> 
> It seems like lp_state_fs.c:lp_llvm_viewport isn't clamping the
> viewport index. Should it be? [Or is there some non-obvious place
> where it's actually being clamped?] It seems like the layer is clamped
> explicitly in lp_setup_tri and lp_setup_point.

This is sort of complicated.
Note that for ordinary fs input handling, you don't see viewport index
handling anywhere in llvmpipe - this is just handled like any other
ordinary attribute (through the interpolation stuff).
The code in lp_state_fs for viewport handling is just to get the correct
depth range, so you are right the viewport index must be clamped. The
clamping is done in lp_setup_tri() however (watch out for
viewport_index), the value then gets passed to rasterization (via
tri->inputs) and there written into the thread_data (in
lp_rast_shade_tile / lp_rast_shade_quads_mask). This is the value the
code in lp_state_fs.c will then pick up to fetch the correct depthrange
values (for depth clamping).

Though now that I look at it, this looks wrong in draw:

draw_clamp_viewport_idx(int idx)
{
   return ((PIPE_MAX_VIEWPORTS > idx || idx < 0) ? idx : 0);
}


The same clamping in llvmpipe looks like this:

lp_clamp_viewport_idx(int idx)
{
   return (PIPE_MAX_VIEWPORTS > idx && idx >= 0) ? idx : 0;
}

Would only make a difference for negative viewport indices though (and
don't ask me what would happen but draw might indeed fetch the viewport
values from "random" memory locations so the result might be quite
unpredictable...).

Roland