Image scaling error

[dadr]

I've been trying all the options in the python3 branch, and came across something I thought was amiss in how the software handles images which don't fill the screen. There are 4 options, and 3 work fine, but do nothing does not work like I think it was meant to. When do nothing is selected, the image is still scaled to fit the screen. I "think" it is meant to scale down if necessary, but if not, then there should be a small image centered in the screen. Looking through the logic, modules/helper.py:makeFullframe() seems to ignore properly, and scale everything else to what it should be. Then the image gets handed back and eventually winds up in modules/display.py:image() where I think that each and every image goes through convert again and is scaled to the screen. (Although I'm not sure I follow all the functions with convert in the display module. What are the x and y offsets for? Is it compensating for rotation?) It would be my suggestion that a case be added to makeFullframe() to scale down do-nothing when needed and pad the image if not - and then determine whether we can remove the scaling in image(). I think that the net result will be to call convert one less time, and to behave properly. It would also be possible to do a mat frame for small photos (e.g.add beveled edges)? In any case, I wanted to make sure that I understood all this correctly, to get some input on the offsets in display.py, and ask if making such a change made sense to others as well?

[dadr]

I tried some things, and actually two options were not working as described! I think that removing the 2 lines:

  '-resize',
   '%dx%d' % (self.width, self.height),

in the function image() in display.py (lines 232,233 in the python3 branch) fixes this issue.

Is it worth trying to refactor into makeFullframe to try to save a convert or two - or should I just do the simple PR with this? it does seem to me that it would make sense to have the image processing unified, and then keep the display module just about displaying. That would make it simpler to add more image processing options over time.

[mrworf]

The trick with the image() call is that it takes into account the physical display and how the memory must be laid out to work. I think the resize was added to make sure it will render properly if it gets an odd size. But it does change the image too which is less than ideal. What image() must guarantee is that the physical size of the data matches the display, so the use of -resize while it accomplishes this, it isn't necessarily the right thing.

It might make more sense to tell convert that the canvas must be X times Y instead, this would leave the image untouched and pad with black where needed or crop where needed.

So I don't think we can avoid that convert call since it also deals with pixel data for the display, and baking this into makeFullframe is bad since the output from that function is (I believe) to some extent cached and reused, which means we cannot have hardware based data there (except for the resolution) since it will make it difficult to deal with later should user change any settings (trying to be smart about fetching data from google since they have limits for how hard you can ping them).

But I'm more than open to a PR which investigates the change of resize to canvas instead.

[dadr]

Thanks for the reply. I think that the change I suggested above does what you describe. I'm no ImageMagick expert, but I thought that the -extent portion of that command does what you say. I can confirm that it adds and crops to make the result match the screen - as you recommend. The following was a 600x600 image hand-run through the command: It comes out formatted well for HD screen, and seems to run properly as modified. Oversized pictures run through do nothing show a central portion. Let me know if I've misunderstood how this works.

So the cache saves the modified pictures? I had the impression it only saved the originals so they would not need to be downloaded again and again. Seems that changing resolution could mess up modified pictures' formatting as well.

But maybe let's leave refactoring for another day. If it's not broken they say...

Let me know if the 2 line change above works for you and I'll turn it into a PR.

[mrworf]

Correction, I doublechecked and the only change done before caching is rotating it to the orientation it SHOULD have according to EXIF data. My bad, been a while since I looked at the code.

And yes, what you're saying and showing looks absolutely correct, please do a PR, also please use this change for 2 days and let me know if you've seen any odd behaviors before I merge it :)

Thanks 😄

[dadr]

This is in the #187 PR for python3. There is also an outstanding PR #183 that has this as well as the fix for #182.

[PhilWareAR]

Looks like this broke the resizing of portrait orientation images larger than the monitors resolution.

Our frame is setup to "do nothing" (don't care for the blur or fill). Previously, the displayed result of resizing was blank-space on either side of the resized large portrait image (complete image displayed with blank space on either side). After the update (?), the resulting images are zoomed in.

Thanks for this project, we love our picture frame zoomed in or not.

debug.txt

[dadr]

I had the same question (see the text just below the motorcycle picture) but came away thinking this is how mrworf intended it to work. Do you think there should be another option? If so what would it be - ideally? It seems you are looking for something that would scale images up or down to fit the screen, but leave size mismatches as blank (black) space. (i.e. what "do nothing" was doing before) Thanks for your suggestions!

[PhilWareAR]

--snip It seems you are looking for something that would scale images up or down to fit the screen, but leave size mismatches as blank (black) space. (i.e. what "do nothing" was doing before) --snip

Yes. This is how the resize worked previously. Panoramas etc. displayed resized to fit the horizontal dimension of the frame (monitor) with black above and below; skinny portraits filled the vertical dimension with black space on either side.

"Do Nothing" is working correctly: images are displayed "as-is" with no resizing, resulting in the zoomed-in effect for images that exceed the dimensions of the screen, and zoomed out as in the motorcycle image. "Blur" is working correctly. The images are resized to fit the screen up to the nearest dimension: both "oversized" portrait and landscape images are resized with blurred image filling the blank space either above/below or to each side the least dimension. "Zoom" is working correctly. Images are resized to fit the screen in the nearest dimension, but are cropped. "Auto" I'll assume is working based on Blur/Zoom.

If I had my druthers, the "Blur" choice would be replaced with "Resize with Blur", and, "Resize without Blur". The resize is what I want, the Blur is what I'd like to unselect.

Thanks again, Phil

[mrworf]

So we're missing an option here then :)

• Do nothing (no change of image size, do your best to show it, cropped or padded)
• Blur (resize to fit, fill blank areas with zoomed in version that is blurred)
• Zoom to fill (resize so it fills the entire screen, crop if necessary)
• Zoom to fit (resize so it shows entire image, pad if necessary)
• Auto (combo of zoom to fill/blur where it picks blur if too much will be cropped)

Does this sound about right? So the option missing here is "zoom to fit"

[dadr]

That's my understanding, but @PhilWareAR can confirm.