d3d11 memleak. RGBA4 / BGRA4 conversion code is not functional in D3D11::update

[jamesfAnet]

https://github.com/bkaradzic/bgfx/blob/a855f1cf2dde73dc26346768b032530ca200c1b3/src/renderer_d3d11.cpp#L4818

@bwrsandman

There's a few issues going on with recent changes to TextureD3D11::update

Here's code for reference, with some interleaved comments

        {
            uint8_t* src = data;
            // (jamesf) this loop is going to happen for every call to this function, regardless of format. I think this is too much pointless work that could be avoided. I think this loop should be in an if (R5G6B5/RGBA4/RGB5A1) check
            for (uint32_t yy = 0, height = _rect.m_height; yy < height; yy += blockHeight)
            {
                switch (m_textureFormat)
                {
                case TextureFormat::R5G6B5:
                    temp = (uint8_t*)BX_ALLOC(g_allocator, rectpitch); // allocating here every loop..
                    bimg::imageConvert(temp, 16, bx::packB5G6R5, src, bx::unpackR5G6B5, rectpitch);
                    data = temp;
                    break;
                case TextureFormat::RGBA4:
                    temp = (uint8_t*)BX_ALLOC(g_allocator, rectpitch);
                    bimg::imageConvert(temp, 16, bx::packBgra4, src, bx::unpackRgba4, rectpitch);
                    data = temp;
                    break;
                case TextureFormat::RGB5A1:
                    temp = (uint8_t*)BX_ALLOC(g_allocator, rectpitch);
                    bimg::imageConvert(temp, 16, bx::packBgr5a1, src, bx::unpackRgb5a1, rectpitch);
                    data = temp;
                    break;
                }
                src += srcpitch;
            }
            // (jamesf) at end of loop, 'data' is essentially pointing at last row of image
        }

        deviceCtx->UpdateSubresource(
              m_ptr
            , subres
            , depth ? NULL : &box
            , data
            , srcpitch
            , TextureD3D11::Texture3D == m_type ? slicepitch : 0
            );

        if (NULL != temp)
        {
            BX_FREE(g_allocator, temp); // (jamesf) freeing once at the end, leaking "loop count - 1"
        }

`

[jamesfAnet]

might be worth adding these formats to 08-update example

[bkaradzic]

Oh wow, that code is quite idiotic...

I deleted it: https://github.com/bkaradzic/bgfx/commit/27602120e9af88ac36913e94824ed5ba8ec84864

[jamesfAnet]

That works. Adding an assert or something might be nice. Now if someone went down this codepath with an RGBA4 it would upload swizzled, but if they provide the mem in createTexture2D it will be correct. See nearly matching bimg::imageConvert calls in TextureD3D11::create.

I am actually using RGBA4 so it was weird to sync latest and see those textures breaking. The fix for me was to just swap to new BGRA4 enum since all my use-cases are greyscale. Seems like that's how the backend actually wants it anyway. Feel free to close this issue if you'd like.

[bkaradzic]

This just need to be re-implemented and tested.

[bwrsandman]

Similar conversion was done in D3D9, there could be an issue there too

[bkaradzic]

@bwrsandman But it doesn't allocate memory per row: https://github.com/bkaradzic/bgfx/blob/9e7aa2d2eff46087d68d801c38d0891d649b63af/src/renderer_d3d9.cpp#L3115-L3141