[sokol_gfx d3d11] update_image is wrong for 3d textures

[jakubtomsu]

I ran into a bug where the uploaded 3d texture was wrong in a standalone application, but it was fine in renderdoc. I believe it's related to how the data is copied into the mapped subresource in update_image. There is also this comment:

https://github.com/floooh/sokol/blob/c54523c078e481d3084fa0b4630d2ce3d3e1e74f/sokol_gfx.h#L11461-L11465

I think I'm gonna work around this issue in my renderer code with sg.d3d11_query_image_info and calling D3D11 mysel, as a quick fix for now. But I could look into fixing this issue later.

[floooh]

Deleted my previous comment because I wrote rubbish :D But maybe the tex3d sample (https://github.com/floooh/sokol-samples/blob/master/sapp/tex3d-sapp.c) can still be helpful even if it doesn't dynamically upload data each frame, but maybe it can still be a good playground to investigate the issue.

One thing that's different from other 3D backends is that there's separate code to populate a new texture, or update an existing texture. For new textures this is the important code:

https://github.com/floooh/sokol/blob/c54523c078e481d3084fa0b4630d2ce3d3e1e74f/sokol_gfx.h#L10408-L10434

...while for updating an existing texture this code is used:

https://github.com/floooh/sokol/blob/c54523c078e481d3084fa0b4630d2ce3d3e1e74f/sokol_gfx.h#L11448-L11483

...both loop over face_index, slice_index and mip_index.

It could be that the bug is in both places, or only in the update-image code, maybe it makes sense to share a bit of code between those two places (not sure yet if that makes sense though).

In the Metal and WebGPU backends, creating and updating a texture uses a common helper function:

• Metal: https://github.com/floooh/sokol/blob/c54523c078e481d3084fa0b4630d2ce3d3e1e74f/sokol_gfx.h#L12288-L12334
• WebGPU: https://github.com/floooh/sokol/blob/c54523c078e481d3084fa0b4630d2ce3d3e1e74f/sokol_gfx.h#L14484-L14528

...which is a lot cleaner (but Metal and WebGPU are different in that the texture object is created without data, and the data is copied into the object after creation), but as I said above, not sure how much the two code paths can be unified in the D3D11 backend.

[jakubtomsu]

Thanks for the example. In my case it's about uploading dynamic texture every few frames but it's still handy.

I just implemented the code for uploading the texture directly with d3d11, and it works correctly now. But I found out update_image works fine with size 128^3 and up, but didn't work for size 32^3.

This is the uploading code I ended up with, note the difference between dst_offset and src_offset. I believe update_image should have something like src_depth_pitch.

        // FIXME:
        // sg.update_image(
        //     ren.blood_volume_img,
        //     sg.Image_Data{subimage = {0 = {0 = {ptr = &game.blood, size = size_of(game.blood)}}}},
        // )

        img := sg.d3d11_query_image_info(ren.blood_volume_img)
        res := transmute(^d3d11.IResource)img.res

        msr: d3d11.MAPPED_SUBRESOURCE
        hr := d3d11_device_context->Map(res, 0, .WRITE_DISCARD, {}, &msr)
        fmt.println(hr)
        fmt.println(msr)
        fmt.println(img)
        if hr == 0 {
            defer d3d11_device_context->Unmap(res, 0)

            // NOTE: this code assumes a pixel is one byte
            for z in 0 ..< BLOOD_BOUNDS_Z {
                for y in 0 ..< BLOOD_BOUNDS_Y {
                    dst_offset := uintptr(z * int(msr.DepthPitch) + y * int(msr.RowPitch))
                    src_offset := uintptr(z * BLOOD_BOUNDS_X * BLOOD_BOUNDS_Y + y * BLOOD_BOUNDS_X)
                    runtime.mem_copy(
                        rawptr(uintptr(msr.pData) + dst_offset),
                        rawptr(uintptr(&game.blood) + src_offset),
                        BLOOD_BOUNDS_X * size_of(u8),
                    )
                }
            }
        }

[floooh]

Okidoki, thanks a lot for the investigation and sample code! I'll try to set aside some time next week for a new sample/regression-test and fixing the issue.

In general, the idea for uploading texture data is that the incoming data should be tightly packed (e.g. no pitch-related gaps between rows and slices - because different backends have different requirements for those anyway), and sokol_gfx.h will take care of 'unwrapping' the data, even if that means doing small per-row memcpy's.

At least that's the idea for now.

At some point in the future I want to make the resource update functions more orthogonal, first by providing more orthogonal functions, at least:

• copy-buffer-to-buffer
• write-mem-to-buffer
• copy-image-to-image
• write-mem-to-image
• ...?

...and ideally also allowing to update only parts of the resource, and with custom row/slice pitch - and implementing "fast paths" if those things match the backend API expectations (e.g. doing a single big copy instead of many small copies).

...it'll be a while until I get to that though, probably after the computer-shader stuff...

[jakubtomsu]

Sounds good, thank you! Fixing this would be great. And more modern uploading and writing functions would be nice to have as well, but I agree the compute stuff is higher priority.

[floooh]

...starting to look into this now. Might take a couple of days because I'll build a new sample around it.

[floooh]

Ok, I can reproduce the issue, and also confirm that it only seems affects the D3D11 backend (not 100% sure though because the texture size where the problem manifests seems to differ between hardware configs, see below).

Interestingly, on my Intel GPU laptop, the cutoff size is 32x32 (with RGBA8 pixels). A RGBA8 3D texture of size 32x32x3 still works (e.g. one depth slice is 32 * 32 * sizeof(uint32_t) = 4096 bytes. The required depth-pitch 4096 matches the slice size.

However going down to a 16x16x3 textures breaks. D3D11 reports a required depth-pitch of 2048, while one slice is 1024 bytes.

I'll work on a fix over the next few evenings (had to squeeze an unexpected PR and fix for sokol-shdc earlier today).

[floooh]

Ok, fixed in https://github.com/floooh/sokol/issues/1063, but before merging I want to make the new sample in sokol-samples a bit more flexible, and also do a bit more testing (ETA tomorrow or the day after).

[floooh]

Ok, I extended the sample to test all sorts of texture sizes and also dynamic vs immutable 3D textures (this is recorded on Mac with the Metal backend, but I also tested on my Windows laptop with Intel GPU, and gaming PC with nvidia rtx2070. Merge and website update incoming (I also stumbled over an unrelated problem in the WebGPU backend, but that's not important enough to delay the merge: https://github.com/floooh/sokol/issues/1066)

https://github.com/floooh/sokol/assets/1699414/1e74f12f-2b49-4194-a163-7ce64981a07a

[floooh]

Ok, merged. Next up is merging the new sample and updating the sample web pages.

Thanks for the bug report!

[jakubtomsu]

Fantastic! Thank you so much for your work:)