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

[Tobias Droste]

Hi,

this does not seem to work on r600g (or I'm too stupid ;-)).
I updated to latest mesa master, patched piglit and this is the result:

$ glxinfo
[...]
OpenGL vendor string: X.Org
OpenGL renderer string: Gallium 0.4 on AMD JUNIPER
OpenGL core profile version string: 3.3 (Core Profile) Mesa 10.3.0-devel 
(git-0c181cd)
OpenGL core profile shading language version string: 3.30
OpenGL core profile context flags: (none)
OpenGL core profile profile mask: core profile
[...]

$ MESA_EXTENSION_OVERRIDE=GL_ARB_fragment_layer_viewport bin/shader_runner 
tests/spec/arb_fragment_layer_viewport/layer-gs-write-simple.shader_test -fbo 
-auto
piglit: info: Requested an OpenGL 3.2 Core Context, and received a matching 
3.3 context

Probe color at (0,0)
  Expected: 0.000000 1.000000 0.000000
  Observed: 1.000000 0.000000 0.000000
PIGLIT: {'result': 'fail' }

$ MESA_EXTENSION_OVERRIDE=GL_ARB_fragment_layer_viewport bin/shader_runner 
tests/spec/arb_fragment_layer_viewport/viewport-gs-write-simple.shader_test -
fbo -auto
piglit: info: Requested an OpenGL 3.2 Core Context, and received a matching 
3.3 context

Probe color at (0,0)
  Expected: 0.000000 1.000000 0.000000
  Observed: 1.000000 0.000000 0.000000
PIGLIT: {'result': 'fail' }

Tobias

Am Mo, 23. Juni 2014, 23:00:15 schrieb Ilia Mirkin:
> On Mon, Jun 23, 2014 at 3:52 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> > On Mon, Jun 23, 2014 at 3:50 PM, Roland Scheidegger <sroland at vmware.com> 
wrote:
> >> 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
> > 
> > And a viewport index of -1 was one of my test cases. That's probably it!
> 
> Just to close the loop here, this was indeed it. With the patch that
> fixes that (which is already in the repo), llvmpipe passes. I've also
> pushed patches 1 + 2 of this series. The only thing left to test is
> r600... anyone with the hw can grab the latest master, grab the piglit
> tests from http://patchwork.freedesktop.org/patch/28375/, and run them
> with
> 
> MESA_EXTENSION_OVERRIDE=GL_ARB_fragment_layer_viewport
> bin/shader_runner ... -fbo -auto
> 
> If it passes, I think it's fine to push this out with a cap. If it fails...
> ugh.
> 
>   -ilia
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev