Bug 106283

If you are referring to MESA_SHADER_DUMP_PATH and MESA_SHADER_READ_PATH, your conclusion seems wrong.

Shader replacement works when application calls glShaderSource(). You need to set the environment variables pointing to existing paths for it to work. Shader replacements don't work for ancient glProgramString() API.

One source of confusion here might be that there exists 2 shader dumping paths in Mesa, created for different purposes.

Shader replacement is done with MESA_SHADER_DUMP_PATH and MESA_SHADER_READ_PATH, this can be used to analyze performance, debug shaders and so on. Then there is MESA_SHADER_CAPTURE_PATH which is used to capture shaders in a special form that can be used with 'shader-db' tool. This supports ARB programs but cannot be used together with MESA_SHADER_DUMP_PATH and MESA_SHADER_READ_PATH, these are separate things and used for different purposes.

Hopefully this helps!

No it does not help.
Yes, I swapped the working and not working functions.
Sorry, my mistake. But the point remains.

As you have said yourself, Shader replacements don't work for ancient glProgramString().

I need that thing to work for glProgramString() too. This is what I request here.

The application is definitely using the ancient API that cannot dump&replace.

(In reply to iive from comment #3)
> No it does not help.
> Yes, I swapped the working and not working functions.
> Sorry, my mistake. But the point remains.
> 
> As you have said yourself, Shader replacements don't work for ancient
> glProgramString().
> 
> I need that thing to work for glProgramString() too. This is what I request
> here.
> 
> The application is definitely using the ancient API that cannot dump&replace.

OK I see .. it looks like it should be pretty straightforward to implement this support by sharing the dumping code. Will take a look at this later.

Hi,

if you are looking at the shader capture code, would you consider looking at this patch too?

https://lists.freedesktop.org/archives/mesa-dev/2018-April/193430.html

I just send it to the list. It changes the naming convention for captured shaders so that is easier to identify them.

FrameRetrace supports gl shader replacement.  I'll enhance it to support glProgramString.  It will be easier to debug shaders with FrameRetrace than using the environment variables.

https://fosdem.org/2018/schedule/event/apitrace/

(In reply to Tapani Pälli from comment #4)
> (In reply to iive from comment #3)
> OK I see .. it looks like it should be pretty straightforward to implement
> this support by sharing the dumping code. Will take a look at this later.

Your words made this seems trivial and something that you could make straight away.
Yet it's been more than a week without any new development (afaik).

I hope you haven't forgotten about it and you could work on it in reasonable time frame.

(In reply to iive from comment #7)
> (In reply to Tapani Pälli from comment #4)
> > (In reply to iive from comment #3)
> > OK I see .. it looks like it should be pretty straightforward to implement
> > this support by sharing the dumping code. Will take a look at this later.
> 
> Your words made this seems trivial and something that you could make
> straight away.
> Yet it's been more than a week without any new development (afaik).
> 
> I hope you haven't forgotten about it and you could work on it in reasonable
> time frame.

I'm working on something different ATM and would not like to context switch but will look at this later. If changes from ben@besd.de make sense, it would be good to land those first. I'm not sure what implications his set has on using shader-db so I'm hoping some active user of shader-db to comment/review his series.

(In reply to Tapani Pälli from comment #8)
> I'm working on something different ATM and would not like to context switch
> but will look at this later. If changes from ben@besd.de make sense, it
> would be good to land those first. I'm not sure what implications his set
> has on using shader-db so I'm hoping some active user of shader-db to
> comment/review his series.

Since the cause of that bug was discovered and there is working patch for it,
my urgency for this feature has decreased.

Just tell me after how many days/weeks I should remind you to check this again.

(In reply to iive from comment #9)
> (In reply to Tapani Pälli from comment #8)
> > I'm working on something different ATM and would not like to context switch
> > but will look at this later. If changes from ben@besd.de make sense, it
> > would be good to land those first. I'm not sure what implications his set
> > has on using shader-db so I'm hoping some active user of shader-db to
> > comment/review his series.
> 
[...]
> 
> Just tell me after how many days/weeks I should remind you to check this
> again.

Just a reminder about the shader replacement feature request.

I hope that you haven't forgotten about it.

(In reply to iive from comment #10)
> Just a reminder about the shader replacement feature request.
> 
> I hope that you haven't forgotten about it.

When I got my first reply I was so hopeful that this
simple request could be fulfilled in a few days.

Yet, here we are, one month after my last reminder
and I don't even get replies anymore.

Please, don't postpone it to infinity.

do you have an apitrace file of your workload, so I can test this functionality in FrameRetrace?

(In reply to Mark Janes from comment #12)
> do you have an apitrace file of your workload, so I can test this
> functionality in FrameRetrace?

The needed functionality was to dig deeper into bug #91808.
You can find the link to my trace here:
https://bugs.freedesktop.org/show_bug.cgi?id=91808#c8

Created attachment 140342 [details] [review]
fix wip

I believe this should do the trick. If this works OK for you I can clean it up and send it forward.

FYI I've sent patch for review:
https://lists.freedesktop.org/archives/mesa-dev/2018-July/200885.html

commit 656ccf4ef890b91debbb72b38957723ca04411d0
Author: Tapani Pälli <tapani.palli@intel.com>
Date:   Tue Jul 24 10:41:46 2018 +0300

    mesa: shader dump/read support for ARB programs
    
    Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106283
    Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>