py5coding / py5generator

Meta-programming project that creates the py5 library code.
https://py5coding.org/
GNU General Public License v3.0
52 stars 13 forks source link

`save_frame()` cannot be called from a mouse or keyboard event function but that works fine in Processing #394

Closed hx2A closed 8 months ago

hx2A commented 9 months ago

From @villares:

About this, being careful of what you do outside the animation thread, I was very surprised today to learn that I can't call py5.save_frame(...) from my key_pressed() event function when using P3D (an OpenGL renderer)! Was that always so? Or is is it something new to Processing 4? Also I would say it is not intuitive to think of saving an image as a "drawing method"...

Here's what's happening:

py5's save_frame() is different from Processing's saveFrame(). py5 uses the PIL image library to save the image, letting you save the image in many formats and all the other nice features PIL has. py5 also lets you save to a BytesIO object. For these work, it needs to access the frame's pixels, calling Processing's updatePixels() method. That call must be triggering that error.

The error message should be improved, and if possible, the functionality should be repaired. Ideally this would work.

The save_frame() method can tell if it is being called from draw() or not. Then it can fall back to Processing's method instead, or perhaps use py5_tools.screenshot().

What about save()? Should test that also.

Originally posted by @hx2A in https://github.com/py5coding/py5generator/discussions/355#discussioncomment-7960774

hx2A commented 8 months ago

Then it can fall back to Processing's method instead

I started working on this. The solution is to fallback to Processing's save() method.

This issue only applies to py5's save() and save_frame() methods. The Py5Graphics and Py5Image objects' save() methods do not have this problem.

One shortcoming with what I have coded is that there is reduced functionality in that Processing's save() method does not save in all the formats that the PIL image library does and that you cannot save an image to a BytesIO object. This limitation would have to be documented.

Alternatively, py5 could save to a temporary file with Processing's save() method, read it back, and then proceed as before, saving to the requested filename with PIL or saving to a BytesIO object. This would make the save() and save_frame() methods' functionality consistent across all use cases at the price of a small performance hit.

I prefer the alternate solution, but want to hear what others think first.

villares commented 8 months ago

My naive solution would be something like this (pseudocode):

if legacy_mode:
    legacy_save(...)
else:
    try:
        pillow_save(...)
    except TroubleSaving: 
        warning('Use legacy_mode=True when outside draw()!')
        legacy_save(...)
hx2A commented 8 months ago

My naive solution would be something like this (pseudocode):

The issue in py5 isn't the saving, its the getting the pixels that will be saved to the image. Processing's save() method takes care of this. When saving with PIL, py5 needs to get the pixels from load_pixels() first.

I thought of a better solution than what is above; py5 can save with Processing's saveFrame() if the user wants to save in a format that Processing can handle. If not it will use PIL, getting the pixels from load_pixels() first.

hx2A commented 8 months ago

+PR #415 for this issue

Fixed! Now py5 will use Processing's save() method when it can't call load_pixels(). If the user wants to save the image in a format Processing supports it will do so directly. If not, it will save to a temporary PNG file, load it back, and then save it again with PIL.