Skip to content
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

atlas,d2d: overdraw the background bitmap by one cell on all sides #17674

Merged
merged 3 commits into from
Aug 6, 2024
Merged

atlas,d2d: overdraw the background bitmap by one cell on all sides #17674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4da9367
atlas,d2d: overdraw the background bitmap by one cell on all sides
DHowett Aug 6, 2024
1d54c80
We don't need to paint off the edge of the swapchain, there's nothing…
DHowett Aug 6, 2024
9f14782
Okay audit boomer
DHowett Aug 6, 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
44 changes: 37 additions & 7 deletions src/renderer/atlas/BackendD2D.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ void BackendD2D::_handleSettingsUpdate(const RenderingPayload& p)
const auto renderTargetChanged = !_renderTarget;
const auto fontChanged = _fontGeneration != p.s->font.generation();
const auto cursorChanged = _cursorGeneration != p.s->cursor.generation();
const auto backgroundColorChanged = _miscGeneration != p.s->misc.generation();
const auto cellCountChanged = _viewportCellCount != p.s->viewportCellCount;

if (renderTargetChanged)
Expand Down Expand Up @@ -125,26 +126,48 @@ void BackendD2D::_handleSettingsUpdate(const RenderingPayload& p)
_builtinGlyphsRenderTargetActive = false;
}

if (renderTargetChanged || fontChanged || cellCountChanged)
if (renderTargetChanged || fontChanged || cellCountChanged || backgroundColorChanged)
{
const D2D1_BITMAP_PROPERTIES props{
.pixelFormat = { DXGI_FORMAT_R8G8B8A8_UNORM, D2D1_ALPHA_MODE_PREMULTIPLIED },
.dpiX = static_cast<f32>(p.s->font->dpi),
.dpiY = static_cast<f32>(p.s->font->dpi),
};
const D2D1_SIZE_U size{
p.s->viewportCellCount.x,
p.s->viewportCellCount.y,
p.s->viewportCellCount.x + 2u,
p.s->viewportCellCount.y + 2u,
};
const D2D1_MATRIX_3X2_F transform{
.m11 = static_cast<f32>(p.s->font->cellSize.x),
.m22 = static_cast<f32>(p.s->font->cellSize.y),
/* Brushes are transformed relative to the render target, not the rect into which they are painted. */
.dx = -static_cast<f32>(p.s->font->cellSize.x),
.dy = -static_cast<f32>(p.s->font->cellSize.y),
};
THROW_IF_FAILED(_renderTarget->CreateBitmap(size, nullptr, 0, &props, _backgroundBitmap.put()));

/*
We're allocating a bitmap that is one pixel wider on every side than the viewport so that we can fill in the gutter
with the background color. D2D doesn't have an equivalent to D3D11_TEXTURE_ADDRESS_BORDER, which we use in the D3D
backend to ensure the colors don't bleed off the edges.

XXXXXXXXXXXXXXXX <- background color
X+------------+X
X| viewport |X
X| |X
X| |X
X+------------+X
XXXXXXXXXXXXXXXX

The translation in `transform` ensures that we render it off the top left of the render target.
*/
auto backgroundFill = std::make_unique_for_overwrite<u32[]>(static_cast<size_t>(size.width) * size.height);
std::fill_n(backgroundFill.get(), size.width * size.height, u32ColorPremultiply(p.s->misc->backgroundColor));
Copy link
Member Author

Choose a reason for hiding this comment

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

A heap alloc for 1440 bytes? At least it only happens when we need to recreate the bitmap.

Copy link
Member

Choose a reason for hiding this comment

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

Thinking about it some more, you could use CreateCompatibleRenderTarget and Clear() to initialize the bitmap. There's some code you could copy in _resizeCursorBitmap.

Another option is that we preallocate the foreground/background bitmap with a 1px margin and adjust the "stride" accordingly so that D3D is unaffected by this. The latter option is potentially the optimal solution, I think. (I'd be happy to help with that if you'd like.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Arguably more expensive than a heap alloc of 1440 bytes, FWIW.

A compatible render target, pointed to the bitmap, based on the current render target, all to just clear it (after converting the background color to D2D's preferred D2D_COLOR_F) and suck the bitmap out to stride-copy the actual background colors in.

Copy link
Member Author

Choose a reason for hiding this comment

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

However, preallocating it down in the Renderer is a good idea. Can we book that for the future, or would you like it in this PR?


THROW_IF_FAILED(_renderTarget->CreateBitmap(size, backgroundFill.get(), size.width * sizeof(u32), &props, _backgroundBitmap.put()));
THROW_IF_FAILED(_renderTarget->CreateBitmapBrush(_backgroundBitmap.get(), _backgroundBrush.put()));
_backgroundBrush->SetInterpolationMode(D2D1_BITMAP_INTERPOLATION_MODE_NEAREST_NEIGHBOR);
_backgroundBrush->SetExtendModeX(D2D1_EXTEND_MODE_MIRROR);
_backgroundBrush->SetExtendModeY(D2D1_EXTEND_MODE_MIRROR);
_backgroundBrush->SetExtendModeX(D2D1_EXTEND_MODE_CLAMP);
_backgroundBrush->SetExtendModeY(D2D1_EXTEND_MODE_CLAMP);
_backgroundBrush->SetTransform(&transform);
_backgroundBitmapGeneration = {};
}
Expand All @@ -158,14 +181,21 @@ void BackendD2D::_handleSettingsUpdate(const RenderingPayload& p)
_generation = p.s.generation();
_fontGeneration = p.s->font.generation();
_cursorGeneration = p.s->cursor.generation();
_miscGeneration = p.s->misc.generation();
_viewportCellCount = p.s->viewportCellCount;
}

void BackendD2D::_drawBackground(const RenderingPayload& p)
{
if (_backgroundBitmapGeneration != p.colorBitmapGenerations[0])
{
THROW_IF_FAILED(_backgroundBitmap->CopyFromMemory(nullptr, p.backgroundBitmap.data(), gsl::narrow_cast<UINT32>(p.colorBitmapRowStride * sizeof(u32))));
const D2D1_RECT_U rect{
1u,
1u,
1u + p.s->viewportCellCount.x,
1u + p.s->viewportCellCount.y,
};
THROW_IF_FAILED(_backgroundBitmap->CopyFromMemory(&rect, p.backgroundBitmap.data(), gsl::narrow_cast<UINT32>(p.colorBitmapRowStride * sizeof(u32))));
_backgroundBitmapGeneration = p.colorBitmapGenerations[0];
}

Expand Down
1 change: 1 addition & 0 deletions src/renderer/atlas/BackendD2D.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ namespace Microsoft::Console::Render::Atlas
til::generation_t _generation;
til::generation_t _fontGeneration;
til::generation_t _cursorGeneration;
til::generation_t _miscGeneration;
u16x2 _viewportCellCount{};

#if ATLAS_DEBUG_SHOW_DIRTY
Expand Down
Loading