Skip to content

Robust GPU error recovery [Windows] #99

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 12 commits into
base: stable
Choose a base branch
from
Draft

Robust GPU error recovery [Windows] #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
223e2b8
WIP
dvijayak Mar 13, 2024
b63b8f5
More WIP
dvijayak Mar 15, 2024
8a3b5fa
Better idea. WIP
dvijayak Mar 27, 2024
33f73bf
Add stubs for non-windows platforms.
dvijayak Mar 27, 2024
be4ad33
Even better idea: use a tlocal queue. Changed error to be scoped to a…
dvijayak Apr 3, 2024
c014061
Cleanup
dvijayak Apr 3, 2024
b4f1412
Total simplification....let matoya fix itself.
dvijayak May 1, 2024
d2b88a9
Added logs so its clear when d3d11 ctx is reiniting.
dvijayak May 6, 2024
1af162e
Upon d3d11 device and ctx reinit due to fatal errors, ensure that the…
dvijayak May 6, 2024
c7c3d66
TEST: logs to help figure out what's going on
dvijayak May 6, 2024
3066a96
MTY_WindowSetUITexture no longer indiscriminately deletes the existin…
dvijayak May 7, 2024
6473bde
Merge remote-tracking branch 'parsec/stable' into gpu-error-handler
dvijayak May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions src/app.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ static void gfx_set_device(struct window_common *cmn, MTY_Device *device)
GFX_UI_API[cmn->api].destroy(&cmn->gfx_ui, cmn->device);

cmn->device = device;
cmn->device_changed = true;
}
}

Expand All @@ -96,8 +97,12 @@ static bool gfx_begin_ui(struct window_common *cmn, MTY_Device *device)
if (!cmn->gfx_ui)
cmn->gfx_ui = GFX_UI_API[cmn->api].create(device);

if (!cmn->ui_textures)
if (!cmn->gfx_ui) {
MTY_HashDestroy(&cmn->ui_textures, NULL);

} else if (!cmn->ui_textures) {
cmn->ui_textures = MTY_HashCreate(0);
}

return cmn->gfx_ui != NULL;
}
Comment on lines 97 to 108
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the gfx_ui state could not be created from the given device, then we want to clean out the hash of textures.

Expand Down Expand Up @@ -213,18 +218,28 @@ bool MTY_WindowSetUITexture(MTY_App *app, MTY_Window window, uint32_t id, const
return false;

MTY_Device *device = gfx_ctx_get_device(cmn);
if (!device)
return false;
Comment on lines 220 to +222
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid a bunch of error scenarios by not allowing the function to proceed if the graphics context doesn't have a good device handle. This scenario occurs when the GPU resets.


bool r = gfx_begin_ui(cmn, device);

if (r) {
void *texture = MTY_HashPopInt(cmn->ui_textures, id);
GFX_UI_API[cmn->api].destroy_texture(cmn->gfx_ui, &texture, device);
void *texture = MTY_HashGetInt(cmn->ui_textures, id);

if (rgba) {
void *texture_new = GFX_UI_API[cmn->api].create_texture(cmn->gfx_ui, device, rgba, width, height);

if (rgba)
texture = GFX_UI_API[cmn->api].create_texture(cmn->gfx_ui, device, rgba, width, height);
if (texture_new) {
GFX_UI_API[cmn->api].destroy_texture(cmn->gfx_ui, &texture, device);
Comment on lines +232 to +233
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check if texture is NULL before destroying it. An up-front check is ideal, even if the individual APIs may check too


if (texture)
MTY_HashSetInt(cmn->ui_textures, id, texture);
texture = texture_new;
MTY_HashSetInt(cmn->ui_textures, id, texture_new);
}

} else {
GFX_UI_API[cmn->api].destroy_texture(cmn->gfx_ui, &texture, device);
MTY_HashPopInt(cmn->ui_textures, id);
}

r = texture != NULL;
}
Comment on lines 225 to 245
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old logic is to first destroy the existing texture for id and get rid of it from the textures hash, and then carry on with creating the new texture with the given pixel data rgba if it was provide, finally updating the hash node for this id with the new texture handle.

The problem with this is that when create_texture fails and/or returns a null texture handle, the existing perfectly-fine texture is removed. This failure can occur when the GPU resets, and because the entry is removed from the hash indiscriminately, MTY_WindowHasUITexture thereafter always returns false for the given id, which is incorrect behavior.

My fix is to replace the old/existing texture with the new one IFF the new texture was successfully created. After this change, even when the GPU resets, textures are rendered properly because the hash still maintains a reference to the original valid handle.

Expand Down
2 changes: 2 additions & 0 deletions src/app.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ struct window_common {
struct gfx *gfx[APP_GFX_LAYERS];
struct gfx_ui *gfx_ui;
struct gfx_ctx *gfx_ctx;

bool device_changed;
};

// App
Expand Down
10 changes: 9 additions & 1 deletion src/windows/appw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2062,7 +2062,15 @@ void MTY_WindowWarpCursor(MTY_App *app, MTY_Window window, uint32_t x, uint32_t

MTY_ContextState MTY_WindowGetContextState(MTY_App *app, MTY_Window window)
{
return MTY_CONTEXT_STATE_NORMAL;
MTY_ContextState state = MTY_CONTEXT_STATE_NORMAL;

struct window_common *cmn = mty_window_get_common(app, window);
if (cmn && cmn->api != MTY_GFX_NONE && cmn->device_changed) {
state = MTY_CONTEXT_STATE_NEW;
cmn->device_changed = false;
}

return state;
}
Comment on lines 2063 to 2074
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the device has changed or been reset, it is a mistake not to inform clients of matoya that textures need to be resubmitted.


void *MTY_WindowGetNative(MTY_App *app, MTY_Window window)
Expand Down
69 changes: 59 additions & 10 deletions src/windows/gfx/d3d11-ctx.c
View file
Open in desktop
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All changes in this file implement a more robust GPU error handling. This portion concerns the main graphics context state.

Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ GFX_CTX_PROTOTYPES(_d3d11_)

#define DXGI_FATAL(e) ( \
(e) == DXGI_ERROR_DEVICE_REMOVED || \
(e) == DXGI_ERROR_DRIVER_INTERNAL_ERROR || \
(e) == DXGI_ERROR_DEVICE_HUNG || \
(e) == DXGI_ERROR_DEVICE_RESET \
)

#define D3D11_CTX_WAIT 2000

struct d3d11_ctx {
bool ready;
HWND hwnd;
struct sync sync;
struct dxgi_sync *dxgi_sync;
Expand All @@ -46,6 +48,8 @@ static void d3d11_ctx_get_size(struct d3d11_ctx *ctx, uint32_t *width, uint32_t

static void d3d11_ctx_free(struct d3d11_ctx *ctx)
{
ctx->ready = false;

if (ctx->back_buffer)
ID3D11RenderTargetView_Release(ctx->back_buffer);

Expand All @@ -55,8 +59,11 @@ static void d3d11_ctx_free(struct d3d11_ctx *ctx)
if (ctx->swap_chain2)
IDXGISwapChain2_Release(ctx->swap_chain2);

if (ctx->context)
if (ctx->context) {
ID3D11DeviceContext_Flush(ctx->context);
ID3D11DeviceContext_ClearState(ctx->context);
ID3D11DeviceContext_Release(ctx->context);
}

if (ctx->device)
ID3D11Device_Release(ctx->device);
Expand Down Expand Up @@ -187,10 +194,32 @@ static bool d3d11_ctx_init(struct d3d11_ctx *ctx)
if (device2)
IDXGIDevice2_Release(device2);

if (e != S_OK)
ctx->ready = e == S_OK;

if (!ctx->ready)
d3d11_ctx_free(ctx);

return e == S_OK;
return ctx->ready;
}

static bool d3d11_ctx_reinit(struct d3d11_ctx *ctx)
{
MTY_Log("Reinitializing d3d11_ctx...");
d3d11_ctx_free(ctx);
bool r = d3d11_ctx_init(ctx);
if (r)
MTY_Log("d3d11_ctx reinit ok.");
else
MTY_Log("d3d11_ctx reinit FAILED!");
Comment on lines +210 to +213
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No braces, and I'm not certain we need to log two times every time


return r;
}

int32_t mty_d3d11_error_handler_default(int32_t e1, int32_t e2, void *opaque)
{
e1; e2; opaque;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually put each of these on a different line


return 0;
}

struct gfx_ctx *mty_d3d11_ctx_create(void *native_window, bool vsync)
Expand Down Expand Up @@ -235,18 +264,27 @@ MTY_Device *mty_d3d11_ctx_get_device(struct gfx_ctx *gfx_ctx)
{
struct d3d11_ctx *ctx = (struct d3d11_ctx *) gfx_ctx;

if (!ctx->ready)
d3d11_ctx_reinit(ctx);

return (MTY_Device *) ctx->device;
}

MTY_Context *mty_d3d11_ctx_get_context(struct gfx_ctx *gfx_ctx)
{
struct d3d11_ctx *ctx = (struct d3d11_ctx *) gfx_ctx;

if (!ctx->ready)
d3d11_ctx_reinit(ctx);

return (MTY_Context *) ctx->context;
}

static void d3d11_ctx_refresh(struct d3d11_ctx *ctx)
{
if (!ctx->ready && !d3d11_ctx_reinit(ctx))
return;

uint32_t width = ctx->width;
uint32_t height = ctx->height;

Expand All @@ -263,8 +301,7 @@ static void d3d11_ctx_refresh(struct d3d11_ctx *ctx)

if (DXGI_FATAL(e)) {
MTY_Log("'IDXGISwapChain2_ResizeBuffers' failed with HRESULT 0x%X", e);
d3d11_ctx_free(ctx);
d3d11_ctx_init(ctx);
ctx->ready = false;
}
}
}
Expand All @@ -273,6 +310,10 @@ MTY_Surface *mty_d3d11_ctx_get_surface(struct gfx_ctx *gfx_ctx)
{
struct d3d11_ctx *ctx = (struct d3d11_ctx *) gfx_ctx;

if (!ctx->ready && !d3d11_ctx_reinit(ctx))
return NULL;

HRESULT e = S_OK;
ID3D11Resource *resource = NULL;

if (!ctx->swap_chain2)
Expand All @@ -281,22 +322,26 @@ MTY_Surface *mty_d3d11_ctx_get_surface(struct gfx_ctx *gfx_ctx)
if (!ctx->back_buffer) {
d3d11_ctx_refresh(ctx);

HRESULT e = IDXGISwapChain2_GetBuffer(ctx->swap_chain2, 0, &IID_ID3D11Resource, &resource);
e = IDXGISwapChain2_GetBuffer(ctx->swap_chain2, 0, &IID_ID3D11Resource, &resource);
if (e != S_OK) {
MTY_Log("'IDXGISwapChain2_GetBuffer' failed with HRESULT 0x%X", e);
goto except;
}

e = ID3D11Device_CreateRenderTargetView(ctx->device, resource, NULL, &ctx->back_buffer);
if (e != S_OK)
if (e != S_OK) {
MTY_Log("'ID3D11Device_CreateRenderTargetView' failed with HRESULT 0x%X", e);
goto except;
}
}

except:

if (resource)
ID3D11Resource_Release(resource);

ctx->ready = e == S_OK;

return (MTY_Surface *) ctx->back_buffer;
}

Expand All @@ -314,6 +359,9 @@ void mty_d3d11_ctx_present(struct gfx_ctx *gfx_ctx)
{
struct d3d11_ctx *ctx = (struct d3d11_ctx *) gfx_ctx;

if (!ctx->ready && !d3d11_ctx_reinit(ctx))
return;

if (ctx->back_buffer) {
int64_t interval = sync_next_interval(&ctx->sync);
UINT flags = interval > 0 ? 0 : DXGI_PRESENT_ALLOW_TEARING;
Expand All @@ -335,13 +383,14 @@ void mty_d3d11_ctx_present(struct gfx_ctx *gfx_ctx)

if (DXGI_FATAL(e)) {
MTY_Log("'IDXGISwapChain2_Present' failed with HRESULT 0x%X", e);
d3d11_ctx_free(ctx);
d3d11_ctx_init(ctx);
ctx->ready = false;

} else {
DWORD we = WaitForSingleObjectEx(ctx->waitable, D3D11_CTX_WAIT, TRUE);
if (we != WAIT_OBJECT_0)
if (we != WAIT_OBJECT_0) {
MTY_Log("'WaitForSingleObjectEx' failed with error 0x%X", we);
ctx->ready = false;
}
}
}
}
Expand Down
28 changes: 21 additions & 7 deletions src/windows/gfx/d3d11-ui.c
View file
Open in desktop
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All changes in this file implement a more robust GPU error handling. This portion concerns state pertaining to rendering UI.

Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ struct d3d11_ui_buffer {
};

struct d3d11_ui {
bool ready;
struct d3d11_ui_buffer vb;
struct d3d11_ui_buffer ib;
ID3D11VertexShader *vs;
Expand Down Expand Up @@ -149,7 +150,9 @@ struct gfx_ui *mty_d3d11_ui_create(MTY_Device *device)

except:

if (e != S_OK)
ctx->ready = e == S_OK;

if (!ctx->ready)
mty_d3d11_ui_destroy((struct gfx_ui **) &ctx, device);

return (struct gfx_ui *) ctx;
Expand Down Expand Up @@ -196,6 +199,9 @@ bool mty_d3d11_ui_render(struct gfx_ui *gfx_ui, MTY_Device *device, MTY_Context
const MTY_DrawData *dd, MTY_Hash *cache, MTY_Surface *dest)
{
struct d3d11_ui *ctx = (struct d3d11_ui *) gfx_ui;
if (!ctx->ready)
return false;

ID3D11Device *_device = (ID3D11Device *) device;
ID3D11DeviceContext *_context = (ID3D11DeviceContext *) context;
ID3D11RenderTargetView *_dest = (ID3D11RenderTargetView *) dest;
Expand All @@ -204,30 +210,32 @@ bool mty_d3d11_ui_render(struct gfx_ui *gfx_ui, MTY_Device *device, MTY_Context
if (dd->displaySize.x <= 0 || dd->displaySize.y <= 0 || dd->cmdListLength == 0)
return false;

bool result = false;

// Resize vertex and index buffers if necessary
HRESULT e = d3d11_ui_resize_buffer(_device, &ctx->vb, dd->vtxTotalLength, GFX_UI_VTX_INCR, sizeof(MTY_Vtx),
D3D11_BIND_VERTEX_BUFFER);
if (e != S_OK)
return false;
goto except;

e = d3d11_ui_resize_buffer(_device, &ctx->ib, dd->idxTotalLength, GFX_UI_IDX_INCR, sizeof(uint16_t),
D3D11_BIND_INDEX_BUFFER);
if (e != S_OK)
return false;
goto except;

// Map both vertex and index buffers and bulk copy the data
D3D11_MAPPED_SUBRESOURCE vtx_map = {0};
e = ID3D11DeviceContext_Map(_context, ctx->vb.res, 0, D3D11_MAP_WRITE_DISCARD, 0, &vtx_map);
if (e != S_OK) {
MTY_Log("'ID3D11DeviceContext_Map' failed with HRESULT 0x%X", e);
return false;
goto except;
}

D3D11_MAPPED_SUBRESOURCE idx_map = {0};
e = ID3D11DeviceContext_Map(_context, ctx->ib.res, 0, D3D11_MAP_WRITE_DISCARD, 0, &idx_map);
if (e != S_OK) {
MTY_Log("'ID3D11DeviceContext_Map' failed with HRESULT 0x%X", e);
return false;
goto except;
}

MTY_Vtx *vtx_dst = (MTY_Vtx *) vtx_map.pData;
Expand Down Expand Up @@ -261,7 +269,7 @@ bool mty_d3d11_ui_render(struct gfx_ui *gfx_ui, MTY_Device *device, MTY_Context
D3D11_MAPPED_SUBRESOURCE cb_map = {0};
e = ID3D11DeviceContext_Map(_context, ctx->cb_res, 0, D3D11_MAP_WRITE_DISCARD, 0, &cb_map);
if (e != S_OK)
return false;
goto except;

struct d3d11_ui_cb *cb = (struct d3d11_ui_cb *) cb_map.pData;
memcpy(&cb->proj, proj, sizeof(proj));
Expand Down Expand Up @@ -338,7 +346,13 @@ bool mty_d3d11_ui_render(struct gfx_ui *gfx_ui, MTY_Device *device, MTY_Context
vtxOffset += cmdList->vtxLength;
}

return true;
result = true;

except:

ctx->ready = e == S_OK;

return result;
}

void *mty_d3d11_ui_create_texture(struct gfx_ui *gfx_ui, MTY_Device *device, const void *rgba,
Expand Down