-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
convert_resize_img should not crash if it can't resize the image #75
Comments
On second thought - if the image can't be clipped to the cell bounds - the exception gives me useful info. I can revert to text display of the custom emoji (it looks like : emojiname : ). |
I also encountered this while testing and provided a solution. What do you see to this? The issue is, in computing the size of the image to fit into a I have considered always rounding computed dimension up instead but I have not decided on whether its a good idea. I've tried it before and it make some images go really off their original aspect ratio when small. Also, I could just use a Anyways, aside small (zero) size, there are also other errors that could occur when converting an image, which are not reasonably remediable. For instance, if an image was initialized with a PIL image instance with an invalid mode. I can't recreate it right now but I remember it was a certain exception I was encountered that caused me to add an exception handler for conversion. |
That's another option. And this exactly (i.e leaving the user to handle the exception as they see fit), is why I intentionally left rendering errors unhandled. You can subclass Sorry, I don't know why/how I mistakenly edited your comment. 😆 |
It'd be interesting to see how it performs with max (dimension, 1). At such small image sizes, aspect ratio may be a bit distorted, but it won't matter. And the error placeholder widget will provide a nice solution in cases where we get an exception. |
Hmm... Okay. Do I implement the quick fix now? Or do I just wait and implement the actual solution? The actual solution is something I had in mind to do already. A means of preserving aspect ratio beyond just the size computation. If you've noticed, images rendered with a small size tend to be way off their original aspect ratio. This is caused by rounding. The actual image image size computation is done in pixels but when converting from pixels to cells, some precision is lost. I intend to store the ratio of the computed pixel size to the rounded pixel size along with the size. Then at render-time, that ratio can be used to compute the actual image size, and the image is composited unto a blank image of the rounded size (i.e pixel-equivalent of the image size in cells). This is not so trivial. So, I wouldn't want to delay this release any longer. Do you think the error placeholder will suffice for now, or I should implement the |
Also, I forgot to mention that My reason for the property was to provide a means by which the aspect ratio of an image can be distorted if wanted but I ask myself every time, who really wants to do so? |
Changed the label because this has actually been the intended behaviour until now. Also, I I'm considering changing the title because any exception raised within Or what do you think should be done if an exception is raised at that point? |
- Fix: Ensure `_valid_size()` never returns zero for either dimension. Refs: #75
- Change: Ensure `_valid_size()` never returns zero for either dimension. Refs: #75
Your idea for a long term fix sounds good. I don't know who's going to want
to have the widget distort their image. The options available now -
display as-provided, or resize-to-fit available space, preserving aspect
ratio, seem right.
I'm going to need to catch these exceptions, so overriding render() seems
like my near term solution.
…On Sat, Feb 25, 2023 at 9:16 PM Toluwaleke Ogundipe < ***@***.***> wrote:
Hmm... Okay.
Do I implement the quick fix now? Or do I just wait and implement the
actual solution?
The actual solution is something I had in mind to do already. A means of
preserving aspect ratio beyond just the size computation. If you've
noticed, images rendered with a small size tend to be way off their
original aspect ratio. This is caused by rounding.
The actual image image size computation is done in pixels but when
converting from pixels to cells, some precision is lost.
I intend to store the ratio of the computed pixel size to the rounded
pixel size along with the size. Then at render-time, that ratio can be used
to compute the actual image size, and the image is composited unto a blank
image of the rounded size (i.e pixel-equivalent of the image size in cells).
—
Reply to this email directly, view it on GitHub
<#75 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAY4FJWLEZOE7GTBRIL4EALWZK4JJANCNFSM6AAAAAAVIEDS34>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
I see, good. Commit 6f6aa39 actually already covers your case. With the changes, it's now impossible to have a zero size. As for the scale (which you don't use), I've also made the necessary changes to disallow a zero render size but i can't push until i update the related tests. |
About the image conversion I mentioned yesterday: from PIL import Image
from term_image.image import *
AutoImage(Image.new("La", (100, 100))).draw() results in:
IMO, there's obviously nothing that could be done to handle such, the exception just has to be raised. |
As for resize errors, any obvious this ones are now prevented by ensuring the image size, rendered size and hence, render size is never null. The rendered size covers the case of small scale that I mentioned. This will keep things together until I get to implement the actual solution later. Can't wait to get images with well preserved aspect ratio 😁. Thanks for the report and suggestion. 🙇🏾♂️ |
One last question... Please, do you think |
Description
I'm trying to render an image (Mastodon custom server emoji) into a 2-cell-wide x 1-cell-tall box. Usually this works fine. Sometimes I encounter a server emoji that can't be resized this small. The error occurred much more frequently when I was using a 1x1 cell box, expanding to 2x1 is fine. Expanding beyond that is not going to work for this application.
To Reproduce
Steps to reproduce the behavior:
Code snippet: This will constrain the UrwidImage to render in a 1x1 cell:
Nest the below widget
cw
in whatever testbed app you like.Make sure url_string points to the url of a suitably large image.
columns = [] img = Image.open(requests.get(url_string, stream=True).raw) columns.append(urwid.BoxAdapter(UrwidImage(AutoImage(img)), 1)) columns.append(("weight", 9999, urwid.Text(""))) cw = urwid.Columns(columns, dividechars=1, min_width=1)
4. See error
File ".../dev/term-image/src/term_image/image/common.py", line 1584, in convert_resize_img raise ValueError("Image size or scale too small") from e
Expected behavior
Ideally the image would be clipped to the available space. If it can't be clipped, at least don't crash.
Screenshots
If applicable, add screenshots to help explain your problem.
The text was updated successfully, but these errors were encountered: