Skip to content

Commit

Permalink
AtlasEngine: Fix several error handling bugs (#17193)
Browse files Browse the repository at this point in the history
This fixes:
* `HRESULT`s not being shown as unsigned hex
* `D2DERR_RECREATE_TARGET` not being handled
* 4 calls not checking their `HRESULT` return
  Out of the 4 only `CreateCompatibleRenderTarget` will throw in
  practice, however it throws `D2DERR_RECREATE_TARGET` which is common.
  Without this error handling, AtlasEngine may crash.

* Set Graphics API to Direct2D
* Use `DXGIAdapterRemovalSupportTest.exe` to trigger
  `D2DERR_RECREATE_TARGET`
* No error message is shown ✅
* If the `D2DERR_RECREATE_TARGET` handling is removed, the application
  never crashes due to `cursorRenderTarget` being `nullptr` ✅

(cherry picked from commit b31059e)
Service-Card-Id: 92500372
Service-Version: 1.20
  • Loading branch information
lhecker authored and DHowett committed May 6, 2024
1 parent 345d266 commit ed2923b
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 8 deletions.
4 changes: 3 additions & 1 deletion src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -995,7 +995,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
winrt::fire_and_forget TermControl::_RendererWarning(IInspectable /*sender*/,
Control::RendererWarningArgs args)
{
const auto hr = static_cast<HRESULT>(args.Result());
// HRESULT is a signed 32-bit integer which would result in a hex output like "-0x7766FFF4",
// but canonically HRESULTs are displayed unsigned as "0x8899000C". See GH#11556.
const auto hr = std::bit_cast<uint32_t>(args.Result());

auto weakThis{ get_weak() };
co_await wil::resume_foreground(Dispatcher());
Expand Down
2 changes: 1 addition & 1 deletion src/renderer/atlas/AtlasEngine.r.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ catch (const wil::ResultException& exception)
{
const auto hr = exception.GetErrorCode();

if (hr == DXGI_ERROR_DEVICE_REMOVED || hr == DXGI_ERROR_DEVICE_RESET)
if (hr == DXGI_ERROR_DEVICE_REMOVED || hr == DXGI_ERROR_DEVICE_RESET || hr == D2DERR_RECREATE_TARGET)
{
_p.dxgi = {};
return E_PENDING;
Expand Down
11 changes: 6 additions & 5 deletions src/renderer/atlas/BackendD2D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,11 @@ void BackendD2D::_handleSettingsUpdate(const RenderingPayload& p)
_viewportCellCount = p.s->viewportCellCount;
}

void BackendD2D::_drawBackground(const RenderingPayload& p) noexcept
void BackendD2D::_drawBackground(const RenderingPayload& p)
{
if (_backgroundBitmapGeneration != p.colorBitmapGenerations[0])
{
_backgroundBitmap->CopyFromMemory(nullptr, p.backgroundBitmap.data(), gsl::narrow_cast<UINT32>(p.colorBitmapRowStride * sizeof(u32)));
THROW_IF_FAILED(_backgroundBitmap->CopyFromMemory(nullptr, p.backgroundBitmap.data(), gsl::narrow_cast<UINT32>(p.colorBitmapRowStride * sizeof(u32))));
_backgroundBitmapGeneration = p.colorBitmapGenerations[0];
}

Expand Down Expand Up @@ -350,7 +350,7 @@ f32r BackendD2D::_getGlyphRunDesignBounds(const DWRITE_GLYPH_RUN& glyphRun, f32
_glyphMetrics = Buffer<DWRITE_GLYPH_METRICS>{ size };
}

glyphRun.fontFace->GetDesignGlyphMetrics(glyphRun.glyphIndices, glyphRun.glyphCount, _glyphMetrics.data(), false);
THROW_IF_FAILED(glyphRun.fontFace->GetDesignGlyphMetrics(glyphRun.glyphIndices, glyphRun.glyphCount, _glyphMetrics.data(), false));

const f32 fontScale = glyphRun.fontEmSize / fontMetrics.designUnitsPerEm;
f32r accumulatedBounds{
Expand Down Expand Up @@ -535,7 +535,8 @@ void BackendD2D::_resizeCursorBitmap(const RenderingPayload& p, const til::size
const D2D1_SIZE_F sizeF{ static_cast<f32>(newSizeInPx.width), static_cast<f32>(newSizeInPx.height) };
const D2D1_SIZE_U sizeU{ gsl::narrow_cast<UINT32>(newSizeInPx.width), gsl::narrow_cast<UINT32>(newSizeInPx.height) };
wil::com_ptr<ID2D1BitmapRenderTarget> cursorRenderTarget;
_renderTarget->CreateCompatibleRenderTarget(&sizeF, &sizeU, nullptr, D2D1_COMPATIBLE_RENDER_TARGET_OPTIONS_NONE, cursorRenderTarget.addressof());
THROW_IF_FAILED(_renderTarget->CreateCompatibleRenderTarget(&sizeF, &sizeU, nullptr, D2D1_COMPATIBLE_RENDER_TARGET_OPTIONS_NONE, cursorRenderTarget.addressof()));

cursorRenderTarget->SetAntialiasMode(D2D1_ANTIALIAS_MODE_ALIASED);

cursorRenderTarget->BeginDraw();
Expand All @@ -547,7 +548,7 @@ void BackendD2D::_resizeCursorBitmap(const RenderingPayload& p, const til::size
}
THROW_IF_FAILED(cursorRenderTarget->EndDraw());

cursorRenderTarget->GetBitmap(_cursorBitmap.put());
THROW_IF_FAILED(cursorRenderTarget->GetBitmap(_cursorBitmap.put()));
_cursorBitmapSize = newSize;
}

Expand Down
2 changes: 1 addition & 1 deletion src/renderer/atlas/BackendD2D.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace Microsoft::Console::Render::Atlas

private:
ATLAS_ATTR_COLD void _handleSettingsUpdate(const RenderingPayload& p);
void _drawBackground(const RenderingPayload& p) noexcept;
void _drawBackground(const RenderingPayload& p);
void _drawText(RenderingPayload& p);
ATLAS_ATTR_COLD f32 _drawTextPrepareLineRendition(const RenderingPayload& p, const ShapedRow* row, f32 baselineY) const noexcept;
ATLAS_ATTR_COLD void _drawTextResetLineRendition(const ShapedRow* row) const noexcept;
Expand Down

0 comments on commit ed2923b

Please sign in to comment.