fix #279967: image resize not correctly honoring aspect ratio #5731
Conversation
mscore/inspector/inspectorImage.cpp
Outdated
| // lockAspectRatioClicked | ||
| //--------------------------------------------------------- | ||
|
|
||
| void InspectorImage::lockAspectRatioClicked(bool clicked) |
There was a problem hiding this comment.
Should be:
void InspectorImage::lockAspectRatioClicked(bool checked)
Also, I suggest that you throw away the aspect ratio (i.e., reset it to a known ”invalid” value such as 0.0) when checked is false.
There was a problem hiding this comment.
It should be clicked because that's what Qt uses.
And is there any real-world usage of throwing away the aspect ratio when it is false? If not, then I don't see the need of making the code more complex.
There was a problem hiding this comment.
It should be
clickedbecause that's what Qt uses.
No, it should be checked because that's what Qt uses and also because it's what makes sense. Think about it: When this function is called, it's always because the checkbox has been clicked (or otherwise manipulated by the user), but the checkbox is not always checked. That's what the parameter is there to tell you.
And is there any real-world usage of throwing away the aspect ratio when it is
false?
It's an additional sanity check, and it will also be useful when saving to the score file (i.e., a value of 0 means don't save it).
There was a problem hiding this comment.
I'm convinced of the first matter.
For the second matter, I think "additional sanity check" should only take place if it can prevent latent failures or crashes. And as I said, this fix has nothing to do with saving to score file. If a variable is intended to be saved to score, it should be defined in libmscore rather than mscore.
Harmoniker1
force-pushed
the
image-resize
branch
3 times, most recently
from
3f564f7
to
493bce7
Compare
Harmoniker1
changed the title
Harmoniker1
force-pushed
the
image-resize
branch
from
493bce7
to
982a9b2
Compare
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.
Harmoniker1
force-pushed
the
image-resize
branch
from
982a9b2
to
e30019a
Compare
|
@Howard-C I would leave inspector related changes to the new inspector. Moreover, this issue has probably been addressed there. What do you think? |
|
Not fixing it for 3.5? |
|
I'd suggest this go to 3.6 |
Resolves: https://musescore.org/node/279967.
A new member variable
_aspectRatioofInspectorImageis added to remember the image's original ratio. It is also updated when "Lock aspect ratio" is checked inlockAspectRatioClicked(). The new two slotswidthChanged()andheightChanged()help changing the value other than the one changed by user.