Skip to content

Commit

Permalink
Code cleanup for review
Browse files Browse the repository at this point in the history
  • Loading branch information
@zadjii-msft
zadjii-msft committed Jun 3, 2020
1 parent f9aae9d commit 9d30b3b
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 148 deletions.
73 changes: 31 additions & 42 deletions src/renderer/base/renderer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -843,12 +843,16 @@ void Renderer::_PaintBufferOutputGridLineHelper(_In_ IRenderEngine* const pEngin
}

// Routine Description:
// - Paint helper to draw the cursor within the buffer.
// - Retrieves information about the cursor, and pack it into a CursorOptions
// that the render engine can use for painting the cursor.
// - If the cursor is "off", or the cursor is out of bounds of the viewport,
// this will return nullopt (indicating the cursor shoudln't be painted this
// frame)
// Arguments:
// - <none>
// Return Value:
// - <none>
void Renderer::_PaintCursor(_In_ IRenderEngine* const pEngine)
// - nullopt if the cursor is off our out-of-frame, otherwise a CursorOptions
[[nodiscard]] std::optional<CursorOptions> Renderer::_GetCursorInfo()
{
if (_pData->IsCursorVisible())
{
Expand Down Expand Up @@ -880,57 +884,42 @@ void Renderer::_PaintCursor(_In_ IRenderEngine* const pEngine)
options.cursorColor = cursorColor;
options.isOn = _pData->IsCursorOn();

// Draw it within the viewport
LOG_IF_FAILED(pEngine->PaintCursor(options));
return { options };
}
}
return std::nullopt;
}

// Routine Description:
// - TODO
// - Paint helper to draw the cursor within the buffer.
// Arguments:
// - <none>
// - engine - The render engine that we're targeting.
// Return Value:
// - <none>
[[nodiscard]] HRESULT Renderer::_PrepareRenderInfo(_In_ IRenderEngine* const pEngine)
void Renderer::_PaintCursor(_In_ IRenderEngine* const pEngine)
{
RenderFrameInfo info;
info.cursorInfo = std::nullopt;

if (_pData->IsCursorVisible())
const auto cursorInfo = _GetCursorInfo();
if (cursorInfo.has_value())
{
// Get cursor position in buffer
COORD coordCursor = _pData->GetCursorPosition();

// GH#3166: Only draw the cursor if it's actually in the viewport. It
// might be on the line that's in that partially visible row at the
// bottom of the viewport, the space that's not quite a full line in
// height. Since we don't draw that text, we shouldn't draw the cursor
// there either.
Viewport view = _pData->GetViewport();
if (view.IsInBounds(coordCursor))
{
// Adjust cursor to viewport
view.ConvertToOrigin(&coordCursor);

COLORREF cursorColor = _pData->GetCursorColor();
bool useColor = cursorColor != INVALID_COLOR;

// Build up the cursor parameters including position, color, and drawing options
CursorOptions options;
options.coordCursor = coordCursor;
options.ulCursorHeightPercent = _pData->GetCursorHeight();
options.cursorPixelWidth = _pData->GetCursorPixelWidth();
options.fIsDoubleWidth = _pData->IsCursorDoubleWidth();
options.cursorType = _pData->GetCursorStyle();
options.fUseColor = useColor;
options.cursorColor = cursorColor;
options.isOn = _pData->IsCursorOn();

info.cursorInfo = options;
}
LOG_IF_FAILED(pEngine->PaintCursor(cursorInfo.value()));
}
}

// Routine Description:
// - Retrieves info from the render data to prepare the engine with, before the
// frame is drawn. Some renderers might want to use this information to affect
// later drawing decisions.
// * Namely, the DX renderer uses this to know the cursor position and state
// before PaintCursor is called, so it can draw the cursor underneath the
// text.
// Arguments:
// - engine - The render engine that we're targeting.
// Return Value:
// - S_OK if the engine prepared successfully, or a relevant error via HRESULT.
[[nodiscard]] HRESULT Renderer::_PrepareRenderInfo(_In_ IRenderEngine* const pEngine)
{
RenderFrameInfo info;
info.cursorInfo = _GetCursorInfo();
return pEngine->PrepareRenderInfo(info);
}

Expand Down
1 change: 1 addition & 0 deletions src/renderer/base/renderer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ namespace Microsoft::Console::Render

[[nodiscard]] HRESULT _PaintTitle(IRenderEngine* const pEngine);

[[nodiscard]] std::optional<CursorOptions> _GetCursorInfo();
[[nodiscard]] HRESULT _PrepareRenderInfo(_In_ IRenderEngine* const pEngine);

// Helper functions to diagnose issues with painting and layout.
Expand Down
53 changes: 47 additions & 6 deletions src/renderer/dx/CustomTextRenderer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,26 @@ using namespace Microsoft::Console::Render;
clientDrawingEffect);
}

[[nodiscard]] HRESULT _DrawCursorHelper(::Microsoft::WRL::ComPtr<ID2D1DeviceContext> d2dContext,
const DrawingContext& drawingContext)
// Function Description:
// - Attempt to draw the cursor. If the cursor isn't visible or on, this
// function will do nothing. If the cursor isn't within the bounds of the
// current run of text, then this function will do nothing.
// - This function will get called twice during a run, once before the text is
// drawn (underneath the text), and again after the text is drawn (above the
// text). Depending on if the cursor wants to be drawn above or below the
// text, this function will do nothing for the first/second pass
// (respectively).
// Arguments:
// - d2dContext - Pointer to the current D2D drawing context
// - textRunBounds - The bounds of the current run of text.
// - drawingContext - Pointer to structure of information required to draw
// - firstPass - true if we're being called before the text is drawn, false afterwards.
// Return Value:
// - S_FALSE if we did nothing, S_OK if we successfully painted, otherwise an appropriate HRESULT
[[nodiscard]] HRESULT _drawCursorHelper(::Microsoft::WRL::ComPtr<ID2D1DeviceContext> d2dContext,
D2D1_RECT_F textRunBounds,
const DrawingContext& drawingContext,
const bool firstPass)
try
{
if (!drawingContext.cursorInfo.has_value())
Expand All @@ -232,13 +250,24 @@ try
}

const auto& options = drawingContext.cursorInfo.value();
const til::size glyphSize{ til::math::flooring, drawingContext.cellSize.width, drawingContext.cellSize.height };
// const til::size glyphSize{ til::math::flooring, drawingContext.cellSize };

// if the cursor is off, do nothing - it should not be visible.
if (!options.isOn)
{
return S_FALSE;
}

// TODO GH#TODO: Add support for `"cursorTextColor": null` for letting the
// cursor draw on top again.
if (!firstPass)
{
return S_FALSE;
}

const til::size glyphSize{ til::math::flooring,
drawingContext.cellSize.width,
drawingContext.cellSize.height };

// Create rectangular block representing where the cursor can fill.
D2D1_RECT_F rect = til::rectangle{ til::point{ options.coordCursor } }.scale_up(glyphSize);

Expand All @@ -248,6 +277,15 @@ try
rect.right += glyphSize.width();
}

// If the cursor isn't within the bounds of this current run of text, do nothing.
if (rect.top > textRunBounds.bottom ||
rect.bottom <= textRunBounds.top ||
rect.left > textRunBounds.right ||
rect.right <= textRunBounds.left)
{
return S_FALSE;
}

CursorPaintType paintType = CursorPaintType::Fill;

switch (options.cursorType)
Expand Down Expand Up @@ -286,7 +324,7 @@ try
return E_NOTIMPL;
}

Microsoft::WRL::ComPtr<ID2D1SolidColorBrush> brush; // = _d2dBrushForeground;
Microsoft::WRL::ComPtr<ID2D1SolidColorBrush> brush;

if (options.fUseColor)
{
Expand Down Expand Up @@ -394,7 +432,7 @@ CATCH_RETURN()

d2dContext->FillRectangle(rect, drawingContext->backgroundBrush);

RETURN_IF_FAILED(_DrawCursorHelper(d2dContext, *drawingContext));
RETURN_IF_FAILED(_drawCursorHelper(d2dContext, rect, *drawingContext, true));

// GH#5098: If we're rendering with cleartype text, we need to always render
// onto an opaque background. If our background _isn't_ opaque, then we need
Expand Down Expand Up @@ -583,6 +621,9 @@ CATCH_RETURN()
drawingContext->foregroundBrush,
clientDrawingEffect));
}

RETURN_IF_FAILED(_drawCursorHelper(d2dContext, rect, *drawingContext, false));

return S_OK;
}
#pragma endregion
Expand Down
115 changes: 18 additions & 97 deletions src/renderer/dx/DxRenderer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1414,108 +1414,18 @@ try
}
CATCH_RETURN()

// Routine Description:
// - Does nothing. Our cursor is drawn in CustomTextRenderer::DrawGlyphRun,
// either above or below the text.
// Arguments:
// - options - unused
// Return Value:
// - S_OK
[[nodiscard]] HRESULT DxEngine::PaintCursor(const CursorOptions& /*options*/) noexcept
{
return S_OK;
}

// // Routine Description:
// // - Draws a block at the given position to represent the cursor
// // - May be a styled cursor at the character cell location that is less than a full block
// // Arguments:
// // - options - Packed options relevant to how to draw the cursor
// // Return Value:
// // - S_OK or relevant DirectX error.
// [[nodiscard]] HRESULT DxEngine::PaintCursor(const CursorOptions& options) noexcept
// try
// {
// // if the cursor is off, do nothing - it should not be visible.
// if (!options.isOn)
// {
// return S_FALSE;
// }
// // Create rectangular block representing where the cursor can fill.
// D2D1_RECT_F rect = til::rectangle{ til::point{ options.coordCursor } }.scale_up(_glyphCell);

// // If we're double-width, make it one extra glyph wider
// if (options.fIsDoubleWidth)
// {
// rect.right += _glyphCell.width();
// }

// CursorPaintType paintType = CursorPaintType::Fill;

// switch (options.cursorType)
// {
// case CursorType::Legacy:
// {
// // Enforce min/max cursor height
// ULONG ulHeight = std::clamp(options.ulCursorHeightPercent, s_ulMinCursorHeightPercent, s_ulMaxCursorHeightPercent);
// ulHeight = (_glyphCell.height<ULONG>() * ulHeight) / 100;
// rect.top = rect.bottom - ulHeight;
// break;
// }
// case CursorType::VerticalBar:
// {
// // It can't be wider than one cell or we'll have problems in invalidation, so restrict here.
// // It's either the left + the proposed width from the ease of access setting, or
// // it's the right edge of the block cursor as a maximum.
// rect.right = std::min(rect.right, rect.left + options.cursorPixelWidth);
// break;
// }
// case CursorType::Underscore:
// {
// rect.top = rect.bottom - 1;
// break;
// }
// case CursorType::EmptyBox:
// {
// paintType = CursorPaintType::Outline;
// break;
// }
// case CursorType::FullBox:
// {
// break;
// }
// default:
// return E_NOTIMPL;
// }

// Microsoft::WRL::ComPtr<ID2D1SolidColorBrush> brush = _d2dBrushForeground;

// if (options.fUseColor)
// {
// // Make sure to make the cursor opaque
// RETURN_IF_FAILED(_d2dRenderTarget->CreateSolidColorBrush(_ColorFFromColorRef(OPACITY_OPAQUE | options.cursorColor), &brush));
// }

// switch (paintType)
// {
// case CursorPaintType::Fill:
// {
// _d2dRenderTarget->FillRectangle(rect, brush.Get());
// break;
// }
// case CursorPaintType::Outline:
// {
// // DrawRectangle in straddles physical pixels in an attempt to draw a line
// // between them. To avoid this, bump the rectangle around by half the stroke width.
// rect.top += 0.5f;
// rect.left += 0.5f;
// rect.bottom -= 0.5f;
// rect.right -= 0.5f;

// _d2dRenderTarget->DrawRectangle(rect, brush.Get());
// break;
// }
// default:
// return E_NOTIMPL;
// }

// return S_OK;
// }
// CATCH_RETURN()

// Routine Description:
// - Paint terminal effects.
// Arguments:
Expand Down Expand Up @@ -2246,6 +2156,17 @@ try
}
CATCH_LOG()

// Method Description:
// - Informs this render engine about certain state for this frame at the
// beginning of this frame. We'll use it to get information about the cursor
// before PaintCursor is called. This enables the DX renderer to draw the
// cursor underneath the text.
// - This is called every frame. When the cursor is Off or out of frame, the
// info's cursorInfo will be set to std::nullopt;
// Arguments:
// - info - a RenderFrameInfo with information about the state of the cursor in this frame.
// Return Value:
// - S_OK
[[nodiscard]] HRESULT DxEngine::PrepareRenderInfo(const RenderFrameInfo& info) noexcept
{
_frameInfo = info;
Expand Down
3 changes: 0 additions & 3 deletions src/renderer/dx/DxRenderer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,6 @@ namespace Microsoft::Console::Render

static std::atomic<size_t> _tracelogCount;

static const ULONG s_ulMinCursorHeightPercent = 25;
static const ULONG s_ulMaxCursorHeightPercent = 100;

// Device-Independent Resources
::Microsoft::WRL::ComPtr<ID2D1Factory> _d2dFactory;
::Microsoft::WRL::ComPtr<IDWriteFactory1> _dwriteFactory;
Expand Down

1 comment on commit 9d30b3b

@github-actions

This comment was marked as resolved.

Sign in to view

Please sign in to comment.