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

Tue Jun 24 09:58:47 PDT 2014

Am Di, 24. Juni 2014, 12:46:39 schrieb Ilia Mirkin:
> On Tue, Jun 24, 2014 at 12:44 PM, Tobias Droste <tdroste at gmx.de> wrote:
> > 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_tes
> > t - 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' }
> 
> Boooo :(
> 
> Could I trouble you to figure out which of the 4 draws (in each) is
> failing? (Just comment out the "probe" lines, leaving only one in for
> each)
> 
> Thanks for testing, BTW!
> 
>   -ilia

All 4 probes in each test fail with the same result:
Expected: 0.000000 1.000000 0.000000
Observed: 1.000000 0.000000 0.000000

(And just to make sure I'm doing the right thing, I removed all 4 probes in 
each test and then it passes).

Tobias

> 
> > 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