Repository: microsoft/terminal
File: src/renderer/atlas/BackendD3D.cpp (C++)

Code before:
}

void BackendD3D::_resetGlyphAtlas(const RenderingPayload& p)
{
    // The index returned by _BitScanReverse is undefined when the input is 0. We can simultaneously guard
    // against that and avoid unreasonably small textures, by clamping the min. texture size to `minArea`.
    // `minArea` results in a 64kB RGBA texture which is the min. alignment for placed memory.
    static constexpr u32 minArea = 128 * 128;
    static constexpr u32 maxArea = D3D10_REQ_TEXTURE2D_U_OR_V_DIMENSION * D3D10_REQ_TEXTURE2D_U_OR_V_DIMENSION;

    const auto cellArea = static_cast<u32>(p.s->font->cellSize.x) * p.s->font->cellSize.y;
    const auto targetArea = static_cast<u32>(p.s->targetSize.x) * p.s->targetSize.y;

    const auto minAreaByFont = cellArea * 95; // Covers all printable ASCII characters
    const auto minAreaByGrowth = static_cast<u32>(_rectPacker.width) * _rectPacker.height * 2;
    const auto min = std::max(minArea, std::max(minAreaByFont, minAreaByGrowth));

    // It's hard to say what the max. size of the cache should be. Optimally I think we should use as much
    // memory as is available, but the rendering code in this project is a big mess and so integrating
    // memory pressure feedback (RegisterVideoMemoryBudgetChangeNotificationEvent) is rather difficult.
    // As an alternative I'm using 1.25x the size of the swap chain. The 1.25x is there to avoid situations, where
    // we're locked into a state, where on every render pass we're starting with a half full atlas, drawing once,
    // filling it with the remaining half and drawing again, requiring two rendering passes on each frame.
    const auto maxAreaByFont = targetArea + targetArea / 4;
    const auto area = std::min(maxArea, std::min(maxAreaByFont, min));

    // This block of code calculates the size of a power-of-2 texture that has an area larger than the given `area`.
    // For instance, for an area of 985x1946 = 1916810 it would result in a u/v of 2048x1024 (area = 2097152).
    // This has 2 benefits: GPUs like power-of-2 textures and it ensures that we don't resize the texture
    // every time you resize the window by a pixel. Instead it only grows/shrinks by a factor of 2.
    unsigned long index;
    _BitScanReverse(&index, area - 1);
    const auto u = static_cast<u16>(1u << ((index + 2) / 2));
    const auto v = static_cast<u16>(1u << ((index + 1) / 2));

    if (u != _rectPacker.width || v != _rectPacker.height)
    {
        _resizeGlyphAtlas(p, u, v);
    }

    stbrp_init_target(&_rectPacker, u, v, _rectPackerData.data(), _rectPackerData.size());

    // This is a little imperfect, because it only releases the memory of the glyph mappings, not the memory held by
    // any DirectWrite fonts. On the other side, the amount of fonts on a system is always finite, where "finite"
    // is pretty low, relatively speaking. Additionally this allows us to cache the boxGlyphs map indefinitely.
    // It's not great, but it's not terrible.
    for (auto& slot : _glyphAtlasMap.container())
    {
        if (slot.inner)
        {
            slot.inner->glyphs.clear();

Review comment (suggestion):
* Avoid reducing `area` below `minArea` at all times

(`minArea` should probably be called `absoluteMinArea` or something.)

Code after:
}

void BackendD3D::_resetGlyphAtlas(const RenderingPayload& p)
{
    // The index returned by _BitScanReverse is undefined when the input is 0. We can simultaneously guard
    // against that and avoid unreasonably small textures, by clamping the min. texture size to `minArea`.
    // `minArea` results in a 64kB RGBA texture which is the min. alignment for placed memory.
    static constexpr u32 minArea = 128 * 128;
    static constexpr u32 maxArea = D3D10_REQ_TEXTURE2D_U_OR_V_DIMENSION * D3D10_REQ_TEXTURE2D_U_OR_V_DIMENSION;

    const auto cellArea = static_cast<u32>(p.s->font->cellSize.x) * p.s->font->cellSize.y;
    const auto targetArea = static_cast<u32>(p.s->targetSize.x) * p.s->targetSize.y;

    const auto minAreaByFont = cellArea * 95; // Covers all printable ASCII characters
    const auto minAreaByGrowth = static_cast<u32>(_rectPacker.width) * _rectPacker.height * 2;

    // It's hard to say what the max. size of the cache should be. Optimally I think we should use as much
    // memory as is available, but the rendering code in this project is a big mess and so integrating
    // memory pressure feedback (RegisterVideoMemoryBudgetChangeNotificationEvent) is rather difficult.
    // As an alternative I'm using 1.25x the size of the swap chain. The 1.25x is there to avoid situations, where
    // we're locked into a state, where on every render pass we're starting with a half full atlas, drawing once,
    // filling it with the remaining half and drawing again, requiring two rendering passes on each frame.
    const auto maxAreaByFont = targetArea + targetArea / 4;

    auto area = std::min(maxAreaByFont, std::max(minAreaByFont, minAreaByGrowth));
    area = clamp(area, minArea, maxArea);

    // This block of code calculates the size of a power-of-2 texture that has an area larger than the given `area`.
    // For instance, for an area of 985x1946 = 1916810 it would result in a u/v of 2048x1024 (area = 2097152).
    // This has 2 benefits: GPUs like power-of-2 textures and it ensures that we don't resize the texture
    // every time you resize the window by a pixel. Instead it only grows/shrinks by a factor of 2.
    unsigned long index;
    _BitScanReverse(&index, area - 1);
    const auto u = static_cast<u16>(1u << ((index + 2) / 2));
    const auto v = static_cast<u16>(1u << ((index + 1) / 2));

    if (u != _rectPacker.width || v != _rectPacker.height)
    {
        _resizeGlyphAtlas(p, u, v);
    }

    stbrp_init_target(&_rectPacker, u, v, _rectPackerData.data(), _rectPackerData.size());

    // This is a little imperfect, because it only releases the memory of the glyph mappings, not the memory held by
    // any DirectWrite fonts. On the other side, the amount of fonts on a system is always finite, where "finite"
    // is pretty low, relatively speaking. Additionally this allows us to cache the boxGlyphs map indefinitely.
    // It's not great, but it's not terrible.
    for (auto& slot : _glyphAtlasMap.container())
    {
        if (slot.inner)
        {