search

microsoft / angle

ANGLE: OpenGL ES to DirectX translation
Other
615 stars 166 forks source link

HLSLCompiler is not initialized if loading shaders with glProgramBinaryOES #128

Closed lunasorcery closed 6 years ago

lunasorcery commented 7 years ago

I have a project where I'm loading all of my shaders from a cache on disk using glProgramBinaryOES. I then get a crash in HLSLCompiler::compileToBinary when attempting to draw geometry, because HLSLCompiler has not been initialized. My callstack inside ANGLE is as follows (parameters trimmed for clarity):

libGLESv2.dll!rx::HLSLCompiler::compileToBinary
libGLESv2.dll!rx::Renderer11::compileToExecutable
libGLESv2.dll!rx::ProgramD3D::getVertexExecutableForInputLayout
libGLESv2.dll!rx::InputLayoutCache::createInputLayout
libGLESv2.dll!rx::InputLayoutCache::updateInputLayout
libGLESv2.dll!rx::InputLayoutCache::applyVertexBuffers
libGLESv2.dll!rx::Renderer11::applyVertexBuffer
libGLESv2.dll!rx::Renderer11::genericDrawElements
libGLESv2.dll!rx::Context11::drawElements
libGLESv2.dll!gl::Context::drawElements
libGLESv2.dll!gl::DrawElements

From my investigation, it seems as though https://github.com/microsoft/angle/commit/01074255c6425b433cd8278f40b527add27011e4 introduced this issue by removing the call to HLSLCompiler::initialize from the top of HLSLCompiler::compileToBinary, replacing it with an assert. As a result, it seems HLSLCompiler::ensureInitialized (formerly HLSLCompiler::initialize) is only ever called from HLSLCompiler::disassembleBinary, or via a deep callstack from glLinkProgram. I presume this issue has gone unnoticed as most users will be calling glLinkProgram at some point before attempting to draw geometry.

I've managed to resolve the issue for my purposes by reintroducing the initialization call to the top of HLSLCompiler::compileToBinary (see: https://github.com/willkirkby/angle/commit/6f73065902d57eaec1e5a1f297a16e7b13364f4e). However, given the prior commit was to introduce threading support, I'm hesitant to submit this as a pull request as I'm uncertain as to this change's effect on threaded shader compilation, and would appreciate another pair of eyes on it.

austinkinross commented 7 years ago

Hi Will, thanks for reporting this!

This issue has been fixed in the master ANGLE repository. It was 'accidentally' fixed in this change while fixing another issue, but a test for your issue was added later that passes after the first change.

We'll try to schedule a merge from the master ANGLE branch into Microsoft/ANGLE soon. In the meantime, hopefully it should be fairly easy to copy the above fix into your working branch- it should just involve adding this to the top of Renderer11::applyShaders(...) in Renderer11.cpp:

    // This method is called single-threaded.
    ANGLE_TRY(ensureHLSLCompilerInitialized());

Hope this helps, Austin

lunasorcery commented 7 years ago

Hi Austin,

Yep, that seems to solve it. I've reverted my change and merged that upstream fix into my fork, and that works nicely. Should this issue be left open until you've merged the fix in from the master ANGLE branch?

~will

austinkinross commented 7 years ago

Yes please, we'll close this issue next time we perform a merge. Thanks!

austinkinross commented 6 years ago

This fix should now be in ms-master. Thanks again for reporting it!