[Mesa-dev] [PATCH 00/13] st/mesa: finally finish ARB_enhanced_layouts

[Ilia Mirkin]

On Fri, Oct 7, 2016 at 3:55 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> Hi everybody,
>
> this series implements the missing piece of ARB_enhanced_layouts, which is
> the component numbers part of the extension.  You can find the full series
> here: https://cgit.freedesktop.org/~nh/mesa/log/?h=ARB_enhanced_layouts
>
> The series follows the approach sketched by Ilia a few months back.  The
> UsageMask part of input and output declarations is put to use, translating
>
> layout(location = 0, component = 1) in vec2 a[4];
> layout(location = 2, component = 3) in float b[4];
> layout(location = 1, component = 3) in float c;
> layout(location = 2, component = 0) in float d;
>
> into
>
> DCL IN[0..3].yz, ARRAY(1), GENERIC[0], PERSPECTIVE
> DCL IN[2..5].w, ARRAY(2), GENERIC[2], PERSPECTIVE
> DCL IN[1].w, GENERIC[1], PERSPECTIVE
> DCL IN[2].x, GENERIC[2], PERSPECTIVE

Hmmm... this seems potentially fragile wrt existing implementations.
Are there any tests where IN[1].w (for example) would be
flat-qualified? In order for that to be supported properly, an
implementation would almost certainly have to look at the UsageMask. I
think in my original proposal, I was suggesting that there would be no
overlapping like this for the inputs, i.e. the above would be

IN[0..3].yz -> GENERIC[0]
IN[4..7].w -> GENERIC[2]
etc

Although your approach has the advantage of not having to futz with
the input/output mapping logic too much. So ultimately I think it's
fine, but tests need to be added for having different interpolation
settings for different components of a "shared" slot.

>
> It turns out that this approach works on radeonsi, softpipe, and llvmpipe
> without any driver changes at all, but I'm introducing a new pipe cap anyway
> just in case.  Unless your driver is doing something particularly fancy with
> arrays, chances are that you only have to flip on the new cap and things
> will Just Work.  (I can squash that into this series before I push if you
> don't want to keep track of the trivial patch for that.)
>
> Most of the work in the series lies in rearranging how we emit the
> declarations for input and output arrays -- that's patch #10, after some
> preparatory work.
>
> There is one caveat to the use of UsageMask: arrays of dvec3 leave the zw
> components free in every other slot, and that space can be used by other
> declarations.  In that case, declarations will not be disjoint; and since
> this problem is unavoidable for dvec3 arrays, I'm not even trying to do
> anything about dvec3 non-arrays.  If your driver has problems with that, my
> suggestion would be to fix the dvec3 non-array case and add an additional
> special case bit to the array declaration token.
>
> So far, I've tested this series on radeonsi with Piglit (plus two new tests
> which I'm going to send out as well).  The enhanced layouts parts pass, and
> I think I fixed all the regressions elsewhere.  I plan to do further tests
> with CTS before this is pushed.
>
> Finally, as a little addition, the last patch turns on GLSL 4.50 for
> radeonsi.  All the bits for OpenGL 4.5 are implemented with this series,
> though we do have to be careful about really claiming support in the
> release because we haven't done the whole CTS results submission dance (and
> there are still a bunch of genuine failures as well).
>
> Please review!
>
> Thanks,
> Nicolai
> --
>  docs/features.txt                            |  16 +-
>  docs/relnotes/12.1.0.html                    |   2 +-
>  src/compiler/glsl/ir_print_visitor.cpp       |  10 +-
>  src/gallium/auxiliary/tgsi/tgsi_scan.c       |   9 +-
>  src/gallium/auxiliary/tgsi/tgsi_ureg.c       | 115 +++--
>  src/gallium/auxiliary/tgsi/tgsi_ureg.h       |  30 ++
>  src/gallium/docs/source/screen.rst           |   8 +
>  .../drivers/freedreno/freedreno_screen.c     |   1 +
>  src/gallium/drivers/i915/i915_screen.c       |   1 +
>  src/gallium/drivers/ilo/ilo_screen.c         |   1 +
>  src/gallium/drivers/llvmpipe/lp_screen.c     |   1 +
>  .../drivers/nouveau/nv30/nv30_screen.c       |   1 +
>  .../drivers/nouveau/nv50/nv50_screen.c       |   1 +
>  .../drivers/nouveau/nvc0/nvc0_screen.c       |   1 +
>  src/gallium/drivers/r300/r300_screen.c       |   1 +
>  src/gallium/drivers/r600/r600_pipe.c         |   1 +
>  src/gallium/drivers/radeonsi/si_pipe.c       |   3 +-
>  src/gallium/drivers/softpipe/sp_screen.c     |   1 +
>  src/gallium/drivers/svga/svga_screen.c       |   1 +
>  src/gallium/drivers/swr/swr_screen.cpp       |   1 +
>  src/gallium/drivers/vc4/vc4_screen.c         |   1 +
>  src/gallium/drivers/virgl/virgl_screen.c     |   1 +
>  src/gallium/include/pipe/p_defines.h         |   1 +
>  src/mesa/state_tracker/st_extensions.c       |   7 +
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp   | 441 +++++++++--------
>  25 files changed, 416 insertions(+), 240 deletions(-)
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev