[Mesa-dev] [PATCH 02/36] i965: Write code to compute a VUE map.

[Paul Berry]

On 2 September 2011 12:47, Eric Anholt <eric at anholt.net> wrote:

> On Fri,  2 Sep 2011 09:06:41 -0700, Paul Berry <stereotype441 at gmail.com>
> wrote:
> > Several places in the i965 code make implicit assumptions about the
> > structure of data in the VUE (vertex URB entry).  This patch adds a
> > function, brw_compute_vue_map(), which computes the structure of the
> > VUE explicitly.  Future patches will modify the rest of the driver to
> > use the explicitly computed map rather than rely on implicit
> > assumptions about it.
> > ---
> > +      if (two_side_color) {
> > +         /* front and back colors need to be consecutive */
> > +         if ((outputs_written & BITFIELD64_BIT(VERT_RESULT_COL1)) &&
> > +             (outputs_written & BITFIELD64_BIT(VERT_RESULT_BFC1))) {
> > +            assert(outputs_written & BITFIELD64_BIT(VERT_RESULT_COL0));
> > +            assert(outputs_written & BITFIELD64_BIT(VERT_RESULT_BFC0));
> > +            assign_vue_slot(vue_map, VERT_RESULT_COL0);
> > +            assign_vue_slot(vue_map, VERT_RESULT_BFC0);
> > +            assign_vue_slot(vue_map, VERT_RESULT_COL1);
> > +            assign_vue_slot(vue_map, VERT_RESULT_BFC1);
> > +         } else if ((outputs_written & BITFIELD64_BIT(VERT_RESULT_COL0))
> &&
> > +                    (outputs_written &
> BITFIELD64_BIT(VERT_RESULT_BFC0))) {
> > +            assign_vue_slot(vue_map, VERT_RESULT_COL0);
> > +            assign_vue_slot(vue_map, VERT_RESULT_BFC0);
> > +         }
>
> This looks like you could move the else if () up above and drop the
> COL0/BFC0 concerns from the COL1/BFC1 path. (and then I don't have to
> think about what can generates COL1/BFC1 and whether the asserts are
> valid).
>

Yeah, I suspect that some of these asserts are incorrect.  I remember
thinking they were suspicious when I copied them from brw_vs_alloc_regs()
and get_attr_override()--for example, what happens if the client code turns
on two-sided color and writes to gl_FrontColorSecondary and
gl_BackColorSecondary but not gl_FrontColor and gl_BackColor?  Then these
assertions would fire.  That would be kind of weird, but it shouldn't be
illegal.

Thinking about it further, what I would actually prefer to do is to always
put front and back colors next to each other in the VUE (at least for
GEN6+), regardless of whether we are doing two-sided color.  Then the code
would look something like this:

if (outputs_written & BITFIELD64_BIT(VERT_RESULT_COL0)
    assign_vue_slot(vue_map, VERT_RESULT_COL0);
if (outputs_written & BITFIELD64_BIT(VERT_RESULT_BFC0)
    assign_vue_slot(vue_map, VERT_RESULT_BFC0);
if (outputs_written & BITFIELD64_BIT(VERT_RESULT_COL1)
    assign_vue_slot(vue_map, VERT_RESULT_COL1);
if (outputs_written & BITFIELD64_BIT(VERT_RESULT_BFC1)
    assign_vue_slot(vue_map, VERT_RESULT_BFC1);

Then we would let get_attr_override() decide whether or not to swizzle the
colors based on the setting of two_side_color.  I think that would address
your comment (since it would remove the assertions).  It would also have the
advantage of making the layout of the VUE easier for humans to comprehend.
I'll try doing that as a follow-up patch.  (I don't want to do it by
altering this patch because in future patches, where I switch the driver
over to using the VUE map, I don't want to cause behavioral changes).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110902/7b1d9256/attachment-0001.htm>