Skip to content

Commit

Permalink
platform: fix initial cursor size on Windows
Browse files Browse the repository at this point in the history
6c0745c introduced the assumption in BufferedDisplay that the cursor is initially hidden, but this is not true on Windows.

A possible solution is to do something similar to what we did for the screen size: we store in DisplayStrategy a variable that represents the actual cursor size, and update it in DisplayStrategy::reloadScreenInfo(). BufferedDisplay now stores the 'future' cursor size instead of the 'last' one.

The only difference between this solution and the one for screen size is that while DisplayStrategy::size is modified exclusively from DisplayStrategy::reloadScreenInfo(), DisplayStrategy::caretSize is also modified from BufferedDisplay::flushScreen(), because flushScreen() does change the terminal's cursor size.
  • Loading branch information
magiblot committed Jan 22, 2021
1 parent 6c0745c commit 188009b
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 37 deletions.
22 changes: 3 additions & 19 deletions include/tvision/internal/buffdisp.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,6 @@ class ScreenCursor;

class BufferedDisplay : public DisplayStrategy {

protected:

// 'caretSize' must be a value in the range 0 to 100. Zero means it is not
// visible.

int caretSize;

// 'getCaretSize()' must return a number in the range 1 to 100,
// because the original THardwareInfo interface maps directly to
// the Win32 Console API (see CONSOLE_CURSOR_INFO).

int getCaretSize() override { return max(caretSize, 1); }
bool isCaretVisible() override { return caretSize > 0; }
void setCaretSize(int size) override { caretSize = size; }

private:

friend struct FlushScreenAlgorithm;

struct Range {
Expand All @@ -45,7 +28,7 @@ class BufferedDisplay : public DisplayStrategy {
const uint widePlaceholder;
bool caretMoved;
TPoint caretPosition;
int lastCaretSize;
int newCaretSize;

bool limitFPS;
int frameDrops;
Expand Down Expand Up @@ -81,7 +64,8 @@ class BufferedDisplay : public DisplayStrategy {
BufferedDisplay();
~BufferedDisplay();

void setCaretPosition(int x, int y);
void setCaretSize(int size) override;
void setCaretPosition(int x, int y) override;
void screenWrite(int x, int y, TScreenCell *buf, int len) override;
void flushScreen() override;
void reloadScreenInfo() override;
Expand Down
33 changes: 23 additions & 10 deletions include/tvision/internal/platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,37 @@ class DisplayStrategy {

virtual ~DisplayStrategy() {}

// 'size' must not change until 'reloadScreenInfo()' is invoked.
// Invariant: 'size' is in sync with the actual screen size after
// 'DisplayStrategy::reloadScreenInfo()' has been invoked.
// Invariant: these variables are is in sync with the actual screen state
// after 'DisplayStrategy::reloadScreenInfo()' has been invoked.

TPoint size {};
int caretSize {};

// This function can be overriden by inheritors in order to update
// internal structs when the display properties change.
// internal structs when the display properties change. But
// 'DisplayStrategy::reloadScreenInfo()' must be invoked eventually.

virtual void reloadScreenInfo() { size = getScreenSize(); };
virtual void reloadScreenInfo()
{
size = getScreenSize();
caretSize = getCaretSize();
}

protected:

// The function is meant to be overriden by inheritors. Its only
// responsibility should be to return the updated screen size.
// These functions are meant to be overriden by inheritors. Their only
// responsibility should be to return updated values.

virtual TPoint getScreenSize() { return size; }

virtual int getCaretSize() { return 0; }
virtual bool isCaretVisible() { return false; }
// 'getCaretSize()' must return a value in the range 0 to 100. Zero means
// the caret is not visible.

virtual int getCaretSize() { return caretSize; }

public:

bool isCaretVisible() { return caretSize != 0; }
virtual void clearScreen() {}
virtual void setCaretPosition(int x, int y) {}
virtual ushort getScreenMode() { return 0; }
Expand Down Expand Up @@ -79,7 +92,7 @@ class PlatformStrategy {
virtual void cursorOn() { if (input) input->cursorOn(); }
virtual void cursorOff() { if (input) input->cursorOff(); }

virtual int getCaretSize() { return display->getCaretSize(); }
virtual int getCaretSize() { return display->caretSize; }
virtual bool isCaretVisible() { return display->isCaretVisible(); }
virtual void clearScreen() { display->clearScreen(); }
virtual int getScreenRows() { return display->size.y; }
Expand Down
1 change: 1 addition & 0 deletions include/tvision/internal/win32con.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ class Win32Display : public BufferedDisplay

void reloadScreenInfo() override;
TPoint getScreenSize() override;
int getCaretSize() override;

void clearScreen() override;
ushort getScreenMode() override;
Expand Down
15 changes: 10 additions & 5 deletions source/platform/buffdisp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ BufferedDisplay::BufferedDisplay() :
screenChanged = true;
caretMoved = false;
caretPosition = {-1, -1};
caretSize = lastCaretSize = 0;
newCaretSize = 0;
}

BufferedDisplay::~BufferedDisplay()
Expand All @@ -61,6 +61,11 @@ void BufferedDisplay::resizeBuffer()
rowDamage.resize(size.y, {INT_MAX, INT_MIN});
}

void BufferedDisplay::setCaretSize(int size)
{
newCaretSize = size;
}

void BufferedDisplay::setCaretPosition(int x, int y)
{
caretPosition = {x, y};
Expand Down Expand Up @@ -156,7 +161,7 @@ void BufferedDisplay::undrawCursors()

bool BufferedDisplay::needsFlush() const
{
return screenChanged || caretMoved || caretSize != lastCaretSize;
return screenChanged || caretMoved || caretSize != newCaretSize;
}

void BufferedDisplay::flushScreen()
Expand All @@ -168,12 +173,12 @@ void BufferedDisplay::flushScreen()
if (caretPosition.x != -1)
lowlevelMoveCursor(caretPosition.x, caretPosition.y);
undrawCursors();
if (caretSize != lastCaretSize)
lowlevelCursorSize(caretSize);
if (caretSize != newCaretSize)
lowlevelCursorSize(newCaretSize);
lowlevelFlush();
screenChanged = false;
caretMoved = false;
lastCaretSize = caretSize;
caretSize = newCaretSize;
}
}

Expand Down
10 changes: 7 additions & 3 deletions source/platform/win32con.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,6 @@ Win32Display::Win32Display(Win32ConsoleStrategy &cnState) :
cnState(cnState),
lastAttr('\x00')
{
CONSOLE_CURSOR_INFO crInfo {};
GetConsoleCursorInfo(cnHandle(), &crInfo);
setCaretSize(crInfo.bVisible ? crInfo.dwSize : 0);
}

void Win32Display::reloadScreenInfo()
Expand All @@ -308,6 +305,13 @@ TPoint Win32Display::getScreenSize()
return {cnState.sbInfo.dwSize.X, cnState.sbInfo.dwSize.Y};
}

int Win32Display::getCaretSize()
{
CONSOLE_CURSOR_INFO crInfo {};
GetConsoleCursorInfo(cnHandle(), &crInfo);
return crInfo.bVisible ? crInfo.dwSize : 0;
}

void Win32Display::lowlevelCursorSize(int size)
{
CONSOLE_CURSOR_INFO crInfo;
Expand Down

0 comments on commit 188009b

Please sign in to comment.