bkaradzic / bgfx

Cross-platform, graphics API agnostic, "Bring Your Own Engine/Framework" style rendering library.
https://bkaradzic.github.io/bgfx/overview.html
BSD 2-Clause "Simplified" License
14.98k stars 1.94k forks source link

Crash in RendererContextGL::isTextureFormatValid for MIP_AUTOGEN #1216

Open hugoam opened 7 years ago

hugoam commented 7 years ago

I just updated to latest changes in master and I'm now getting a crash when initializing OpenGL renderer. The crash happens inside here :

At line 2153 RendererContextGL::init when checking for mip map generation capabilities :

supported |= isTextureFormatValid(TextureFormat::Enum(ii), false, true)
    ? BGFX_CAPS_FORMAT_TEXTURE_MIP_AUTOGEN
    : BGFX_CAPS_FORMAT_TEXTURE_NONE
    ;

It crashes at line 1505 in RendererContextGL::isTextureFormatValid :

    if (0 == err
    &&  _mipAutogen)
    {
        glGenerateMipmap(target); // Exception thrown at 0x04E50920 (ig9icd32.dll) in editor_d.exe: 0xC0000005: Access violation reading location 0x0000800B
        err = glGetError();
    }

Specifically it happens when checking format TextureFormat::ETC1 (but if I manually skip this one it happens for other formats as well)

Not sure you can do much about it since it looks to be hardware related (crashes in an intel graphics dll ig9icd32.dll) My gpu is Intel HD Graphics 520

I just comment out the MIP_AUTOGEN check for a quick kludge workaround, for the rest all seems to work fine

bkaradzic commented 7 years ago

Is glGenerateMipmap NULL?

hugoam commented 7 years ago

No, it' not NULL, the call succeeds for the 7 first formats before ETC1. I also just checked that the samples run fine but that's because they select D3D11 by default. If I force openGL, it crashes just the same as my app.

jamesmintram commented 7 years ago

This has just happened to me on iOS using openGLES at approximately the same point.

This call fails:

glCompressedTexImage2D(GL_TEXTURE_2D, 0, internalFmt, 16, 16, 0, size, data);

With a crash in:

#0  0x33b80224 in glrSetTextureDataType(GLDShareGroupRec*, GLDTextureRec*) ()
#1  0x33b7efe4 in glrUpdateTexture ()
#2  0x30e0ebce in gldModifyTexSubImage ()
#3  0x340d2210 in glCompressedTexImage2D_Exec ()
#4  0x26306be0 in glCompressedTexImage2D ()
#5  0x000e9fec in bgfx::gl::initTestTexture(bgfx::TextureFormat::Enum, bool, bool) at /Users/james/bgfx/bgfx/bgfx/src/renderer_gl.cpp:1183
#6  0x00107f30 in bgfx::gl::isTextureFormatValid(bgfx::TextureFormat::Enum, bool, bool) at /Users/james/bgfx/bgfx/bgfx/src/renderer_gl.cpp:1233
#7  0x000ebbd8 in bgfx::gl::RendererContextGL::init() at /Users/james/bgfx/bgfx/bgfx/src/renderer_gl.cpp:1861
#8  0x000ea4a8 in bgfx::gl::rendererCreate() at /Users/james/bgfx/bgfx/bgfx/src/renderer_gl.cpp:3564

Full log output: https://gist.github.com/jamesmintram/e3cde5b15790602057e32c9059a21a4c Revision: 24569934a17119fa5432a50438055021573e9ff0

jamesmintram commented 7 years ago

Just tried a recent revision of BGFX - and it also crashes in a similar fashion:

Call:

            glCompressedTexImage2D(
                  _target
                , _level
                , _internalformat
                , _width
                , _height
                , _border
                , _imageSize
                , _data
                );

Callstack:

#0  0x33b80224 in glrSetTextureDataType(GLDShareGroupRec*, GLDTextureRec*) ()
#1  0x33b7efe4 in glrUpdateTexture ()
#2  0x30e0ebce in gldModifyTexSubImage ()
#3  0x340d2210 in glCompressedTexImage2D_Exec ()
#4  0x26306be0 in glCompressedTexImage2D ()
#5  0x00112cf0 in bgfx::gl::compressedTexImage(unsigned int, int, unsigned int, int, int, int, int, int, void const*) at /Users/james/bgfx_new/bgfx/src/renderer_gl.cpp:1387
#6  0x00112a88 in bgfx::gl::initTestTexture(bgfx::TextureFormat::Enum, bool, bool, bool, int) at /Users/james/bgfx_new/bgfx/src/renderer_gl.cpp:1438
#7  0x0012fcc4 in bgfx::gl::isTextureFormatValid(bgfx::TextureFormat::Enum, bool, bool, bool, int) at /Users/james/bgfx_new/bgfx/src/renderer_gl.cpp:1492

Full output log: https://gist.github.com/jamesmintram/16f6983611f068f607ae7c0289afe092 Revision: 0c204d33809a29f3146d3732d778aba599813547

jamesmintram commented 7 years ago

OK so I "Fixed it" - but I doubt it is a proper fix :)

I added updated the loop further down to also start at TextureFormat::Unkown on iOS.

           const int first_format = BX_ENABLED(BX_PLATFORM_IOS) ? TextureFormat::Unknown : 0;// skip test on iOS!
            for (uint32_t ii = first_format
                ; ii < TextureFormat::Count
                ; ++ii
                )
            {
                if (TextureFormat::Unknown != ii
                &&  TextureFormat::UnknownDepth != ii)
                {
                    s_textureFormat[ii].m_supported = isTextureFormatValid(TextureFormat::Enum(ii) );
                }
            }

            if (BX_ENABLED(0) )
            {
                // Disable all compressed texture formats. For testing only.
                for (uint32_t ii = 0; ii < TextureFormat::Unknown; ++ii)
                {
                    s_textureFormat[ii].m_supported = false;
                }
            }

            const bool computeSupport = false
                || !!(BGFX_CONFIG_RENDERER_OPENGLES >= 31)
                || s_extension[Extension::ARB_compute_shader].m_supported
                ;

            for (uint32_t ii = first_format; ii < TextureFormat::Count; ++ii)
            {
                uint16_t supported = BGFX_CAPS_FORMAT_TEXTURE_NONE;
bkaradzic commented 7 years ago

Doubt it's proper fix, since it will skip all compressed textures, which will lead decompression in runtime.

jamesmintram commented 7 years ago

I thought as much. So I guess there is some issue the iOS OpenGLES drivers are having when enumerating all of those different compressed textures. I played around a bit before applying that fix hack/workaround. I tried to recreate the crash by passing ETC1 in every time, alternating between BC1 and ETC1 and neither caused it to crash.

I was going to try mixing up the order in which the formats are tested to see if that could change the outcome - but I ran out of time and applied that workaround.

I could take another look tomorrow?

BTW It only happens on device - ios simulator is fine,

bkaradzic commented 7 years ago

For iOS probably proper fix is just to hardcode available formats and parameters, or alternatively just disable test for those that we know crash driver 100%, and let test those that are fine.