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.

Sign up for GitHub

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

Jump to bottom

[graph] terminals smaller than 80x25 attempt to draw offscreen #2171

Closed
midichef opened this issue Dec 13, 2023 · 3 comments · Fixed by #2485
Closed

[graph] terminals smaller than 80x25 attempt to draw offscreen #2171

midichef opened this issue Dec 13, 2023 · 3 comments · Fixed by #2485
Assignees
@midichef
Labels
bug cosmetic

Comments

@midichef
Copy link
Contributor

Small description
If a window is smaller than the default startup window, canvas.py thinks the screen is bigger than it is, and attempts to draw offscreen at first. This causes errors to show up from various calls to scr.addstr():

scr.addstr(y, x, disp_column_fill*actualw, cattr.attr)  # clear whole area before displaying
_curses.error: addwstr() returned ERR

Steps to reproduce with sample data and a .vd
Open a terminal narrower than 80 columns or shorter than 25 columns, and plot any data set.

Additional context
saul.pw/VisiData v3.0dev

I'm looking into the cause and a fix.
@midichef midichef added the bug label Dec 13, 2023
@saulpw
Copy link
Owner

saulpw commented Dec 21, 2023

Thanks for looking into this, @midichef. It's probably entirely cosmetic but it would be good to clean it up anyway.

@midichef
Copy link
Contributor Author

I did a bisect, and it seems the problem started in the 4 commits after b252375, and in or before e1dd8c4, in the reworking of the color attributes in October. I have looked at the changes a few times but surprisingly, nothing looks like a likely cause.

I'll keep investigating.

midichef added a commit to midichef/visidata that referenced this issue Aug 5, 2024
@midichef
[canvas-] change mock window size from 80x25 to 1x1 saulpw#2171
ce8b65f 
Plotter/Canvas/GraphSheet do not have a screen when __init__() is called.
Until they have a screen, DrawablePane.windowWidth and .windowHeight
will give 25 and 80. If the actual terminal is smaller than 80x25.
errors will be caused by drawing past the edges of the terminal.

This commit uses a temporary size of 1x1 in __init__().
True canvas dimensions will be calculated later, with the real
curses screen, by first calling Canvas.refresh(), then waiting
for the next screen draw. During that draw, the sheet's screen
will be provided by drawSheet():
drawSheet(scr, sheet) -> draw(scr) -> render(h, w) -> resetCanvasDimensions(w, h)
@midichef
Copy link
Contributor Author

midichef commented Aug 5, 2024 •
edited
Loading

surprisingly, nothing [in these commits] looks like a likely cause.

My best guess is that these commits did not cause the bug. They merely revealed it. The bug is sensitive to timing. While investigating, I accidentally made the problem disappear by adding vd.status() calls. My tentative working hypothesis is: the bug causes no problems if refresh() can be reached before draw_all() runs. (The particular refresh() call to reach may be the one at the end of reload().)

I've got a patch for Canvas to handle the case when the terminal window is very small.

@midichef midichef mentioned this issue Aug 5, 2024
Merged
midichef added a commit to midichef/visidata that referenced this issue Aug 6, 2024
@midichef
[canvas-] change mock window size from 80x25 to 1x1 saulpw#2171
f31f1cb 
Plotter/Canvas/GraphSheet do not have a screen when __init__() is called.
Until they have a screen, DrawablePane.windowWidth and .windowHeight
will give 25 and 80. If the actual terminal is smaller than 80x25.
errors will be caused by drawing past the edges of the terminal.

This commit uses a temporary size of 1x1 in __init__().
True canvas dimensions will be calculated later, with the real
curses screen, by first calling Canvas.refresh(), then waiting
for the next screen draw. During that draw, the sheet's screen
will be provided by drawSheet():
drawSheet(scr, sheet) -> draw(scr) -> render(h, w) -> resetCanvasDimensions(w, h)
anjakefala added a commit that referenced this issue Oct 6, 2024
@anjakefala
Merge pull request #2485 from midichef/graph_window_init
3bdda47 
[canvas-] change mock window size from 80x25 to 1x1 #2171
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@midichef midichef

Labels
bug cosmetic
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[canvas-] change mock window size from 80x25 to 1x1 #2171 midichef/visidata
2 participants
@saulpw @midichef