[Mesa-dev] [PATCH 2/5] meta: fix meta clear of layered framebuffers
Paul Berry
stereotype441 at gmail.com
Thu Nov 21 07:52:25 PST 2013
On 21 November 2013 05:21, Marek Olšák <maraeo at gmail.com> wrote:
> I currently work on a simpler solution for Gallium. I'm using
> AMD_vertex_shader_layer and this vertex shader (I wrote it in TGSI
> though):
>
> void main()
> {
> gl_Position = gl_Vertex;
> gl_TexCoord[0] = MultiTexCoord0; // clear color
> gl_Layer = gl_InstanceID;
> }
>
> Then I render the quad with DrawArraysInstanced and set the instance
> count = NumLayers. That way each instance (each quad) is sent to a
> different layer and everything is cleared in one draw call.
>
> Drivers not supporting AMD_vertex_shader_layer will have to use a GS
> though.
>
Your approach potentially has better performance, but I'm concerned about
risk, because I'm hoping to get this patch cherry-picked into 10.0 (which
is due to be released in a week). Currently GLSL clear uses 4 different
shaders:
1. A vertex shader that just sets gl_Position (compilable for either
desktop GLSL 1.10 or GLSL ES 1.00)
2. A vertex shader that just sets gl_Position (compilable for either
desktop GLSL 1.30 or GLSL ES 3.00)
3. A fragment shader that outputs floating point color (compilable for
either desktop GLSL or GLSL ES)
4. A fragment shader that outputs integer color (compilable for either
desktop GLSL 1.30 or GLSL ES 3.00)
My proposal adds a fifth:
5. A geometry shader that sets gl_Position and gl_Layer
If we want to use gl_InstanceID and AMD_vertex_shader_layer, we would have
to add two more:
6. A vertex shader that sets gl_Position and gl_Layer (for drivers that
support AMD_vertex_shader_layer)
7. A vertex shader that sets gl_Position and passes through gl_InstanceID
to the GS (for drivers that support GS but not AMD_vertex_shader_layer)
Testing on the Meta clear path is already pretty thin (at least for i965)
because most i965 clears can be done by faster mechanisms (fast color
clear, fast depth clear, or the "blorp" color clear). I'm worried about
introducing 3 new shaders to a poorly-tested Meta path this late in the
release process.
Anyone want to weigh in on this subject from a risk perspective?
I'm also wondering how important performance really is in this patch. I
suspect it's not a big issue for i965, since i965 usually does clears using
faster mechanisms. It's not an issue for i915, since i915 doesn't support
geometry shaders. Are there other drivers that use Meta other than those
two? If not, I'm tempted to leave this patch as is, based on the reasoning
that since Meta is a fallback mechanism, correctness is more important than
performance.
>
> Marek
>
>
> On Wed, Nov 20, 2013 at 5:47 AM, Paul Berry <stereotype441 at gmail.com>
> wrote:
> > From section 4.4.7 (Layered Framebuffers) of the GLSL 3.2 spec:
> >
> > When the Clear or ClearBuffer* commands are used to clear a
> > layered framebuffer attachment, all layers of the attachment are
> > cleared.
> >
> > This patch fixes meta clears to properly clear all layers of a layered
> > framebuffer attachment. We accomplish this by adding a geometry
> > shader to the meta clear program which sets gl_Layer to a uniform
> > value. When clearing a layered framebuffer, we execute in a loop,
> > setting the uniform to point to each layer in turn.
> >
> > Cc: "10.0" <mesa-stable at lists.freedesktop.org>
> > ---
> > src/mesa/drivers/common/meta.c | 51
> +++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 48 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/mesa/drivers/common/meta.c
> b/src/mesa/drivers/common/meta.c
> > index 99b02ba..a6d5098 100644
> > --- a/src/mesa/drivers/common/meta.c
> > +++ b/src/mesa/drivers/common/meta.c
> > @@ -241,9 +241,11 @@ struct clear_state
> > GLuint VBO;
> > GLuint ShaderProg;
> > GLint ColorLocation;
> > + GLint LayerLocation;
> >
> > GLuint IntegerShaderProg;
> > GLint IntegerColorLocation;
> > + GLint IntegerLayerLocation;
> > };
> >
> >
> > @@ -2145,6 +2147,19 @@ meta_glsl_clear_init(struct gl_context *ctx,
> struct clear_state *clear)
> > "{\n"
> > " gl_Position = position;\n"
> > "}\n";
> > + const char *gs_source =
> > + "#version 150\n"
> > + "layout(triangles) in;\n"
> > + "layout(triangle_strip, max_vertices = 4) out;\n"
> > + "uniform int layer;\n"
> > + "void main()\n"
> > + "{\n"
> > + " for (int i = 0; i < 3; i++) {\n"
> > + " gl_Layer = layer;\n"
> > + " gl_Position = gl_in[i].gl_Position;\n"
> > + " EmitVertex();\n"
> > + " }\n"
> > + "}\n";
> > const char *fs_source =
> > "#ifdef GL_ES\n"
> > "precision highp float;\n"
> > @@ -2154,7 +2169,7 @@ meta_glsl_clear_init(struct gl_context *ctx,
> struct clear_state *clear)
> > "{\n"
> > " gl_FragColor = color;\n"
> > "}\n";
> > - GLuint vs, fs;
> > + GLuint vs, gs = 0, fs;
> > bool has_integer_textures;
> >
> > if (clear->ArrayObj != 0)
> > @@ -2176,6 +2191,12 @@ meta_glsl_clear_init(struct gl_context *ctx,
> struct clear_state *clear)
> > _mesa_ShaderSource(vs, 1, &vs_source, NULL);
> > _mesa_CompileShader(vs);
> >
> > + if (_mesa_has_geometry_shaders(ctx)) {
> > + gs = _mesa_CreateShaderObjectARB(GL_GEOMETRY_SHADER);
> > + _mesa_ShaderSource(gs, 1, &gs_source, NULL);
> > + _mesa_CompileShader(gs);
> > + }
> > +
> > fs = _mesa_CreateShaderObjectARB(GL_FRAGMENT_SHADER);
> > _mesa_ShaderSource(fs, 1, &fs_source, NULL);
> > _mesa_CompileShader(fs);
> > @@ -2183,6 +2204,8 @@ meta_glsl_clear_init(struct gl_context *ctx,
> struct clear_state *clear)
> > clear->ShaderProg = _mesa_CreateProgramObjectARB();
> > _mesa_AttachShader(clear->ShaderProg, fs);
> > _mesa_DeleteObjectARB(fs);
> > + if (gs != 0)
> > + _mesa_AttachShader(clear->ShaderProg, gs);
> > _mesa_AttachShader(clear->ShaderProg, vs);
> > _mesa_DeleteObjectARB(vs);
> > _mesa_BindAttribLocation(clear->ShaderProg, 0, "position");
> > @@ -2190,6 +2213,10 @@ meta_glsl_clear_init(struct gl_context *ctx,
> struct clear_state *clear)
> >
> > clear->ColorLocation = _mesa_GetUniformLocation(clear->ShaderProg,
> > "color");
> > + if (gs != 0) {
> > + clear->LayerLocation = _mesa_GetUniformLocation(clear->ShaderProg,
> > + "layer");
> > + }
> >
> > has_integer_textures = _mesa_is_gles3(ctx) ||
> > (_mesa_is_desktop_gl(ctx) && ctx->Const.GLSLVersion >= 130);
> > @@ -2227,6 +2254,8 @@ meta_glsl_clear_init(struct gl_context *ctx,
> struct clear_state *clear)
> > clear->IntegerShaderProg = _mesa_CreateProgramObjectARB();
> > _mesa_AttachShader(clear->IntegerShaderProg, fs);
> > _mesa_DeleteObjectARB(fs);
> > + if (gs != 0)
> > + _mesa_AttachShader(clear->IntegerShaderProg, gs);
> > _mesa_AttachShader(clear->IntegerShaderProg, vs);
> > _mesa_DeleteObjectARB(vs);
> > _mesa_BindAttribLocation(clear->IntegerShaderProg, 0, "position");
> > @@ -2240,7 +2269,13 @@ meta_glsl_clear_init(struct gl_context *ctx,
> struct clear_state *clear)
> >
> > clear->IntegerColorLocation =
> > _mesa_GetUniformLocation(clear->IntegerShaderProg, "color");
> > + if (gs != 0) {
> > + clear->IntegerLayerLocation =
> > + _mesa_GetUniformLocation(clear->IntegerShaderProg, "layer");
> > + }
> > }
> > + if (gs != 0)
> > + _mesa_DeleteObjectARB(gs);
> > }
> >
> > static void
> > @@ -2371,8 +2406,18 @@ _mesa_meta_glsl_Clear(struct gl_context *ctx,
> GLbitfield buffers)
> > _mesa_BufferData(GL_ARRAY_BUFFER_ARB, sizeof(verts), verts,
> > GL_DYNAMIC_DRAW_ARB);
> >
> > - /* draw quad */
> > - _mesa_DrawArrays(GL_TRIANGLE_FAN, 0, 4);
> > + /* draw quad(s) */
> > + if (fb->Layered) {
> > + for (unsigned layer = 0; layer < fb->NumLayers; layer++) {
> > + if (fb->_IntegerColor)
> > + _mesa_Uniform1i(clear->IntegerLayerLocation, layer);
> > + else
> > + _mesa_Uniform1i(clear->LayerLocation, layer);
> > + _mesa_DrawArrays(GL_TRIANGLE_FAN, 0, 4);
> > + }
> > + } else {
> > + _mesa_DrawArrays(GL_TRIANGLE_FAN, 0, 4);
> > + }
> >
> > _mesa_meta_end(ctx);
> > }
> > --
> > 1.8.4.2
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131121/d4c65faf/attachment.html>
More information about the mesa-dev
mailing list