Bypass glDetachShader and glDeleteShader when dumping state #89

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
@gregory38
Contributor

gregory38 commented Jul 6, 2012

Follow issue 78:

Due to a driver bug, I recreate glCreateShaderProgramv in my application but I keep the part that delete/detach the shader part. Unfortunately apitrace isn't able anymore to retrieve shader code source

I think, it would be better to bypass glDetachShader and glDeleteShader too in dumping state.

@ghost ghost assigned jrfonseca Jul 13, 2012

@jrfonseca

This comment has been minimized.

Show comment
Hide comment
@jrfonseca

jrfonseca Jul 13, 2012

Member

I'm on the fence for this one.

glDetachShader and glDeleteShader are used a lot. My concern is that, by not freeing shaders, shader names will never be reused, therefore shader names will be very different from the trace file, causing bugs/confusion when dumping state.

Probably the only way to fix this would be to keep a cache of the source when dumping state.

Member

jrfonseca commented Jul 13, 2012

I'm on the fence for this one.

glDetachShader and glDeleteShader are used a lot. My concern is that, by not freeing shaders, shader names will never be reused, therefore shader names will be very different from the trace file, causing bugs/confusion when dumping state.

Probably the only way to fix this would be to keep a cache of the source when dumping state.

@gregory38

This comment has been minimized.

Show comment
Hide comment
@gregory38

gregory38 Jul 21, 2012

Contributor

Hello,

I try to implement a shader cache: https://github.com/gregory38/apitrace/tree/separate-shader_V2

As you can see, I add a new flag (attached to glLinkProgram). The function that cache the source is a duplicate of dumpProgram without JSon stuff. I implement the cache as a map: Program->ShaderMap.

Code can potentially be extended to glCreateShaderProgramv, in this case the shader string must be build from the call parameters.

Contributor

gregory38 commented Jul 21, 2012

Hello,

I try to implement a shader cache: https://github.com/gregory38/apitrace/tree/separate-shader_V2

As you can see, I add a new flag (attached to glLinkProgram). The function that cache the source is a duplicate of dumpProgram without JSon stuff. I implement the cache as a map: Program->ShaderMap.

Code can potentially be extended to glCreateShaderProgramv, in this case the shader string must be build from the call parameters.

@jrfonseca

This comment has been minimized.

Show comment
Hide comment
@jrfonseca

jrfonseca Jan 29, 2015

Member

Sorry for not taking action all this time. I was reluctant between both solutions: the complexity of a shader cache, vs the shader leak/renumbering.

But I have good news: it's not necessary to skip glDeleteShader. Skipping glDetachShader suffices.

And by not skipping glDeleteShader, my concern of shaders being massively renumbered no longer applies. Shaders are still deleted, but merely their desctruction is deferred until the program gets deleted.

So I'll take you'r original patch. And modify it to only skip glDetachShader.

Member

jrfonseca commented Jan 29, 2015

Sorry for not taking action all this time. I was reluctant between both solutions: the complexity of a shader cache, vs the shader leak/renumbering.

But I have good news: it's not necessary to skip glDeleteShader. Skipping glDetachShader suffices.

And by not skipping glDeleteShader, my concern of shaders being massively renumbered no longer applies. Shaders are still deleted, but merely their desctruction is deferred until the program gets deleted.

So I'll take you'r original patch. And modify it to only skip glDetachShader.

@jrfonseca jrfonseca closed this in a468a7a Jan 29, 2015

jrfonseca added a commit to apitrace/apitrace-tests that referenced this pull request Jan 29, 2015

@gregory38

This comment has been minimized.

Show comment
Hide comment
@gregory38

gregory38 Jan 31, 2015

Contributor

Cool. Thank you.

Contributor

gregory38 commented Jan 31, 2015

Cool. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment