date:20100127

On Tue, 2010-01-26 at 16:26 -0800, Brian Paul wrote:
 Stephane Marchesin wrote:
  On Tue, Jan 26, 2010 at 15:13, Ian Romanick i...@freedesktop.org wrote:
  -BEGIN PGP SIGNED MESSAGE-
  Hash: SHA1
 
  José Fonseca wrote:
  mesa_7_7_branch and master are becoming quite different, because of all
  the gallium interface changes that have been going into master, so
  merging fixes from mesa_7_7_branch into master is becoming less and less
  of a trivial exercise.
 
  This is aggravated by the fact we are basing a release from the
  mesa_7_7_branch, so it's likely that we'll need to have temporary
  non-invasive bugfixes that should not go into master (which should
  receive instead the proper and potentially invasive fix).
 
  I see a few alternatives here:
 
  a) stop merging mesa_7_7_branch - master. bugfixes should be applied to
  both branches. preferably by the person that wrote the patch.
 
  b) when applying a non-trivial bugfix to mesa_7_7_branch, the same
  person should do the merge into master, and undo any undesired change,
  or update for interface changes in master. Note however that it's better
  to give a few days between applying to mesa_7_7_branch and merging into
  master, to allow for wider testing, otherwise we'll be merging like
  crazy.
 
  c) do not apply any non trivial bugfix to mesa_7_7_branch anymore, and
  use a separate branch for those.
 
  I don't feel strongly about any of these alternatives for now. We'll
  eventually need to setup a private branch for our release so c) is bound
  to happen anyway. But I also think we can't keep merging mesa_7_7_branch
  into master like this forever -- after so much thought was put into the
  gallium interface changes (feature branches, peer review, etc) but
  whenever a mesa_7_7_branch - master happens all sort of wrong code is
  merged automatically.
  This was my primary argument *against* our current commit / merge model.
 
  The ideal would be to peer-review the merges before committing, but it
  seems difficult to do that with git, while preserving merge history and
  not redoing merges.
  It sounds like we want to copy the Linux kernel model:
 
  - - Each developer has a local tree.
  - - Each developer sends out:
- A patch series
- A pull request
- A list of commit IDs to cherry-pick
  - - Based on review comments, the maintainer applies the patches / pulls
  the tree.
  
  More bureaucracy. Just what we need in our understaffed world.
 
 I'm not too crazy about this either.  We can barely keep up with the 
 patches submitted for review now.  I certainly don't have time to 
 review everything that comes along and very few other people are 
 reviewing/testing/committing patches either.
 
 My plate is already full.

One thing worth noting is how well the branch-master merges have been
working.  We've *never* managed to put so many fixes into the stable
branch and successfully propagate them to master.  Think of the hundreds
of commits Vinson has made fixing errors from static analysis.

The system has worked better than anything else before, but is now
starting to reach its limits.  That seems to me to call for a minor
adjustment rather than a total overhaul.

Maybe the approach should be to minimise now the amount of stuff going
into the stable branch - ask Vinson to do his work on master now, for
instance, and let the stable branch only take fixes for user-visible
bugs, which are hopefully smaller in volume.

Keith


--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] mesa_7_7_branch - master merges

On Wed, 2010-01-27 at 00:59 -0800, Keith Whitwell wrote:
 On Tue, 2010-01-26 at 16:26 -0800, Brian Paul wrote:
  Stephane Marchesin wrote:
   On Tue, Jan 26, 2010 at 15:13, Ian Romanick i...@freedesktop.org wrote:
   -BEGIN PGP SIGNED MESSAGE-
   Hash: SHA1
  
   José Fonseca wrote:
   mesa_7_7_branch and master are becoming quite different, because of all
   the gallium interface changes that have been going into master, so
   merging fixes from mesa_7_7_branch into master is becoming less and less
   of a trivial exercise.
  
   This is aggravated by the fact we are basing a release from the
   mesa_7_7_branch, so it's likely that we'll need to have temporary
   non-invasive bugfixes that should not go into master (which should
   receive instead the proper and potentially invasive fix).
  
   I see a few alternatives here:
  
   a) stop merging mesa_7_7_branch - master. bugfixes should be applied to
   both branches. preferably by the person that wrote the patch.
  
   b) when applying a non-trivial bugfix to mesa_7_7_branch, the same
   person should do the merge into master, and undo any undesired change,
   or update for interface changes in master. Note however that it's better
   to give a few days between applying to mesa_7_7_branch and merging into
   master, to allow for wider testing, otherwise we'll be merging like
   crazy.
  
   c) do not apply any non trivial bugfix to mesa_7_7_branch anymore, and
   use a separate branch for those.
  
   I don't feel strongly about any of these alternatives for now. We'll
   eventually need to setup a private branch for our release so c) is bound
   to happen anyway. But I also think we can't keep merging mesa_7_7_branch
   into master like this forever -- after so much thought was put into the
   gallium interface changes (feature branches, peer review, etc) but
   whenever a mesa_7_7_branch - master happens all sort of wrong code is
   merged automatically.
   This was my primary argument *against* our current commit / merge model.
  
   The ideal would be to peer-review the merges before committing, but it
   seems difficult to do that with git, while preserving merge history and
   not redoing merges.
   It sounds like we want to copy the Linux kernel model:
  
   - - Each developer has a local tree.
   - - Each developer sends out:
 - A patch series
 - A pull request
 - A list of commit IDs to cherry-pick
   - - Based on review comments, the maintainer applies the patches / pulls
   the tree.
   
   More bureaucracy. Just what we need in our understaffed world.
  
  I'm not too crazy about this either.  We can barely keep up with the 
  patches submitted for review now.  I certainly don't have time to 
  review everything that comes along and very few other people are 
  reviewing/testing/committing patches either.
  
  My plate is already full.
 
 One thing worth noting is how well the branch-master merges have been
 working.  We've *never* managed to put so many fixes into the stable
 branch and successfully propagate them to master.  Think of the hundreds
 of commits Vinson has made fixing errors from static analysis.
 
 The system has worked better than anything else before, but is now
 starting to reach its limits.  That seems to me to call for a minor
 adjustment rather than a total overhaul.

Yes. I agree.

Indeed reviews only work if there are enough reviewers, and the review
traffic is pretty overwhelming as it is. I should have thought of that
before mentioning it.

BTW, Brian thanks for all the hard work you've been doing on taking
patches. I should and will try to do more on this subject.

 Maybe the approach should be to minimise now the amount of stuff going
 into the stable branch - ask Vinson to do his work on master now, for
 instance, and let the stable branch only take fixes for user-visible
 bugs, which are hopefully smaller in volume.

Yes, that's probably a good compromise.

Jose


--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [RFC] gallium-multiple-constant-buffers merge

On Mon, 2010-01-25 at 06:05 -0800, michal wrote:
 Brian Paul wrote on 2010-01-22 17:56:
  michal wrote:

  Brian Paul wrote on 2010-01-21 21:57:
  
  michal wrote:


  Hi,
 
  This simple feature branch adds support for two-dimensional constant 
  buffers in TGSI.
 
  An example shader would look like this:
 
  FRAG
 
  DCL IN[0], COLOR, LINEAR
  DCL OUT[0], COLOR
  DCL CONST[1][1..2]
 
  MAD OUT[0], IN[0], CONST[1][2], CONST[1][1]
 
  END
 
  For this to work, one needs to bind a buffer to slot nr 1 containing at 
  least 3 vectors.
  
  
  Just a terminology thing: this feature really implements arrays of 
  constant buffers, not really two-dimensional buffers, right?
 


  That's correct -- the access to constbuf data is two-dimensional, but 
  the constbufs themselves are an array of differently-sized constat buffers.
 
  
  In p_state.h we should probably rename PIPE_MAX_CONSTANT to 
  PIPE_MAX_CONSTANT_BUFFERS to be clearer.
 
  Don't we need a new PIPE_CAP_MAX_CONSTANT_BUFFERS query?  Maybe even a 
  query per shader stage?
 


  What about maximum size of a single constant buffer? I would think this 
  is a more crtical parameter than the number of constbuf slots the driver 
  support.
  
 
  Yeah, I thought we already had a query for that, but we don't.
 
  I'd suggest:
 
  PIPE_CAP_MAX_CONST_BUFFERS
  PIPE_CAP_MAX_CONST_BUFFER_SIZE  (in bytes)
 

 
 All,
 
 Thanks for your comments, I have commited my changes to the branch and 
 am awaiting for more comments.

Michal,

Looks good to me.  Go ahead and merge when you're ready.

Keith


--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] perrtblend merge

Roland,

This is looking good too - feel free to merge when ready.

Ketih

On Tue, 2010-01-26 at 08:03 -0800, Roland Scheidegger wrote:
 Oh, I should have added the PIPE_CAP bits (even if not supported) to all
 drivers. Good catch. I'll do that for the other drivers now.
 
 Roland
 
 (btw, I think r500 could do separate colormasks, but not separate blend
 enables, and there might be more hardware like that. However, this is
 not exposed by GL, it might be supported by some DX9 cap bit, but it
 didn't seem worthwile to add a separate gallium cap bit for supporting
 per-rt blend enables and colormasks, respectively.)
 
 On 26.01.2010 16:37, Corbin Simpson wrote:
  Yeah, r300 doesn't but r600 does. I've read through the branch, and
  the r300g patch looks perfect. I've pushed another patch on top for
  the pipe caps, to avoid post-merge cleanups for myself.
  
  On Tue, Jan 26, 2010 at 7:00 AM, Alex Deucher alexdeuc...@gmail.com wrote:
  On Tue, Jan 26, 2010 at 9:44 AM, Roland Scheidegger srol...@vmware.com 
  wrote:
  Hi,
 
  I'm planning on merging this branch to master soon.
  This will make it possible to do per render target blend enables,
  colormasks, and also per rendertarget blend funcs (with a different CAP
  bit for the latter, and this one isn't actually used in mesa state
  tracker yet).
  None of the drivers other than softpipe implement any of it, but they
  were adapted to the interface changes so should continue to run.
  Apparently, that functionality is only interesting for drivers
  supporting multiple render targets, and the hw probably needs to be
  quite new (I know that i965 could support it (well not the multiple
  blend funcs but the rest), but the driver currently only supports 1
  render target).
  FWIW, AMD R6xx+ hw supports MRTs and per-MRT blends as well, although
  at the moment the driver also only supports 1 RT.
 
  Alex
 
  --
  The Planet: dedicated and managed hosting, cloud storage, colocation
  Stay online with enterprise data centers and the best network in the 
  business
  Choose flexible plans and management services without long-term contracts
  Personal 24x7 support from experience hosting pros just a phone call away.
  http://p.sf.net/sfu/theplanet-com
  ___
  Mesa3d-dev mailing list
  Mesa3d-dev@lists.sourceforge.net
  https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
 
  
  
  
 
 
 --
 The Planet: dedicated and managed hosting, cloud storage, colocation
 Stay online with enterprise data centers and the best network in the business
 Choose flexible plans and management services without long-term contracts
 Personal 24x7 support from experience hosting pros just a phone call away.
 http://p.sf.net/sfu/theplanet-com
 ___
 Mesa3d-dev mailing list
 Mesa3d-dev@lists.sourceforge.net
 https://lists.sourceforge.net/lists/listinfo/mesa3d-dev



--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [RFC] gallium-multiple-constant-buffers merge

Keith Whitwell wrote:
 On Mon, 2010-01-25 at 06:05 -0800, michal wrote:
 Brian Paul wrote on 2010-01-22 17:56:
 michal wrote:
   
 Brian Paul wrote on 2010-01-21 21:57:
 
 michal wrote:
   
   
 Hi,

 This simple feature branch adds support for two-dimensional constant 
 buffers in TGSI.

 An example shader would look like this:

 FRAG

 DCL IN[0], COLOR, LINEAR
 DCL OUT[0], COLOR
 DCL CONST[1][1..2]

 MAD OUT[0], IN[0], CONST[1][2], CONST[1][1]

 END

 For this to work, one needs to bind a buffer to slot nr 1 containing at 
 least 3 vectors.
 
 
 Just a terminology thing: this feature really implements arrays of 
 constant buffers, not really two-dimensional buffers, right?

   
   
 That's correct -- the access to constbuf data is two-dimensional, but 
 the constbufs themselves are an array of differently-sized constat buffers.

 
 In p_state.h we should probably rename PIPE_MAX_CONSTANT to 
 PIPE_MAX_CONSTANT_BUFFERS to be clearer.

 Don't we need a new PIPE_CAP_MAX_CONSTANT_BUFFERS query?  Maybe even a 
 query per shader stage?

   
   
 What about maximum size of a single constant buffer? I would think this 
 is a more crtical parameter than the number of constbuf slots the driver 
 support.
 
 Yeah, I thought we already had a query for that, but we don't.

 I'd suggest:

 PIPE_CAP_MAX_CONST_BUFFERS
 PIPE_CAP_MAX_CONST_BUFFER_SIZE  (in bytes)

   
 All,

 Thanks for your comments, I have commited my changes to the branch and 
 am awaiting for more comments.
 
 Michal,
 
 Looks good to me.  Go ahead and merge when you're ready.

Yes, I reviewed the branch too and it looks good.  Thanks.

-Brian


--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] mesa_7_7_branch - master merges

José Fonseca wrote:
 On Wed, 2010-01-27 at 00:59 -0800, Keith Whitwell wrote:
 On Tue, 2010-01-26 at 16:26 -0800, Brian Paul wrote:
 Stephane Marchesin wrote:
 On Tue, Jan 26, 2010 at 15:13, Ian Romanick i...@freedesktop.org wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1

 José Fonseca wrote:
 mesa_7_7_branch and master are becoming quite different, because of all
 the gallium interface changes that have been going into master, so
 merging fixes from mesa_7_7_branch into master is becoming less and less
 of a trivial exercise.

 This is aggravated by the fact we are basing a release from the
 mesa_7_7_branch, so it's likely that we'll need to have temporary
 non-invasive bugfixes that should not go into master (which should
 receive instead the proper and potentially invasive fix).

 I see a few alternatives here:

 a) stop merging mesa_7_7_branch - master. bugfixes should be applied to
 both branches. preferably by the person that wrote the patch.

 b) when applying a non-trivial bugfix to mesa_7_7_branch, the same
 person should do the merge into master, and undo any undesired change,
 or update for interface changes in master. Note however that it's better
 to give a few days between applying to mesa_7_7_branch and merging into
 master, to allow for wider testing, otherwise we'll be merging like
 crazy.

 c) do not apply any non trivial bugfix to mesa_7_7_branch anymore, and
 use a separate branch for those.

 I don't feel strongly about any of these alternatives for now. We'll
 eventually need to setup a private branch for our release so c) is bound
 to happen anyway. But I also think we can't keep merging mesa_7_7_branch
 into master like this forever -- after so much thought was put into the
 gallium interface changes (feature branches, peer review, etc) but
 whenever a mesa_7_7_branch - master happens all sort of wrong code is
 merged automatically.
 This was my primary argument *against* our current commit / merge model.

 The ideal would be to peer-review the merges before committing, but it
 seems difficult to do that with git, while preserving merge history and
 not redoing merges.
 It sounds like we want to copy the Linux kernel model:

 - - Each developer has a local tree.
 - - Each developer sends out:
   - A patch series
   - A pull request
   - A list of commit IDs to cherry-pick
 - - Based on review comments, the maintainer applies the patches / pulls
 the tree.
 More bureaucracy. Just what we need in our understaffed world.
 I'm not too crazy about this either.  We can barely keep up with the 
 patches submitted for review now.  I certainly don't have time to 
 review everything that comes along and very few other people are 
 reviewing/testing/committing patches either.

 My plate is already full.
 One thing worth noting is how well the branch-master merges have been
 working.  We've *never* managed to put so many fixes into the stable
 branch and successfully propagate them to master.  Think of the hundreds
 of commits Vinson has made fixing errors from static analysis.

 The system has worked better than anything else before, but is now
 starting to reach its limits.  That seems to me to call for a minor
 adjustment rather than a total overhaul.
 
 Yes. I agree.
 
 Indeed reviews only work if there are enough reviewers, and the review
 traffic is pretty overwhelming as it is. I should have thought of that
 before mentioning it.
 
 BTW, Brian thanks for all the hard work you've been doing on taking
 patches. I should and will try to do more on this subject.
 
 Maybe the approach should be to minimise now the amount of stuff going
 into the stable branch - ask Vinson to do his work on master now, for
 instance, and let the stable branch only take fixes for user-visible
 bugs, which are hopefully smaller in volume.
 
 Yes, that's probably a good compromise.

I've already asked Vinson to switch over to master for his clean-ups.

-Brian

--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH] hack around commas in macro argument

José Fonseca wrote:
 On Tue, 2010-01-26 at 05:49 -0800, Roland Scheidegger wrote:
 On 26.01.2010 09:18, Marvin wrote:
 Jose, Brian,

 Marc,

 Why is this necessary? It has been working fine so far. Which gcc version
  are you using? What commas are you referring to?
 the PIPE_ALIGN_TYPE macro is so far only used in the cell driver in 
 src/gallium/drivers/cell/spu/spu_main.c  (this is probably why no one 
 noticed it).

 The marco takes a type, a stuct in this case, which can include commas:

 PIPE_ALIGN_TYPE(16,
 struct spu_framebuffer
 {
void *color_start;  /** addr of color surface in main 
 memory */
void *depth_start;  /** addr of depth surface in main 
 memory */
enum pipe_format color_format;
enum pipe_format depth_format;
uint width, height; /** size in pixels */
 ^^^

uint width_tiles, height_tiles; /** width and height in tiles */
   ^^^

uint color_clear_value;
uint depth_clear_value;

uint zsize; /** 0, 2 or 4 bytes per Z */
float zscale;   /** 65535.0, 2^24-1 or 2^32-1 */
 });

 This will cause a problem, as the macro will thread each comma as an 
 argument 
 seperator and thus the number of arguments is larger than 2.
 Hmm, maybe could just avoid the problem by not using commas in the
 struct declaration?
 
 I agree with Roland. It seems the lesser evil.
 
 Marc, thanks for the detailed explanation.

I've fixed this in the Cell driver.

-Brian


--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 1/4] tgsi: add properties for fragment coord conventions (v2)

Luca Barbieri wrote:
 Changes in v2:
 - Caps are added in a separate, subsequent patch
 
 This adds two TGSI fragment program properties that indicate the
 fragment coord conventions.
 
 The properties behave as described in the extension spec for
 GL_ARB_fragment_coord_conventions, but the default origin in
 upper left instead of lower left as in OpenGL.
 
 The syntax is:
 PROPERTY FS_COORD_ORIGIN [UPPER_LEFT|LOWER_LEFT]
 PROPERTY FS_COORD_PIXEL_CENTER [HALF_INTEGER|INTEGER]
 
 The names have been chosen for consistency with the GS properties
 and the OpenGL extension spec.
 
 The defaults are of course the previously assumed conventions:
 UPPER_LEFT and HALF_INTEGER.

Sorry for the slow reply on this.

It looks like you've solved the fragcoord problems in a clean/logical 
way.  I haven't seen any objections, so I think we can move forward 
with this.

This patch looks good, just a minor comment below.



 ---
  src/gallium/auxiliary/tgsi/tgsi_dump.c |   22 +-
  src/gallium/auxiliary/tgsi/tgsi_text.c |   63 
 +++-
  src/gallium/auxiliary/tgsi/tgsi_ureg.c |   32 ++
  src/gallium/auxiliary/tgsi/tgsi_ureg.h |7 +++
  src/gallium/include/pipe/p_shader_tokens.h |   10 -
  5 files changed, 131 insertions(+), 3 deletions(-)
 
 diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c 
 b/src/gallium/auxiliary/tgsi/tgsi_dump.c
 index c254a72..307d1f7 100644
 --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c
 +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c
 @@ -159,7 +159,9 @@ static const char *property_names[] =
  {
 GS_INPUT_PRIMITIVE,
 GS_OUTPUT_PRIMITIVE,
 -   GS_MAX_OUTPUT_VERTICES
 +   GS_MAX_OUTPUT_VERTICES,
 +   FS_COORD_ORIGIN,
 +   FS_COORD_PIXEL_CENTER
  };
 
  static const char *primitive_names[] =
 @@ -176,6 +178,18 @@ static const char *primitive_names[] =
 POLYGON
  };
 
 +static const char *fs_coord_origin_names[] =
 +{
 +   UPPER_LEFT,
 +   LOWER_LEFT
 +};
 +
 +static const char *fs_coord_pixel_center_names[] =
 +{
 +   HALF_INTEGER,
 +   INTEGER
 +};
 +
 
  static void
  _dump_register_decl(
 @@ -372,6 +386,12 @@ iter_property(
case TGSI_PROPERTY_GS_OUTPUT_PRIM:
   ENM(prop-u[i].Data, primitive_names);
   break;
 +  case TGSI_PROPERTY_FS_COORD_ORIGIN:
 + ENM(prop-u[i].Data, fs_coord_origin_names);
 + break;
 +  case TGSI_PROPERTY_FS_COORD_PIXEL_CENTER:
 + ENM(prop-u[i].Data, fs_coord_pixel_center_names);
 + break;
default:
   SID( prop-u[i].Data );
   break;
 diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c 
 b/src/gallium/auxiliary/tgsi/tgsi_text.c
 index 7fe5dad..0062e9d 100644
 --- a/src/gallium/auxiliary/tgsi/tgsi_text.c
 +++ b/src/gallium/auxiliary/tgsi/tgsi_text.c
 @@ -1117,7 +1117,9 @@ static const char *property_names[] =
  {
 GS_INPUT_PRIMITIVE,
 GS_OUTPUT_PRIMITIVE,
 -   GS_MAX_OUTPUT_VERTICES
 +   GS_MAX_OUTPUT_VERTICES,
 +   FS_COORD_ORIGIN,
 +   FS_COORD_PIXEL_CENTER
  };
 
  static const char *primitive_names[] =
 @@ -1134,6 +1136,19 @@ static const char *primitive_names[] =
 POLYGON
  };
 
 +static const char *fs_coord_origin_names[] =
 +{
 +   UPPER_LEFT,
 +   LOWER_LEFT
 +};
 +
 +static const char *fs_coord_pixel_center_names[] =
 +{
 +   HALF_INTEGER,
 +   INTEGER
 +};
 +
 +
  static boolean
  parse_primitive( const char **pcur, uint *primitive )
  {
 @@ -1151,6 +1166,40 @@ parse_primitive( const char **pcur, uint *primitive )
 return FALSE;
  }
 
 +static boolean
 +parse_fs_coord_origin( const char **pcur, uint *fs_coord_origin )
 +{
 +   uint i;
 +
 +   for (i = 0; i  sizeof(fs_coord_origin_names) / 
 sizeof(fs_coord_origin_names[0]); i++) {
 +  const char *cur = *pcur;
 +
 +  if (str_match_no_case( cur, fs_coord_origin_names[i])) {
 + *fs_coord_origin = i;
 + *pcur = cur;
 + return TRUE;
 +  }
 +   }
 +   return FALSE;
 +}
 +
 +static boolean
 +parse_fs_coord_pixel_center( const char **pcur, uint *fs_coord_pixel_center )
 +{
 +   uint i;
 +
 +   for (i = 0; i  sizeof(fs_coord_pixel_center_names) / 
 sizeof(fs_coord_pixel_center_names[0]); i++) {
 +  const char *cur = *pcur;
 +
 +  if (str_match_no_case( cur, fs_coord_pixel_center_names[i])) {
 + *fs_coord_pixel_center = i;
 + *pcur = cur;
 + return TRUE;
 +  }
 +   }
 +   return FALSE;
 +}
 +
 
  static boolean parse_property( struct translate_ctx *ctx )
  {
 @@ -1192,6 +1241,18 @@ static boolean parse_property( struct translate_ctx 
 *ctx )
   ctx-implied_array_size = u_vertices_per_prim(values[0]);
}
break;
 +   case TGSI_PROPERTY_FS_COORD_ORIGIN:
 +  if (!parse_fs_coord_origin(ctx-cur, values[0] )) {
 + report_error( ctx, Unknown coord origin as property: must be 
 UPPER_LEFT or LOWER_LEFT! );
 + return FALSE;
 +  }
 +  break;
 +   case TGSI_PROPERTY_FS_COORD_PIXEL_CENTER:
 +  if (!parse_fs_coord_pixel_center(ctx-cur,

Re: [Mesa3d-dev] [PATCH 2/4] tgsi: add caps for fragment coord conventions (v2)

Luca Barbieri wrote:
 Changes in v2:
 - Split from properties patch
 - Use positive caps instead of negative caps
 
 This adds 4 caps to indicate support of each of the fragment coord
 conventions.
 
 All drivers are also modifed to add the appropriate caps (3 lines each).
 
 Some drivers were incorrectly using non-Gallium-default conventions,
 and caps for them have them set so that they will behave correctly
 after the later state tracker patches.
 
 This drivers are softpipe/llvmpipe (uses integer rather than half
 integer) and pre-nv50 Nouveau (uses lower left rather than upper left).
 
 Other drivers might be broken. With this patchset, fixing them is
 only a matter of exposing the appropriate caps that match the behavior
 of the existing code.
 
 Drivers are encouraged to support all conventions themselves for better
 performance, and this feature is added to softpipe in a later patch.

Looks good.

-Brian

--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 4/4] softpipe: support all TGSI fragment coord conventions (v3)

Luca Barbieri wrote:
 Changes in v3:
 - Use positive caps instead of negative caps
 
 Changes in v2:
 - Now takes the fragment convention directly from the fragment shader
 
 Adds internal support for all fragment coord conventions to softpipe.
 
 This patch is not required for use with the current state trackers, but it
 allows softpipe to run any TGSI program and enhances performance.

Looks good.

-Brian

--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 3/4] st/mesa: Gallium support for ARB_fragment_coord_conventions (v3)

Luca Barbieri wrote:
 Changes in v3:
 - Use positive caps instead of negative ones
 
 Changes in v2:
 - Updated formatting
 
 The state tracker will use the TGSI convention properties if the hardware
 exposes the appropriate capability, and otherwise adjust WPOS itself.
 
 This will also fix some drivers that were previously broken due to their
 incorrect, inadvertent, use of conventions other than upper_left+half_integer.

Minor comments below.


 ---
  src/mesa/state_tracker/st_extensions.c   |1 +
  src/mesa/state_tracker/st_mesa_to_tgsi.c |   74 -
  2 files changed, 72 insertions(+), 3 deletions(-)
 
 diff --git a/src/mesa/state_tracker/st_extensions.c 
 b/src/mesa/state_tracker/st_extensions.c
 index 60732f3..982a1fb 100644
 --- a/src/mesa/state_tracker/st_extensions.c
 +++ b/src/mesa/state_tracker/st_extensions.c
 @@ -147,6 +147,7 @@ void st_init_extensions(struct st_context *st)
  * Extensions that are supported by all Gallium drivers:
  */
 ctx-Extensions.ARB_copy_buffer = GL_TRUE;
 +   ctx-Extensions.ARB_fragment_coord_conventions = GL_TRUE;

We can't really expose this extension until we've updated GLSL to 
understand layout qualifiers.  And, unfortunately, that builds on GLSL 
1.40 which we don't support yet.


 ctx-Extensions.ARB_fragment_program = GL_TRUE;
 ctx-Extensions.ARB_half_float_vertex = GL_TRUE;
 ctx-Extensions.ARB_map_buffer_range = GL_TRUE;
 diff --git a/src/mesa/state_tracker/st_mesa_to_tgsi.c 
 b/src/mesa/state_tracker/st_mesa_to_tgsi.c
 index 05b56c9..366729a 100644
 --- a/src/mesa/state_tracker/st_mesa_to_tgsi.c
 +++ b/src/mesa/state_tracker/st_mesa_to_tgsi.c
 @@ -34,8 +34,10 @@
  #include pipe/p_compiler.h
  #include pipe/p_shader_tokens.h
  #include pipe/p_state.h
 +#include pipe/p_context.h
  #include tgsi/tgsi_ureg.h
  #include st_mesa_to_tgsi.h
 +#include st_context.h
  #include shader/prog_instruction.h
  #include shader/prog_parameter.h
  #include shader/prog_print.h
 @@ -665,6 +667,22 @@ compile_instruction(
 }
  }
  
 +/**
 + * Emit the TGSI instructions to adjust the WPOS pixel center convention
 + */
 +static void
 +emit_adjusted_wpos( struct st_translate *t,
 +const struct gl_program *program, GLfloat value)
 +{
 +   struct ureg_program *ureg = t-ureg;
 +   struct ureg_dst wpos_temp = ureg_DECL_temporary(ureg);
 +   struct ureg_src wpos_input = t-inputs[t-inputMapping[FRAG_ATTRIB_WPOS]];
 +
 +   ureg_ADD(ureg, ureg_writemask(wpos_temp, TGSI_WRITEMASK_X | 
 TGSI_WRITEMASK_Y),
 +wpos_input, ureg_imm1f(ureg, value));
 +
 +   t-inputs[t-inputMapping[FRAG_ATTRIB_WPOS]] = ureg_src(wpos_temp);
 +}
  
  /**
   * Emit the TGSI instructions for inverting the WPOS y coordinate.
 @@ -690,12 +708,17 @@ emit_inverted_wpos( struct st_translate *t,
 winSizeState);
  
 struct ureg_src winsize = ureg_DECL_constant( ureg, winHeightConst );
 -   struct ureg_dst wpos_temp = ureg_DECL_temporary( ureg );
 +   struct ureg_dst wpos_temp;
 struct ureg_src wpos_input = t-inputs[t-inputMapping[FRAG_ATTRIB_WPOS]];
  
 /* MOV wpos_temp, input[wpos]
  */
 -   ureg_MOV( ureg, wpos_temp, wpos_input );
 +   if (wpos_input.File == TGSI_FILE_TEMPORARY)
 +  wpos_temp = ureg_dst(wpos_input);
 +   else {
 +  wpos_temp = ureg_DECL_temporary( ureg );
 +  ureg_MOV( ureg, wpos_temp, wpos_input );
 +   }
  
 /* SUB wpos_temp.y, winsize_const, wpos_input
  */
 @@ -801,6 +824,7 @@ st_translate_mesa_program(
  * Declare input attributes.
  */
 if (procType == TGSI_PROCESSOR_FRAGMENT) {
 +  struct gl_fragment_program* fp = (struct gl_fragment_program*)program;
for (i = 0; i  numInputs; i++) {
   t-inputs[i] = ureg_DECL_fs_input(ureg,
 inputSemanticName[i],
 @@ -812,7 +836,51 @@ st_translate_mesa_program(
   /* Must do this after setting up t-inputs, and before
* emitting constant references, below:
*/
 - emit_inverted_wpos( t, program );
 + struct pipe_screen* pscreen = st_context(ctx)-pipe-screen;
 + int invert = 0;

This should be boolean invert = FALSE.


 + if (!fp-OriginUpperLeft) {
 +if (pscreen-get_param(pscreen, 
 PIPE_CAP_TGSI_FS_COORD_ORIGIN_LOWER_LEFT))
 +   ureg_property_fs_coord_origin(ureg, 
 TGSI_FS_COORD_ORIGIN_LOWER_LEFT);
 +else if (pscreen-get_param(pscreen, 
 PIPE_CAP_TGSI_FS_COORD_ORIGIN_UPPER_LEFT))
 +   invert = 1;
 +else
 +   assert(0);
 + }
 + else {
 +if (pscreen-get_param(pscreen, 
 PIPE_CAP_TGSI_FS_COORD_ORIGIN_UPPER_LEFT)) {
 +}
 +else if (pscreen-get_param(pscreen, 
 PIPE_CAP_TGSI_FS_COORD_ORIGIN_LOWER_LEFT)) {
 +   ureg_property_fs_coord_origin(ureg, 
 TGSI_FS_COORD_ORIGIN_LOWER_LEFT);
 +   invert = 1;
 +

Re: [Mesa3d-dev] Mesa (master): intel: Remove dead code from having to clip copyteximage source rect.

Eric Anholt wrote:
Module: Mesa
Branch: master
Commit: 2792baec343e5773ff51e93c1b6df8b63d3af4af
URL:
http://cgit.freedesktop.org/mesa/mesa/commit/?id=2792baec343e5773ff51e93c1b6df8b63d3af4af

Author: Eric Anholt e...@anholt.net
Date: Tue Jan 26 18:04:03 2010 -0800

intel: Remove dead code from having to clip copyteximage source rect.

mesa core does it now. If only it did so for other entrypoints.

There's actually a subtle reason why core Mesa doesn't do clipping for
all the glDrawPixels/CopyPixels/etc functions.

The GL spec says that pixel-transfer ops such as histograms and minmax
are applied before fragments are clipped/culled. Mesa allows the
driver functions to take a crack at applying any/all pixel transfer
operations (scale/bias, LUT, histogram, etc). If the driver can do
those things, great. Otherwise a sw fallback is used.

If we clipped glDrawPixels (for example) in core Mesa before doing
pixel transfer ops and before calling the driver function that would
upset the histogram/minmax outcome.

That said, this is a pretty obscure case and we've never had a driver
implement histograms. But that's why.

There are utility functions such as _mesa_clip_pixels() that drivers
can use to do clipping.

-Brian

--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 1/4] tgsi: add properties for fragment coord conventions (v2)

On Wed, 2010-01-27 at 08:49 -0800, Brian Paul wrote:
 Luca Barbieri wrote:
  Changes in v2:
  - Caps are added in a separate, subsequent patch
 
  This adds two TGSI fragment program properties that indicate the
  fragment coord conventions.
 
  The properties behave as described in the extension spec for
  GL_ARB_fragment_coord_conventions, but the default origin in
  upper left instead of lower left as in OpenGL.
 
  The syntax is:
  PROPERTY FS_COORD_ORIGIN [UPPER_LEFT|LOWER_LEFT]
  PROPERTY FS_COORD_PIXEL_CENTER [HALF_INTEGER|INTEGER]
 
  The names have been chosen for consistency with the GS properties
  and the OpenGL extension spec.
 
  The defaults are of course the previously assumed conventions:
  UPPER_LEFT and HALF_INTEGER.
 
 Sorry for the slow reply on this.
 
 It looks like you've solved the fragcoord problems in a clean/logical
 way.  I haven't seen any objections, so I think we can move forward
 with this.

Didn't Keith had objections on this, on another thread? Luca re-sent the
patch but I don't see the remark being addressed.

 Forwarded Message 
 From: Keith Whitwell kei...@vmware.com
 To: Luca Barbieri l...@luca-barbieri.com
 Cc: mesa3d-dev@lists.sourceforge.net
 mesa3d-dev@lists.sourceforge.net
 Subject: Re: [Mesa3d-dev] [PATCH 3/7] tgsi: add properties for
 fragment coord conventions
 Date: Tue, 26 Jan 2010 03:11:51 -0800
 
 Luca,
 
 I would have expected fragment coord conventions to be device state, not
 a part of the shader.
 
 It seems like these new flags are really peers (or replacements?) of the
 gl_rasterization_rules flag in pipe_rasterizer_state, and that the
 shaders should remain unchanged.
 
 Keith

Jose


--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 1/4] tgsi: add properties for fragment coord conventions (v2)

On Wed, 2010-01-27 at 09:09 -0800, José Fonseca wrote:
 On Wed, 2010-01-27 at 08:49 -0800, Brian Paul wrote:
  Luca Barbieri wrote:
   Changes in v2:
   - Caps are added in a separate, subsequent patch
  
   This adds two TGSI fragment program properties that indicate the
   fragment coord conventions.
  
   The properties behave as described in the extension spec for
   GL_ARB_fragment_coord_conventions, but the default origin in
   upper left instead of lower left as in OpenGL.
  
   The syntax is:
   PROPERTY FS_COORD_ORIGIN [UPPER_LEFT|LOWER_LEFT]
   PROPERTY FS_COORD_PIXEL_CENTER [HALF_INTEGER|INTEGER]
  
   The names have been chosen for consistency with the GS properties
   and the OpenGL extension spec.
  
   The defaults are of course the previously assumed conventions:
   UPPER_LEFT and HALF_INTEGER.
  
  Sorry for the slow reply on this.
  
  It looks like you've solved the fragcoord problems in a clean/logical
  way.  I haven't seen any objections, so I think we can move forward
  with this.
 
 Didn't Keith had objections on this, on another thread? Luca re-sent the
 patch but I don't see the remark being addressed.

I don't think my concerns are fatal to this approach, I just need to
understand the issues a bit better before signing off on it.

Keith


--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 1/4] tgsi: add properties for fragment coord conventions (v2)

2010-01-27 Thread Luca Barbieri

 Didn't Keith had objections on this, on another thread? Luca re-sent the
 patch but I don't see the remark being addressed.

  Forwarded Message 
 From: Keith Whitwell kei...@vmware.com
 To: Luca Barbieri l...@luca-barbieri.com
 Cc: mesa3d-dev@lists.sourceforge.net
 mesa3d-dev@lists.sourceforge.net
 Subject: Re: [Mesa3d-dev] [PATCH 3/7] tgsi: add properties for
 fragment coord conventions
 Date: Tue, 26 Jan 2010 03:11:51 -0800

 Luca,

 I would have expected fragment coord conventions to be device state, not
 a part of the shader.

 It seems like these new flags are really peers (or replacements?) of the
 gl_rasterization_rules flag in pipe_rasterizer_state, and that the
 shaders should remain unchanged.

I answered that concern, as reported below.
Any disagreements with the following analysis and conclusion that
making the flags part of the shader is appropriate?

--

This flags are different from gl_rasterization_rules, as they only
affect fragment.position as seen by the shader, while
gl_rasterization_rules affects the trigger point for rasterization
itself only. I quoted the answer to this I already gave at the end of
the message.

OpenGL makes the convention part of the shader.

Of course, we could design Gallium in a different way, but I don't see
any advantage that would justify the inconvenience.
Also, since the flags only affect the fragment shader (as discussed
below), it makes sense for it to be part of it.

--

gl_rasterization_rules affects the way fragments are rasterized, i.e.
the set of fragments which a primitive is mapped to.
Changing it is equivalent to adding/subtracting a subpixel offset to
the viewport (which seemingly depends on the primitive type).

The pixel center convention instead sets how the values look like in
the fragment shader.
Changing it is equivalent to adding/subtracting 0.5 to the
fragment.position in the fragment shader.

In other words, yes, if you set gl_rasterization_rules and the pixel
center in a mismatched way, fragment.position will not be the
coordinate of the rasterization center.

As another example, suppose you do a blit with the 3D engine using
fragment.position to sample from a texture rectangle with bilinear
filtering.
A wrong rasterization convention may cause 1 pixel black bars at the borders.
A wrong pixel center convention will cause a 2x2 blur filter to be
applied to the texture.

BTW, gl_rasterization_rules is ignored by almost all drivers

From the spec:

   The scope of this extension deals *only* with how the fragment
   coordinate XY location appears during programming fragment processing.
   Beyond the scope of this extension are coordinate conventions used
   for rasterization or transformation.

--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 3/4] st/mesa: Gallium support for ARB_fragment_coord_conventions (v3)

2010-01-27 Thread Luca Barbieri

On Wed, Jan 27, 2010 at 5:49 PM, Brian Paul bri...@vmware.com wrote:
 Luca Barbieri wrote:

 Changes in v3:
 - Use positive caps instead of negative ones

 Changes in v2:
 - Updated formatting

 The state tracker will use the TGSI convention properties if the hardware
 exposes the appropriate capability, and otherwise adjust WPOS itself.

 This will also fix some drivers that were previously broken due to their
 incorrect, inadvertent, use of conventions other than
 upper_left+half_integer.

 Minor comments below.


 ---
  src/mesa/state_tracker/st_extensions.c   |    1 +
  src/mesa/state_tracker/st_mesa_to_tgsi.c |   74
 -
  2 files changed, 72 insertions(+), 3 deletions(-)

 diff --git a/src/mesa/state_tracker/st_extensions.c
 b/src/mesa/state_tracker/st_extensions.c
 index 60732f3..982a1fb 100644
 --- a/src/mesa/state_tracker/st_extensions.c
 +++ b/src/mesa/state_tracker/st_extensions.c
 @@ -147,6 +147,7 @@ void st_init_extensions(struct st_context *st)
     * Extensions that are supported by all Gallium drivers:
     */
    ctx-Extensions.ARB_copy_buffer = GL_TRUE;
 +   ctx-Extensions.ARB_fragment_coord_conventions = GL_TRUE;

 We can't really expose this extension until we've updated GLSL to understand
 layout qualifiers.  And, unfortunately, that builds on GLSL 1.40 which we
 don't support yet.
I think we shoud do this by keeping the quoted line, but changing Mesa
so that ctx-Extensions.ARB_fragment_coord_conventions does not
actually cause the extension string to be returned by the user until
the GLSL part is implemented.

Otherwise, if we remove that line, the ARBfp parser will refuse to
recognize the new keywords.

BTW, I think the layout keyword can be added to the current GLSL, as
the extension spec says:

   RESOLVED: Yes. This has been recast to use layout qualifiers
   originally introduced in GLSL 1.40 and extended in GLSL 1.50.
   However note that it is the intent of this extension to stand
   separately from the GLSL 1.40/1.50 and it is desinged to be
   implementable against GLSL 1.10 or 1.20.


--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 1/4] tgsi: add properties for fragment coord conventions (v2)

Keith Whitwell wrote:
 On Wed, 2010-01-27 at 09:09 -0800, José Fonseca wrote:
 On Wed, 2010-01-27 at 08:49 -0800, Brian Paul wrote:
 Luca Barbieri wrote:
 Changes in v2:
 - Caps are added in a separate, subsequent patch

 This adds two TGSI fragment program properties that indicate the
 fragment coord conventions.

 The properties behave as described in the extension spec for
 GL_ARB_fragment_coord_conventions, but the default origin in
 upper left instead of lower left as in OpenGL.

 The syntax is:
 PROPERTY FS_COORD_ORIGIN [UPPER_LEFT|LOWER_LEFT]
 PROPERTY FS_COORD_PIXEL_CENTER [HALF_INTEGER|INTEGER]

 The names have been chosen for consistency with the GS properties
 and the OpenGL extension spec.

 The defaults are of course the previously assumed conventions:
 UPPER_LEFT and HALF_INTEGER.
 Sorry for the slow reply on this.

 It looks like you've solved the fragcoord problems in a clean/logical
 way.  I haven't seen any objections, so I think we can move forward
 with this.
 Didn't Keith had objections on this, on another thread? Luca re-sent the
 patch but I don't see the remark being addressed.
 
 I don't think my concerns are fatal to this approach, I just need to
 understand the issues a bit better before signing off on it.

I meant I didn't see objections to Luca's latest patch series.

After studying the issues for a while I think Luca's on the right 
track.  I'll try to summarize.


Re: coord origin upper/lower-left:

A GPU may natively implement lower-left origin or upper-left origin 
(or both).  With GL_ARB_fragment_coord_conventions, the user may want 
to use either of those origins.  By asking the driver what it supports 
we can avoid extra Y-inversion code in the shader.  Drivers don't have 
to do anything special other than tell the state tracker (via new cap 
bits) what it can do.


Re: pixel center integer:

Again, depending on what the GPU implements we may need to 
add/subtract a 0.5 bias to the fragment coord register in a fragment 
shader.  This is orthogonal to pipe_rasterizer_state:: 
gl_rasterization_rules.

With Luca's patch we query what the GPU can support and submit TGSI 
shaders which either tell the driver which convention to use, or 
submit shaders that implement the bias themselves.

The new TGSI shader properties are needed because the origin/center 
state can change per-shader (it's not per-context) and for the sake of 
drivers that can support both conventions.


I too was concerned whether this was all really needed but I think it is.

-Brian

--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 3/4] st/mesa: Gallium support for ARB_fragment_coord_conventions (v3)

Luca Barbieri wrote:
 On Wed, Jan 27, 2010 at 5:49 PM, Brian Paul bri...@vmware.com wrote:
 Luca Barbieri wrote:
 Changes in v3:
 - Use positive caps instead of negative ones

 Changes in v2:
 - Updated formatting

 The state tracker will use the TGSI convention properties if the hardware
 exposes the appropriate capability, and otherwise adjust WPOS itself.

 This will also fix some drivers that were previously broken due to their
 incorrect, inadvertent, use of conventions other than
 upper_left+half_integer.
 Minor comments below.


 ---
  src/mesa/state_tracker/st_extensions.c   |1 +
  src/mesa/state_tracker/st_mesa_to_tgsi.c |   74
 -
  2 files changed, 72 insertions(+), 3 deletions(-)

 diff --git a/src/mesa/state_tracker/st_extensions.c
 b/src/mesa/state_tracker/st_extensions.c
 index 60732f3..982a1fb 100644
 --- a/src/mesa/state_tracker/st_extensions.c
 +++ b/src/mesa/state_tracker/st_extensions.c
 @@ -147,6 +147,7 @@ void st_init_extensions(struct st_context *st)
 * Extensions that are supported by all Gallium drivers:
 */
ctx-Extensions.ARB_copy_buffer = GL_TRUE;
 +   ctx-Extensions.ARB_fragment_coord_conventions = GL_TRUE;
 We can't really expose this extension until we've updated GLSL to understand
 layout qualifiers.  And, unfortunately, that builds on GLSL 1.40 which we
 don't support yet.
 I think we shoud do this by keeping the quoted line, but changing Mesa
 so that ctx-Extensions.ARB_fragment_coord_conventions does not
 actually cause the extension string to be returned by the user until
 the GLSL part is implemented.

Hmmm, I'd really rather not special-case the extension code for this 
one thing.



 Otherwise, if we remove that line, the ARBfp parser will refuse to
 recognize the new keywords.
 
 BTW, I think the layout keyword can be added to the current GLSL, as
 the extension spec says:
 
RESOLVED: Yes. This has been recast to use layout qualifiers
originally introduced in GLSL 1.40 and extended in GLSL 1.50.
However note that it is the intent of this extension to stand
separately from the GLSL 1.40/1.50 and it is desinged to be
implementable against GLSL 1.10 or 1.20.

OK, I missed that part.

I guess we could just expose the extension and keep in mind that lack 
of GLSL support is a known issue.  I could try to implement it in GLSL 
as soon as I find some spare time...

-Brian

--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 1/4] tgsi: add properties for fragment coord conventions (v2)


 --
 
 This flags are different from gl_rasterization_rules, as they only
 affect fragment.position as seen by the shader, while
 gl_rasterization_rules affects the trigger point for rasterization
 itself only. I quoted the answer to this I already gave at the end of
 the message.
 
 OpenGL makes the convention part of the shader.
 
 Of course, we could design Gallium in a different way, but I don't see
 any advantage that would justify the inconvenience.
 Also, since the flags only affect the fragment shader (as discussed
 below), it makes sense for it to be part of it.

I think this is the most convincing point -- no other API than GL will
make this configurable, and GL puts it in the shader.

Unless implementing it this way runs us into major problems, I don't see
any justification for introducing an extra gap between GL and gallium
semantics, which means I'm basically OK with Luca's approach.

Keith


--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 3/4] st/mesa: Gallium support for ARB_fragment_coord_conventions (v3)

2010-01-27 Thread Luca Barbieri

 Hmmm, I'd really rather not special-case the extension code for this one
 thing.
Isn't it possible to accomplish this by commenting out the following
line from extensions.c:
+ { OFF, GL_ARB_fragment_coord_conventions,
F(ARB_fragment_coord_conventions) },

Then swrast and Gallium can set ARB_fragment_coord_conventions to
true, and, once the GLSL part is done, the line can be uncommented to
actually expose the extension.

--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 1/4] tgsi: add properties for fragment coord conventions (v2)

On Wed, 2010-01-27 at 09:24 -0800, Brian Paul wrote:
 Keith Whitwell wrote:
  On Wed, 2010-01-27 at 09:09 -0800, José Fonseca wrote:
  On Wed, 2010-01-27 at 08:49 -0800, Brian Paul wrote:
  Luca Barbieri wrote:
  Changes in v2:
  - Caps are added in a separate, subsequent patch
 
  This adds two TGSI fragment program properties that indicate the
  fragment coord conventions.
 
  The properties behave as described in the extension spec for
  GL_ARB_fragment_coord_conventions, but the default origin in
  upper left instead of lower left as in OpenGL.
 
  The syntax is:
  PROPERTY FS_COORD_ORIGIN [UPPER_LEFT|LOWER_LEFT]
  PROPERTY FS_COORD_PIXEL_CENTER [HALF_INTEGER|INTEGER]
 
  The names have been chosen for consistency with the GS properties
  and the OpenGL extension spec.
 
  The defaults are of course the previously assumed conventions:
  UPPER_LEFT and HALF_INTEGER.
  Sorry for the slow reply on this.
 
  It looks like you've solved the fragcoord problems in a clean/logical
  way.  I haven't seen any objections, so I think we can move forward
  with this.
  Didn't Keith had objections on this, on another thread? Luca re-sent the
  patch but I don't see the remark being addressed.
  
  I don't think my concerns are fatal to this approach, I just need to
  understand the issues a bit better before signing off on it.
 
 I meant I didn't see objections to Luca's latest patch series.
 
 After studying the issues for a while I think Luca's on the right 
 track.  I'll try to summarize.
 
 
 Re: coord origin upper/lower-left:
 
 A GPU may natively implement lower-left origin or upper-left origin 
 (or both).  With GL_ARB_fragment_coord_conventions, the user may want 
 to use either of those origins.  By asking the driver what it supports 
 we can avoid extra Y-inversion code in the shader.  Drivers don't have 
 to do anything special other than tell the state tracker (via new cap 
 bits) what it can do.
 
 
 Re: pixel center integer:
 
 Again, depending on what the GPU implements we may need to 
 add/subtract a 0.5 bias to the fragment coord register in a fragment 
 shader.  This is orthogonal to pipe_rasterizer_state:: 
 gl_rasterization_rules.
 
 With Luca's patch we query what the GPU can support and submit TGSI 
 shaders which either tell the driver which convention to use, or 
 submit shaders that implement the bias themselves.
 
 The new TGSI shader properties are needed because the origin/center 
 state can change per-shader (it's not per-context) and for the sake of 
 drivers that can support both conventions.
 
 
 I too was concerned whether this was all really needed but I think it is.

It appears everybody was in agreement after all.

I really don't have an opinion on this, as I don't understand the
different rules. From what I could gather it seems that even in the most
limited hardware it is always possible to expose this new functionality
either by tweaking shaders or flushing contexts before switching
hardware global device settings.

I'm a bit confused what will
pipe_rasterizer_state::gl_rasterization_rules mean at the end of the
day. Is it still necessary?

Jose


--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 1/4] tgsi: add properties for fragment coord conventions (v2)

José Fonseca wrote:
 On Wed, 2010-01-27 at 09:24 -0800, Brian Paul wrote:
 Keith Whitwell wrote:
 On Wed, 2010-01-27 at 09:09 -0800, José Fonseca wrote:
 On Wed, 2010-01-27 at 08:49 -0800, Brian Paul wrote:
 Luca Barbieri wrote:
 Changes in v2:
 - Caps are added in a separate, subsequent patch

 This adds two TGSI fragment program properties that indicate the
 fragment coord conventions.

 The properties behave as described in the extension spec for
 GL_ARB_fragment_coord_conventions, but the default origin in
 upper left instead of lower left as in OpenGL.

 The syntax is:
 PROPERTY FS_COORD_ORIGIN [UPPER_LEFT|LOWER_LEFT]
 PROPERTY FS_COORD_PIXEL_CENTER [HALF_INTEGER|INTEGER]

 The names have been chosen for consistency with the GS properties
 and the OpenGL extension spec.

 The defaults are of course the previously assumed conventions:
 UPPER_LEFT and HALF_INTEGER.
 Sorry for the slow reply on this.

 It looks like you've solved the fragcoord problems in a clean/logical
 way.  I haven't seen any objections, so I think we can move forward
 with this.
 Didn't Keith had objections on this, on another thread? Luca re-sent the
 patch but I don't see the remark being addressed.
 I don't think my concerns are fatal to this approach, I just need to
 understand the issues a bit better before signing off on it.
 I meant I didn't see objections to Luca's latest patch series.

 After studying the issues for a while I think Luca's on the right 
 track.  I'll try to summarize.


 Re: coord origin upper/lower-left:

 A GPU may natively implement lower-left origin or upper-left origin 
 (or both).  With GL_ARB_fragment_coord_conventions, the user may want 
 to use either of those origins.  By asking the driver what it supports 
 we can avoid extra Y-inversion code in the shader.  Drivers don't have 
 to do anything special other than tell the state tracker (via new cap 
 bits) what it can do.


 Re: pixel center integer:

 Again, depending on what the GPU implements we may need to 
 add/subtract a 0.5 bias to the fragment coord register in a fragment 
 shader.  This is orthogonal to pipe_rasterizer_state:: 
 gl_rasterization_rules.

 With Luca's patch we query what the GPU can support and submit TGSI 
 shaders which either tell the driver which convention to use, or 
 submit shaders that implement the bias themselves.

 The new TGSI shader properties are needed because the origin/center 
 state can change per-shader (it's not per-context) and for the sake of 
 drivers that can support both conventions.


 I too was concerned whether this was all really needed but I think it is.
 
 It appears everybody was in agreement after all.
 
 I really don't have an opinion on this, as I don't understand the
 different rules. From what I could gather it seems that even in the most
 limited hardware it is always possible to expose this new functionality
 either by tweaking shaders or flushing contexts before switching
 hardware global device settings.

Right.  We ask the driver what it can do and the state tracker emits 
extra instructions to invert/bias the fragcoord as needed.


 I'm a bit confused what will
 pipe_rasterizer_state::gl_rasterization_rules mean at the end of the
 day. Is it still necessary?

Yes, it's still needed.  gl_rasterization_rules controls which pixels 
are hits/written when rasterizing a triangle regardless of the shader.

The pixel_center_integer controls what value the fragment shader gets 
when it reads gl_FragCoord (either x.0 or x.5).

-Brian


--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 3/4] st/mesa: Gallium support for ARB_fragment_coord_conventions (v3)

Luca Barbieri wrote:
 Hmmm, I'd really rather not special-case the extension code for this one
 thing.
 Isn't it possible to accomplish this by commenting out the following
 line from extensions.c:
 + { OFF, GL_ARB_fragment_coord_conventions,
 F(ARB_fragment_coord_conventions) },
 
 Then swrast and Gallium can set ARB_fragment_coord_conventions to
 true, and, once the GLSL part is done, the line can be uncommented to
 actually expose the extension.

Yes, that should work (but haven't tried).  Thanks.

-Brian

--
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
___
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Re: [Mesa3d-dev] [PATCH 1/4] tgsi: add properties for fragment coord conventions (v2)