[Mesa-dev] [PATCH-RFC] draw: Rewrite primitive decomposer

[Chia-I Wu]

On Tue, Aug 3, 2010 at 6:48 AM, Zack Rusin <zackr at vmware.com> wrote:
> On Monday 02 August 2010 12:33:50 Chia-I Wu wrote:
>> While studying the draw module, I noticed that multiple primitive
>> decomposers exist: draw_pt_vcache_tmp.h, draw_gs_tmp.h,
>> draw_so_emit_tmp.h, and draw_pt_decompose.h.  The differences between
>> them are small, yet some of them support primitive modes that others
>> don't.
> Yes, that's because they all operate on a different set of primitives and
> presence of any of the others should result in an assert since it signifies
> that something went seriously wrong.
draw_pt_vcache_tmp.h needs to handle primitive modes with adjacency.  When GS
is not active, draw_pt_decompose.h also needs to handle them.  With OpenGL, GS
never sees QUADS or POLYGON;  Stream output never sees those modes with
adjacency.

There is FUNC_ENTER that is used to insert arbitrary code before the decompose
loop.  It is used by some decomposers to add assertions.  One way is to use it
to insert assertions like

  if (mode == GL_QUADS || mode == GL_POLYGON /* || ... */) {
    assert(!"unexpected primitve type");
    return;
  }

The compiler should hopefully optimize away the code for the unexpected modes
so the object code will still be clean.  Otherwise, we need to add macros to
control the supported primitive modes.
>> As for PIPE_PRIM_TRIANGLE_STRIP_ADJACENCY, the GS decomposer does not
>> seem to follow the vertex ordering of GL_TRIANGLE_STRIP_ADJACENCY.  I
>> write that part from scratch.  I am not 100% sure here as I don't have
>> a test case to verify.  It would be great if someone can clarify.
>
> That's very vague, what's not right? The only thing I see in the old code is
> that we possibly flip the first and second adjacency vertex.
Sorry, I was not clear here.  It should be easier to describe with an example.

When there are 8 vertices, the two decomposed triangles should be

  TRI_ADJ(0, 1, 2, 6, 4, 3);
  TRI_ADJ(4, 0, 2, 5, 6, 7);

while the old code would emit

  TRI_ADJ(0, 1, 2, 3, 4, 5);
  TRI_ADJ(2, 5, 6, 3, 4, 7);

(Table 2.4 of glspec33.core.20100311.pdf)
>> I haven't done a thorough (read: piglit) test yet.  I would like to
>> get early feedback before continuing.  Suggestions?
>
> Obviously having multiple paths that decompose primitives isn't ideal so it
> would be nice to fix it. The new code is harder to read and misses those extra
Hmm, I thought it was easier to read ;p
> asserts that we had, plus it would be good if fixes, if any, came in separate
> patches, not during refactoring and given that pretty much all of the new
> decompose code is copy and paste it would nice if the old copyright notices
> were preserved.
There were no copyright notices.  The first commit for draw_pt_vcache_tmp.h
dated back to 2008.  I could add that.  I will add a patch to fix the edge
flags for flatshade_first POLYGON if I have a green light on this patch series
in the end of the discussion.

-- 
olv at LunarG.com