Bill-Gray/PDCursesMod

Panels only partially refresh; maybe rewrite panel.c?

GitMensch opened this issue · 9 comments

comparing ptest in wincon and wingui showed one big difference:

  • wingui disallows resizing
  • wincon allows resizing (not sure if it can prevent this)

Also: the wincon version creates resize events and those are handled like any key events - resizing "a bit" therefore ends ptest, which is a bit confusing.

Suggestion:

  • prevent resize in wincon if possible
  • otherwise don't catch the resize events (or catch them for redraw, but not "as if a key was pressed")

Relying on a window being "un-resizable", or on it always being able to be resized, isn't a good idea. You don't always have control over either of those.

Correct behavior here is : if the window is resized, redraw the background at the new size and update the panels. I've modified ptest.c to do that, and it works... with ncurses. PDCursesMod and PDCurses garble the panels, on all platforms. Will investigate.

I believe the "wingui disallows resizing" is the same as this issue I just posted: #257

This does appear to be a different issue. Actually, two issues.

I've just fixed the first (easy) one : when you resize the window, wait_a_while() would say that a key had been hit and you should move on to the next panel display in the series.

However, if I resize the ptest window, the update only refreshes parts of most panels :
ptest

(In the above, panel 1 vanished completely. I've not figured out the pattern yet as to which lines from which panels get overwritten.)

This happens on all PDCursesMod platforms (and on PDCurses, at least on the X11 port.) The issue appears to be that the panels don't realize they've been 'touched'. If I add a couple of touchwin() statements within update_panels() to say that every panel has been touched, thereby forcing all of them to be refreshed, the problem goes away. This seems a little excessive, however.

Updated to note : I've looked a bit more at the panel.c code and recoiled in mild horror. I can't figure out much of the underlying logic, and there's a good bit of redundant code.

I think this will be best fixed by rewriting the panel library, almost from scratch. That's not really a big job, but I've been short of time lately; I can't say when it'll happen.

Maybe you find a better issue title than the current one?

Revised for (I think) a better issue title. Feel free to alter it.

A note for future reference, probably by me (don't have much time to work on this right now, and am about to be away for a while anyway).

Let's say you have ptest set up to show the following panels :
image
If you resize, the lines in any panel that go over a lower panel get redrawn. In this case, that means the bottom five lines of panel 1, the top line of panel 2, and the bottom seven lines of panel 4 :
image
Not sure yet what all that means. I'm still thinking the best route will probably be to rewrite large chunks of, and possibly all of, panel.c.

don't have much time to work on this right now, and am about to be away for a while anyway

Unrelated, but could you trigger a release before in this case, please?

Okay, I've figured out and fixed the underlying bug. See commit e2b2205.

In the _override() function in panel.c, we check a panel against other, overlapping/underlying panels to see if it needs to be updated. A panel that overlaps two panels and has three overlapping it would be checked against all five (though if show == -1, it would be checked only against the overlapping three.)

Anyway. Each time we check against an overlapping/underlying panel, we also check against stdscr. If there are no overlapping/underlying panels, we don't do that check, resulting in the errors you saw and which appear in the screenshot above.

As noted above, the bug was in both PDCursesMod and PDCurses, and therefore may be of interest to @wmcbrine as well.

There are still some baffling/poorly written bits in panel.c, and a rewrite may yet be in order. But it's very low on the to-do list, and the original bug is fixed, so... I'm closing this issue as 'fixed', which is mostly true.