Re: [Mesa-dev] [PATCH 2/6] gallium: implement clamping controls (ARB_color_buffer_float)
On Mon, 2011-03-21 at 02:12 +0100, Marek Olšák wrote: diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h index cf6c5b5..f6ad456 100644 --- a/src/gallium/include/pipe/p_state.h +++ b/src/gallium/include/pipe/p_state.h @@ -81,6 +81,8 @@ struct pipe_rasterizer_state { unsigned flatshade:1; unsigned light_twoside:1; + unsigned clamp_vertex_color:1; + unsigned clamp_fragment_color:1; unsigned front_ccw:1; unsigned cull_face:2; /** PIPE_FACE_x */ unsigned fill_front:2; /** PIPE_POLYGON_MODE_x */ Don't know if this affects the overall packing of the struct. Have you been able to check? Otherwise the interface changes look good to me. Keith ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/6] gallium: implement clamping controls (ARB_color_buffer_float)
On 03/21/2011 02:12 AM, Marek Olšák wrote: diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h index cf6c5b5..f6ad456 100644 --- a/src/gallium/include/pipe/p_state.h +++ b/src/gallium/include/pipe/p_state.h @@ -81,6 +81,8 @@ struct pipe_rasterizer_state { unsigned flatshade:1; unsigned light_twoside:1; + unsigned clamp_vertex_color:1; + unsigned clamp_fragment_color:1; unsigned front_ccw:1; unsigned cull_face:2; /** PIPE_FACE_x */ unsigned fill_front:2; /** PIPE_POLYGON_MODE_x */ Hadn't you put clamp_fragment_color in the blend state initially ? It seems like a more logical place to me. Beyond that I don't mind where it ends up though, both CSOs are growing a little too large for my taste. Christoph ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/6] gallium: implement clamping controls (ARB_color_buffer_float)
On Mon, 2011-03-21 at 16:23 +0100, Christoph Bumiller wrote: On 03/21/2011 02:12 AM, Marek Olšák wrote: diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h index cf6c5b5..f6ad456 100644 --- a/src/gallium/include/pipe/p_state.h +++ b/src/gallium/include/pipe/p_state.h @@ -81,6 +81,8 @@ struct pipe_rasterizer_state { unsigned flatshade:1; unsigned light_twoside:1; + unsigned clamp_vertex_color:1; + unsigned clamp_fragment_color:1; unsigned front_ccw:1; unsigned cull_face:2; /** PIPE_FACE_x */ unsigned fill_front:2; /** PIPE_POLYGON_MODE_x */ Hadn't you put clamp_fragment_color in the blend state initially ? It seems like a more logical place to me. Indeed you're right. Fragment color clamping takes place in the part of the pipeline governed by the blend CSO. Keith ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/6] gallium: implement clamping controls (ARB_color_buffer_float)
On 03/21/2011 09:10 AM, Keith Whitwell wrote: On Mon, 2011-03-21 at 02:12 +0100, Marek Olšák wrote: diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h index cf6c5b5..f6ad456 100644 --- a/src/gallium/include/pipe/p_state.h +++ b/src/gallium/include/pipe/p_state.h @@ -81,6 +81,8 @@ struct pipe_rasterizer_state { unsigned flatshade:1; unsigned light_twoside:1; + unsigned clamp_vertex_color:1; + unsigned clamp_fragment_color:1; unsigned front_ccw:1; unsigned cull_face:2; /** PIPE_FACE_x */ unsigned fill_front:2; /** PIPE_POLYGON_MODE_x */ Don't know if this affects the overall packing of the struct. Have you been able to check? Otherwise the interface changes look good to me. How do most GPUs handle this clamping? Is it done with extra shader code or is it a discrete operation? If most hardware does it with shader code, maybe we should just do it that way too in Gallium and avoid the new cap bits, etc. OpenGL 3.0 deprecates the per-vertex and per-fragment clamp controls so people will have to do it in their shaders eventually. Re: your comment in rasterizer.rst: XXX: this happens _before_ the geometry shader and thus might not belong here. So is there some question about where the per-vertex clamp is to be done? It seems to me that it should be done after the geometry shader. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/6] gallium: implement clamping controls (ARB_color_buffer_float)
Yeah I moved it from the rasterizer state to the blend state, but then I realized the rasterizer state had been a better place. The thing is the fragment color clamping is done right after the fragment shader output is written, that is before the alpha test, before the depth/stencil test, and therefore long before blending. So I don't know where to put it. Obviously, the depth-stencil-alpha state is a better candidate than the blend state. Also you can do some nasty things with it. For example, you can use the clamping to clamp the fragment color after the fragment shader and then multiply it by 2 in the blending stage, effectively outputting a value in the range [0, 2]. Marek On Mon, Mar 21, 2011 at 4:36 PM, Keith Whitwell kei...@vmware.com wrote: On Mon, 2011-03-21 at 16:23 +0100, Christoph Bumiller wrote: On 03/21/2011 02:12 AM, Marek Olšák wrote: diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h index cf6c5b5..f6ad456 100644 --- a/src/gallium/include/pipe/p_state.h +++ b/src/gallium/include/pipe/p_state.h @@ -81,6 +81,8 @@ struct pipe_rasterizer_state { unsigned flatshade:1; unsigned light_twoside:1; + unsigned clamp_vertex_color:1; + unsigned clamp_fragment_color:1; unsigned front_ccw:1; unsigned cull_face:2; /** PIPE_FACE_x */ unsigned fill_front:2; /** PIPE_POLYGON_MODE_x */ Hadn't you put clamp_fragment_color in the blend state initially ? It seems like a more logical place to me. Indeed you're right. Fragment color clamping takes place in the part of the pipeline governed by the blend CSO. Keith ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/6] gallium: implement clamping controls (ARB_color_buffer_float)
On Mon, Mar 21, 2011 at 4:50 PM, Brian Paul bri...@vmware.com wrote: On 03/21/2011 09:10 AM, Keith Whitwell wrote: On Mon, 2011-03-21 at 02:12 +0100, Marek Olšák wrote: diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h index cf6c5b5..f6ad456 100644 --- a/src/gallium/include/pipe/p_state.h +++ b/src/gallium/include/pipe/p_state.h @@ -81,6 +81,8 @@ struct pipe_rasterizer_state { unsigned flatshade:1; unsigned light_twoside:1; + unsigned clamp_vertex_color:1; + unsigned clamp_fragment_color:1; unsigned front_ccw:1; unsigned cull_face:2; /** PIPE_FACE_x */ unsigned fill_front:2; /** PIPE_POLYGON_MODE_x */ Don't know if this affects the overall packing of the struct. Have you been able to check? Otherwise the interface changes look good to me. How do most GPUs handle this clamping? Is it done with extra shader code or is it a discrete operation? If most hardware does it with shader code, maybe we should just do it that way too in Gallium and avoid the new cap bits, etc. There is a bit in the CB_COLOR[0-7]_INFO register on R600 which controls color clamping before blending. I am not sure whether it can be used for ARB_color_buffer_float or whether it's strictly dependent on the colorbuffer format. Christopher, do you happen to know whether NV50/NVC0 has a hardware state for it too? OpenGL 3.0 deprecates the per-vertex and per-fragment clamp controls so people will have to do it in their shaders eventually. Re: your comment in rasterizer.rst: XXX: this happens _before_ the geometry shader and thus might not belong here. So is there some question about where the per-vertex clamp is to be done? It seems to me that it should be done after the geometry shader. You're right. I will remove that comment. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/6] gallium: implement clamping controls (ARB_color_buffer_float)
On 03/21/2011 05:43 PM, Marek Olšák wrote: On Mon, Mar 21, 2011 at 4:50 PM, Brian Paul bri...@vmware.com wrote: On 03/21/2011 09:10 AM, Keith Whitwell wrote: On Mon, 2011-03-21 at 02:12 +0100, Marek Olšák wrote: diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h index cf6c5b5..f6ad456 100644 --- a/src/gallium/include/pipe/p_state.h +++ b/src/gallium/include/pipe/p_state.h @@ -81,6 +81,8 @@ struct pipe_rasterizer_state { unsigned flatshade:1; unsigned light_twoside:1; + unsigned clamp_vertex_color:1; + unsigned clamp_fragment_color:1; unsigned front_ccw:1; unsigned cull_face:2; /** PIPE_FACE_x */ unsigned fill_front:2; /** PIPE_POLYGON_MODE_x */ Don't know if this affects the overall packing of the struct. Have you been able to check? Otherwise the interface changes look good to me. How do most GPUs handle this clamping? Is it done with extra shader code or is it a discrete operation? If most hardware does it with shader code, maybe we should just do it that way too in Gallium and avoid the new cap bits, etc. There is a bit in the CB_COLOR[0-7]_INFO register on R600 which controls color clamping before blending. I am not sure whether it can be used for ARB_color_buffer_float or whether it's strictly dependent on the colorbuffer format. Christoph, do you happen to know whether NV50/NVC0 has a hardware state for it too? Sure, there's a command to control per-RT fragment colour clamping and one to enable/disable vertex colour clamping. No fiddling with shaders. Not sure where exactly the clamping happens, but it's supposed to behave like OpenGL requires it to. (So we just call them FRAG_COLOR_CLAMP_EN and VERT_COLOR_CLAMP_EN, on nv50 the latter is part of the color semantic/linkage setup, on nvc0 it always applies to the dedicated colour outputs.) OpenGL 3.0 deprecates the per-vertex and per-fragment clamp controls so people will have to do it in their shaders eventually. Re: your comment in rasterizer.rst: XXX: this happens _before_ the geometry shader and thus might not belong here. So is there some question about where the per-vertex clamp is to be done? It seems to me that it should be done after the geometry shader. You're right. I will remove that comment. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/6] gallium: implement clamping controls (ARB_color_buffer_float)
From: Luca Barbieri l...@luca-barbieri.com BTW this changes the gallium interface. Some rather cosmetic changes by Marek. Squashed commit of the following: commit 513b37d484f0318311e84bb86ed4c93cdff71f13 Author: Luca Barbieri l...@luca-barbieri.com Date: Thu Aug 26 18:17:54 2010 +0200 mesa/st: respect fragment clamping in st_DrawPixels commit 546a31e42cad459d7a7a10ebf77fc5ffcf89e9b8 Author: Luca Barbieri l...@luca-barbieri.com Date: Thu Aug 26 18:17:28 2010 +0200 mesa/st: support fragment and vertex color clamping commit c406514a1fbee6891da4cf9ac3eebe4e4407ec13 Author: Luca Barbieri l...@luca-barbieri.com Date: Tue Aug 24 21:56:37 2010 +0200 mesa/st: expose ARB_color_buffer_float if unclamping is supported commit d0c5ea11b6f75f3da2f4ca989115f150ebc7cf8d Author: Luca Barbieri l...@luca-barbieri.com Date: Thu Aug 26 17:53:41 2010 +0200 mesa/st: use unclamped colors This assumes that Gallium is to be interpreted as given drivers the responsibility to clamp these colors if necessary. commit aef5c3c6be6edd076e955e37c80905bc447f8a82 Author: Luca Barbieri l...@luca-barbieri.com Date: Thu Aug 26 18:12:34 2010 +0200 mesa, mesa/st: handle read color clamping properly We set IMAGE_CLAMP_BIT in the caller based on _ClampReadColor, where the operation mandates it. (see the removed XXX comment. -Marek) TODO: did I get the set of operations mandating it right? commit 76bdfcfe3ff4145a1818e6cb6e227b730a5f12d8 Author: Luca Barbieri l...@luca-barbieri.com Date: Thu Aug 26 18:18:25 2010 +0200 gallium: add color clamping to the interface --- src/gallium/docs/source/cso/rasterizer.rst | 27 +++ src/gallium/include/pipe/p_defines.h|2 ++ src/gallium/include/pipe/p_state.h |2 ++ src/mesa/state_tracker/st_atom_blend.c |2 +- src/mesa/state_tracker/st_atom_depth.c |2 +- src/mesa/state_tracker/st_atom_rasterizer.c |8 +++- src/mesa/state_tracker/st_cb_clear.c|4 ++-- src/mesa/state_tracker/st_cb_drawpixels.c |1 + src/mesa/state_tracker/st_cb_readpixels.c |7 +-- src/mesa/state_tracker/st_extensions.c |7 +++ 10 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/gallium/docs/source/cso/rasterizer.rst b/src/gallium/docs/source/cso/rasterizer.rst index d547055..c04cd2b 100644 --- a/src/gallium/docs/source/cso/rasterizer.rst +++ b/src/gallium/docs/source/cso/rasterizer.rst @@ -7,6 +7,33 @@ The rasterizer state controls the rendering of points, lines and triangles. Attributes include polygon culling state, line width, line stipple, multisample state, scissoring and flat/smooth shading. +Linkage +--- + +clamp_vertex_color +^^ + +If set, TGSI_SEMANTIC_COLOR registers are clamped to the [0, 1] range after +the execution of the vertex shader, before being passed to the geometry +shader or fragment shader. + +OpenGL: glClampColor(GL_CLAMP_VERTEX_COLOR) in GL 3.0 or GL_ARB_color_buffer_float + +D3D11: seems always disabled + +XXX: this happens _before_ the geometry shader and thus might not belong here. + +clamp_fragment_color + + +Controls whether TGSI_SEMANTIC_COLOR outputs of the fragment shader +are clamped to [0, 1]. + +OpenGL: glClampColor(GL_CLAMP_FRAGMENT_COLOR) in GL 3.0 or ARB_color_buffer_float + +D3D11: seems always disabled + + Shading --- diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h index bac3300..4f6daa8 100644 --- a/src/gallium/include/pipe/p_defines.h +++ b/src/gallium/include/pipe/p_defines.h @@ -463,6 +463,8 @@ enum pipe_cap { PIPE_CAP_SHADER_STENCIL_EXPORT, PIPE_CAP_TGSI_INSTANCEID, PIPE_CAP_VERTEX_ELEMENT_INSTANCE_DIVISOR, + PIPE_CAP_VERTEX_COLOR_CLAMP_CONTROL, + PIPE_CAP_FRAGMENT_COLOR_CLAMP_CONTROL }; /* Shader caps not specific to any single stage */ diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h index cf6c5b5..f6ad456 100644 --- a/src/gallium/include/pipe/p_state.h +++ b/src/gallium/include/pipe/p_state.h @@ -81,6 +81,8 @@ struct pipe_rasterizer_state { unsigned flatshade:1; unsigned light_twoside:1; + unsigned clamp_vertex_color:1; + unsigned clamp_fragment_color:1; unsigned front_ccw:1; unsigned cull_face:2; /** PIPE_FACE_x */ unsigned fill_front:2; /** PIPE_POLYGON_MODE_x */ diff --git a/src/mesa/state_tracker/st_atom_blend.c b/src/mesa/state_tracker/st_atom_blend.c index fb1c7a4..d1844e1 100644 --- a/src/mesa/state_tracker/st_atom_blend.c +++ b/src/mesa/state_tracker/st_atom_blend.c @@ -286,7 +286,7 @@ update_blend( struct st_context *st ) { struct pipe_blend_color bc; - COPY_4FV(bc.color, st-ctx-Color.BlendColor); + COPY_4FV(bc.color, st-ctx-Color.BlendColorUnclamped); cso_set_blend_color(st-cso_context, bc); } } diff --git a/src/mesa/state_tracker/st_atom_depth.c