GNOME Bugzilla – Bug 315051
Image preview in transforms keeps original version visible
Last modified: 2017-05-05 23:09:09 UTC
Image and Image+grid keep the original version fully visible, thus making adjustments based in other layers impossible or hard, as the original version blocks the view.
Woops, typo in summary.
It might be a simple change to turn off visibility of the layer that is being transformed.
Created attachment 70730 [details] [review] Hide active drawable during transform This is my attempt at resolving this bug. The patch works by making the active layer, mask, or channel invisible while the transform preview is present. This is performed in the 'gimp_transform_tool_button_press' function because the preview is not drawn before this point. If a visible drawable is hidden, the drawable's pointer is stored in an element named 'hidden_item' which was added to the '_GimpTransformTool' structure. (If the transform is a horizontal or vertical flip, the drawable's visibility is not changed as there is no interactive preview.) If there is a selection present -- or the transform is operating on paths or the selection mask -- there is no need to hide anything. Nor is there a need to do anything if the active drawable was hidden to begin with. In all of these cases, 'hidden_item' gets set to NULL and the operation proceeds as in unpatched versions. The restoration of the drawable's visibility is handled in the function 'gimp_transform_tool_response' (this seems to work but perhaps there is a more logical place to perform this). It should be noted that any additional views will also have the active drawable temporarily hidden (and will not have the transformed preview displayed). This doesn't seem entirely unreasonable. Finally, for the case of transforming a layer mask, my patch only hides the mask from the projection; ideally it should probably be disabled as well. This could be accomplished by adding another entry to the '_GimpTransformTool' structure but perhaps there is a better solution (or a preferable approach to this whole thing).
I haven't tried the patch yet, but a couple of comments: First, it is not good to refer to the flip tool inside the transform tool code: object code should only refer to ancestor objects, not descendants. If you need to distinguish between tools, the way to do it is to create a variable in the tool or tool class struct, probably boolean, set to TRUE for one type of tool, and FALSE for the other. Second, it seems to me that the only case you really need to handle is proper layers -- not, for example, layer masks. The transform tool preview is only shown floating on top of an image, not playing its proper role in the image, so it doesn't seem to me that removing a layer mask is very helpful. I wonder if this would allow you to simplify the code.
Thank you for your comments. I agree with your first criticism; I felt that what I had done for the FLIP tool was unkosher but could not figure out what the callback was when the mouse was clicked. The ideal would be to not treat the FLIP tool specially but it doesn't call 'gimp_transform_tool_response' and I couldn't figure out where to handle the visibility restoration. If there is a callback function when the FLIP tool is effected, the visibility could be handled like the other tools and its state restored in that function. Regarding the second criticism: if the mask or channel is visible when the transform is called then the same problem exists as for layers; the difference only being that channels and masks are not commonly visible. The code wasn't really complicated by handling this case, only the (otherwise boolean) "flag" to restore the drawable had to be changed to an appropriate item pointer. ---- Is it feasible to modify the item's visibility without having the "eyeball" follow suit? Hiding the drawable during the preview is an internal mechanism and, ideally, the user shouldn't be distracted by it. (There is a similar concern with the border of the layermask changing state.)
There is no reason why GimpFlipTool shouldn't use gimp_transform_tool_response(). If it makes the code cleaner, it probably use it. But it would be best to do such cleanup in a separate patch so that we can apply it individually.
The change to use response() no matter how the tool is confirmed has been taken care of in CVS.
Created attachment 75043 [details] [review] Updated patch to hide item during transform Removes the special (and distasteful) treatment of the Flip Tool and moves the restoration of visibility to the 'gimp_transform_halt' fumction (in case the transform is aborted). I would point out the following concerns: A) The visibility indicator (eyeball or layermask border) changes during the transform. B) If a selected region is being transformed, the selected region of the background drawable still shows the drawable's contents (the same behavior as an unpatched version). C) If a layermask is being transformed, the background reflects the original drawable as it appeared at invocation and does not get modified to represent how the projection would appear with the transformed mask. D) The same issue as "C" only for channel transformations. I could address the first concern if it were deemed desirable; although the solution would probably be somewhat convoluted and, perhaps, complicated. Resolving the other concerns is beyond my capabilities at this time and would probably require a lower level approach to handling the background projection.
Created attachment 77955 [details] [review] Updated patch No effective changes from previous version other than being made to work with the current version of the GIMP (2.3.14 at the time of this post).
Why not changing the original visibility to, say 10%, to keep seeing the original shape, size and position for reference, but avoiding the visibility problems that the current method implies?
*** Bug 504074 has been marked as a duplicate of this bug. ***
*** Bug 520460 has been marked as a duplicate of this bug. ***
Bumping this, the new unified transformation tool might be a good point to implement this. Just add a new checkbox to "show image preview" which says something like "Hide original".
The unified transform tool uses the very same parent class and is no different from the othe transform tools. We get this for all tools, or for none.
*** Bug 734389 has been marked as a duplicate of this bug. ***
fixing this is dead isn't it :(
Old bugs like these are usually an indication that the current implementation works for many people, at least good enough to not cause enough motivation to fix them asap.
*** Bug 635940 has been marked as a duplicate of this bug. ***
I have completed a video detailing the problems and suggested fixes, which are in line with what is mentioned above. The video is here: https://youtu.be/yfPxaeiYSZ4 A common photo-editing task is transforming a selection in relation to what's under it. Thus, if you want to transform the layer in relation to what's under it, you have to follow these steps: 1.Start the transform (scale, rotate, unified transform, etc.) 2.Hide the current layer. 3.Adjust the opacity of the transformation preview in the transform tool options. 4.Set grid to zero lines 5.complete transformation 6.Un-hide layer to see the results. Additionally, you have to do this for each tool you want to use for transforming at least once per gimp session. This is a lot of work, when all you want to do is see what's under your transformation while transforming. The steps could be reduced dramatically by changing some of gimps defaults: 1.Start the transformation (GIMP automatically sets transforming layer display to hidden. This is necessary to see the result of your transformation in can see relation to what's below it. GIMP could also set the transform visibility to 75% opacity by default, giving an even better view of what the transformation is covering up. GIMP should hide grid lines by default during transformation unless the user asks for them. They have only ever really gotten in the way, and I have yet to find any good use for them.) 2.Complete the transformation (GIMP unhides the layer, thus showing the transformation in its complete state at full opacity) So as you can see, this would cut down the work the user has to do a lot when transforming. It would also bring GIMP's transforming into the same ease of use as every other graphics program I've ever used professionally. :) After having lived with it like this every working day for the last 6 years, I have to say, it's still one of my biggest gripes about GIMP's GUI. Can we fix it? Please? Pretty pretty please? :)
Fixed in master, but only hide layers and channels (not layer masks): commit 32239a25886b0062fb866cbd63fedb9d3693798e Author: Michael Natterer <mitch@gimp.org> Date: Thu Apr 6 00:29:30 2017 +0200 Bug 315051 - Image preview in transforms keeps original version visible Argh, didn't commit everything... app/tools/gimptransformtool.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) commit f03051143831b9053c6d6c791886425f8bc688f8 Author: Michael Natterer <mitch@gimp.org> Date: Thu Apr 6 00:15:42 2017 +0200 Bug 315051 - Image preview in transforms keeps original version visible If the transformed item is a layer, and we are transforming the entire layer (if there is now selection), hide the original layer during the interactive transform. Based on a 2.8 patch from saulgoode. app/tools/gimptransformtool.c | 43 +++++++++++++++++++++++++++++++++++++++++++ app/tools/gimptransformtool.h | 3 +++ 2 files changed, 46 insertions(+)
Comment on attachment 77955 [details] [review] Updated patch Too outdated to be used on master.
Forgot this: commit 6561dec3df987d1ccbdc1648676d42642810bfb8 Author: Michael Natterer <mitch@gimp.org> Date: Sat May 6 01:07:53 2017 +0200 Bug 315051 - Image preview in transforms keeps original version visible Only hide the layer if the transform preview is visible. app/tools/gimptransformtool.c | 1 + 1 file changed, 1 insertion(+)