On 08/21/2013 07:54 AM, Paul Berry wrote:
On 20 August 2013 11:58, Kenneth Graunke <kenn...@whitecape.org <mailto:kenn...@whitecape.org>> wrote:On 08/20/2013 11:30 AM, Paul Berry wrote: --- src/mesa/drivers/dri/i965/brw___program.h | 8 ++++++++ src/mesa/drivers/dri/i965/brw___shader.cpp | 2 +- src/mesa/drivers/dri/i965/brw___shader.h | 7 ++++++- src/mesa/drivers/dri/i965/brw___vec4.h | 12 ++++++++++-- src/mesa/drivers/dri/i965/brw___vs.h | 8 ++++++++ 5 files changed, 33 insertions(+), 4 deletions(-) I like the change to brw_program.h and brw_vs.h. But I'm confused by the changes to brw_shader.h and brw_vec4.h---those are already only used by C++ code, and full of C++ stuff. Perhaps you're making them safe to include from C, for some reason? Yeah, sorry. I got confused at some point while I was rebasing this series and squashed together two unrelated patches. You're right that the changes to brw_shader.h and brw_vec4.h are to make them safe to include from C. They're needed for patch 6 (i965/vec4: Move vec4 data structures and functions to brw_vec4.{cpp,h}). How about if I split those changes out to their own patch, put it just before patch 6, and make the commit message something like this? Make brw_{shader,vec4}.h safe to include from C. The patch that follows will move the definition of struct brw_vec4_prog_key from brw_vs.h to brw_vec4.h, making it necessary for brw_vs.h to include brw_vec4.h (because brw_vs.h defines struct brw_vs_prog_key, which contains brw_vec4_prog_key as a member). Since brw_vs.h is included from C source files, that means that brw_vec4.h will need to be safe to include from C. Same for brw_shader.h, since it is included by brw_vec4.h.
Sounds good to me. I don't know that you really need to split it into two patches, but some comments might be nice.
--Ken _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
