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

Add VT100 ranged scroll #10110

Merged
merged 7 commits into from
Mar 5, 2025
Merged

Add VT100 ranged scroll #10110

merged 7 commits into from
Mar 5, 2025

Conversation

RetiredWizard
Copy link

This adds VT100 ranged scrolling support (ESC [###;###r). This can be useful for keeping a status line at the bottom and/or top of the screen while scrolling.

I'm realizing that submitting these incrementally is probably creating extra review work so I'll try not to do this again going forward. Sorry. 😦 The good news is that I don't plan on adding any more VT100 sequences until the inverted tile support is available.

I think this PR adds a bit more code than the earlier ones. I also wasn't sure about the decision to implement two scrolling methods, one for full screen (faster?) and a second for ranged scrolling. The second method could be used exclusively with less code.

In any case, I'm open to input, suggestions or even skipping this particular feature.

@RetiredWizard
Copy link
Author

As a side note, the formatting of the terminalio documentation is not working right https://docs.circuitpython.org/en/latest/shared-bindings/terminalio/

Is there somewhere I can look to figure out how to fix the formatting?

@RetiredWizard
Copy link
Author

The ja builds of the two samd51 boards overflowed slightly, I can disable the new VT100 features on those boards but the Pimoroni board seems to be something else, I can compile the ja firmware locally without an issue, perhaps it's a CI glitch?

@RetiredWizard RetiredWizard marked this pull request as draft March 5, 2025 12:23
@RetiredWizard
Copy link
Author

Drafting while I test out some speed/memory improvements

@RetiredWizard RetiredWizard marked this pull request as ready for review March 5, 2025 14:23
@RetiredWizard
Copy link
Author

RetiredWizard commented Mar 5, 2025

Okay, I think this is ready for review now.

I was able to build the two samd51 ja translations locally, however given how close they are, I think it makes sense to leave this feature disabled for them.

@RetiredWizard
Copy link
Author

I was looking at why simply setting the top of screen pointer slowed down when scrolling and realized that the screen is marked dirty and has to be redrawn when this occurs. Given that, there actually is no speed advantage to leaving the pointer version of the scroll code in place, however I'm wondering if I might be able to add optional hardware scrolling support to the display driver libraries (I've mostly been testing on an ili9341).

If I give up on the hardware scrolling approach, it might be worth coming back and removing the lines of code supporting the "pointer" scrolling method.

@bill88t
Copy link

bill88t commented Mar 5, 2025

I think it's worthwhile implementing full ANSI since we can. I'll be doing full color support and cursor once this is merged in.
(I have the cursors implemented in python already in my displayio virtual console objects.)

@tannewt
Copy link
Member

tannewt commented Mar 5, 2025

I'm realizing that submitting these incrementally is probably creating extra review work so I'll try not to do this again going forward. Sorry. 😦

More PRs tend to be smaller in scope and are easier to review. So, please keep doing small PRs.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks fine. Thanks for adding this!

@tannewt
Copy link
Member

tannewt commented Mar 5, 2025

I'm wondering if I might be able to add optional hardware scrolling support to the display driver libraries (I've mostly been testing on an ili9341).

I wanted to do this since I knew that SPI displays can. However, I've never come up with a good displayio internal system to do it. Furthermore, often the SPI displays are oriented 90 degrees from the direction we actually want to scroll. In those cases, it doesn't help text rendering.

The picodvi output would be able to do this too though. We can change the framebuffer pointers for each output row pretty easily.

@tannewt tannewt merged commit e8de36a into adafruit:main Mar 5, 2025
533 checks passed
@RetiredWizard RetiredWizard deleted the rangescroll branch March 5, 2025 22:13
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.

3 participants