Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-23 Thread Jose Fonseca
Ok. I've moved the docs to src/gallium/docs/source/cso/rasterizer.rst , and 
renamed `lower_left_origin` to `bottom_edge_rule`.

This is how it looks like:

  
http://people.freedesktop.org/~jrfonseca/gl_rasterization_rules/cso/rasterizer.html#other-members

Jose

- Original Message -
 Yeah, I was confused when reading the comment and the diagrams. It
 probably shouldn't mention the screen origin at all and instead should
 say which one of the top and bottom edges is inclusive and which one
 is exclusive when determining pixel ownership.
 
 Anyway, thank you for fixing this. I would have probably never knew
 how to fix the triangle rasterization tests if you didn't bring this
 up.
 
 Marek
 
 On Sun, Apr 21, 2013 at 7:54 PM, Jose Fonseca jfons...@vmware.com wrote:
  - Original Message -
  Some suggestions for the name:
 
  lower_left_edge_rule
  lower_left_rasterization_edge_rule
  gl_edge_rule
  gl_rasterization_edge_rule
 
  In this case, the name is not as important as the documentation which
  defines the behavior of the state.
 
  On that note, I thought that James' diagrams were pretty good.  Maybe the
  axis is misleading.
 
 
  +   /**
  +* Triangle rasterization always uses a 'top,left' rule for pixel
  ownership,
  +* this just alters what we consider to be the top edge for that test.
  +*
  +* When true, screen coordinates origin is considered to be at
  bottom-left
  +* (e.g., OpenGL drawables):
  +*
  +*  y ^
  +*|
  +*|  +=+ - top edge
  +*|  | |
  +*|  | |
  +*|  | |
  +*|  +-+
  +*|
  +*  0 +-
  +*0x
  +*
  +*  When false, screen coordinates origin is considered to be at
  top-left
  +*  (e.g., OpenGL FBOs, D3D):
  +*
  +*0x
  +*  0 +-
  +*|
  +*|  +=+ - top edge
  +*|  | |
  +*|  | |
  +*|  | |
  +*|  +-+
  +*|
  +*  y V
   *
  -* Triangle rasterization always uses a 'top,left' rule for pixel
  -* ownership, this just alters which point we consider the pixel
  -* center for that test.
  +* See also:
  +* -
  http://www.opengl.org/registry/specs/ARB/fragment_coord_conventions.txt
  +* -
  http://msdn.microsoft.com/en-us/library/windows/desktop/cc627092.aspx
  +* -
  http://msdn.microsoft.com/en-us/library/windows/desktop/bb147314.aspx
   */
  -   unsigned gl_rasterization_rules:1;
  +   unsigned lower_left_origin:1;
 
  /**
   * When true, rasterization is disabled and no pixels are written.
 
  Jose
 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-23 Thread Jose Fonseca
My follow on changes can be seen at 
http://cgit.freedesktop.org/~jrfonseca/mesa/log/?h=gl-rasterization-rules

Jose

- Original Message -
 Ok. I've moved the docs to src/gallium/docs/source/cso/rasterizer.rst , and
 renamed `lower_left_origin` to `bottom_edge_rule`.
 
 This is how it looks like:
 
   
 http://people.freedesktop.org/~jrfonseca/gl_rasterization_rules/cso/rasterizer.html#other-members
 
 Jose
 
 - Original Message -
  Yeah, I was confused when reading the comment and the diagrams. It
  probably shouldn't mention the screen origin at all and instead should
  say which one of the top and bottom edges is inclusive and which one
  is exclusive when determining pixel ownership.
  
  Anyway, thank you for fixing this. I would have probably never knew
  how to fix the triangle rasterization tests if you didn't bring this
  up.
  
  Marek
  
  On Sun, Apr 21, 2013 at 7:54 PM, Jose Fonseca jfons...@vmware.com wrote:
   - Original Message -
   Some suggestions for the name:
  
   lower_left_edge_rule
   lower_left_rasterization_edge_rule
   gl_edge_rule
   gl_rasterization_edge_rule
  
   In this case, the name is not as important as the documentation which
   defines the behavior of the state.
  
   On that note, I thought that James' diagrams were pretty good.  Maybe the
   axis is misleading.
  
  
   +   /**
   +* Triangle rasterization always uses a 'top,left' rule for pixel
   ownership,
   +* this just alters what we consider to be the top edge for that
   test.
   +*
   +* When true, screen coordinates origin is considered to be at
   bottom-left
   +* (e.g., OpenGL drawables):
   +*
   +*  y ^
   +*|
   +*|  +=+ - top edge
   +*|  | |
   +*|  | |
   +*|  | |
   +*|  +-+
   +*|
   +*  0 +-
   +*0x
   +*
   +*  When false, screen coordinates origin is considered to be at
   top-left
   +*  (e.g., OpenGL FBOs, D3D):
   +*
   +*0x
   +*  0 +-
   +*|
   +*|  +=+ - top edge
   +*|  | |
   +*|  | |
   +*|  | |
   +*|  +-+
   +*|
   +*  y V
*
   -* Triangle rasterization always uses a 'top,left' rule for pixel
   -* ownership, this just alters which point we consider the pixel
   -* center for that test.
   +* See also:
   +* -
   http://www.opengl.org/registry/specs/ARB/fragment_coord_conventions.txt
   +* -
   http://msdn.microsoft.com/en-us/library/windows/desktop/cc627092.aspx
   +* -
   http://msdn.microsoft.com/en-us/library/windows/desktop/bb147314.aspx
*/
   -   unsigned gl_rasterization_rules:1;
   +   unsigned lower_left_origin:1;
  
   /**
* When true, rasterization is disabled and no pixels are written.
  
   Jose
  
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-23 Thread Christoph Bumiller
On 23.04.2013 18:28, Jose Fonseca wrote:
 Ok. I've moved the docs to src/gallium/docs/source/cso/rasterizer.rst , and 
 renamed `lower_left_origin` to `bottom_edge_rule`.
 

Well, that doesn't work for NV, but it's at least less invasive for
radeon since you don't have to change the state tracker (using
lower_left_origin instead of flipping viewport + bottom_edge_rule) to
get things working correctly.

/me breathes, tries not to care, too much stuff on my plate already

 This is how it looks like:
 
   
 http://people.freedesktop.org/~jrfonseca/gl_rasterization_rules/cso/rasterizer.html#other-members
 
 Jose
 
 - Original Message -
 Yeah, I was confused when reading the comment and the diagrams. It
 probably shouldn't mention the screen origin at all and instead should
 say which one of the top and bottom edges is inclusive and which one
 is exclusive when determining pixel ownership.

 Anyway, thank you for fixing this. I would have probably never knew
 how to fix the triangle rasterization tests if you didn't bring this
 up.

 Marek

 On Sun, Apr 21, 2013 at 7:54 PM, Jose Fonseca jfons...@vmware.com wrote:
 - Original Message -
 Some suggestions for the name:

 lower_left_edge_rule
 lower_left_rasterization_edge_rule
 gl_edge_rule
 gl_rasterization_edge_rule

 In this case, the name is not as important as the documentation which
 defines the behavior of the state.

 On that note, I thought that James' diagrams were pretty good.  Maybe the
 axis is misleading.


 +   /**
 +* Triangle rasterization always uses a 'top,left' rule for pixel
 ownership,
 +* this just alters what we consider to be the top edge for that test.
 +*
 +* When true, screen coordinates origin is considered to be at
 bottom-left
 +* (e.g., OpenGL drawables):
 +*
 +*  y ^
 +*|
 +*|  +=+ - top edge
 +*|  | |
 +*|  | |
 +*|  | |
 +*|  +-+
 +*|
 +*  0 +-
 +*0x
 +*
 +*  When false, screen coordinates origin is considered to be at
 top-left
 +*  (e.g., OpenGL FBOs, D3D):
 +*
 +*0x
 +*  0 +-
 +*|
 +*|  +=+ - top edge
 +*|  | |
 +*|  | |
 +*|  | |
 +*|  +-+
 +*|
 +*  y V
  *
 -* Triangle rasterization always uses a 'top,left' rule for pixel
 -* ownership, this just alters which point we consider the pixel
 -* center for that test.
 +* See also:
 +* -
 http://www.opengl.org/registry/specs/ARB/fragment_coord_conventions.txt
 +* -
 http://msdn.microsoft.com/en-us/library/windows/desktop/cc627092.aspx
 +* -
 http://msdn.microsoft.com/en-us/library/windows/desktop/bb147314.aspx
  */
 -   unsigned gl_rasterization_rules:1;
 +   unsigned lower_left_origin:1;

 /**
  * When true, rasterization is disabled and no pixels are written.

 Jose


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


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-23 Thread Marek Olšák
This looks good to me. bottom_edge_rule should probably also be set
conditionally for Bitmap, Clear, and DrawPixels like this:

bottom_edge_rule = st_fb_orientation(ctx-DrawBuffer) == Y_0_TOP;

Marek

On Tue, Apr 23, 2013 at 6:39 PM, Jose Fonseca jfons...@vmware.com wrote:
 My follow on changes can be seen at 
 http://cgit.freedesktop.org/~jrfonseca/mesa/log/?h=gl-rasterization-rules

 Jose

 - Original Message -
 Ok. I've moved the docs to src/gallium/docs/source/cso/rasterizer.rst , and
 renamed `lower_left_origin` to `bottom_edge_rule`.

 This is how it looks like:

   
 http://people.freedesktop.org/~jrfonseca/gl_rasterization_rules/cso/rasterizer.html#other-members

 Jose

 - Original Message -
  Yeah, I was confused when reading the comment and the diagrams. It
  probably shouldn't mention the screen origin at all and instead should
  say which one of the top and bottom edges is inclusive and which one
  is exclusive when determining pixel ownership.
 
  Anyway, thank you for fixing this. I would have probably never knew
  how to fix the triangle rasterization tests if you didn't bring this
  up.
 
  Marek
 
  On Sun, Apr 21, 2013 at 7:54 PM, Jose Fonseca jfons...@vmware.com wrote:
   - Original Message -
   Some suggestions for the name:
  
   lower_left_edge_rule
   lower_left_rasterization_edge_rule
   gl_edge_rule
   gl_rasterization_edge_rule
  
   In this case, the name is not as important as the documentation which
   defines the behavior of the state.
  
   On that note, I thought that James' diagrams were pretty good.  Maybe the
   axis is misleading.
  
  
   +   /**
   +* Triangle rasterization always uses a 'top,left' rule for pixel
   ownership,
   +* this just alters what we consider to be the top edge for that
   test.
   +*
   +* When true, screen coordinates origin is considered to be at
   bottom-left
   +* (e.g., OpenGL drawables):
   +*
   +*  y ^
   +*|
   +*|  +=+ - top edge
   +*|  | |
   +*|  | |
   +*|  | |
   +*|  +-+
   +*|
   +*  0 +-
   +*0x
   +*
   +*  When false, screen coordinates origin is considered to be at
   top-left
   +*  (e.g., OpenGL FBOs, D3D):
   +*
   +*0x
   +*  0 +-
   +*|
   +*|  +=+ - top edge
   +*|  | |
   +*|  | |
   +*|  | |
   +*|  +-+
   +*|
   +*  y V
*
   -* Triangle rasterization always uses a 'top,left' rule for pixel
   -* ownership, this just alters which point we consider the pixel
   -* center for that test.
   +* See also:
   +* -
   http://www.opengl.org/registry/specs/ARB/fragment_coord_conventions.txt
   +* -
   http://msdn.microsoft.com/en-us/library/windows/desktop/cc627092.aspx
   +* -
   http://msdn.microsoft.com/en-us/library/windows/desktop/bb147314.aspx
*/
   -   unsigned gl_rasterization_rules:1;
   +   unsigned lower_left_origin:1;
  
   /**
* When true, rasterization is disabled and no pixels are written.
  
   Jose
 
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-23 Thread Jose Fonseca
- Original Message -
 This looks good to me. 

Thanks.

 bottom_edge_rule should probably also be set
 conditionally for Bitmap, Clear, and DrawPixels like this:
 
 bottom_edge_rule = st_fb_orientation(ctx-DrawBuffer) == Y_0_TOP;

I will do that in a follow on change.

Jose

 Marek
 
 On Tue, Apr 23, 2013 at 6:39 PM, Jose Fonseca jfons...@vmware.com wrote:
  My follow on changes can be seen at
  http://cgit.freedesktop.org/~jrfonseca/mesa/log/?h=gl-rasterization-rules
 
  Jose
 
  - Original Message -
  Ok. I've moved the docs to src/gallium/docs/source/cso/rasterizer.rst ,
  and
  renamed `lower_left_origin` to `bottom_edge_rule`.
 
  This is how it looks like:
 

  http://people.freedesktop.org/~jrfonseca/gl_rasterization_rules/cso/rasterizer.html#other-members
 
  Jose
 
  - Original Message -
   Yeah, I was confused when reading the comment and the diagrams. It
   probably shouldn't mention the screen origin at all and instead should
   say which one of the top and bottom edges is inclusive and which one
   is exclusive when determining pixel ownership.
  
   Anyway, thank you for fixing this. I would have probably never knew
   how to fix the triangle rasterization tests if you didn't bring this
   up.
  
   Marek
  
   On Sun, Apr 21, 2013 at 7:54 PM, Jose Fonseca jfons...@vmware.com
   wrote:
- Original Message -
Some suggestions for the name:
   
lower_left_edge_rule
lower_left_rasterization_edge_rule
gl_edge_rule
gl_rasterization_edge_rule
   
In this case, the name is not as important as the documentation which
defines the behavior of the state.
   
On that note, I thought that James' diagrams were pretty good.  Maybe
the
axis is misleading.
   
   
+   /**
+* Triangle rasterization always uses a 'top,left' rule for pixel
ownership,
+* this just alters what we consider to be the top edge for that
test.
+*
+* When true, screen coordinates origin is considered to be at
bottom-left
+* (e.g., OpenGL drawables):
+*
+*  y ^
+*|
+*|  +=+ - top edge
+*|  | |
+*|  | |
+*|  | |
+*|  +-+
+*|
+*  0 +-
+*0x
+*
+*  When false, screen coordinates origin is considered to be at
top-left
+*  (e.g., OpenGL FBOs, D3D):
+*
+*0x
+*  0 +-
+*|
+*|  +=+ - top edge
+*|  | |
+*|  | |
+*|  | |
+*|  +-+
+*|
+*  y V
 *
-* Triangle rasterization always uses a 'top,left' rule for pixel
-* ownership, this just alters which point we consider the pixel
-* center for that test.
+* See also:
+* -
http://www.opengl.org/registry/specs/ARB/fragment_coord_conventions.txt
+* -
http://msdn.microsoft.com/en-us/library/windows/desktop/cc627092.aspx
+* -
http://msdn.microsoft.com/en-us/library/windows/desktop/bb147314.aspx
 */
-   unsigned gl_rasterization_rules:1;
+   unsigned lower_left_origin:1;
   
/**
 * When true, rasterization is disabled and no pixels are written.
   
Jose
  
  ___
  mesa-dev mailing list
  mesa-dev@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/mesa-dev
 
 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-22 Thread Christoph Bumiller
On 21.04.2013 14:35, Jose Fonseca wrote:
 
 
 - Original Message -
 On 21.04.2013 13:18, Jose Fonseca wrote:

 I think that drivers can just report all 4 CAPs as supported and do the
 adjustment in the shader themselves (no need for recompilation, just use
 uniforms, the st already does it like that), provided that the state
 tracker actually uses the rasterizer origin bit instead of changing the
 viewport and applies no transformation to the fragment coordinate
 whatsoever.
 
 I'm not sure how much that simplifies in the end. If the drivers need to 
 resort to uniforms to deal with all combinations, then how will making the 
 gl_Fragcoord/viewport transformation depend on lower_left_origin simplify 
 things? 
 
 Is it really true that for all hardware gl_FragCoord will depend on the 
 lower_left_origin rasterizer state?
 

I don't know about all hardware. R600 doesn't have that origin switch,
but the half-integers switch might have an effect.

My suggestion about letting the driver modify the coordinate was to
avoid having a dependency in the gallium interface between the shader
setting, or worse, yet another cap about whether it exists.

The only (small) issue is, if a driver does handle the origin switch and
compensates for the effect on FragCoord, and the state tracker decides
to not use that switch and just flips the viewport, it has to do its own
transformation on FragCoord, we get to do 2 transformations.

 Finally, I think this is precisely what Marek was concerned; so to allow 
 existing drivers to opt out from having to deal with this, we'll need a cap.

Which is, I guess, why we have to add both versions depending on a CAP
once again, i.e. for some drivers the origin switch in the rasterizer is
used (nouveau at least; this should affect the edge rule; I think I
looked for an independent switch way back and didn't find one) and for
other drivers the viewport is flipped in combination with changing a
separate edge rule rasterizer state.
Maybe some drivers even support both (independent change of edge rule
and origin) ...

 
 
 That said, I don't oppose any of this if it make HW driver implementer lives 
 easier.
 
 But how seriously/quickly are you and other hardware drivers maintainers 
 actually aiming at implementing this? I don't wanna go through all that 
 trouble if nobody will care.
 

Well, there's not much code (in terms of lines) to write on the driver
side, but code that uglifies things always takes a bit longer to become
comfortable with ...

 
 Either way, I think that this patch series already is a good improvement over 
 the ugly one-bit-fit-all-needs gl_rasterization_rules state, and should 
 cause no regressions whatsoever.  I'd like to tackle the entanglement of 
 lower_left_origin with other bits of state in a follow-on gallium change 
 after there is a clearer understanding/consensus if/how will HW implement 
 this.
 
 Jose
 

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


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Jose Fonseca
- Original Message -
 Do we really need the lower_left_origin state? I think I can't
 implement it for radeon and it's the kind of stuff that should be
 taken care of by the state tracker anyway. 

My understanding is that hardware had switches for this sort of thing. It's 
really hard to provide fully-conforming rasterization for opengl, dx9  dx10 
without it.

If your hardware allows to put a negative pitch on rendertargets, then that 
should also do it.

If you know what is the hardware's sub-pixel rasterization resolution, then 
adding a vertical bias equal to that amount, depending on this state, would 
give a very close approximation. (This would get the top/bottom edges right, at 
expense of small inaccuracies on non-horizontal edges)

 Isn't it sufficient to just
 set a viewport which is upside down, like we do now?

I'm not aware of rasterization top-left rule being affected by the viewport 
flipping.

Do both

 ./bin/triangle-rasterization -auto
 ./bin/triangle-rasterization -use_fbo -auto

currently work for you?


If drivers don't provide this state, the only way to workaround it I know would 
be to store textures (or drawables?) up-side down, and flip them on 
gl(Get)TexImage  friends.  This would be like using a cannon to shoot a fly (a 
lot of work and a lot of overheads for a small correctness detail).  I think 
the drivers are better equipped to handle this.

And you always have the option of merely ignoring this state.  Top-left rule 
correct rasterization has, after all, been ignored till date, and nobody cared.


For the record, my motivation here is simple: llvmpipe gets the right behavior 
on GL drawables, and fails on GL FBOs  D3D 9/10. I want to get the right 
behavior on D3D 9/10 without causing regressions on GL drawables.

BTW, I'd imagine that if hardware rasterizer behavior is hardcoded to anything, 
it would be to D3D 9/10 behavior. That is, they would get GL FBO right, but 
drawables wrong.


Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Jose Fonseca


- Original Message -
 - Original Message -
  Do we really need the lower_left_origin state? I think I can't
  implement it for radeon and it's the kind of stuff that should be
  taken care of by the state tracker anyway.
 
 My understanding is that hardware had switches for this sort of thing. It's
 really hard to provide fully-conforming rasterization for opengl, dx9  dx10
 without it.
 
 If your hardware allows to put a negative pitch on rendertargets, then that
 should also do it.
 
 If you know what is the hardware's sub-pixel rasterization resolution, then
 adding a vertical bias equal to that amount, depending on this state, would
 give a very close approximation. (This would get the top/bottom edges right,
 at expense of small inaccuracies on non-horizontal edges)
 
  Isn't it sufficient to just
  set a viewport which is upside down, like we do now?
 
 I'm not aware of rasterization top-left rule being affected by the viewport
 flipping.
 
 Do both
 
  ./bin/triangle-rasterization -auto
  ./bin/triangle-rasterization -use_fbo -auto
 
 currently work for you?

Forgot to say -- these are piglit tests.

Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Christoph Bumiller
On 21.04.2013 09:36, Jose Fonseca wrote:
 - Original Message -
 Do we really need the lower_left_origin state? I think I can't
 implement it for radeon and it's the kind of stuff that should be
 taken care of by the state tracker anyway. 
 My understanding is that hardware had switches for this sort of thing. It's 
 really hard to provide fully-conforming rasterization for opengl, dx9  dx10 
 without it.

 If your hardware allows to put a negative pitch on rendertargets, then that 
 should also do it.

I have a switch for the upside down thing, but maybe it could be
framebuffer state instead of rasterizer state (since it's going to
either not change (D3D) or only change with the famebuffer, and I have
to set WINDOW_OFFSET_Y to 0 / fb height depending on the setting of Y
direction (the latter won't work with MRTs, but that's the non-FBO case
anyway)) ?

R600 seems to have PA_SU_VTX_CNTL.PIX_CENTER but no state to change the
window origin / direction ... and I'd rather not have to bother with it
myself either.

Also, note that this state and the pixel center one might (or maybe I
should say will) affect the values of hardware's gl_FragCoord and hence
PIPE_CAP_TGSI_FS_COORD_ORIGIN/PIXEL_CENTER*, i.e. the shader
transformation of that input must be adjusted according to this state.
I'd probably be OK with making this the driver's task.

 If you know what is the hardware's sub-pixel rasterization resolution, then 
 adding a vertical bias equal to that amount, depending on this state, would 
 give a very close approximation. (This would get the top/bottom edges right, 
 at expense of small inaccuracies on non-horizontal edges)

 Isn't it sufficient to just
 set a viewport which is upside down, like we do now?
 I'm not aware of rasterization top-left rule being affected by the viewport 
 flipping.

 Do both

  ./bin/triangle-rasterization -auto
  ./bin/triangle-rasterization -use_fbo -auto

 currently work for you?


 If drivers don't provide this state, the only way to workaround it I know 
 would be to store textures (or drawables?) up-side down, and flip them on 
 gl(Get)TexImage  friends.  This would be like using a cannon to shoot a fly 
 (a lot of work and a lot of overheads for a small correctness detail).  I 
 think the drivers are better equipped to handle this.

 And you always have the option of merely ignoring this state.  Top-left rule 
 correct rasterization has, after all, been ignored till date, and nobody 
 cared.


 For the record, my motivation here is simple: llvmpipe gets the right 
 behavior on GL drawables, and fails on GL FBOs  D3D 9/10. I want to get the 
 right behavior on D3D 9/10 without causing regressions on GL drawables.

 BTW, I'd imagine that if hardware rasterizer behavior is hardcoded to 
 anything, it would be to D3D 9/10 behavior. That is, they would get GL FBO 
 right, but drawables wrong.


 Jose
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Dave Airlie
On Sun, Apr 21, 2013 at 5:36 PM, Jose Fonseca jfons...@vmware.com wrote:
 - Original Message -
 Do we really need the lower_left_origin state? I think I can't
 implement it for radeon and it's the kind of stuff that should be
 taken care of by the state tracker anyway.

 My understanding is that hardware had switches for this sort of thing. It's 
 really hard to provide fully-conforming rasterization for opengl, dx9  dx10 
 without it.

 If your hardware allows to put a negative pitch on rendertargets, then that 
 should also do it.

 If you know what is the hardware's sub-pixel rasterization resolution, then 
 adding a vertical bias equal to that amount, depending on this state, would 
 give a very close approximation. (This would get the top/bottom edges right, 
 at expense of small inaccuracies on non-horizontal edges)

 Isn't it sufficient to just
 set a viewport which is upside down, like we do now?

 I'm not aware of rasterization top-left rule being affected by the viewport 
 flipping.

 Do both

  ./bin/triangle-rasterization -auto
  ./bin/triangle-rasterization -use_fbo -auto

 currently work for you?


just FYI, on my evergreen, the first fails the second passes, maybe
someone could try on fglrx, I'd be sorta willing to guess AMD hw just
does DX10 :)

and I think I've heard some complaints about our rendering offseting
being wrong somewhere in the past on r600.

Dave.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Christoph Bumiller
On 21.04.2013 12:34, Dave Airlie wrote:
 On Sun, Apr 21, 2013 at 5:36 PM, Jose Fonseca jfons...@vmware.com wrote:
 - Original Message -
 Do we really need the lower_left_origin state? I think I can't
 implement it for radeon and it's the kind of stuff that should be
 taken care of by the state tracker anyway.
 My understanding is that hardware had switches for this sort of thing. It's 
 really hard to provide fully-conforming rasterization for opengl, dx9  dx10 
 without it.

 If your hardware allows to put a negative pitch on rendertargets, then that 
 should also do it.

 If you know what is the hardware's sub-pixel rasterization resolution, then 
 adding a vertical bias equal to that amount, depending on this state, would 
 give a very close approximation. (This would get the top/bottom edges right, 
 at expense of small inaccuracies on non-horizontal edges)

 Isn't it sufficient to just
 set a viewport which is upside down, like we do now?
 I'm not aware of rasterization top-left rule being affected by the viewport 
 flipping.

 Do both

  ./bin/triangle-rasterization -auto
  ./bin/triangle-rasterization -use_fbo -auto

 currently work for you?

 just FYI, on my evergreen, the first fails the second passes, maybe
 someone could try on fglrx, I'd be sorta willing to guess AMD hw just
 does DX10 :)

 and I think I've heard some complaints about our rendering offseting
 being wrong somewhere in the past on r600.

Same on nouveau. On NV blob it's the other way around, it fails for
-use_fbo. So clearly, both can work.

 Dave.
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Marek Olšák
Same as Dave. The first one fails, the second one (-use_fbo) passes.

I just hope lower_left_origin doesn't affect the Y axis of the
viewport, scissor and gl_FragCoord. If it does, a PIPE_CAP would be
useful to keep the current behavior.

Marek

On Sun, Apr 21, 2013 at 9:36 AM, Jose Fonseca jfons...@vmware.com wrote:
 - Original Message -
 Do we really need the lower_left_origin state? I think I can't
 implement it for radeon and it's the kind of stuff that should be
 taken care of by the state tracker anyway.

 My understanding is that hardware had switches for this sort of thing. It's 
 really hard to provide fully-conforming rasterization for opengl, dx9  dx10 
 without it.

 If your hardware allows to put a negative pitch on rendertargets, then that 
 should also do it.

 If you know what is the hardware's sub-pixel rasterization resolution, then 
 adding a vertical bias equal to that amount, depending on this state, would 
 give a very close approximation. (This would get the top/bottom edges right, 
 at expense of small inaccuracies on non-horizontal edges)

 Isn't it sufficient to just
 set a viewport which is upside down, like we do now?

 I'm not aware of rasterization top-left rule being affected by the viewport 
 flipping.

 Do both

  ./bin/triangle-rasterization -auto
  ./bin/triangle-rasterization -use_fbo -auto

 currently work for you?


 If drivers don't provide this state, the only way to workaround it I know 
 would be to store textures (or drawables?) up-side down, and flip them on 
 gl(Get)TexImage  friends.  This would be like using a cannon to shoot a fly 
 (a lot of work and a lot of overheads for a small correctness detail).  I 
 think the drivers are better equipped to handle this.

 And you always have the option of merely ignoring this state.  Top-left rule 
 correct rasterization has, after all, been ignored till date, and nobody 
 cared.


 For the record, my motivation here is simple: llvmpipe gets the right 
 behavior on GL drawables, and fails on GL FBOs  D3D 9/10. I want to get the 
 right behavior on D3D 9/10 without causing regressions on GL drawables.

 BTW, I'd imagine that if hardware rasterizer behavior is hardcoded to 
 anything, it would be to D3D 9/10 behavior. That is, they would get GL FBO 
 right, but drawables wrong.


 Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Jose Fonseca


- Original Message -
 On 21.04.2013 09:36, Jose Fonseca wrote:
  - Original Message -
  Do we really need the lower_left_origin state? I think I can't
  implement it for radeon and it's the kind of stuff that should be
  taken care of by the state tracker anyway.
  My understanding is that hardware had switches for this sort of thing. It's
  really hard to provide fully-conforming rasterization for opengl, dx9 
  dx10 without it.
 
  If your hardware allows to put a negative pitch on rendertargets, then that
  should also do it.
 
 I have a switch for the upside down thing, but maybe it could be
 framebuffer state instead of rasterizer state (since it's going to
 either not change (D3D) 

You're right, they should never change at higher frequency than per-framebuffer.

But due to auxiliary modules like u_blit, u_blitter, u_gen_mipmap, this state 
will eventually change even for D3D state trackers.  (This is however fixable, 
if there are performance implications switching this state, we could enhance 
these helper modules so that they switch it often. But I doubt this is a 
problem in practice)

 or only change with the famebuffer, and I have
 to set WINDOW_OFFSET_Y to 0 / fb height depending on the setting of Y
 direction (the latter won't work with MRTs, but that's the non-FBO case
 anyway)) ?

Yes, it could go in theory, and truth is rasterizer state is full of bits that 
apply to other stages of the pipeline, but the practical hurdle of moving this 
to pipe_framebuffer is that pipe_framebuffer has no discrete state beyond 
surfaces so far (it is little more than a tuple of surfaces), so a lot of code 
would need to be updated to fill, propagate, and consider such state in 
pipe_framebuffer...

I presume your concern is that rasterizer state changes frequently where as 
framebuffer state changes infrequently, so adding a dependency would cause 
framebuffer to be processed more often than desired.  You can avoid that by 
keeping track of the lower_left_origin state independently at 
nvc0_rasterizer_state_bind:

diff --git a/src/gallium/drivers/nvc0/nvc0_state.c 
b/src/gallium/drivers/nvc0/nvc0_state.c
index cba076f..2a6fabf 100644
--- a/src/gallium/drivers/nvc0/nvc0_state.c
+++ b/src/gallium/drivers/nvc0/nvc0_state.c
@@ -324,6 +324,12 @@ nvc0_rasterizer_state_bind(struct pipe_context *pipe, void 
*hwcso)
 
nvc0-rast = hwcso;
nvc0-dirty |= NVC0_NEW_RASTERIZER;
+
+   if (nvc0-rast 
+   nvc0-lower_left_origin != nvc0-rast-pipe.lower_left_origin) {
+  nvc0-lower_left_origin = nvc0-rast-pipe.lower_left_origin;
+  nvc0-dirty |= NVC0_NEW_FRAMEBUFFER;
+   }
 }

 static void

This means you won't need to validate framebuffer anymore often than strictly 
necessary. You could also have a new NVC0_NEW_FRAMEBUFFER_ORIGIN flag, just for 
tidyness.

 R600 seems to have PA_SU_VTX_CNTL.PIX_CENTER but no state to change the
 window origin / direction ... and I'd rather not have to bother with it
 myself either.

I need to get this working flawlessly on llvmpipe, but I really see no much 
need for hw driver developers to rush and get this handled properly.  There is 
probably much bigger fish to fry.

If people care enough to devise a state tracker workaround, we could have this 
on a PIPE_CAP.  I'd be all for it.  But even in that case, I think that nudging 
the coordinates slightly would probably get the most bang for buck.

 Also, note that this state and the pixel center one might (or maybe I
 should say will) affect the values of hardware's gl_FragCoord and hence
 PIPE_CAP_TGSI_FS_COORD_ORIGIN/PIXEL_CENTER*, i.e. the shader
 transformation of that input must be adjusted according to this state.
 I'd probably be OK with making this the driver's task.

The FS_COORD_PIXEL_CENTER spec in src/gallium/docs/source/tgsi.rst already 
stated that these are independent: 

  Note that this does not affect the set of fragments generated by
  rasterization, which is instead controlled by gl_rasterization_rules in the
  rasterizer.

And I'm not changing the semantics.  That also seems the spirit of 
GL_ARB_fragment_coord_conventions spec. 

I wouldn't object to add to Gallium a dependency betwen these state if it helps 
hw driver developers, but I don't see how we could define it in such way that 
it would work well for all cases. And I suspect that different hardware 
probably handles this slightly differently (ie, what is orthogonal to some is 
not to others).

Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Jose Fonseca


- Original Message -
 Same as Dave. The first one fails, the second one (-use_fbo) passes.
 
 I just hope lower_left_origin doesn't affect the Y axis of the
 viewport, scissor and gl_FragCoord. If it does, a PIPE_CAP would be
 useful to keep the current behavior.

Maybe I'm not explaining myself properly -- one can keep the current behavior 
merely by ignoring the lower_left_origin bit -- so I really don't see any need 
for concern.

 Marek
 

Jose


 On Sun, Apr 21, 2013 at 9:36 AM, Jose Fonseca jfons...@vmware.com wrote:
  - Original Message -
  Do we really need the lower_left_origin state? I think I can't
  implement it for radeon and it's the kind of stuff that should be
  taken care of by the state tracker anyway.
 
  My understanding is that hardware had switches for this sort of thing. It's
  really hard to provide fully-conforming rasterization for opengl, dx9 
  dx10 without it.
 
  If your hardware allows to put a negative pitch on rendertargets, then that
  should also do it.
 
  If you know what is the hardware's sub-pixel rasterization resolution, then
  adding a vertical bias equal to that amount, depending on this state,
  would give a very close approximation. (This would get the top/bottom
  edges right, at expense of small inaccuracies on non-horizontal edges)
 
  Isn't it sufficient to just
  set a viewport which is upside down, like we do now?
 
  I'm not aware of rasterization top-left rule being affected by the viewport
  flipping.
 
  Do both
 
   ./bin/triangle-rasterization -auto
   ./bin/triangle-rasterization -use_fbo -auto
 
  currently work for you?
 
 
  If drivers don't provide this state, the only way to workaround it I know
  would be to store textures (or drawables?) up-side down, and flip them on
  gl(Get)TexImage  friends.  This would be like using a cannon to shoot a
  fly (a lot of work and a lot of overheads for a small correctness detail).
  I think the drivers are better equipped to handle this.
 
  And you always have the option of merely ignoring this state.  Top-left
  rule correct rasterization has, after all, been ignored till date, and
  nobody cared.
 
 
  For the record, my motivation here is simple: llvmpipe gets the right
  behavior on GL drawables, and fails on GL FBOs  D3D 9/10. I want to get
  the right behavior on D3D 9/10 without causing regressions on GL
  drawables.
 
  BTW, I'd imagine that if hardware rasterizer behavior is hardcoded to
  anything, it would be to D3D 9/10 behavior. That is, they would get GL FBO
  right, but drawables wrong.
 
 
  Jose

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


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Christoph Bumiller
On 21.04.2013 13:18, Jose Fonseca wrote:

 - Original Message -
 On 21.04.2013 09:36, Jose Fonseca wrote:
 - Original Message -
 Do we really need the lower_left_origin state? I think I can't
 implement it for radeon and it's the kind of stuff that should be
 taken care of by the state tracker anyway.
 My understanding is that hardware had switches for this sort of thing. It's
 really hard to provide fully-conforming rasterization for opengl, dx9 
 dx10 without it.

 If your hardware allows to put a negative pitch on rendertargets, then that
 should also do it.
 I have a switch for the upside down thing, but maybe it could be
 framebuffer state instead of rasterizer state (since it's going to
 either not change (D3D) 
 You're right, they should never change at higher frequency than 
 per-framebuffer.

 But due to auxiliary modules like u_blit, u_blitter, u_gen_mipmap, this state 
 will eventually change even for D3D state trackers.  (This is however 
 fixable, if there are performance implications switching this state, we could 
 enhance these helper modules so that they switch it often. But I doubt this 
 is a problem in practice)

 or only change with the famebuffer, and I have
 to set WINDOW_OFFSET_Y to 0 / fb height depending on the setting of Y
 direction (the latter won't work with MRTs, but that's the non-FBO case
 anyway)) ?
 Yes, it could go in theory, and truth is rasterizer state is full of bits 
 that apply to other stages of the pipeline, but the practical hurdle of 
 moving this to pipe_framebuffer is that pipe_framebuffer has no discrete 
 state beyond surfaces so far (it is little more than a tuple of surfaces), so 
 a lot of code would need to be updated to fill, propagate, and consider such 
 state in pipe_framebuffer...

 I presume your concern is that rasterizer state changes frequently where as 
 framebuffer state changes infrequently, so adding a dependency would cause 
 framebuffer to be processed more often than desired.  You can avoid that by 
 keeping track of the lower_left_origin state independently at 
 nvc0_rasterizer_state_bind:

 diff --git a/src/gallium/drivers/nvc0/nvc0_state.c 
 b/src/gallium/drivers/nvc0/nvc0_state.c
 index cba076f..2a6fabf 100644
 --- a/src/gallium/drivers/nvc0/nvc0_state.c
 +++ b/src/gallium/drivers/nvc0/nvc0_state.c
 @@ -324,6 +324,12 @@ nvc0_rasterizer_state_bind(struct pipe_context *pipe, 
 void *hwcso)
  
 nvc0-rast = hwcso;
 nvc0-dirty |= NVC0_NEW_RASTERIZER;
 +
 +   if (nvc0-rast 
 +   nvc0-lower_left_origin != nvc0-rast-pipe.lower_left_origin) {
 +  nvc0-lower_left_origin = nvc0-rast-pipe.lower_left_origin;
 +  nvc0-dirty |= NVC0_NEW_FRAMEBUFFER;
 +   }
  }

  static void

 This means you won't need to validate framebuffer anymore often than strictly 
 necessary. You could also have a new NVC0_NEW_FRAMEBUFFER_ORIGIN flag, just 
 for tidyness.

 R600 seems to have PA_SU_VTX_CNTL.PIX_CENTER but no state to change the
 window origin / direction ... and I'd rather not have to bother with it
 myself either.
 I need to get this working flawlessly on llvmpipe, but I really see no much 
 need for hw driver developers to rush and get this handled properly.  There 
 is probably much bigger fish to fry.

 If people care enough to devise a state tracker workaround, we could have 
 this on a PIPE_CAP.  I'd be all for it.  But even in that case, I think that 
 nudging the coordinates slightly would probably get the most bang for buck.

 Also, note that this state and the pixel center one might (or maybe I
 should say will) affect the values of hardware's gl_FragCoord and hence
 PIPE_CAP_TGSI_FS_COORD_ORIGIN/PIXEL_CENTER*, i.e. the shader
 transformation of that input must be adjusted according to this state.
 I'd probably be OK with making this the driver's task.
 The FS_COORD_PIXEL_CENTER spec in src/gallium/docs/source/tgsi.rst already 
 stated that these are independent: 

   Note that this does not affect the set of fragments generated by
   rasterization, which is instead controlled by gl_rasterization_rules in the
   rasterizer.

 And I'm not changing the semantics.  That also seems the spirit of 
 GL_ARB_fragment_coord_conventions spec. 

 I wouldn't object to add to Gallium a dependency betwen these state if it 
 helps hw driver developers, but I don't see how we could define it in such 
 way that it would work well for all cases. And I suspect that different 
 hardware probably handles this slightly differently (ie, what is orthogonal 
 to some is not to others).

I think that drivers can just report all 4 CAPs as supported and do the
adjustment in the shader themselves (no need for recompilation, just use
uniforms, the st already does it like that), provided that the state
tracker actually uses the rasterizer origin bit instead of changing the
viewport and applies no transformation to the fragment coordinate
whatsoever.

 Jose

___
mesa-dev mailing list

Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Jose Fonseca


- Original Message -
 On 21.04.2013 13:18, Jose Fonseca wrote:
 
  - Original Message -
  On 21.04.2013 09:36, Jose Fonseca wrote:
  - Original Message -
  Do we really need the lower_left_origin state? I think I can't
  implement it for radeon and it's the kind of stuff that should be
  taken care of by the state tracker anyway.
  My understanding is that hardware had switches for this sort of thing.
  It's
  really hard to provide fully-conforming rasterization for opengl, dx9 
  dx10 without it.
 
  If your hardware allows to put a negative pitch on rendertargets, then
  that
  should also do it.
  I have a switch for the upside down thing, but maybe it could be
  framebuffer state instead of rasterizer state (since it's going to
  either not change (D3D)
  You're right, they should never change at higher frequency than
  per-framebuffer.
 
  But due to auxiliary modules like u_blit, u_blitter, u_gen_mipmap, this
  state will eventually change even for D3D state trackers.  (This is
  however fixable, if there are performance implications switching this
  state, we could enhance these helper modules so that they switch it often.
  But I doubt this is a problem in practice)
 
  or only change with the famebuffer, and I have
  to set WINDOW_OFFSET_Y to 0 / fb height depending on the setting of Y
  direction (the latter won't work with MRTs, but that's the non-FBO case
  anyway)) ?
  Yes, it could go in theory, and truth is rasterizer state is full of bits
  that apply to other stages of the pipeline, but the practical hurdle of
  moving this to pipe_framebuffer is that pipe_framebuffer has no discrete
  state beyond surfaces so far (it is little more than a tuple of surfaces),
  so a lot of code would need to be updated to fill, propagate, and consider
  such state in pipe_framebuffer...
 
  I presume your concern is that rasterizer state changes frequently where as
  framebuffer state changes infrequently, so adding a dependency would cause
  framebuffer to be processed more often than desired.  You can avoid that
  by keeping track of the lower_left_origin state independently at
  nvc0_rasterizer_state_bind:
 
  diff --git a/src/gallium/drivers/nvc0/nvc0_state.c
  b/src/gallium/drivers/nvc0/nvc0_state.c
  index cba076f..2a6fabf 100644
  --- a/src/gallium/drivers/nvc0/nvc0_state.c
  +++ b/src/gallium/drivers/nvc0/nvc0_state.c
  @@ -324,6 +324,12 @@ nvc0_rasterizer_state_bind(struct pipe_context *pipe,
  void *hwcso)
   
  nvc0-rast = hwcso;
  nvc0-dirty |= NVC0_NEW_RASTERIZER;
  +
  +   if (nvc0-rast 
  +   nvc0-lower_left_origin != nvc0-rast-pipe.lower_left_origin) {
  +  nvc0-lower_left_origin = nvc0-rast-pipe.lower_left_origin;
  +  nvc0-dirty |= NVC0_NEW_FRAMEBUFFER;
  +   }
   }
 
   static void
 
  This means you won't need to validate framebuffer anymore often than
  strictly necessary. You could also have a new NVC0_NEW_FRAMEBUFFER_ORIGIN
  flag, just for tidyness.
 
  R600 seems to have PA_SU_VTX_CNTL.PIX_CENTER but no state to change the
  window origin / direction ... and I'd rather not have to bother with it
  myself either.
  I need to get this working flawlessly on llvmpipe, but I really see no much
  need for hw driver developers to rush and get this handled properly.
  There is probably much bigger fish to fry.
 
  If people care enough to devise a state tracker workaround, we could have
  this on a PIPE_CAP.  I'd be all for it.  But even in that case, I think
  that nudging the coordinates slightly would probably get the most bang for
  buck.
 
  Also, note that this state and the pixel center one might (or maybe I
  should say will) affect the values of hardware's gl_FragCoord and hence
  PIPE_CAP_TGSI_FS_COORD_ORIGIN/PIXEL_CENTER*, i.e. the shader
  transformation of that input must be adjusted according to this state.
  I'd probably be OK with making this the driver's task.
  The FS_COORD_PIXEL_CENTER spec in src/gallium/docs/source/tgsi.rst already
  stated that these are independent:
 
Note that this does not affect the set of fragments generated by
rasterization, which is instead controlled by gl_rasterization_rules in
the
rasterizer.
 
  And I'm not changing the semantics.  That also seems the spirit of
  GL_ARB_fragment_coord_conventions spec.
 
  I wouldn't object to add to Gallium a dependency betwen these state if it
  helps hw driver developers, but I don't see how we could define it in such
  way that it would work well for all cases. And I suspect that different
  hardware probably handles this slightly differently (ie, what is
  orthogonal to some is not to others).
 
 I think that drivers can just report all 4 CAPs as supported and do the
 adjustment in the shader themselves (no need for recompilation, just use
 uniforms, the st already does it like that), provided that the state
 tracker actually uses the rasterizer origin bit instead of changing the
 viewport and applies no transformation to the fragment 

Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Marek Olšák
I have managed to make the triangle-rasterization test pass. Let's
forget about what the origin is, because it's not really important.
What is actually important is what happens when an edge falls exactly
on a sample point. Radeons have a state which determines what happens
for the left, right, top, and bottom edge, and it does not affect the
coordinate system, which is always top-left. So the issue is fixable
for radeon drivers as long as the origin is always top-left (i.e. no
changes are made to the viewport and scissor states).

Registers:
r300 - SC_EDGERULE
r600 - PA_SC_EDGERULE

Marek

On Sun, Apr 21, 2013 at 2:35 PM, Jose Fonseca jfons...@vmware.com wrote:


 - Original Message -
 On 21.04.2013 13:18, Jose Fonseca wrote:
 
  - Original Message -
  On 21.04.2013 09:36, Jose Fonseca wrote:
  - Original Message -
  Do we really need the lower_left_origin state? I think I can't
  implement it for radeon and it's the kind of stuff that should be
  taken care of by the state tracker anyway.
  My understanding is that hardware had switches for this sort of thing.
  It's
  really hard to provide fully-conforming rasterization for opengl, dx9 
  dx10 without it.
 
  If your hardware allows to put a negative pitch on rendertargets, then
  that
  should also do it.
  I have a switch for the upside down thing, but maybe it could be
  framebuffer state instead of rasterizer state (since it's going to
  either not change (D3D)
  You're right, they should never change at higher frequency than
  per-framebuffer.
 
  But due to auxiliary modules like u_blit, u_blitter, u_gen_mipmap, this
  state will eventually change even for D3D state trackers.  (This is
  however fixable, if there are performance implications switching this
  state, we could enhance these helper modules so that they switch it often.
  But I doubt this is a problem in practice)
 
  or only change with the famebuffer, and I have
  to set WINDOW_OFFSET_Y to 0 / fb height depending on the setting of Y
  direction (the latter won't work with MRTs, but that's the non-FBO case
  anyway)) ?
  Yes, it could go in theory, and truth is rasterizer state is full of bits
  that apply to other stages of the pipeline, but the practical hurdle of
  moving this to pipe_framebuffer is that pipe_framebuffer has no discrete
  state beyond surfaces so far (it is little more than a tuple of surfaces),
  so a lot of code would need to be updated to fill, propagate, and consider
  such state in pipe_framebuffer...
 
  I presume your concern is that rasterizer state changes frequently where as
  framebuffer state changes infrequently, so adding a dependency would cause
  framebuffer to be processed more often than desired.  You can avoid that
  by keeping track of the lower_left_origin state independently at
  nvc0_rasterizer_state_bind:
 
  diff --git a/src/gallium/drivers/nvc0/nvc0_state.c
  b/src/gallium/drivers/nvc0/nvc0_state.c
  index cba076f..2a6fabf 100644
  --- a/src/gallium/drivers/nvc0/nvc0_state.c
  +++ b/src/gallium/drivers/nvc0/nvc0_state.c
  @@ -324,6 +324,12 @@ nvc0_rasterizer_state_bind(struct pipe_context *pipe,
  void *hwcso)
 
  nvc0-rast = hwcso;
  nvc0-dirty |= NVC0_NEW_RASTERIZER;
  +
  +   if (nvc0-rast 
  +   nvc0-lower_left_origin != nvc0-rast-pipe.lower_left_origin) {
  +  nvc0-lower_left_origin = nvc0-rast-pipe.lower_left_origin;
  +  nvc0-dirty |= NVC0_NEW_FRAMEBUFFER;
  +   }
   }
 
   static void
 
  This means you won't need to validate framebuffer anymore often than
  strictly necessary. You could also have a new NVC0_NEW_FRAMEBUFFER_ORIGIN
  flag, just for tidyness.
 
  R600 seems to have PA_SU_VTX_CNTL.PIX_CENTER but no state to change the
  window origin / direction ... and I'd rather not have to bother with it
  myself either.
  I need to get this working flawlessly on llvmpipe, but I really see no much
  need for hw driver developers to rush and get this handled properly.
  There is probably much bigger fish to fry.
 
  If people care enough to devise a state tracker workaround, we could have
  this on a PIPE_CAP.  I'd be all for it.  But even in that case, I think
  that nudging the coordinates slightly would probably get the most bang for
  buck.
 
  Also, note that this state and the pixel center one might (or maybe I
  should say will) affect the values of hardware's gl_FragCoord and hence
  PIPE_CAP_TGSI_FS_COORD_ORIGIN/PIXEL_CENTER*, i.e. the shader
  transformation of that input must be adjusted according to this state.
  I'd probably be OK with making this the driver's task.
  The FS_COORD_PIXEL_CENTER spec in src/gallium/docs/source/tgsi.rst already
  stated that these are independent:
 
Note that this does not affect the set of fragments generated by
rasterization, which is instead controlled by gl_rasterization_rules in
the
rasterizer.
 
  And I'm not changing the semantics.  That also seems the spirit of
  GL_ARB_fragment_coord_conventions spec.
 
  I wouldn't 

Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Jose Fonseca
- Original Message -
 I have managed to make the triangle-rasterization test pass. Let's
 forget about what the origin is, because it's not really important.
 What is actually important is what happens when an edge falls exactly
 on a sample point. Radeons have a state which determines what happens
 for the left, right, top, and bottom edge, and it does not affect the
 coordinate system, which is always top-left. So the issue is fixable
 for radeon drivers as long as the origin is always top-left (i.e. no
 changes are made to the viewport and scissor states).
 
 Registers:
 r300 - SC_EDGERULE
 r600 - PA_SC_EDGERULE
 
 Marek
 

That's great.

Spite the name, this was precisely the intent of lower_left_origin -- whether 
top edges (horizontal edges exactly along the sample point) are inclusive or 
exclusive.  I'm open for better name suggestions.

Jose


 On Sun, Apr 21, 2013 at 2:35 PM, Jose Fonseca jfons...@vmware.com wrote:
 
 
  - Original Message -
  On 21.04.2013 13:18, Jose Fonseca wrote:
  
   - Original Message -
   On 21.04.2013 09:36, Jose Fonseca wrote:
   - Original Message -
   Do we really need the lower_left_origin state? I think I can't
   implement it for radeon and it's the kind of stuff that should be
   taken care of by the state tracker anyway.
   My understanding is that hardware had switches for this sort of thing.
   It's
   really hard to provide fully-conforming rasterization for opengl, dx9
   
   dx10 without it.
  
   If your hardware allows to put a negative pitch on rendertargets, then
   that
   should also do it.
   I have a switch for the upside down thing, but maybe it could be
   framebuffer state instead of rasterizer state (since it's going to
   either not change (D3D)
   You're right, they should never change at higher frequency than
   per-framebuffer.
  
   But due to auxiliary modules like u_blit, u_blitter, u_gen_mipmap, this
   state will eventually change even for D3D state trackers.  (This is
   however fixable, if there are performance implications switching this
   state, we could enhance these helper modules so that they switch it
   often.
   But I doubt this is a problem in practice)
  
   or only change with the famebuffer, and I have
   to set WINDOW_OFFSET_Y to 0 / fb height depending on the setting of Y
   direction (the latter won't work with MRTs, but that's the non-FBO case
   anyway)) ?
   Yes, it could go in theory, and truth is rasterizer state is full of
   bits
   that apply to other stages of the pipeline, but the practical hurdle of
   moving this to pipe_framebuffer is that pipe_framebuffer has no discrete
   state beyond surfaces so far (it is little more than a tuple of
   surfaces),
   so a lot of code would need to be updated to fill, propagate, and
   consider
   such state in pipe_framebuffer...
  
   I presume your concern is that rasterizer state changes frequently where
   as
   framebuffer state changes infrequently, so adding a dependency would
   cause
   framebuffer to be processed more often than desired.  You can avoid that
   by keeping track of the lower_left_origin state independently at
   nvc0_rasterizer_state_bind:
  
   diff --git a/src/gallium/drivers/nvc0/nvc0_state.c
   b/src/gallium/drivers/nvc0/nvc0_state.c
   index cba076f..2a6fabf 100644
   --- a/src/gallium/drivers/nvc0/nvc0_state.c
   +++ b/src/gallium/drivers/nvc0/nvc0_state.c
   @@ -324,6 +324,12 @@ nvc0_rasterizer_state_bind(struct pipe_context
   *pipe,
   void *hwcso)
  
   nvc0-rast = hwcso;
   nvc0-dirty |= NVC0_NEW_RASTERIZER;
   +
   +   if (nvc0-rast 
   +   nvc0-lower_left_origin != nvc0-rast-pipe.lower_left_origin) {
   +  nvc0-lower_left_origin = nvc0-rast-pipe.lower_left_origin;
   +  nvc0-dirty |= NVC0_NEW_FRAMEBUFFER;
   +   }
}
  
static void
  
   This means you won't need to validate framebuffer anymore often than
   strictly necessary. You could also have a new
   NVC0_NEW_FRAMEBUFFER_ORIGIN
   flag, just for tidyness.
  
   R600 seems to have PA_SU_VTX_CNTL.PIX_CENTER but no state to change the
   window origin / direction ... and I'd rather not have to bother with it
   myself either.
   I need to get this working flawlessly on llvmpipe, but I really see no
   much
   need for hw driver developers to rush and get this handled properly.
   There is probably much bigger fish to fry.
  
   If people care enough to devise a state tracker workaround, we could
   have
   this on a PIPE_CAP.  I'd be all for it.  But even in that case, I think
   that nudging the coordinates slightly would probably get the most bang
   for
   buck.
  
   Also, note that this state and the pixel center one might (or maybe I
   should say will) affect the values of hardware's gl_FragCoord and hence
   PIPE_CAP_TGSI_FS_COORD_ORIGIN/PIXEL_CENTER*, i.e. the shader
   transformation of that input must be adjusted according to this state.
   I'd probably be OK with making this the driver's task.
   The 

Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Jose Fonseca


- Original Message -
 - Original Message -
  I have managed to make the triangle-rasterization test pass. Let's
  forget about what the origin is, because it's not really important.
  What is actually important is what happens when an edge falls exactly
  on a sample point. Radeons have a state which determines what happens
  for the left, right, top, and bottom edge, and it does not affect the
  coordinate system, which is always top-left. So the issue is fixable
  for radeon drivers as long as the origin is always top-left (i.e. no
  changes are made to the viewport and scissor states).
  
  Registers:
  r300 - SC_EDGERULE
  r600 - PA_SC_EDGERULE
  
  Marek
  
 
 That's great.
 
 Spite the name, this was precisely the intent of lower_left_origin --
 whether top edges (horizontal edges exactly along the sample point) are
 inclusive or exclusive.  I'm open for better name suggestions.

Actually, it's not just horizontal edges -- the rule also applies to any edge  
45 degrees.

Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Marek Olšák
Some suggestions for the name:

lower_left_edge_rule
lower_left_rasterization_edge_rule
gl_edge_rule
gl_rasterization_edge_rule

In this case, the name is not as important as the documentation which
defines the behavior of the state.

Marek

On Sun, Apr 21, 2013 at 3:56 PM, Jose Fonseca jfons...@vmware.com wrote:
 - Original Message -
 I have managed to make the triangle-rasterization test pass. Let's
 forget about what the origin is, because it's not really important.
 What is actually important is what happens when an edge falls exactly
 on a sample point. Radeons have a state which determines what happens
 for the left, right, top, and bottom edge, and it does not affect the
 coordinate system, which is always top-left. So the issue is fixable
 for radeon drivers as long as the origin is always top-left (i.e. no
 changes are made to the viewport and scissor states).

 Registers:
 r300 - SC_EDGERULE
 r600 - PA_SC_EDGERULE

 Marek


 That's great.

 Spite the name, this was precisely the intent of lower_left_origin -- 
 whether top edges (horizontal edges exactly along the sample point) are 
 inclusive or exclusive.  I'm open for better name suggestions.

 Jose


 On Sun, Apr 21, 2013 at 2:35 PM, Jose Fonseca jfons...@vmware.com wrote:
 
 
  - Original Message -
  On 21.04.2013 13:18, Jose Fonseca wrote:
  
   - Original Message -
   On 21.04.2013 09:36, Jose Fonseca wrote:
   - Original Message -
   Do we really need the lower_left_origin state? I think I can't
   implement it for radeon and it's the kind of stuff that should be
   taken care of by the state tracker anyway.
   My understanding is that hardware had switches for this sort of thing.
   It's
   really hard to provide fully-conforming rasterization for opengl, dx9
   
   dx10 without it.
  
   If your hardware allows to put a negative pitch on rendertargets, then
   that
   should also do it.
   I have a switch for the upside down thing, but maybe it could be
   framebuffer state instead of rasterizer state (since it's going to
   either not change (D3D)
   You're right, they should never change at higher frequency than
   per-framebuffer.
  
   But due to auxiliary modules like u_blit, u_blitter, u_gen_mipmap, this
   state will eventually change even for D3D state trackers.  (This is
   however fixable, if there are performance implications switching this
   state, we could enhance these helper modules so that they switch it
   often.
   But I doubt this is a problem in practice)
  
   or only change with the famebuffer, and I have
   to set WINDOW_OFFSET_Y to 0 / fb height depending on the setting of Y
   direction (the latter won't work with MRTs, but that's the non-FBO case
   anyway)) ?
   Yes, it could go in theory, and truth is rasterizer state is full of
   bits
   that apply to other stages of the pipeline, but the practical hurdle of
   moving this to pipe_framebuffer is that pipe_framebuffer has no discrete
   state beyond surfaces so far (it is little more than a tuple of
   surfaces),
   so a lot of code would need to be updated to fill, propagate, and
   consider
   such state in pipe_framebuffer...
  
   I presume your concern is that rasterizer state changes frequently where
   as
   framebuffer state changes infrequently, so adding a dependency would
   cause
   framebuffer to be processed more often than desired.  You can avoid that
   by keeping track of the lower_left_origin state independently at
   nvc0_rasterizer_state_bind:
  
   diff --git a/src/gallium/drivers/nvc0/nvc0_state.c
   b/src/gallium/drivers/nvc0/nvc0_state.c
   index cba076f..2a6fabf 100644
   --- a/src/gallium/drivers/nvc0/nvc0_state.c
   +++ b/src/gallium/drivers/nvc0/nvc0_state.c
   @@ -324,6 +324,12 @@ nvc0_rasterizer_state_bind(struct pipe_context
   *pipe,
   void *hwcso)
  
   nvc0-rast = hwcso;
   nvc0-dirty |= NVC0_NEW_RASTERIZER;
   +
   +   if (nvc0-rast 
   +   nvc0-lower_left_origin != nvc0-rast-pipe.lower_left_origin) {
   +  nvc0-lower_left_origin = nvc0-rast-pipe.lower_left_origin;
   +  nvc0-dirty |= NVC0_NEW_FRAMEBUFFER;
   +   }
}
  
static void
  
   This means you won't need to validate framebuffer anymore often than
   strictly necessary. You could also have a new
   NVC0_NEW_FRAMEBUFFER_ORIGIN
   flag, just for tidyness.
  
   R600 seems to have PA_SU_VTX_CNTL.PIX_CENTER but no state to change the
   window origin / direction ... and I'd rather not have to bother with it
   myself either.
   I need to get this working flawlessly on llvmpipe, but I really see no
   much
   need for hw driver developers to rush and get this handled properly.
   There is probably much bigger fish to fry.
  
   If people care enough to devise a state tracker workaround, we could
   have
   this on a PIPE_CAP.  I'd be all for it.  But even in that case, I think
   that nudging the coordinates slightly would probably get the most bang
   for
   buck.
  
   Also, note that this state and the 

Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Jose Fonseca
- Original Message -
 Some suggestions for the name:
 
 lower_left_edge_rule
 lower_left_rasterization_edge_rule
 gl_edge_rule
 gl_rasterization_edge_rule
 
 In this case, the name is not as important as the documentation which
 defines the behavior of the state.

On that note, I thought that James' diagrams were pretty good.  Maybe the axis 
is misleading.


+   /**
+* Triangle rasterization always uses a 'top,left' rule for pixel ownership,
+* this just alters what we consider to be the top edge for that test.
+*
+* When true, screen coordinates origin is considered to be at bottom-left
+* (e.g., OpenGL drawables):
+*
+*  y ^
+*|
+*|  +=+ - top edge
+*|  | |
+*|  | |
+*|  | |
+*|  +-+
+*|
+*  0 +-
+*0x
+*
+*  When false, screen coordinates origin is considered to be at top-left
+*  (e.g., OpenGL FBOs, D3D):
+*
+*0x
+*  0 +-
+*|
+*|  +=+ - top edge
+*|  | |
+*|  | |
+*|  | |
+*|  +-+
+*|
+*  y V
 *
-* Triangle rasterization always uses a 'top,left' rule for pixel
-* ownership, this just alters which point we consider the pixel
-* center for that test.
+* See also:
+* - http://www.opengl.org/registry/specs/ARB/fragment_coord_conventions.txt
+* - http://msdn.microsoft.com/en-us/library/windows/desktop/cc627092.aspx
+* - http://msdn.microsoft.com/en-us/library/windows/desktop/bb147314.aspx
 */
-   unsigned gl_rasterization_rules:1;
+   unsigned lower_left_origin:1;
 
/**
 * When true, rasterization is disabled and no pixels are written.

Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.

2013-04-21 Thread Marek Olšák
Yeah, I was confused when reading the comment and the diagrams. It
probably shouldn't mention the screen origin at all and instead should
say which one of the top and bottom edges is inclusive and which one
is exclusive when determining pixel ownership.

Anyway, thank you for fixing this. I would have probably never knew
how to fix the triangle rasterization tests if you didn't bring this
up.

Marek

On Sun, Apr 21, 2013 at 7:54 PM, Jose Fonseca jfons...@vmware.com wrote:
 - Original Message -
 Some suggestions for the name:

 lower_left_edge_rule
 lower_left_rasterization_edge_rule
 gl_edge_rule
 gl_rasterization_edge_rule

 In this case, the name is not as important as the documentation which
 defines the behavior of the state.

 On that note, I thought that James' diagrams were pretty good.  Maybe the 
 axis is misleading.


 +   /**
 +* Triangle rasterization always uses a 'top,left' rule for pixel 
 ownership,
 +* this just alters what we consider to be the top edge for that test.
 +*
 +* When true, screen coordinates origin is considered to be at bottom-left
 +* (e.g., OpenGL drawables):
 +*
 +*  y ^
 +*|
 +*|  +=+ - top edge
 +*|  | |
 +*|  | |
 +*|  | |
 +*|  +-+
 +*|
 +*  0 +-
 +*0x
 +*
 +*  When false, screen coordinates origin is considered to be at top-left
 +*  (e.g., OpenGL FBOs, D3D):
 +*
 +*0x
 +*  0 +-
 +*|
 +*|  +=+ - top edge
 +*|  | |
 +*|  | |
 +*|  | |
 +*|  +-+
 +*|
 +*  y V
  *
 -* Triangle rasterization always uses a 'top,left' rule for pixel
 -* ownership, this just alters which point we consider the pixel
 -* center for that test.
 +* See also:
 +* - 
 http://www.opengl.org/registry/specs/ARB/fragment_coord_conventions.txt
 +* - http://msdn.microsoft.com/en-us/library/windows/desktop/cc627092.aspx
 +* - http://msdn.microsoft.com/en-us/library/windows/desktop/bb147314.aspx
  */
 -   unsigned gl_rasterization_rules:1;
 +   unsigned lower_left_origin:1;

 /**
  * When true, rasterization is disabled and no pixels are written.

 Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev