On 06/22/2011 08:51 PM, Bryan Cain wrote:
On 06/16/2011 12:43 PM, Brian Paul wrote:
On 06/16/2011 10:34 AM, Bryan Cain wrote:
On Thu, Jun 16, 2011 at 9:08 AM, Brian Paul<bri...@vmware.com
<mailto:bri...@vmware.com>>  wrote:


     Looks like nice work, Bryan.

     Just a few minor questions/comments for now:

     1. The st_fragment/vertex/geometry_program structs now have a
     glsl_to_tgsi field.  I did a grep, but I couldn't find where that
     field is assigned.  Can you clue me in?

It's assigned at the end of the get_mesa_program function in
st_glsl_to_tgsi.cpp.

Thanks.  I was using grep glsl_to_tgsi *.[ch]   Duh.


     2. The above mentioned program structs contains an old Mesa
     instruction program AND/OR(?) a GLSL IR.  Do both types of
     representations co-exist sometimes?  Perhaps you could update the
     comments on those structs to explain that.

They used to co-exist, because after my first commit, st_glsl_to_tgsi
still generated Mesa IR in addition to TGSI.  But I removed the Mesa
IR generation in "st/mesa: stop generating Mesa IR in
st_glsl_to_tgsi", so now it has either one or the other -
glsl_to_tgsi_visitor for GLSL shaders, Mesa IR programs for everything
else.

OK, can you update the comments with this info?


     3. Kind of a follow-on: for glDrawPixels and glBitmap we take the
     original program code (in Mesa form) and prepend extra
     instructions for fetching the fragment color or doing the fragment
     kill.  Do we always have the Mesa instructions for this?  It seems
     we don't normally want to generate Mesa instructions all the time
     but we still need them sometimes.

No, I didn't realize Mesa did that, and we don't have the Mesa
instructions for GLSL programs anymore.  I'm not sure what the
right way to handle that is.

How hard would it be to edit the IR to insert the extra operations?

For glBitmaps it's basically sample a texture and then do a
conditional fragment kill.  For glDrawPixels we need to sample the
texture representing the image, then apply the pixel transfer ops
(scale/bias, table lookup, etc).  We generate the code for that in
st_atom_pixeltransfer.c.  That program is then concatenated with the
current fragment program with _mesa_combine_program().

If we ever propogate GLSL IR through the gallium interface there's
lots of places where we'll need to do other kinds of IR editing.

-Brian


I've been working on this for the last few days, but there are some
things I still haven't figured out yet.

I've written a function to generate the shader for
glDrawPixels/glCopyPixels in the form of glsl_to_tgsi_instructions, and
I doubt I'll have any problems writing the code to merge those
instructions with an existing shader.    However, for the shader to work
correctly, I need to set stfp->glsl_to_tgsi in st_fragment_program, but
all of the fragment program variables in st_atom_pixeltransfer.c and
st_cb_drawpixels.c have the type gl_program or gl_fragment_program.
When do these become st_fragment_program so they can be processed in
st_program.c?

Currently, the extra fields in st_fragment_program are 'tgsi' and 'variants'. They only get set/used after the base program's translated into tgsi so they're not relevant in the drawpixels / pixeltransfer code.


Will I need to move the glsl_to_tgsi attribute into
gl_fragment program and give it a name not specific to the state tracker?

Can't you just replace gl_fragment_program with st_fragment_program in the drawpixels and pixeltransfer code?


My other question is that if I don't have this part done before the
merge window closes, would it still be possible to merge the GLSL->TGSI
translator for 7.11 tomorrow or Friday, and for me to fix the
glBitmap/glDrawPixels/glCopyPixels issue in the stable branch before
Mesa 7.11 is released?  It's the only remaining issue I know of with the
GLSL->TGSI translator, and even it's something of a corner case.

I'm kind of on the fence. Does this bring some real value to the 7.11 release? Is there a downside to putting it in 7.12?

I'm going to be on vacation next week and may not be of much help if there's any problems.

-Brian
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to