Image resize not correctly honoring aspect ratio and staff space unit settings

I didn't have any trouble resizing images with or without the aspect ratio option, and my changes were saved. Can you attach your score and precise steps to reproduce the problem?

OK, I see, "Lock aspect ratio" works fine for dragging but it doesn't apply to adjustments made in the Inspector.

The save/reload issue I cannot reproduce with a current build, however. I followed your steps and on reload got exactly what I expected. Can you try again with today's beta update?

Thanks, that explains things and should help in fixing the issue.

Any chance this can be fixed in 3.3? Not being able to resize inserted images without distorting them is a major pain with no work-around, as far as I know.

For me, editing the image directly - double-click and drag handles - works fine. So does placing the image in a frame and using the "scale to frame size" button. So that's a couple of workarounds, no?

Meanwhile, though, should be a pretty easy fix I would have to imagine, although image processing isn't my forte.

Drag-handles seems to work (didn't last time I complained about this). Frame-size good, too - thanks!

Came up again in #301282: Changing size values in the inspector for an image does not obey "Lock aspect ratio"

I have a functional fix for this, but I don't like it because it accumulates rounding errors, so the aspect ratio starts to drift as you type in new values or spin then up and down. Sadly, this happens even if they start out with the exact same value (e.g., 100.0 sp × 100.0 sp).

The main reason for this is that the image does not have any memory of its “real” aspect ratio, so the code has to continually recalculate it on the fly based on whatever the most recent width and height values were.

I would really like to change the way this feature works by always locking to the original aspect ratio of the image rather than whatever the aspect ratio was when the user decided to check the “lock aspect ratio” box (as well as whatever the aspect ratio morphs to over time due to accumulated rounding errors).

Another option is to “lock in” the desired aspect ratio by taking a snapshot of it at the moment the user checks the “lock aspect ratio” box, and then use it for all subsequent calculations. If we do this, this cached aspect ratio will have to be stored in the score file.

Thoughts?

@Spire42 I've managed to fix this, opening a PR now.

Hang on. What approach did you take?

Adding a _ratio member variable to InspectorImage.

Ah, good. Are you persisting this in the score file?

> Another option is to “lock in” the desired aspect ratio by taking a snapshot of it at the moment the user checks the “lock aspect ratio box”

Yes, I did that

> Ah, good. Are you persisting this in the score file?

Unfortunately VS basically started a rebuild after I switched to a new branch to commit it. It's gonna take some while.

OK. Definitely needs to be stored in the score file, or we end up with the original problem again next time the score is opened.

Also, don't forget to apply the locked aspect ratio to the mouse-drag-resize calculations as well.

Just keep in mind anything changed about how the file is written comes with a compatibility concern, so make sure reasonable things happen in both directions.

@Spire42 The edits survived save-and-reopen.
@Marc Sabatella I didn't even touch libmscore code, so nothing changed about how the file is written.

If you're not saving the aspect ratio in the score file, where is the value coming from when you open the file? Is it being recalculated from the current width and height? If so, you'll get rounding errors — really bad rounding errors if the image has been resized to a tiny size prior to saving.

"Really tiny size"? Tiny enough to make 0.01sp look very wide? Why would you do that anyway...

Anyway, here's the PR: https://github.com/musescore/MuseScore/pull/5731.

> "Really tiny size"? Tiny enough to make 0.01sp look very wide? Why would you do that anyway...

I used an extreme example to help illustrate that the fix is broken/incomplete.

I can see a need for "really small" images, e.g., substituting for typography/unavailable fonts.

An extreme example doesn't make the fix broken or incomplete.

@BSG But even the smallest images are by the order of magnitude of 0.1sp, right? The fix works completely fine for things over 1sp, and drifts a tiny bit (about 0.01sp) for things below 1sp but over 0.1sp. I don't think that makes the fix broken.

> An extreme example doesn't make the fix broken or incomplete.

Of course not. The extreme example is supposed to make it easier to understand that the fix is broken/incomplete, which it is.

Why did you create a variable to store the aspect ratio? Because otherwise you would quickly accumulate rounding errors. If you throw away the value of this variable at any point while it's still in use (such as when closing and reopening the score), you get a rounding error when you try to recalculate it. Repeatedly closing the file, reopening it, and resizing the image will still cause you to accumulate rounding errors, only more slowly. Mathematically speaking, it's the same as not having the aspect-ratio variable at all.

Yes, they would be bigger than order of 0.1sp; if this is the way to go, then limit the minimum size a priori.

Tiny size is a red herring (and I regret using it as an example), because the rounding-error accumulation occurs regardless of size.

I think you misunderstood what's going on in the fix I provided. The ratio variable has to be defined even if there would be no rounding errors, because we need a way to remember the ratio. If there's no such variable, we cannot retrieve the original ratio value after one of the size values has been changed. (Although, if you have a way to retrieve it, please do tell me :-) )

> The extreme example is supposed to make it easier to understand that the fix is broken/incomplete, which it is.

It is not, if it only fails at extreme cases which no one would create other than for showing "a fix is broken/incomplete". The failure doesn't lead to a crash, nor a corruption, nor anything critical.

> then limit the minimum size a priori.

It is indeed what should also be done in my opinion.

What I'm saying is that the rounding errors that you're preventing by introducing the aspect-ratio variable are the exact same rounding errors that will occur when you close and reopen the file and then resize the image (without saving the aspect-ratio variable to the score). This is not an extreme or artificial case; it's 100% normal usage when working with any image at any size.

Aren't the rounding errors because of the omission of digits way behind decimal point? If so, what can you do to prevent the rounding errors?

@M.Thum @Marc Sabatella I cannot reproduce the second part, which is about staff space unit settings. I mean, there're some glitches around toggling the reset button for the setting, but there's no visual jump as far as I can tell.

> Aren't the rounding errors because of the omission of digits way behind decimal point?

Yes, but with accumulation, the errors can very quickly propagate to the visible range of the number (i.e., what the user sees in the spinboxes). This is compounded by the fact that the spin buttons themselves introduce further rounding errors because those buttons increment/decrement by 0.1 (which is not representable in floating point) and then “round” the resulting value to the nearest 0.01 (also not representable).

In my testing I was easily able to introduce numerically visible rounding errors by clicking the spin buttons fewer than five times in some cases (again, using normal-sized images with reasonable aspect ratios, including 1:1).

With my fix, I failed to introduce visible rounding errors by just continually scrolling the spin boxes up and/or down, or closing and reopening the file.

> Also, don't forget to apply the locked aspect ratio to the mouse-drag-resize calculations as well.

@Howard-C, are you planning to do this? The relevant code is inside Image::editDrag(), which is in libmscore.

FWIW, in my version of this fix, I made all of my changes in the Image class, and I left the InspectorImage class untouched.

Is this bugged as well?!

In my opinion it isn't the best choice to fix this in libmscore because the issue is all about inspector control instead of element behaviour.

> Is this bugged as well?!

In the sense that it has the same kind of accumulated rounding errors that the other one has, yes.

> In my opinion it isn't the best choice to fix this in libmscore because the issue is all about inspector control instead of element behaviour.

I disagree. “Lock aspect ratio” is a property of the element, not the Inspector. The Inspector is just one of multiple ways for the user to change the image's properties (including size). Enforcement of the locked aspect ratio should happen centrally, inside the actual element.

@Spire42 You can submit your fix as well and let's compare.

The issue emerges when an inspector action is done, and more precisely, it is about the relation between two values when one of them is changed in the inspector. I feel it's more natural to use the signal-slot system of Qt to do this.

Thanks. I didn't want to step on your toes since you beat me to the punch with the PR, but I'll go ahead and do that. I'm working on IRL stuff at the moment so I won't be able to get it completely done and tested for maybe a day, so I hope yours doesn't get merged in the meantime. (Is there a way for you to request that it not be merged yet?)

An admin can add the "under discussion" label to it, but I'm not an admin :)

You can request for it in the developers' chat, I'm having trouble connecting to Telegram recently.

And I didn't mean to "beat you to the punch", it's just I'd already finished the commit before I saw this post. I was looking at a duplicate thread before.

came up again in #302154: Graphics - lock aspect ratio

OS: Windows 10 (10.0), Arch.: x86_64, MuseScore version (64-bit): 3.4.2.9788, revision: 148e43f

The "Lock aspect ratio" box, even when checked, always behaves as if unchecked. See, also, #303235: UNDO doesn't restore image's original size after checking "Scale to frame size".

Came up again in https://musescore.org/de/node/307675

Fixed in branch 3.x, commit 17e709a477

_fix #279967: image resize not correctly honoring aspect ratio

A new member variable _aspectRatio of InspectorImage is added to remember the image's original ratio. It is also updated when "Lock aspect ratio" is checked in lockAspectRatioClicked(). The new two slots widthChanged() and heightChanged() help changing the value other than the one changed by user._

Automatically closed -- issue fixed for 2 weeks with no activity.