Re: [Mesa-dev] [PATCH 2/2] gallium: Replaced gl_rasterization_rules with lower_left_origin and half_pixel_center.
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.
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.
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.
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.
- 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.
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.
- 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.
- 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.
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.
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.
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.
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.
- 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.
- 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.
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.
- 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.
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.
- 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.
- 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.
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.
- 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.
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