[Mesa-dev] [PATCH 1/2] glsl: Properly count doubles as taking twice the number of attrib slots.

[Kenneth Graunke]

On Tuesday, September 01, 2015 07:03:58 PM Ilia Mirkin wrote:
> On Tue, Sep 1, 2015 at 6:53 PM, Matt Turner <mattst88 at gmail.com> wrote:
> > On Tue, Sep 1, 2015 at 3:57 PM, Matt Turner <mattst88 at gmail.com> wrote:
> >> ---
> >> I checked the uses of count_attribute_slots() and it looks like they're
> >> expecting this already, but these two patches definitely need testing on
> >> a driver that supports fp64.
> >
> > Whoops, assuming things go well this should also be Cc'd to 10.6 and 11.0.
> 
> Sooo.... I can't tell whether your patch upsets this, but just want to
> make sure you're aware of this behaviour: Even though the driver is
> allowed to count doubles as double-wide on the back end for the
> purposes of max attributes, double vertex attributes are still only
> supposed to count as a single slot where bindings are concerned.
> 
> I'll run these through a piglit -t fp64 though, hopefully that picks
> up any piglit tests that were supposed to catch this sort of thing.
> 
>   -ilia

You're right - the behavior is quite a bit different than I thought.
Issue 3 of GL_ARB_vertex_attrib_64bit lays out the rules clearly:

    (3) How should we handle vertex attributes for three- and four-component
        vectors with double-precision components?  How do we support these
        with vertex arrays?

      RESOLVED:  Double-precision attributes may consume twice as much
      internal storage as their single-precision counterparts.  For the
      purposes of determining if a vertex shader uses "too many" attribute
      vectors in LinkProgram, implementations are permitted (but not required)
      to count "dvec3" and "dvec4" vertex shader inputs as consuming twice as
      many input vectors as corresponding single-precision types.
      Implementations are required to count inputs of type "double" and
      "dvec2" as a single vector, since these types require no more storage
      than a "vec4".

      Note however, that for the purposes of mapping inputs to generic vertex
      attributes, "dvec3" and "dvec4" inputs are counted as consuming one
      attribute/location.  For example, if a vertex shader specifies:

        layout(location=4) in dvec4 attribs[4];

      the values for the four elements of "attribs" will be taken from vertex
      attributes 4-7, though "attribs" may be counted as consuming eight
      vectors worth of attributes.

So I think we actually want count_attribute_slots() to stay the same, so
that dvecs continue mapping to one attribute/location slot.

That then leaves the question of whether to revert ad208d975a, which
breaks deQP functional.attribute_location.bind_aliasing.max_cond_*.

According to the above text, we're /allowed/ to try and count double
types larger than 128-bits as twice as large, but not required to.
So I believe we could simply revert Dave's patch and have a valid
implementation.

Alternatively, we could try and fix the code to handle attribute
location aliasing.  That actually looks pretty easy, and I've written
a patch I think will fix this properly.  Assuming it passes Piglit,
I'll send it out shortly and CC the three of you.

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150902/b3a92bed/attachment.sig>