`min ratio to crop` site setting should respect w/h ratio as defined in markdown

Images with high h/w ratio do not respect the manually set size in post (composer preview shows correct size, when post is submitted the size is briefly correct but is then resized to screen width)

![image|50x50](upload://dO5YfHKxcWVcelI12ypCQpOhc3A.png) 

![image|50x50](upload://dBlV2poMFtso5zGLgXpcBraTVxg.png) 

2 Likes

Over the years we’ve had a few topics on tall images (high ratio of height to width). AFAIK, the standard behaviour is what you are encountering. There is a site setting to change this:

3 Likes

This would indeed be the case, if the image was inserted without manually cropping the size.

My issue is not wanting the image to display in its default size (with higher height than width) but sized to parameters entered manually (please observe that the size is set as 50x50 for both! images, but only one is respected)

I still think this is a bug because the image was not inserted with auto size set up by system (and then adjusted according to site settings), the system actually didn’t respect the size that was entered purposefully by hand.

2 Likes

I understood what you said: I said that is the standard behaviour as I understand it.

In other words, you didn’t manually crop the size for the saved post. I don’t think that you can but one of the team can confirm this.

See the following post which explains how resizing is handled and you’ll see that setting both the height and width measurements won’t work unless they are the actual dimensions:

1 Like

Respectfully, I believe this is not the case. Perhaps I didn’t explain it properly, so I will try again:

  • the original image in question has low w/h pixel ratio (this is not disputed)
  • if the picture is inserted directly into the composer it will be sized with auto width and height in markdown like this ![image|164x500](upload://) and it will retain the same low w/h ratio and thus be displayed according to min ratio to crop
  • but, when the image is manually resized in markdown as ! [image|50x50](upload://): this new w/h ratio is 1, and so it shouldn’t trigger the min ratio to crop site setting.

The original image cannot be cropped, as all the info it contains is important, the desired outcome is to create a small 50x50 thumbnail, which points to the original.

The properly rephrased issue is therefore:

min ratio to crop site setting should respect w/h ratio as defined in markdown instead of physical pixel w/h ratio.

@dax, this was moved from #bug to #support, should I open new topic in #bug or edit OP in this one?

3 Likes

The rephrased issue looks right. But if you had read the topics I linked to then you would understand why it probably won’t get any traction.

The site setting was developed to prevent individuals overriding it whereas you want to override it. Instead the site setting must be changed sufficient to make the image aspect ratio acceptable for resizing.

In other words, images that are too elongated are not acceptable by default and must be explicitly allowed by the site admin. Individual users cannot override this.

1 Like

What would this thumbnail look like? Using your 50x50 example, I see three options:

  • The top/bottom of the image is cropped to make the thumbnail a perfect 50x50 square.
  • The left/right side would have black (or similar) filler so that you see the “full” image in the original aspect ratio within the 50x50 thumbnail.
  • The image is stretched to make the complete (but distorted) image take up a perfect 50x50 square.

Is there another option I’m not seeing?

Please check the OP: the first image is actually resized to 50x50, and the way it is done is just fine and should work for any ratio (I believe it crops the picture from the top center with full width, and with height that is adjusted for the declared size ratio, then resized).

The problem is that for the image which has a low w/h pixel ratio but an acceptable manually set screen ratio the global setting shouldn’t kick in.

Manually set ratio should take precendence, as this setting is meant to prevent the picture from dominating the large portion of the display (on account of its height), which it clearly doesn’t at 50x50.

Yeah, I saw that. But it seemed to contradict your other statement:

Reading it again, I realize that you said the original shouldn’t be cropped. As far as I know, the original image is never cropped, so there’s nothing to worry about there.

Regardless, I agree with your general concern/suggestion. The stated reason for only showing partial images is to prevent the high height-to-width ratio images from dominating the page. When setting the dimensions to something like 50x50 in your example, this is obviously not the case. So there’s no reason to ignore the specified markdown dimensions.

1 Like

I’m not one of the Discourse team so I’m only repeating what I’ve seen.

There has been more than one reason given for the current default. The reasons I remember seeing here are:

  • provide control of this feature for the site owner
  • prevent elongated images dominating by taking too much space vertically
  • prevent elongated images becoming horizontal slivers or breaks
  • provide legible previews
  • discourage unusual unoptimized images (including elongated images) which are sometimes due to unintended stretching in one dimension

There is also the issue with the composer preview which lets you think that you can manually resize the display image. This has been reported before but apparently hasn’t been considered a priority to fix:

I think you’re missing the point. Why should a 200x1000 image specified as 200x200 in markdown be treated differently from a 200x300 image also specified as 200x200 in markdown?

1 Like

I’m not missing the point because I’m not arguing for or against changing the default. I repeat that I’m just reporting what already exists in the forum. I’m not in the Discourse team so I have no role in deciding what happens.

The only opinion I have expressed is that I consider it is unlikely that there will be any change in the current default. After five years reading nearly every new topic in this forum, I am far more comfortable with how the team make decisions. At the moment, the lack of input from the Discourse team is telling.

Perhaps we should give the team some space to respond then, as this has been derailed from a simple bug report into a full off-topic opinion piece.

This straw man has been beaten to death, perhaps it is time to lay it to rest?

No one here is arguing that this default needs changing, actually it is quite sensible as it is:

What I’m arguing is that min ratio to crop should only operate using the dimensions defined in the post itself, rather than those from the physical file.

If the image with the low w/h pixel ratio is resized manually inside the post, it surely doesn’t dominate the discussion any more?

And before the question arises, why this constitutes a bug:

it’s because I’m unable to format only a specific subset of images, which would dominate discussion if left unformatted, thus creating a catch-22.

If you still don’t belive me that this setting breaks the expected functionallity, please try and resize the second picture in the OP to 50% using the composer scaling tool.

The result is that all pictures in the post can be narrowed to half width, except the tall narrow ones.

1 Like

It’s not a bug because it is the normal and default Discourse behaviour. It is a known limitation by design. Plus there is a site setting to allow what you want and it is the site owner’s prerogative to lift that restriction.

There are site settings for image sizes, file extensions, etc. which get similar requests for the default to be changed. They’re not bugs if you only have to petition the site owner to try to get a setting changed.

This is not a bug. If you set it back to bug, you will rather abruptly find that you will no longer be welcome here.

2 Likes

Hi Jeff, thank you for taking interest in this issue.

I can assure you I didn’t (and wouldn’t ever) recategorize this topic, after it was moved to another category by a member of the team (please note that the recategorization back to bug category was not done by me). I’m quite happy to call this a feature request.

My purpose for posting here was to state the problem I’m having with the regular usage of the software, and to solicit a response from the team.

I’m in no way demanding anything, not even a response, but I would at least like to be heard by the team (and not just dismissed several times in a row by the same—undoubtedly well meaning—member of this forum with the same argument, that doesn’t fully address the issue).

I hope you were able to wade through the entire topic and are familiar with the issue, but just for the benefit of clarity, here is the recap:

  • not all images can be set to desired size using markdown (unexpected behaviour from a user standpoint)
  • this is governed by min ratio to crop, which uses image pixel w/h ratio, even when image is manually resized via markdown to acceptable w/h ratio
  • if it is not too much of an investment, would it be possible to use w/h ratio as defined in markdown instead

This would solve the unexpected image scaling behaviour such as this (all images set to 50x50):

Thank you for your consideration, and thanks again to the entire team for all the effort that goes into this marvellous piece of software!

7 Likes

I’m not sure how complex it would be to change, but I’d like to see this too.

I usually run into this when I’m posting a screenshot of the UI… when you draft a post there’s no indication that the image is going to be cropped, so I end up posting it, seeing the cropped image, and then editing the post to avoid the crop. A few times I’ve tried to edit the dimensions in the markdown, but of course that doesn’t work… so I ultimately will go back and crop the image a re-upload it.

8 Likes

@zogstrip how do you feel about


    if crop
      cropped_width, cropped_height = ImageSizer.crop(original_width, original_height)

      if cropped_width < width
        width = cropped_width
        img["width"] = width
      end

      if cropped_height < height
        height = cropped_height
        img["height"] = height
      end
    end

VS the existing:

https://github.com/discourse/discourse/blob/609625fa180715fcf253dcbe31d6f8daf8f8c5f8/lib/cooked_post_processor.rb#L324-L328

It is certainly less surprising than the current behavior and would make @awesomerobot happy, only real annoyance I can see is that this test is very very very mocky.

https://github.com/discourse/discourse/blob/609625fa180715fcf253dcbe31d6f8daf8f8c5f8/spec/components/cooked_post_processor_spec.rb#L546-L572

If you like feel free to commit.

7 Likes

Recommending a minor change:

Changing one dimension but not the other might result in some… oddities

2 Likes

From what I can tell it does an aspect ratio fix afterwards, at least on dev

5 Likes