Skip to content

Conversation

@dimitriskres
Copy link

Type of changes

  • Bug fix
  • New feature
  • Documentation / docstrings
  • Tests
  • Other

Checklist

  • I've run the latest black with default args on new code.
  • I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate (see note about typos above).
  • I've added tests for new code.
  • I accept that @willmcgugan may be pedantic in the code review.

Description

I was met with flickering on renderables that automatically refresh. This was not fixed with reducing the refresh rate.

see

After digging through the code, I found the reason. Rendering involves the following steps:

  1. Move to start of last (current, if just drawn) line.
  2. Erase the line
  3. Move up
  4. Erase the line
  5. Repeat 3. and 4. for as as many lines except one (handled on steps 1. and 2.)
  6. Draw everything

This leads to an unavoidable case whereby the screen will be cleared after the origin before drawing again. This may not be noticeable or an issue on:

  • Really fast systems
  • Single/seldom refreshing renderables
  • Renderables with a few runes (characters + ansi sequences)

My changes completely fix the flickering problem by taking a more efficient and direct approach to rendering:

  1. Move up (using a single \e[nF instead of repeating \e[nA) the amount of lines
  2. When rendering, every line is appended with \e[0K, erasing any line residuals
  3. After rendering all lines, and before drawing, the entire buffer is appended with \e[0J, clearing any screen residuals

This is more efficient because less ansi sequences are used for moving the cursor. It also completely eliminated flickering as the screen is never fully cleared between draw calls.

To achieve this, I had to introduce \e[nF (move to the start of n lines up) in ControlType and I also included the not-used \e[nG (move to the start of n lines down) for completeness. I also had to modify ControlType.CLEAR to accept a parameter and backfixed any cases where it was already used.

see
A lot of tests fail as a result of this change due to the fact that every render now has a different inherent structure. I am not sure whether a pull request expects me to implement those tests from scratch, but I am not able to regardless due to time constraints at this time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant