Skip to content

Conversation

@RT2Code
Copy link
Contributor

@RT2Code RT2Code commented Oct 20, 2025

Add optional CheckHrResultFn member to ImGui_ImplDX12_InitInfo for custom error handling, matching Vulkan backend. Also fix a couple of missing error checks.

In the example file, I changed CreateDeviceD3D to return HRESULT instead of bool, preserving existing error and cleanup logic. However, I think direct error checks and early aborts would be better here. It would simplify the code and cleanup is not handled properly anyway and is very difficult to manage without RAII.

To be clear, I suggest replacing this:

    HRESULT hr = CreateDeviceD3D(hwnd);
    if (FAILED(hr))
    {
        CleanupDeviceD3D();
        ::UnregisterClassW(wc.lpszClassName, wc.hInstance);
        check_hr_result(hr);
    }
    [...]
    HRESULT CreateDeviceD3D(HWND hWnd)
    {
        [...]
        HRESULT hr = D3D12CreateDevice(nullptr, featureLevel, IID_PPV_ARGS(&g_pd3dDevice));
        if (FAILED(hr))
            return hr;
        [...]
    }

By this:

    CreateDeviceD3D(hwnd);
    [...]
    void CreateDeviceD3D(HWND hWnd)
    {
        [...]
        HRESULT hr = D3D12CreateDevice(nullptr, featureLevel, IID_PPV_ARGS(&g_pd3dDevice));
        check_hr_result(hr);
        [...]
    }

If you agree, I will make these changes.

@ocornut
Copy link
Owner

ocornut commented Oct 22, 2025

  • What's your context and what were you actually trying to solve? Was it in a general reaction to you toying with DX12 code and wanting more thorough reports?
  • If your overall goal was to target developer rather than end-user I wonder if the approach used in imgui_impl_opengl3 backend might work better? (I'm not sure, but wondering)
  • Backends: there are a few failure points but many never effectively fail, and it's possible that some of the silent one are here for a reason and turning them non-silent is not desirable. returns have been turned into opaque call to error handlers I'm not sure that's right. Not sure why some the returns were removed, it will lead to silence and/or printout + crash. Whereas in principle we want to crash on programming errors, the renderer backend choking on unexpected outside event should probably not crash (but it could report crash).
  • If anything there should be a good default in the backend, right now there's none.
  • Examples should not use std::string.
  • Not very thrilled that the example file is +50 lines. This is meant to be an example to be read by user to learn/understand the library. I'd rather have it crash in debugger in 0.1% cases rather than be cluttered in endless checks that will also simply crash just differently.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants