[Mesa-stable] [Mesa-dev] [PATCH 2/5] meta: fix meta clear of layered framebuffers
Marek Olšák
maraeo at gmail.com
Thu Nov 21 08:12:54 PST 2013
Considering you're the only user of meta_clear who cares about it,
it's all up to you. If your hardware can do AMD_vertex_shader_layer,
you won't ever have to implement any other code path.
It's silly to have multiple GLSL shaders for different versions of the
API. I think this should be API-independent and the GLSL compiler
should allow you to compile any shader for internal purposes. If we
adopted the GLSL IR as the main IR in Gallium, we would make it
API-agnostic and allow compilation of all shaders the GLSL compiler
knows how to compile.
I don't think you need a separate shader for outputting a float vs
integer color. What we do in Gallium is that we set the clear color as
a vertex input which is R32G32B32A32_FLOAT, which is passed to the
fragment shader as a flat varying and written to gl_FragColor. If it's
an integer clear, it doesn't matter. The bits of the clear color are
not changed anywhere, so integer clears work too.
Marek
On Thu, Nov 21, 2013 at 4:52 PM, Paul Berry <stereotype441 at gmail.com> wrote:
> 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
>
>
More information about the mesa-stable
mailing list