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.

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

Use memory pool for PolyTextOut items in GDI Renderer #8619

Merged
3 commits merged into from
Jan 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .github/actions/spell-check/dictionary/apis.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ otms
OUTLINETEXTMETRICW
overridable
PAGESCROLL
pmr
RETURNCMD
rfind
roundf
Expand Down
1 change: 1 addition & 0 deletions src/inc/LibraryIncludes.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <deque>
#include <list>
#include <memory>
#include <memory_resource>
#include <map>
#include <mutex>
#include <shared_mutex>
Expand Down
7 changes: 7 additions & 0 deletions src/renderer/gdi/gdirenderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,13 @@ namespace Microsoft::Console::Render
COLORREF _lastBg;
bool _lastFontItalic;

// Memory pooling to save alloc/free work to the OS for things
// frequently created and dropped.
// It's important the pool is first so it can be given to the others on construction.
std::pmr::unsynchronized_pool_resource _pool;
std::pmr::vector<std::pmr::wstring> _polyStrings;
std::pmr::vector<std::pmr::basic_string<int>> _polyWidths;

[[nodiscard]] HRESULT _InvalidCombine(const RECT* const prc) noexcept;
[[nodiscard]] HRESULT _InvalidOffset(const POINT* const ppt) noexcept;
[[nodiscard]] HRESULT _InvalidRestrict() noexcept;
Expand Down
42 changes: 15 additions & 27 deletions src/renderer/gdi/paint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -308,13 +308,11 @@ using namespace Microsoft::Console::Render;

const auto pPolyTextLine = &_pPolyText[_cPolyText];

auto pwsPoly = std::make_unique<wchar_t[]>(cchLine);
RETURN_IF_NULL_ALLOC(pwsPoly);
auto& polyString = _polyStrings.emplace_back(cchLine, UNICODE_NULL);

COORD const coordFontSize = _GetFontSize();

auto rgdxPoly = std::make_unique<int[]>(cchLine);
RETURN_IF_NULL_ALLOC(rgdxPoly);
auto& polyWidth = _polyWidths.emplace_back(cchLine, 0);

// Sum up the total widths the entire line/run is expected to take while
// copying the pixel widths into a structure to direct GDI how many pixels to use per character.
Expand All @@ -327,9 +325,9 @@ using namespace Microsoft::Console::Render;

// Our GDI renderer hasn't and isn't going to handle things above U+FFFF or sequences.
// So replace anything complicated with a replacement character for drawing purposes.
pwsPoly[i] = cluster.GetTextAsSingle();
rgdxPoly[i] = gsl::narrow<int>(cluster.GetColumns()) * coordFontSize.X;
cchCharWidths += rgdxPoly[i];
polyString[i] = cluster.GetTextAsSingle();
polyWidth[i] = gsl::narrow<int>(cluster.GetColumns()) * coordFontSize.X;
cchCharWidths += polyWidth[i];
}

// Detect and convert for raster font...
Expand All @@ -338,15 +336,15 @@ using namespace Microsoft::Console::Render;
// dispatch conversion into our codepage

// Find out the bytes required
int const cbRequired = WideCharToMultiByte(_fontCodepage, 0, pwsPoly.get(), (int)cchLine, nullptr, 0, nullptr, nullptr);
int const cbRequired = WideCharToMultiByte(_fontCodepage, 0, polyString.data(), (int)cchLine, nullptr, 0, nullptr, nullptr);

if (cbRequired != 0)
{
// Allocate buffer for MultiByte
auto psConverted = std::make_unique<char[]>(cbRequired);

// Attempt conversion to current codepage
int const cbConverted = WideCharToMultiByte(_fontCodepage, 0, pwsPoly.get(), (int)cchLine, psConverted.get(), cbRequired, nullptr, nullptr);
int const cbConverted = WideCharToMultiByte(_fontCodepage, 0, polyString.data(), (int)cchLine, psConverted.get(), cbRequired, nullptr, nullptr);

// If successful...
if (cbConverted != 0)
Expand All @@ -356,22 +354,22 @@ using namespace Microsoft::Console::Render;

if (cchRequired != 0)
{
auto pwsConvert = std::make_unique<wchar_t[]>(cchRequired);
std::pmr::wstring polyConvert(cchRequired, UNICODE_NULL, &_pool);

// Then do the actual conversion.
int const cchConverted = MultiByteToWideChar(CP_ACP, 0, psConverted.get(), cbRequired, pwsConvert.get(), cchRequired);
int const cchConverted = MultiByteToWideChar(CP_ACP, 0, psConverted.get(), cbRequired, polyConvert.data(), cchRequired);

if (cchConverted != 0)
{
// If all successful, use this instead.
pwsPoly.swap(pwsConvert);
polyString.swap(polyConvert);
}
}
}
}
}

pPolyTextLine->lpstr = pwsPoly.release();
pPolyTextLine->lpstr = polyString.data();
pPolyTextLine->n = gsl::narrow<UINT>(clusters.size());
pPolyTextLine->x = ptDraw.x;
pPolyTextLine->y = ptDraw.y;
Expand All @@ -380,7 +378,7 @@ using namespace Microsoft::Console::Render;
pPolyTextLine->rcl.top = pPolyTextLine->y;
pPolyTextLine->rcl.right = pPolyTextLine->rcl.left + ((SHORT)cchCharWidths * coordFontSize.X);
pPolyTextLine->rcl.bottom = pPolyTextLine->rcl.top + coordFontSize.Y;
pPolyTextLine->pdx = rgdxPoly.release();
pPolyTextLine->pdx = polyWidth.data();

if (trimLeft)
{
Expand Down Expand Up @@ -417,20 +415,10 @@ using namespace Microsoft::Console::Render;
hr = E_FAIL;
}

for (size_t iPoly = 0; iPoly < _cPolyText; iPoly++)
{
if (nullptr != _pPolyText[iPoly].lpstr)
{
delete[] _pPolyText[iPoly].lpstr;
_pPolyText[iPoly].lpstr = nullptr;
}
_polyStrings.clear();
_polyWidths.clear();

if (nullptr != _pPolyText[iPoly].pdx)
{
delete[] _pPolyText[iPoly].pdx;
_pPolyText[iPoly].pdx = nullptr;
}
}
ZeroMemory(_pPolyText, sizeof(_pPolyText));

_cPolyText = 0;
}
Expand Down
5 changes: 4 additions & 1 deletion src/renderer/gdi/state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ GdiEngine::GdiEngine() :
_lastFontItalic(false),
_fPaintStarted(false),
_hfont(nullptr),
_hfontItalic(nullptr)
_hfontItalic(nullptr),
_pool{}, // It's important the pool is first so it can be given to the others on construction.
_polyStrings{ &_pool },
_polyWidths{ &_pool }
{
ZeroMemory(_pPolyText, sizeof(POLYTEXTW) * s_cPolyTextCache);
_rcInvalid = { 0 };
Expand Down