Version: 0.8.2-rc1 (using KDE KDE 3.5.2) Installed from: Debian testing/unstable Packages OS: Linux When I attempt to use Aspect Ratio Crop to crop my pictures to square, they are often only approximately square, off by a few pixels. For example, when I open a 1944 by 2592 image, and select "1:1" aspect ratio, then press "Max Aspect", I obtain a crop that is 1944 by 1942. (Also, the combo boxes display incorrect values initially.) More generally, I often cannot select exact pixel sizes; for example, on 1:1 aspect ratio, clicking the "down by one" button usually reduces the size by many. In the example above, clicking down-by-one on the width (or the height) yields a crop that is 1938 by 1937. Typing "1944" into the height appears to work, until focus leaves the group of combo boxes. I think the problem is that the slider uses increments that are larger than one pixel; also, for non 1-to-1 aspect ratios, the user may want to specify an exact width or an exact height, so the algorithm by which the sizes are recalculated needs to be more sophisticated. Also more accurate; a carefully-thought-out integer algorithm is most appropriate, I think.
Created attachment 17912 [details] demonstration screencast: ratio crop refuses given width/heigth values same here - also occurs with digikam 0.9.0-beta2. The "ratio crop" tool has difficulties to accept values if a fixed ratio (e.g. 1:1) is set. Changing the values for width or height (with the spin boxes, the text entry field or the sliders) always leads to a recalculation plus/minus some pixels of the changed value (after approx. 0.5s). Therefore no exact cropping to a given size is possible. Decreasing one dimension therefore alwalys results in an additional decrease. The attached screencast should explain the described behaviour (should be played with only 5fps 'cause of dropped frames during encoding). It seems that the values are veryfied/recalculated in a wrong way...
I've been looking into this problem. I found that 640x480, 800x600, 1200x900, 1280x960 always work correctly but 1024x768 and 1152x864 (3:4 aspect ratio, i.e. same as a standard PC screen) never work. If you resize, e.g. maximize, the Aspect Ratio Crop dialogue window then you get different errors and even 640x480, 800x600 etc. have errors too. Looking at the code, what seems to be happening is that it takes the value you enter in the Height or Width spin-box and the selected aspect ratio then draws the selection rectangle in the preview window. Since the preview is not full-size the values have to be scaled. It does this calculation using floating point values then casts the results back to integers. Then, it takes the actual size of the selection rectangle in the preview window and scales the size back up to full-size (again as a floating point calculation with the results cast back to integers) and uses these values to populate the Height and Width spin-boxes. Both of these calculations have rounding errors so you don't get the values you expect. What it should do is to apply the aspect ratio to the value - Height or Width - that you enter to calculate the other value and use those to draw the selection rectangle (which will only be an approximation as the preview is not full-size). I am working to produce a patch to fix this.
I also came across this problem last week. Presumably, the problem of the golden mean helper lines changing their position slightly on horizontal/vertical flip, reported in http://bugs.kde.org/show_bug.cgi?id=120450, is related to this. Mark, have you been able produce a patch (I would very much appreciate one ;-)?
Setup: Fedora Core 6, Showphoto 0.5.0, Digikam 0.9.1 What I do: I define 4x3 or 800x600 aspect ratio. What I get: 798x600 or 600x602 crop boxes. I mention both Digikam and Showphoto, as the Digikam editor seems to be Showphoto.
Mark, could you give a brief update on the status of the patch you mentioned in #2? I also had a look at the code, and I think your analysis is absolutely correct. Best, Arnd
This bug still exists in .9.3-final. The accuracy of aspect-ratio crops is off by as much as 9 pixels. All one needs to do is click the "Max Aspect" button to see that the crop tool is not working correctly. This is problematic for anyone who needs accurate aspect-ratio prints. UFRaw's crop tool is a good example of one that is accurate and works correctly IMHO.
Scott, you could have a look at the code in http://websvn.kde.org/branches/extragear/kde3/graphics/digikam/imageplugins/coreplugin/ratiocrop/imageeffect_ratiocrop.cpp?view=log and see if you can fix the problem. Apparantly none of the developers has managed to find time to hunt down this bug. So if you could help in solving this issue, this would surely very much appreciated! Best, Arnd
The real problem is with ImageSelectionWidget class. I'm working on it to use real image dimensions when computing values. The patch is almost working: I will post it here within few days for testing.
Thanks for the hard work, Roberto. It is appreciated.
Created attachment 23118 [details] digikam-0.9.3-ratiocrop.patch Please test! Changes: - for any ImageSelectionWidget class method, real image dimensions are used to compute values (aspect-ratio, ...) - convertion from real to local region selection is performed only when updating the pixmap. - when the mouse is used, his pointer position is first converted to a virtual position within real image coordinates and the latter is used to compute values (opposite corner, ...). Local pointer position is still used to check if it is within a corner to select the cursor icon. No more needed to convert from local to real region selection. - the range for the width|height spin-boxes is from 0 to maxRange where the latter is computed taking care of top-left selection position and his aspect-ratio. When the width|height is 0, the selection border is not drawn and the OK button is disabled. - minor improvements for the region selection when it is|starts out of the image border.
Roberto, Thanks for the patch. I very appreciate your work... I have not really work on Ratio crop tool implementation since a long time. So all contributions are welcome. Thanks again. I let's Arnd review your patch in details. I'm too busy to finalize the new Time-Line view for next 0.9.4 release: http://digikam3rdparty.free.fr/Screenshots/timelineconcept3.png Regards Gilles Caulier
Roberto, I have tested the patch and I do think that the approach is really the correct one. However, there are some issues, which are caused by the underlying problem of cropping to a specific aspect ratio. Namely, this can only be achieved properly for certain combinations of height/width: For example, consider a 3 to 2 ratio. Changing the rectangle, one may get to 1111/740 = 1.501351... In contrast 1110/740 = 1.5 or 1119/746 =1.5 are ok. So the question is, whether one wants to achieve the aspect ratio only approximately, or precisely. If we want to do it precisely, we have to think a bit harder. Essentially it boils down to a restriction of the possible values for height and width, in dependence on the ratio: height = i * r_h width = i * r_w where the ratio = r_h : r_w . But would it be possible to restrict the motion of the mouse to just these values? Maybe one could just draw the rectangle with the right height and width, which match closest to the current mouse selection? The same problem also arises with the input boxes: Here all values are possible, but not necessarily lead to the precise aspect ratio.I am not sure whether there is any good way to ensure this properly for the text input? One approach might be: after changing a value in the input box the closest matching correct values for H and W are determined and used. This might give a slightly unusual feel, that the values in the input box magically adapt. But maybe that's the only way of ensuring precise values for the aspect ratio. Another issue: Changes to the text input boxes are not reflected in changes of the selected region. I think that's what the timers were used for (can't confirm this right now, because I only have the patched svn version on my machine ;-). So currently one has to use the arrows near the entry box to activate the change. Also, Dotan's example from #c4 with the aspect ratio 4:3: Specifying 400 for the height, gives 299 for the width, and 300 for the width 400 gives for the height. Another minor issue: why do the sliders for width/height change their position, if the selection region is moved with the mouse? First we have to decide if we want to only use the precise aspect ratio. I think we should, what do you think? Best, Arnd
Arnd, I'd recommend a checkbox for "Force exact aspect ratio" or "Allow aspect ratio approximation" with a good description of what it's for. I've been in situations where I've needed both.
Hm, sounds like a good idea, but I think that first we should try to fix this bug keeping the approximation behaviour and ensuring that if ratio=r_h:r_w and we specify height=i*r_h, we will always get width=i*r_w. After that we can work on it. > Specifying 400 for the height, gives 299 for the width What are dimensions of the image you are using for test? I'm using a 2592x1728 image and I cannot reproduce it. > Changes to the text input boxes are not reflected in changes of the selected > region. I think that's what the timers were used for. It seems to me that input box widget sends valueChanged signal only when he lose the focus. As soon as I press Tab key, changes are reflected in the selection region. I think the times were used to let users to select the value with the spin-box, waiting a little before to change it again. With my patch, when you select the width (height), only the height (width) is changed so times are no more needed. > Why do the sliders for width/height change their position, if the selection > region is moved with the mouse? Because the max range is changed. Best, Roberto
The image dimensions are: 3456 x 2304 Input boxes: ok (have to admit that I wasn't aware of the TAB; well I usually only use the mouse anyway). Slider position change: yes, does make sense - thanks for the explanation. Actually, one more remark: Sometimes it would be nice to do really precise crops which would require to zoom into the image.While there is a widget to zoom into an image, this might be really difficult to implement. (If we really would like to have this, this would be better discussed in a separate wish - still I wanted to mention it here ... ;-)
Created attachment 23349 [details] ratiocrop-width-height-ratio.patch (add-on) To be applied after digikam-0.9.3-ratiocrop.patch. With this patch, aspect ratio is computed using width ratio value and height ratio value. This should ensure that if ratio=r_h:r_w and we specify height=i*r_h, we will always get width=i*r_w solving an issue with custom aspect ratio Arnd reported to me. Further it changes Aspect ratio ComboBox text to take orientation into account: if current orientation is Landscape, you will read 3:2 - 4:3 - 5:4 - 7:5 - 10 :7; if current orientation is Portrait, you will read 2:3 - 3:4 - 4:5 - 5:7 - 7:10. Even custom aspect ratio take orientation and auto checkButton into account. Suppose current custom ratio is 800:600 (landscape orientation): - if you change orientation, ratio values order is reversed (600:800) - if you change first value to 400 and auto is checked, custom ratio became 400:600 and orientation became Portrait - if you change first value to 400 and auto is unchecked, second ratio value is changed to 400 (ratio became 400:400) to ensure that with landscape orientation first value >= second value. At last, I moved width and height center buttons near to X and Y input widgets (by now they are near Width and Height input widgets) since they change selection position and not his size. Please test and let me know if it introduces new bugs.
Created attachment 23400 [details] ratiocrop-aspect-ratio-fix.patch (for rev. 768751) To be applied with current svn revision version (768751 for ratiocrop plugin). It merges previous patches and fixes few minor issues I have introduced with my last patch.
SVN commit 770107 by abaecker: When cropping images, fullfill the given aspect ratio as good as possible (patch by Roberto Castagnola). CCBUGS: 128293 TODO:KDE4PORT M +134 -65 imageeffect_ratiocrop.cpp M +15 -16 imageeffect_ratiocrop.h M +363 -357 imageselectionwidget.cpp M +16 -17 imageselectionwidget.h WebSVN link: http://websvn.kde.org/?view=rev&revision=770107
Thanks a lot for this nice patch, Roberto! I applied this patch together with the one you sent me by mail to ensure that the current selection values are only saved when the selection is applied (i.e not also on Cancel). All tests I did worked fine, but due to the complexity and number of possibilities there still good be some overlooked cases. As last step, before this bug can be finally closed, Roberto plans to provide a patch which (optionally) ensures precise aspect ratios as discussed in #c12 above. Best, Arnd
Ok Arnd, Thanks to Roberto. Excelent job... Tomorow, i will backport patch to KDE4 as well Gille Caulier
Arnd, Gilles, I think it will take longer to ensures precise aspect ratios as discussed in #c12 above. However I consider it as a new feature and I will open a new bug/wish as soon as the patch is ready. In the meantime, I think this bug can be closed. Anyway, feel free to add me to CC list if a new bug/wish is opened for this plugin. Best, Roberto
SVN commit 770517 by cgilles: digiKam from trunk: backport commit #770107 from KDE3 branch BUG: 128293 M +169 -107 imageeffect_ratiocrop.cpp M +21 -21 imageeffect_ratiocrop.h M +364 -361 imageselectionwidget.cpp M +30 -34 imageselectionwidget.h WebSVN link: http://websvn.kde.org/?view=rev&revision=770517