On 08/05/17 06:29, Nicolai Hähnle wrote:
On 04.05.2017 16:28, Nicolai Hähnle wrote:
On 04.05.2017 05:34, Timothy Arceri wrote:
On 04/05/17 13:31, Dave Airlie wrote:
+/* 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."
+ */
+#define
USE_PROGRAM(no_error) \
+ 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##no_error(ctx->Pipeline.Current->Name); \
+
}
\
+
}
\
+
why the macro, inline functions are a thing, or just one common
function that both entrypoints call.
So that we can avoid adding an if to call
_mesa_BindProgramPipeline_no_error vs _mesa_BindProgramPipeline
I have to say I also don't like this very much. Do you have benchmarks
to back up the usefulness of this?
This is where C++ templates would be great, because there could just be
a boolean no_error template parameter. Maybe there's a way to get
somewhere similar in C, even if it requires some compiler extensions?
I thought about this some more, and I feel that at least with gcc/clang,
the always_inline attributes provides a reasonably elegant solution. I
mean this code pattern:
__attribute((always_inline))
static inline void useprogram(GLuint program, bool check_error)
{
GET_CURRENT_CTX(ctx);
...
}
void GLAPIENTRY
_mesa_UseProgram(GLuint program)
{
useprogram(program, true);
}
void GLAPIENTRY
_mesa_UseProgram_no_error(GLuint program)
{
useprogram(program, false);
}
This would avoid the code duplication and macro magic that I'm finding a
bit worrisome while still getting fully optimized code in both paths.
This could also be used to clean up some of the other _no_error variants. >
There's a bit of a question of other compiler support. At least judging
from the online docs, MSVC's __forceinline should have the same effect,
so it looks like all the relevant bases are covered.
What do you think?
I'm happy with this solution, thanks. I'll send out a new version for
review.
Cheers,
Nicolai
Cheers,
Nicolai
Dave.
+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);
+ }
+
+ USE_PROGRAM(_no_error)
+}
+
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 +1915,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);
- }
- }
+ USE_PROGRAM()
}
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 *);
--
2.9.3
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
