PowerToys icon indicating copy to clipboard operation
PowerToys copied to clipboard

Image Resizer - Ignore Orientation Not Working

Open spiceygas opened this issue 3 years ago • 24 comments

Microsoft PowerToys version

0.51.1

Running as admin

  • [X] Yes

Area(s) with issue?

Image Resizer

Steps to reproduce

According to the documentation, the "ignore orientation" checkbox should affect whether the specified width x height dimensions are strictly enforced or swapped for portrait-oriented pictures. However, it's not working. The resizer always swaps the dimensions.

  1. Testing with an image that is 4000 x 6000 pixels. (Portrait orientation)
  2. Resize the image.
  • Resize Type: Fit
  • Width: 60
  • Height: 40
  • Unit: Pixel
  • [Check] Make pictures smaller but not larger
  • [Check] Ignore the orientation of pictures
  • [Check] Resize the original pictures
  • [Uncheck] Remove metadata that doesn't affect rendering
  1. Resize. The output image has swapped the dimensions, and is 40x60 instead of 60x40. It remains in portrait mode.
  2. Repeat step 2, but this time uncheck the box to "ignore the orientation of pictures."
  3. Resize. The output image has swapped the dimensions, and is 40x60 instead of 60x40. It remains in portrait mode.

The checkbox state is not correctly controlling whether the orientation is forced or rotated.

✔️ Expected Behavior

When unchecked, the documentation says that "ignore the orientation of pictures" should force it to take the literal dimensions provided (60x40).

❌ Actual Behavior

Image Resizer is swapping the dimensions from 60x40 ==> 40x60 regardless of whether or not the checkbox is checked.

Other Software

N/A

spiceygas avatar Dec 08 '21 12:12 spiceygas

Sounds like a bug. Has the code changed recently? I'm pretty sure I had a test for this.

bricelam avatar Dec 08 '21 17:12 bricelam

Could it be explained by this documented behavior?

If Ignore the orientation of pictures is checked, the width and height of the specified size may be swapped to match the orientation (portrait/landscape) of the current image. In other words: If checked, the smallest number (in width/height) in the settings will be applied to the smallest dimension of the picture. Regardless if this is declared as width or height. The idea is that different photos with different orientations will still be the same size.

Image Resizer doc, see "Note" Box

CleanCodeDeveloper avatar Dec 11 '21 19:12 CleanCodeDeveloper

The same behaviour occurs regardless of whether or not the box is checked. One state should swap to keep orientation, the other shouldn't

However, no matter what you do you get the same outcome. In other words, the checkbox appears to be doing nothing at all.

spiceygas avatar Dec 11 '21 21:12 spiceygas

Sounds like a bug. Has the code changed recently? I'm pretty sure I had a test for this.

The code in question was not modified since the initial commit in March 2020. The relevant piece of code is:

if (_settings.IgnoreOrientation
    && !_settings.SelectedSize.HasAuto
    && _settings.SelectedSize.Unit != ResizeUnit.Percent
    && originalWidth < originalHeight != (width < height))
{
    var temp = width;
    width = height;
    height = temp;
}

I'm not sure if we share the same understanding about the feature in question. I have hopes that @bricelam can give us some insights.

In my opinion the checkbox only makes sense if you use absolute values and resize more than one image.

Lets say you have 2 images: 520 x 360px (Landscape) Landscape

360 x 520px (Portrait) Portrait

and want to resize them to halve of the size 520 x 360px --> 260 x 180px (Landscape) 360 x 520px --> 180 x 260px (Portrait)

using the following resize settings: image

Result: First try without enabling the "Ignore the orientation of pictures" checkbox: 260 x 180px (Landscape) Landscape (Custom) (Ignore the orientation of pictures disabled)

260 x 180px (was previously a "Portrait" picture) Portrait (Custom) (Ignore the orientation of pictures disabled)

Second try with "Ignore the orientation of pictures" checkbox enabled: 260 x 180px (Landscape) Landscape (Custom) (Ignore the orientation of pictures enabled)

180 x 260px (Portrait) Portrait (Custom) (Ignore the orientation of pictures enabled)

IMHO the feature works as intended but the docs need some love to avoid further confusion.

CleanCodeDeveloper avatar Dec 29 '21 20:12 CleanCodeDeveloper

Question: Did you actually make your pictures by running the test case through the utility? Or did you draw those pictures independently? Because that's not how it behaves right now when I run the test case.

In your first scenario (without checking the box), the pictures you show both come out in landscape format. You labeled the second output picture in the first scenario incorrectly as 180x260, but the picture you showed is 260x180.

In my steps to reproduce (#3 and #5 in the initial post) you get the exact same dimensions REGARDLESS of whether or not you check the box. How you're describing the behaviour is NOT how it works right now (v0.51.1).

It seems that one of the checkbox states should keep the orientation (and letterbox the picture), while the other should swap the orientation parameters to accommodate landscape/portrait. And that's what you described. But it's not what's happening right now.

spiceygas avatar Dec 29 '21 21:12 spiceygas

@CleanCodeDeveloper thanks for the message. What you say is correct and what you show is the intended result. I think the problem for @spiceygas is just that it doesn't seem to work over there. So the thing is, we need to know why, and I am willing to label this issue with Needs-Repro.

Jay-o-Way avatar Dec 29 '21 21:12 Jay-o-Way

@spiceygas is this problem occuring to all images you have tried, or maybe one or a few specific?

Jay-o-Way avatar Dec 29 '21 21:12 Jay-o-Way

Question: Did you actually make your pictures by running the test case through the utility? Or did you draw those pictures independently? Because that's not how it behaves right now when I run the test case.

I used the Image Resizer v0.51.1 for this.

In your first scenario (without checking the box), the pictures you show both come out in landscape format. You labeled the second output picture in the first scenario incorrectly as 180x260, but the picture you showed is 260x180.

Fixed this in my comment, thanks for pointing out.

But it's not what's happening right now.

Can you please confirm that you used the exact same settings (Resize type: Fill, Width: 260, Height: 180, Unit: Pixel)?

@Jay-o-Way: Could you try to reproduce the issue using my sample images?

CleanCodeDeveloper avatar Dec 29 '21 21:12 CleanCodeDeveloper

Could you try to reproduce the issue using my sample images?

Fit 260 × 180 pixels

Fit

Fill 260 × 180 pixels

Orentation test - FILL

Works fine here...

(off-topic) @bricelam Notice the increase from 15.3 to 21.3 kB in the only cropped image? I suppose this is normal?

Jay-o-Way avatar Dec 30 '21 12:12 Jay-o-Way

Notice the increase from 15.3 to 21.3 kB in the only cropped image? I suppose this is normal?

Weird. I know nothing about PNG encoding. 🤷‍♂️ Could be some quirk with the compression. Or maybe some metadata is inadvertently being duplicated.... no idea.

bricelam avatar Jan 04 '22 22:01 bricelam

@spiceygas can you share your original images and a screenshot of the exact GUI with settings?

Jay-o-Way avatar Jan 04 '22 22:01 Jay-o-Way

Ok, using the image you provided above (portrait, dimensions = 360 x 520). My goal is to have it resize down to 120 x 80 with cropping.

Scenario 1: Fill, 120 x 80 pixel, no boxes checked Result: Landscape oriented photo, 120 x 80.

Scenario 2: Fill, 120 x 80 pixel, ignore orientation of pictures Result: Portrait oriented picture, 80 x 120.

Scenario 3: Fit, 120 x 80 pixel, no boxes checked Result: Portrait oriented photo, 55 x 80

Scenario 4: Fit, 120 x 80 pixel, ignore orientation of pictures Result: Portrait oriented picture, 80 x 116

Summary

  1. I think there's some human error here on my part. I thought I had tried all of these permutations, but clearly I missed one of them. Scenario 1 matches my use case.
  2. Is it correct that 3 of 4 scenarios output portrait-oriented images? If so, then this issue can be closed as user error.

spiceygas avatar Jan 10 '22 12:01 spiceygas

Thanks for the message.

  1. That is always a possibility :-)
  2. Yes, that's correct. Only "fill" will crop.

Jay-o-Way avatar Jan 10 '22 15:01 Jay-o-Way

still not fixed.

cjwijtmans avatar Feb 13 '23 23:02 cjwijtmans

still not fixed.

We were not able to reproduce the issue above (see conversation above). I believe the feature works as intended but the docs need some love to avoid further confusion.

CleanCodeDeveloper avatar Feb 14 '23 19:02 CleanCodeDeveloper

still not fixed.

We were not able to reproduce the issue above (see conversation above). I believe the feature works as intended but the docs need some love to avoid further confusion.

Try again. It is not doing what i tell it to. Portrait images are incorrectly cropped even when i tell it to ignore orientation or not. When i tell it to fill 200x50 it should produce that image and not 50x200.

cjwijtmans avatar Feb 15 '23 02:02 cjwijtmans

Try again. It is not doing what i tell it to. Portrait images are incorrectly cropped even when i tell it to ignore orientation or not. When i tell it to fill 200x50 it should produce that image and not 50x200.

That's what I meant with documentation. If i remember correctly, the ignore orientation option does not have any effect until you resize multiple images at the same time. If you still feel this is a bug please provide detailed repro steps.

CleanCodeDeveloper avatar Feb 15 '23 06:02 CleanCodeDeveloper

the ignore orientation option does not have any effect until you resize multiple images at the same time.

@CleanCodeDeveloper meh, the devil is in the details here. The setting applies the highest value to the largest dimension, but the number of images doesn't matter.

Jay-o-Way avatar Feb 15 '23 07:02 Jay-o-Way

@cjwijtmans i think the FILL setting is crucial here. Imagine you have a portrait image and you want to fill it (e.g.) 400×300px. What would be the expected result? So if I'm correct, Fill will always crop and the "ignore" setting should only be visible for Fit. Pinging @bricelam

Jay-o-Way avatar Feb 15 '23 07:02 Jay-o-Way

Here is my brain dump from the last time I thought about the interaction between these two settings: https://github.com/bricelam/ImageResizer/issues/38#issuecomment-368962399

bricelam avatar Feb 16 '23 19:02 bricelam

@cjwijtmans i think the FILL setting is crucial here. Imagine you have a portrait image and you want to fill it (e.g.) 400×300px. What would be the expected result? So if I'm correct, Fill will always crop and the "ignore" setting should only be visible for Fit. Pinging @bricelam

The issue is not the cropping. The issue is that when i tell it to create a 500x400 image it will create a 400x500 instead because portrait. Its essentially not doing what i am telling it to. I have resorted to manually cropping instead which is fine. But its still a bug.

cjwijtmans avatar Feb 17 '23 04:02 cjwijtmans

@cjwijtmans Do you have Ignore the orientation of pictures checked? If so, why? Does it work as expected when you uncheck it?

bricelam avatar Feb 28 '23 19:02 bricelam

This comment illustrates the intent of that checkbox:

You may have landscape and portrait photos in 16:9 ratio that you want to fill/crop to 4"x6". The portrait photos should be 6" high and the landscape ones should be 6" wide.

bricelam avatar Feb 28 '23 19:02 bricelam

@cjwijtmans Do you have Ignore the orientation of pictures checked? If so, why? Does it work as expected when you uncheck it?

Simply read my comments again it contains your answer.

cjwijtmans avatar Mar 04 '23 06:03 cjwijtmans

@cjwijtmans I'm having a hard time following your comments. All I'm hearing is "it's wrong", "it's not fixed", and "it's not doing what I tell it to."

Can you provide some clear repro steps along with your expectations? For example, what are the dimensions of the input images, what are all the settings you've specified? What are the output dimensions your expecting for each of the input images?

bricelam avatar Mar 06 '23 18:03 bricelam

image

I have made this to describe the current behavior of the "ignore orientation checkbox" specifically the confusion around the Unchecked Image 2 quadrant. I also think the term ignore orientation is confusing and should probably be a toggle between fit or fill. But then that opens the option to change how you fill: center zoom, top left, bottom right... etc.

Does this lay everything out clearly? How do we want to solve for this? /needinfo

TheJoeFin avatar Sep 19 '23 18:09 TheJoeFin

Your graphic is good, @TheJoeFin . The confusion lies in the fact that - when checked - the words "width" and "height" do not apply anymore. The highest input value applies to the longest dimension for each image file. Using words like "long(est) side" and "short(est) side" sounds a bit odd, but are much more accurate.

I think the UI should have a choice between

  • width & height
  • long side & short side

Yet, then the code should have more logic to handle all this (like, check which value is actually bigger). This is indeed very complicated. 🤯

Jay-o-Way avatar Sep 19 '23 18:09 Jay-o-Way

I agree, it's complicated. And it stems from users having an overly-simplified mental model. If they specify 5"x7", they expect all their photos to be 5x7 regardless of whether they were oriented portrait or landscape when they took them. But when they're resizing just one digital image, they don't expect the dimensions they specify to suddenly be flipped because that checkbox is checked.

Maybe a warning icon with a tooltip should show up when:

  1. They've only selected one image
  2. The checkbox is checked
  3. The input dimensions are oriented different from the source dimmensions

bricelam avatar Sep 19 '23 20:09 bricelam

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 5 days. It will be closed if no further activity occurs within 5 days of this comment.

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 5 days. It will be closed if no further activity occurs within 5 days of this comment.