Bug 332208

Summary: Crop with Ratio setting is bad implemented
Product: [Applications] gwenview Reporter: Richard Llom <richard.llom>
Component: generalAssignee: Gwenview Bugs <gwenview-bugs-null>
Status: RESOLVED FIXED    
Severity: normal CC: danielroschka, huon, mdione, myriam, nate, null
Priority: NOR    
Version: 4.12.2   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: https://invent.kde.org/graphics/gwenview/commit/31ac40afbc8c2206556ade5a2c025605bf307a87 Version Fixed In: 22.08
Sentry Crash Report:

Description Richard Llom 2014-03-16 13:54:39 UTC 
For the Crop operation in Gwenview one can set a image ratio (ie. 3:2, 4:5, etc). However when this is set several things do not work:

1. It is not possible to scroll increase the size, ie. hover with the mouse over a size field and scroll up -> nothing happens (scroll down works).

2. It is not possible to enter custom values in the size field, instead they are rounded on input, which isn't helpful at all. For instance set ratio to 3:2 and try to enter 555 in the width field:
It will always be rendered to 333, although 555:370 would be a valid 3:2 ratio.

Reproducible: Always
Comment 1 Huon 2018-03-15 04:17:30 UTC 
This is definitely still a problem. One solution would be to only round/calculate when leaving the field, rather than every time the value is changed.
Comment 2 null 2018-05-26 19:45:26 UTC 
*** Bug 394714 has been marked as a duplicate of this bug. ***
Comment 3 Marcos Dione 2018-05-26 22:25:45 UTC 
For info, there's no need to select a custom ratio; any of the predefined ratios has the same problem.

I think what is happening is that when one makes a change in one of the fields, the other one is computed based on the aspect ratio, and because that one was changed, it propagates back to the first one.

Based on Huon's comment, I found the culprit 2 lines:

https://bugs.kde.org/show_bug.cgi?id=332208

I wish I had more time to pour on this, sounds like a very easy change.
Comment 4 Nate Graham 2018-05-26 22:32:27 UTC 
Wrong link?
Comment 6 null 2018-05-27 09:42:40 UTC 
If the easy change you are referring to is about simply deleting those lines, then I'm afraid it would break updating the canvas in reaction to spinning the value up and down.

Perhaps we might need something more complicated like "mUpdateFrom*" as found in "resizeimagedialog.cpp".
Comment 7 Marcos Dione 2018-05-27 18:35:00 UTC 
No, I think they need to connect another signal, like what Huon said here:

https://bugs.kde.org/show_bug.cgi?id=332208#c1
Comment 8 Huon 2018-05-28 10:10:25 UTC 
(In reply to Marcos Dione from comment #7)
> No, I think they need to connect another signal, like what Huon said here:
> 
> https://bugs.kde.org/show_bug.cgi?id=332208#c1

That was a high level thought I had at the time. I now think copying the logic in the resize dialog is better, in order to keep the update-as-you-type behaviour, which IMO is better UX.
Comment 9 Nate Graham 2022-05-12 14:43:19 UTC 
Git commit 31ac40afbc8c2206556ade5a2c025605bf307a87 by Nate Graham, on behalf of Alban Boissard.
Committed on 12/05/2022 at 14:43.
Pushed by ngraham into branch 'master'.

Make spinboxes in Crop tool behave properly with fixed aspect ratio

When the aspect ratio is fixed in the crop tool, if we change
the value of a spinbox, the other spinbox is set accordingly but
trigger a signal that change the value of the first spinbox.

This commit fixes that wrong behavior.
FIXED-IN: 22.08

Signed-off-by: Alban Boissard <albanboissard@gmail.com>

M  +10   -2    lib/crop/cropwidget.cpp

https://invent.kde.org/graphics/gwenview/commit/31ac40afbc8c2206556ade5a2c025605bf307a87
Comment 10 Marcos Dione 2022-05-13 07:03:25 UTC 
Looks good! Thanks for your time :)