PaperTTY icon indicating copy to clipboard operation
PaperTTY copied to clipboard

Give error if `scrub` is used on a screen without partial refresh suppport

Open colin-nolan opened this issue 4 years ago • 6 comments

It took me a while to realise why scrub didn't work on the device that I have, which doesn't support partial refresh.

This PR aims to make the issue clear to anyone trying the same thing in the future.

colin-nolan avatar May 03 '20 21:05 colin-nolan

Hmm... if all is well, I think it should work for full refresh too, but it'll be very slow because the display is refreshed after each bar. That said, there could well be circumstances where it doesn't work (for example with my broken 2.13" which doesn't seem to be able to do a full refresh no matter what). I think I did try it with the 7.5" and 4.2", but could try again while testing the other PR.

joukos avatar May 04 '20 05:05 joukos

The issue is that the code doesn't support it. The stack trace for the failure goes:

scrub (size=16) https://github.com/joukos/PaperTTY/blob/e8661895193df5bfd518591cd24fd4c8d39d3749/papertty.py#L458-L463 scrub (fillsize=16) https://github.com/joukos/PaperTTY/blob/e8661895193df5bfd518591cd24fd4c8d39d3749/drivers/drivers_base.py#L58-L60 fill (fillsize=16) https://github.com/joukos/PaperTTY/blob/e8661895193df5bfd518591cd24fd4c8d39d3749/drivers/drivers_base.py#L63-L67 draw (image.size=(16,self.height)) https://github.com/joukos/PaperTTY/blob/e8661895193df5bfd518591cd24fd4c8d39d3749/drivers/drivers_full.py#L64-L66 get_frame_buffer (imwidth != self.width -> 16 != 800) https://github.com/joukos/PaperTTY/blob/e8661895193df5bfd518591cd24fd4c8d39d3749/drivers/drivers_full.py#L73-L76

colin-nolan avatar May 04 '20 21:05 colin-nolan

Ah, I see, you're right, the code basically ignores the X,Y for full refresh drivers and expects a full-sized image when drawing with it. I think I must've tried it on partial support displays but simply told it to use full refresh.

I "fixed" it now by changing the full refresh driver's draw code to keep an internal frame buffer where it pastes the provided image and then draws the result. Running the thing with a big display takes ages though, so having a warning is probably a good idea anyway and perhaps a --force if one really wants to do that (I feel that scrub is sort of a hardware "degauss" function).

Need to wait until the scrubbing completes on my 7.5" and I'll see if anything goes wrong :)

Update: well it took about 36 minutes and apparently I got the orientation wrong for this display, so it cleared it sideways and not completely. No errors though - pasting to a buffer has the nice benefit of not crashing when doing something dumb :) I'll try to look at this a bit later, today perhaps.

joukos avatar May 09 '20 04:05 joukos

Hmm, another little hurdle with this is that the scrubbing can be initiated basically in three different ways: specifying it on the command line or in terminal mode by either sending a SIGUSR1 to the process or by selecting it from the interactive menu. In the latter two an uncaught exception is not desired, so there should be a somewhat more holistic approach to this.

I tried it out a bit and did the following changes:

  • when invoking scrub with SIGUSR1, the request is ignored (and a message shown) if the display does not support partial refresh or is set to full refresh only
  • when invoking it from the interactive menu and the same conditions apply, a confirmation prompt is shown
  • when invoking it via the command instead, an error is shown and --force is required to do it anyway
  • with the frame buffer scrub now works with full refresh displays too, it should probably be added it for partial refresh screens too, but right now I have very limited time to test it properly
  • bumped the "arbitrary" limit of 32 pixel wide bands to 2048 (because I didn't want to wait fifteen minutes to test :joy:) and modified the function to always draw the bands along major axis, since X/Y orientations differ between displays

Perhaps doing these changes is a bit unnecessary for what is essentially a debugging function for "dirty" screens (the maximum width was there for a reason - larger values didn't produce a good result with my 2.13") and the whole function could almost be removed entirely. At least it shouldn't be mentioned as the first command to try in the documentation.

I still need to clean up a bit before merging all of that. This PR opened a little can of worms it seems :)

joukos avatar May 10 '20 20:05 joukos

Sorry about this PR taking so long, I've been too occupied with other things lately. I'll try to keep it near the top of the stack of "things to do when I'm not busy or too exhausted".

joukos avatar May 31 '20 18:05 joukos

Don't worry about it - I only added the PR to make the error clearer!

colin-nolan avatar May 31 '20 19:05 colin-nolan

PR #106 essentially "fixes" this in the sense that scrub just fills the display and doesn't crash, so I think I'll close this.

joukos avatar Sep 27 '23 17:09 joukos