[PATCH] glretrace: Add a new --stub-fs to stub out fragment shaders
José Fonseca
jose.r.fonseca at gmail.com
Fri Aug 3 08:10:57 PDT 2012
On Thu, Aug 2, 2012 at 11:39 PM, Carl Worth <cworth at cworth.org> wrote:
> With this option, all fragment shaders will be replaced with a stub
> shader that simply draws solid magenta. This can be useful for
> measuring the time spent in all other portions of the pipeline.
> ---
>
> José Fonseca <jose.r.fonseca at gmail.com> writes:
> > I don't object this, but it's not easy to do GLSL shader manipulation
> > reliably (without human intervention), without at least a GLSL
> > pre-processor and tokenizer.
>
> What difficulties do you envision here? This patch was easy to write
> and seems to work well, (no parsing of GLSL needed).
>
> I suppose that if you wanted to do modification of a shader, without
> outright replacement, then yes, that would be quite a bit trickier.
Yes, that's what I was thinking but it may be overkill indeed.
> (Eric, give this patch a try and see what you think. Next I'll write
> the script to make two runs with and without --stub-fs, then combine
> the results, do the subtraction, and print a sorted list of individual
> fragment and vertex shaders.)
One thing to watch out is that the vertex shader could be potentially
simplified because its outputs don't get used by the fragment shader.
> > But at the end of the day, ARB_timer_query is just too cumbersome for
> > accurate profiling, and we should have another extension that provides
> > fined grained measurements. I think that Intel is well positioned to
> > design and prototype such extension.
>
> I don't think we need any new extension. There are already a couple of
> exsting extensions to make available more detailed information than
> ARB_timer_query. The ones I am thinking of are AMD_performance_monitor
> and GL_INTEL_performance_queries. Of these, AMD_performance_monitor is
> more interesting to me. Most significantly, it there exists public
> documentation for the specification:
>
> http://www.opengl.org/registry/specs/AMD/performance_monitor.txt
>
> And it looks to me like it would give us all the information we really
> need.
>
> So, yes, this is something that we do want to implement in our driver.
Thanks for the links. I confess I didn't know about them. AMD Looks
quite generic. INTEL's seem unnofficial (
http://zaynar.co.uk/docs/gl-intel-performance-queries.html ). It's a
pity that NVIDIA provides nothing like this.
Concerning your change, would it be possible to use a
glGetShaderiv(GL_SHADER_TYPE) instead of the fragmentShaders set? It
shouldn't affect performance, and it avoids maintaining yet another
mutable global container which we'll need to fix if one day we want to
add multithreaded retracing.
Jose
>
> retrace/glretrace.hpp | 3 ++-
> retrace/glretrace.py | 32 ++++++++++++++++++++++++++++++++
> retrace/glretrace_main.cpp | 2 +-
> retrace/retrace.hpp | 5 +++++
> retrace/retrace_main.cpp | 4 +++-
> 5 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/retrace/glretrace.hpp b/retrace/glretrace.hpp
> index fea187c..95756a2 100644
> --- a/retrace/glretrace.hpp
> +++ b/retrace/glretrace.hpp
> @@ -29,12 +29,13 @@
> #include "glws.hpp"
> #include "retrace.hpp"
>
> +#include <set>
>
> namespace glretrace {
>
>
> extern bool insideGlBeginEnd;
> -
> +extern std::set<unsigned> fragmentShaders;
>
> extern glws::Drawable *currentDrawable;
> extern glws::Context *currentContext;
> diff --git a/retrace/glretrace.py b/retrace/glretrace.py
> index 08fc6a0..186282c 100644
> --- a/retrace/glretrace.py
> +++ b/retrace/glretrace.py
> @@ -289,10 +289,41 @@ class GlRetracer(Retracer):
> print r' if (pipeline) {'
> print r' _pipelineHasBeenBound = true;'
> print r' }'
> +
> + # We use the original shader ID values in the fragmentShader
> + # set for convenience.
> + if (function.name in ('glCreateShader', 'glCreateShaderObjectARB')):
> + print r' if (static_cast<GLenum>((call.arg(0)).toSInt()) == GL_FRAGMENT_SHADER) {'
> + print r' glretrace::fragmentShaders.insert((*call.ret).toUInt());'
> + print r' }'
> +
> + if (function.name in ('glDeleteShader', 'glDeleteObjectARB')):
> + print r' glretrace::fragmentShaders.erase((call.arg(0)).toUInt());'
> +
> + if function.name in ('glShaderSource', 'glShaderSourceARB'):
> + print r' if (retrace::stubFragmentShaders && glretrace::fragmentShaders.find((call.arg(0)).toUInt()) != glretrace::fragmentShaders.end()) {'
> + print r' count = 1;'
> + print r' string[0] = "#version 120\n\n"'
> + print r' "void main()\n"'
> + print r' "{\n"'
> + print r' " /* Magenta! */\n"'
> + print r' " gl_FragColor = vec4(1, 0, 1, 1);\n"'
> + print r' "}";'
> + print r' length[0] = strlen(string[0]);'
> + print r' }'
>
> if function.name == 'glCreateShaderProgramv':
> # When dumping state, break down glCreateShaderProgramv so that the
> # shader source can be recovered.
> + print r' if (retrace::stubFragmentShaders && type == GL_FRAGMENT_SHADER) {'
> + print r' count = 1;'
> + print r' strings[0] = "#version 120\n\n"'
> + print r' "void main()\n"'
> + print r' "{\n"'
> + print r' " /* Magenta! */\n"'
> + print r' " gl_FragColor = vec4(1, 0, 1, 1);\n"'
> + print r' "}";'
> + print r' }'
> print r' if (retrace::dumpingState) {'
> print r' GLuint _shader = glCreateShader(type);'
> print r' if (_shader) {'
> @@ -477,6 +508,7 @@ class GlRetracer(Retracer):
> if __name__ == '__main__':
> print r'''
> #include <string.h>
> +#include <set>
>
> #include "glproc.hpp"
> #include "glretrace.hpp"
> diff --git a/retrace/glretrace_main.cpp b/retrace/glretrace_main.cpp
> index a434d4b..88ec1bf 100644
> --- a/retrace/glretrace_main.cpp
> +++ b/retrace/glretrace_main.cpp
> @@ -35,7 +35,7 @@
> namespace glretrace {
>
> bool insideGlBeginEnd = false;
> -
> +std::set<unsigned> fragmentShaders;
>
> void
> checkGlError(trace::Call &call) {
> diff --git a/retrace/retrace.hpp b/retrace/retrace.hpp
> index 0ee0e55..be49156 100644
> --- a/retrace/retrace.hpp
> +++ b/retrace/retrace.hpp
> @@ -152,6 +152,11 @@ extern bool profiling;
> */
> extern bool dumpingState;
>
> +/**
> + * Stubbing out fragment shaders (to measure vertex shader contribution)
> + */
> +extern bool stubFragmentShaders;
> +
>
> extern bool doubleBuffer;
> extern bool coreProfile;
> diff --git a/retrace/retrace_main.cpp b/retrace/retrace_main.cpp
> index 093ab17..086ef42 100644
> --- a/retrace/retrace_main.cpp
> +++ b/retrace/retrace_main.cpp
> @@ -55,7 +55,7 @@ int verbosity = 0;
> bool debug = true;
> bool profiling = false;
> bool dumpingState = false;
> -
> +bool stubFragmentShaders = false;
>
> bool doubleBuffer = true;
> bool coreProfile = false;
> @@ -319,6 +319,8 @@ int main(int argc, char **argv)
> retrace::debug = false;
> retrace::profiling = true;
> retrace::verbosity = -1;
> + } else if (!strcmp(arg, "--stub-fs")) {
> + retrace::stubFragmentShaders = true;
> } else if (!strcmp(arg, "-c")) {
> comparePrefix = argv[++i];
> if (compareFrequency.empty()) {
> --
> 1.7.10
>
More information about the apitrace
mailing list