On 08/05/17 19:36, Nicolai Hähnle wrote:
On 08.05.2017 02:25, Timothy Arceri wrote:
V3: use always_inline attribute (Suggested by Nicolai)
Cc: Nicolai Hähnle <nhaeh...@gmail.com>
---
src/mapi/glapi/gen/gl_API.xml | 2 +-
src/mesa/main/shaderapi.c | 75
+++++++++++++++++++++++++++++--------------
src/mesa/main/shaderapi.h | 2 ++
3 files changed, 54 insertions(+), 25 deletions(-)
diff --git a/src/mapi/glapi/gen/gl_API.xml
b/src/mapi/glapi/gen/gl_API.xml
index a304ac0..50d60f5 100644
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -5493,21 +5493,21 @@
</function>
<function name="ShaderSource" es2="2.0" marshal="custom">
<param name="shader" type="GLuint"/>
<param name="count" type="GLsizei"/>
<param name="string" type="const GLchar * const *"/>
<param name="length" type="const GLint *"/>
<glx ignore="true"/>
</function>
- <function name="UseProgram" es2="2.0">
+ <function name="UseProgram" es2="2.0" no_error="true">
<param name="program" type="GLuint"/>
<glx ignore="true"/>
</function>
<function name="Uniform1f" es2="2.0">
<param name="location" type="GLint"/>
<param name="v0" type="GLfloat"/>
<glx ignore="true"/>
</function>
<function name="Uniform2f" es2="2.0">
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index eb75a3e..f91cc89 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -1841,20 +1841,70 @@ _mesa_ShaderSource(GLuint shaderObj, GLsizei
count,
free(source);
source = replacement;
}
#endif /* ENABLE_SHADER_CACHE */
shader_source(sh, source);
free(offsets);
}
+/* The ARB_separate_shader_object spec says:
+ *
+ * "The executable code for an individual shader stage is taken from
+ * the current program for that stage. If there is a current
program
+ * object established by UseProgram, that program is considered
current
+ * for all stages. Otherwise, if there is a bound program pipeline
+ * object (section 2.14.PPO), the program bound to the appropriate
+ * stage of the pipeline object is considered current."
+ */
+static ALWAYS_INLINE void
+reference_pipeline_and_use_program(struct gl_context *ctx,
+ struct gl_shader_program *shProg,
+ bool no_error)
+{
+ if (shProg) {
+ /* Attach shader state to the binding point */
+ _mesa_reference_pipeline_object(ctx, &ctx->_Shader, &ctx->Shader);
+ /* Update the program */
+ _mesa_use_shader_program(ctx, shProg);
+ } else {
+ /* Must be done first: detach the progam */
+ _mesa_use_shader_program(ctx, shProg);
+ /* Unattach shader_state binding point */
+ _mesa_reference_pipeline_object(ctx, &ctx->_Shader,
+ ctx->Pipeline.Default);
+ /* If a pipeline was bound, rebind it */
+ if (ctx->Pipeline.Current) {
+ if (no_error)
+
_mesa_BindProgramPipeline_no_error(ctx->Pipeline.Current->Name);
+ else
+ _mesa_BindProgramPipeline(ctx->Pipeline.Current->Name);
+ }
+ }
+}
+
+ \
+
+void GLAPIENTRY
+_mesa_UseProgram_no_error(GLuint program)
+{
+ GET_CURRENT_CONTEXT(ctx);
+ struct gl_shader_program *shProg = NULL;
+
+ if (program) {
+ shProg = _mesa_lookup_shader_program(ctx, program);
+ }
+
+ reference_pipeline_and_use_program(ctx, shProg, true);
+}
+
Unless reference_pipeline_and_use_program has a separate use, I'd
actually feel more comfortable if the *entire* body of _mesa_UseProgram
was moved into the ALWAYS_INLINE function. This is better for
maintainability because it would guarantee that any future change will
automatically update both variants of the functions.
That would just result in some ugly code and would not actually reduce
the number of lines of code. The code takes different paths so nothing
would be updated automatically.
I'd really rather not do that, the no error path is just a single
look-up call, this is a pattern that is going to become very common as
more no_error support is added, I'd rather we be aware that we should
check the no_error variants in future when updating/reviewing api
changes rather than adding a bunch of ugly and hard to follow inline
functions everywhere.
For example this is what the body of the function will look like:
GET_CURRENT_CONTEXT(ctx);
struct gl_shader_program *shProg = NULL;
if (no_error) {
if (program) {
shProg = _mesa_lookup_shader_program(ctx, program);
} else {
if (_mesa_is_xfb_active_and_unpaused(ctx)) {
_mesa_error(ctx, GL_INVALID_OPERATION,
"glUseProgram(transform feedback active)");
return;
}
if (program) {
shProg = _mesa_lookup_shader_program_err(ctx, program,
"glUseProgram");
if (!shProg) {
return;
}
if (!shProg->data->LinkStatus) {
_mesa_error(ctx, GL_INVALID_OPERATION,
"glUseProgram(program %u not linked)", program);
return;
}
if (ctx->_Shader->Flags & GLSL_USE_PROG) {
print_shader_info(*shProg);
}
}
}
if (shProg) {
/* Attach shader state to the binding point */
_mesa_reference_pipeline_object(ctx, &ctx->_Shader, &ctx->Shader);
/* Update the program */
_mesa_use_shader_program(ctx, shProg);
} else {
/* Must be done first: detach the progam */
_mesa_use_shader_program(ctx, shProg);
/* Unattach shader_state binding point */
_mesa_reference_pipeline_object(ctx, &ctx->_Shader,
ctx->Pipeline.Default);
/* If a pipeline was bound, rebind it */
if (ctx->Pipeline.Current) {
if (no_error)
_mesa_BindProgramPipeline_no_error(ctx->Pipeline.Current->Name);
else
_mesa_BindProgramPipeline(ctx->Pipeline.Current->Name);
}
}
Thoughts?
Cheers,
Nicolai
void GLAPIENTRY
_mesa_UseProgram(GLuint program)
{
GET_CURRENT_CONTEXT(ctx);
struct gl_shader_program *shProg = NULL;
if (MESA_VERBOSE & VERBOSE_API)
_mesa_debug(ctx, "glUseProgram %u\n", program);
@@ -1875,44 +1925,21 @@ _mesa_UseProgram(GLuint program)
return;
}
#ifdef DEBUG
if (ctx->_Shader->Flags & GLSL_USE_PROG) {
print_shader_info(shProg);
}
#endif
}
- /* The ARB_separate_shader_object spec says:
- *
- * "The executable code for an individual shader stage is
taken from
- * the current program for that stage. If there is a current
program
- * object established by UseProgram, that program is
considered current
- * for all stages. Otherwise, if there is a bound program
pipeline
- * object (section 2.14.PPO), the program bound to the
appropriate
- * stage of the pipeline object is considered current."
- */
- if (program) {
- /* Attach shader state to the binding point */
- _mesa_reference_pipeline_object(ctx, &ctx->_Shader, &ctx->Shader);
- /* Update the program */
- _mesa_use_shader_program(ctx, shProg);
- } else {
- /* Must be done first: detach the progam */
- _mesa_use_shader_program(ctx, shProg);
- /* Unattach shader_state binding point */
- _mesa_reference_pipeline_object(ctx, &ctx->_Shader,
ctx->Pipeline.Default);
- /* If a pipeline was bound, rebind it */
- if (ctx->Pipeline.Current) {
- _mesa_BindProgramPipeline(ctx->Pipeline.Current->Name);
- }
- }
+ reference_pipeline_and_use_program(ctx, shProg, false);
}
void GLAPIENTRY
_mesa_ValidateProgram(GLuint program)
{
GET_CURRENT_CONTEXT(ctx);
validate_program(ctx, program);
}
diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h
index 99b4fe8..0a28185 100644
--- a/src/mesa/main/shaderapi.h
+++ b/src/mesa/main/shaderapi.h
@@ -120,20 +120,22 @@ _mesa_IsProgram(GLuint name);
extern GLboolean GLAPIENTRY
_mesa_IsShader(GLuint name);
extern void GLAPIENTRY
_mesa_LinkProgram(GLuint programObj);
extern void GLAPIENTRY
_mesa_ShaderSource(GLuint, GLsizei, const GLchar* const *, const
GLint *);
+void GLAPIENTRY
+_mesa_UseProgram_no_error(GLuint);
extern void GLAPIENTRY
_mesa_UseProgram(GLuint);
extern void GLAPIENTRY
_mesa_ValidateProgram(GLuint);
extern void GLAPIENTRY
_mesa_BindAttribLocation(GLuint program, GLuint, const GLchar *);
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
