After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 784802 - Crop and rectangle-select tools incorrectly detect current aspect ratio
Crop and rectangle-select tools incorrectly detect current aspect ratio
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Tools
git master
Other All
: Normal blocker
: 2.10
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2017-07-11 15:15 UTC by Tomasz Goliński
Modified: 2018-02-25 15:39 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Tomasz Goliński 2017-07-11 15:15:00 UTC
Recently, instead of correctly setting the current aspect ratio for crop tool, I get weird ratios, usually close to 1. Sometimes I got results much more off, like 500:1.
Comment 1 Michael Natterer 2017-07-11 17:57:04 UTC
Hmm, I get "Current" or whatever I entered.

This was broken while I changed all things rectangle, but I think
now it works like it did before.

What exactly do you expect to happen when you do exactly what?
Step-by-step please, I'm sure something still not works as before,
but I don't see it.
Comment 2 Tomasz Goliński 2017-07-11 19:10:26 UTC
I'm testing on commit 77f1424777.

I open some images and when I try to crop them (with "current" enabled by default), the crop region I can get has distinctly different aspect ratio than the picture itself. I was understanding that "current" meant "aspect ratio of the active image". 

The situation is a bit random, sometimes I get correct aspect ratio, and sometimes (without changing anything) I get it wrong. It is enough for me to create a few new images (possibly with different shapes, but not always necessary) and try to crop in them.

It somehow seems that the value of "current" aspect ratio are not refreshed after changing an active image. I just have now 3 images of different aspect ratio and trying to crop in each of them gives me the same ratio and after several iterations, ratio changes to a different one. I get usually 3 types of ratios: nearly 1:1, nearly 2:3 portrait and at least 500:1 portrait. However it is not limited to this list.
Comment 3 Michael Natterer 2017-07-11 19:18:26 UTC
Yes indeed. Thanks, I didn't quite know what to fix :)
Comment 4 Michael Natterer 2017-07-11 19:37:30 UTC
This should fix it:

commit 4f2c0dd846ecd538308d1a97f76239ae6bf0fbc9
Author: Michael Natterer <mitch@gimp.org>
Date:   Tue Jul 11 21:35:32 2017 +0200

    Bug 784802 - crop tool incorrectly detects current aspect ratio
    
    When starting the crop tool, set its default aspect ratio to that of
    the current layer/image. Dunno what change exactly broke it, but it
    was broken.

 app/tools/gimpcroptool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 5 Tomasz Goliński 2017-07-11 20:42:27 UTC
Thanks. Indeed, it works as expected now.
Comment 6 Tomasz Goliński 2017-07-14 17:14:35 UTC
I'm afraid the issue is not yet fully resolved. When I last wrote it seemed to work OK for several example pictures, but now in "real life cases" I get weird, but different behaviour. The tool seems to switch for no reason between 1:1 and current. I can't pinpoint the pattern. 

Sometimes I have "current" in tool setting but when I start to drag the selection it changes to "1:1", and reverts to "current" when I release mouse button. Focusing a different image usually corrects the behaviour for some time.
Comment 7 Michael Natterer 2017-07-15 19:27:08 UTC
Argh, indeed. Seems the new code is just as tricky, just in its own way...
Comment 8 Jehan 2017-12-04 23:04:01 UTC
Setting to blocker because that's a regression because of some new code. We should fix it because stable release.
Comment 9 Akkana Peck 2018-02-11 17:50:32 UTC
I just saw something that might be related. In Ellipse select I checked Fixed aspect ratio. The ratio said 1:1 which was what I wanted, so I didn't change it. When I dragged in the canvas, I got an ellipse that was more like 5:1. But typing in a new aspect ratio, 2:1, worked (when I dragged, I got the expected 2:1 ellipse, not a 5:1) and then when I changed it back to 1:1 everything worked fine. So it seems like the ratio it was actually using somehow got out of sync with the ratio it was displaying in Tool Options. It didn't happen again after a restart, so unfortunately I don't know how it got out of sync or how to reproduce it, sorry. I was using "Expand from center" initially in ellipse select but I doubt that's related.
Comment 10 Ell 2018-02-15 22:28:29 UTC
Should be fixed in mater:

commit 547d3149d53d2457d9733d2a1a978a94fd34dd73
Author: Ell <ell_se@yahoo.com>
Date:   Thu Feb 15 15:33:45 2018 -0500

    Bug 784802 - Crop and rectangle-select tools incorrectly detect ...
    
    ... current aspect ratio
    
    When starting the rectangle-select (and ellipse-select) tools,
    properly reset their default aspect ratio to 1:1.  This fixes an
    issue where the tool would use the aspect ratio of the last
    rectangle when there's no user-overriden aspect ratio specified.
    
    This restores the 2.8 behavior, except for the fact that the aspect
    ratio resets to 1:1 when the tool is commited (if there's no user-
    overriden ratio), rather than keeping the aspect ratio of the last
    rectangle (i.e. "Current"); in 2.8 this only happend when halting.
    The current behavior seems more consistent anyway.

 app/tools/gimprectangleselecttool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

commit 49b3695e4647d851522d895bac4f4a2a0f3921b8
Author: Ell <ell_se@yahoo.com>
Date:   Thu Feb 15 16:56:05 2018 -0500

    Bug 784802 - Crop and rectangle-select tools incorrectly detect ...
    
    ... current aspect ratio
    
    When updating the default aspect ratio of a widget-less crop tool,
    construct a temporary GimpToolRectangle widget, so that we can use
    it to call gimp_tool_rectangle_constraint_size_set() and pick the
    correct ratio, instead of just bailing.
    
    When halting the crop tool, update the default aspect ratio, which
    now does the right thing, as per the above.
    
    Update the default aspect ratio upon changes to the active layer of
    the current image, and to the size of the active layer, which
    affect the default aspect ratio when "current layer only" is
    toggled.

 app/tools/gimpcroptool.c | 197 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------
 app/tools/gimpcroptool.h |   1 +
 2 files changed, 140 insertions(+), 58 deletions(-)

The way the aspect ratio entry works is a bit tricky.  To clear some confusion, it has two states: either you type an explicit ratio (e.g. 1:1), in which case this bug shouldn't be triggered, or you clear the entry, in which case the "default" ratio is used (indicated by the entry using an italic font).

When the tool is inactive, the default ratio for the rectangle-select tool is 1:1, and for the crop tool is either the size of the current image, or of the current layer (depending on the tool options).  When you start using the tool, the default ratio changes to "Current", which refers to the ratio of the current *rectangle*, not the current image.  If you restrict the ratio while the the entry shows "Current" you would get the same ratio as the existing rectangle, which isn't necessarily the same as the initial ratio.  Once you stop the tool, the default ratio goes back to what it was at the beginning.

Confusing?  Hell yes, but this is how it's expected to work right now (and how it used to work in 2.8, modulo some minor differences).

Tomasz, I couldn't reproduce what you describe in comment 6, so I'm closing this bug as fixed.  If you still run into cases where the behavior is different from what's described above, please reopen.
Comment 11 Tomasz Goliński 2018-02-24 13:41:28 UTC
Sorry for delay in replying, I had to gather courage to upgrade my system glib to a version unsupported yet by my distro in order to compile current HEAD ;)

The behaviour I see now for crop tool is to remember any previously hand-entered aspect ratio (I think it is a new feature, but it is consistent with behaviour of other tool options). 

However when I erase the ratio value, it gets automatically filled in with width:height of current image/layer (not in italics). I would expect it to mean that it will use ratio of the image. But when I start to draw a crop area, value changes to 1:1. When I release the mouse button it changes again to "current" (like it should according to your description). When I unselect the crop area, ratio gets back to width:height of the image.

I also noticed that when I hand enter a width:height ratio into the box (5361:3574 in my case), it sometimes gets ignored and tool still uses previously set value. It also doesn't happen each time but I reproduced it many times. I think it is only a consequence of the problem with using default crop ratio.
Comment 12 Ell 2018-02-25 08:23:37 UTC
(In reply to Tomasz Goliński from comment #11)
> [...]
>
> However when I erase the ratio value, it gets automatically filled in with
> width:height of current image/layer (not in italics).

The font being non-italic was a bug, and should be fixed now by:

commit 0de25a145a3ffa97a4f802d0abdaa37172d8efad
Author: Ell <ell_se@yahoo.com>
Date:   Sun Feb 25 03:04:39 2018 -0500

    libgimpwidgets: in GimpNumberEntryPair, properly set user-override ...
    
    ... when focus is lost/enter is pressed
    
    When a GimpNumberPairEntry loses focus, or when enter is pressed,
    set its user-override property by calling
    gimp_number_pair_entry_set_user_override(), instead of setting the
    corresponding member directly, so that the entry's font is updated
    correctly.

 libgimpwidgets/gimpnumberpairentry.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

However, if was only a visual glitch -- functionality-wise it should have worked correctly.

> I would expect it to
> mean that it will use ratio of the image. But when I start to draw a crop
> area, value changes to 1:1. [...]
> 
> I also noticed that when I hand enter a width:height ratio into the box
> (5361:3574 in my case), it sometimes gets ignored and tool still uses
> previously set value. It also doesn't happen each time but I reproduced it
> many times. I think it is only a consequence of the problem with using
> default crop ratio.

I can't reproduce either of those :P  The default crop ratio should only ever be 1:1 when no images are open, or when "current layer only" is checked, "allow growing" is unchecked, and the current image has no active layer.

Any chance you could provide exact steps to reproduce this?
Comment 13 Tomasz Goliński 2018-02-25 13:08:21 UTC
I confirm that italics work now as expected. 

Exact steps are extremely straightforward:

1. Open image (tried jpg and tiff, small or big)
2. Select crop tool
3. If ratio is not default, erase it.
4. Ratio shows width:height in italics.
5. Start to drag the selection - ratio immediately changes to "1:1" (in italics).

I don't have anything checked except "Fixed aspect ratio" and "Highlight". I retried it with clean configuration and got the same results.
Comment 14 Ell 2018-02-25 15:32:35 UTC
Ok, got it.  The key was that the width/height tool options had to be 0 when starting to drag to trigger this.  Should be fixed by:

commit 6b4abf7b4cc834c61931cc7171515d1ed0f5dea5
Author: Ell <ell_se@yahoo.com>
Date:   Sun Feb 25 10:15:59 2018 -0500

    Bug 784802 - Crop and rectangle-select tools incorrectly detect ...
    
    ... current aspect ratio
    
    In gimp_{rectangle_select,crop}_tool_start(), move
    GimpToolRectangle signal connection to the end of the function.  In
    particular, connect the signals *after* the call to
    gimp_{rectangle_select,crop}_tool_update_option_defaults(), since
    it may result in an emission of a "change-complete" signal, whose
    handler calls the function again with ignore_pending == FALSE, which
    would override the ratio set with ignore_pending == TRUE.

 app/tools/gimpcroptool.c            | 20 ++++++++++----------
 app/tools/gimprectangleselecttool.c | 14 +++++++-------
 2 files changed, 17 insertions(+), 17 deletions(-)

This leaves the other bug, which I still can't reproduce.  Does it still happen?
Comment 15 Tomasz Goliński 2018-02-25 15:39:20 UTC
Thanks, it is indeed fixed.

As for the other problem, now I can't reproduce it. It was probably some interaction from the first problem. If I see it again, I'll reopen.