Skip to content

/darktable

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crop to exact size #2849

Open
mdebooy opened this issue Aug 11, 2019 · 24 comments

Comments

@mdebooy
Copy link

commented Aug 11, 2019 •
edited

Is your feature request related to a problem? Please describe.
When I want to crop my picture to an exact size then this is not easy. It is not always possible to 'drag' the right size. Also on some pictures the size is not easy to read. Sometimes the selected size is not used when you do an export. The picture is sometimes 1 pixel too small.

Describe the solution you'd like
It would be nice if the user can enter the size by entering the x and/or y size manually. The option to enter values is available in many modules. In the 'Crop and Resize' module it is already possible to enter the degrees to rotate but not the x and/or y crop size.

Describe alternatives you've considered
I asked the question on darktable-user@lists.darktable.org. I now crop to a size a bit bigger than needed and do another crop in GIMP.

Additional context
Add any other context or screenshots about the feature request here.
@TurboGit

This comment has been minimized.

Sign in to view
Copy link
Member

commented Aug 12, 2019

Crop is about ratio not size. You crop the are you're seeing then you export it at the size you want. So I'm not sure what you want this for.
@TurboGit TurboGit added the invalid label Aug 12, 2019
@MStraeten

This comment has been minimized.

Sign in to view
Copy link

commented Aug 12, 2019

the actual implementation ist about ratio and size. It allows cropping by seeing and dragging, but not by defining. For example, you cannot define that you want to cut a 1:1 FullHD extract from the image.
So in my opinion the labeling as ‚invalid‘ needs discussion.
@parafin

This comment has been minimized.

Sign in to view
Copy link
Member

commented Aug 12, 2019

Cutting FullHD frame from picture is not a valid use-case for darktable IMHO, it's more of GIMP or even ImageMagick area. But getting 1:1 output might be desired in case user don't want to involve any scaling. And sometimes he may want to fit the resulting image in some resolution. So I would say use-cases for this feature are not very well defined, but still it's a valid feature request.
@lesnake

This comment has been minimized.

Sign in to view
Copy link

commented Aug 23, 2019 •
edited

I started working on that ~15 days ago. if you want to try it :
https://github.com/lesnake/darktable/tree/numeric-crop-ctrl
I need to translate strings to french at least, and I request for review.
@lesnake

This comment has been minimized.

Sign in to view
Copy link

commented Aug 23, 2019

Capture d’écran du 2019-08-23 22-35-37
@mdebooy

This comment has been minimized.

Sign in to view
Copy link
Author

commented Aug 25, 2019

@lesnake

This comment has been minimized.

Sign in to view
Copy link

commented Aug 25, 2019

1 - This is not in my hands. I don't have started the review process (which I have never done so far).
I'm on the localization stuff.
I think that I can defend the function so that is it is accepted.

2 - You can find directions do build at the project root on github :
https://github.com/darktable-org/darktable
Just pull my repository https://github.com/lesnake/darktable instead
and checkout branch numeric-crop-ctrl

3 - I don't know
You can install it in another folder using a different prefix.
I suggest that you use another picture database and config folders, just in case, by using --datadir --configdir --library
There is no directions on good/best development practices (container ?).
This is what I did.
@mdebooy

This comment has been minimized.

Sign in to view
Copy link
Author

commented Aug 25, 2019

@mdebooy

This comment has been minimized.

Sign in to view
Copy link
Author

commented Aug 26, 2019

@lesnake

This comment has been minimized.

Sign in to view
Copy link

commented Aug 26, 2019

That means that I have to tidy-up my development environment because the problems occurs on lines that I have not modified to make my function ... but which are indeed modified !
@mdebooy

This comment has been minimized.

Sign in to view
Copy link
Author

commented Aug 26, 2019

@lesnake

This comment has been minimized.

Sign in to view
Copy link

commented Aug 27, 2019

It should be good now. I made my branch from the wrong starting point when getting into the git process.
@mdebooy

This comment has been minimized.

Sign in to view
Copy link
Author

commented Aug 27, 2019

@lesnake

This comment has been minimized.

Sign in to view
Copy link

commented Aug 27, 2019

I had to struggle a bit for configuration.
I configured the build to install in a subdirectory named "prefix", in which I created data and config dirs. share subdir is created by install process.
I think that populating data with pixmaps, rawspeed directories and darktablerc file. All are can be found in the share subdirectory

Here is my full command line :
prefix/bin/darktable --datadir /home/pierre/Build/dt-numeric-crop-ctrl/prefix/data --configdir /home/pierre/Build/dt-numeric-crop-ctrl/prefix/config --moduledir /home/pierre/Build/dt-numeric-crop-ctrl/prefix/lib/darktable/ --localedir /home/pierre/Build/dt-numeric-crop-ctrl/prefix/share/local

As I wrote, I had to try hard, so some info may be missing.
@mdebooy

This comment has been minimized.

Sign in to view
Copy link
Author

commented Sep 2, 2019

@lesnake

This comment has been minimized.

Sign in to view
Copy link

commented Sep 2, 2019

My version says "this is darktable 2.7.0+2~g036ae4b62-dirty"
I suppose something went wrong at checkout.

git log says:
$ git log
commit 036ae4b (HEAD -> numeric-crop-ctrl, origin/numeric-crop-ctrl)
Author: Pierre lesnake le.snake@gmail.com
Date: Tue Aug 27 23:04:10 2019 +0200

Improve tooltips and generate french translation

commit 40c54f9
Author: Pierre lesnake le.snake@gmail.com
Date: Mon Aug 26 23:21:04 2019 +0200

Add controls for pixel size precision cropping.

commit 475e529 (tag: release-2.7.0)

You can also grep the tree:
grep -r "numcrop" .
Binary file ./build/src/iop/CMakeFiles/clipping.dir/introspection_clipping.c.o matches
Binary file ./build/src/iop/libclipping.so matches
Binary file ./prefix/lib/darktable/plugins/libclipping.so matches
and a lot of results in clipping.c

You can also look at https://github.com/lesnake/darktable/blob/numeric-crop-ctrl/src/iop/clipping.c and search for numcrop text.

Capture d’écran du 2019-09-02 22-01-11
Capture d’écran du 2019-09-02 22-02-07
@mdebooy

This comment has been minimized.

Sign in to view
Copy link
Author

commented Sep 2, 2019

@mdebooy

This comment has been minimized.

Sign in to view
Copy link
Author

commented Sep 3, 2019

@lesnake

This comment has been minimized.

Sign in to view
Copy link

commented Sep 5, 2019

Glad you made it and find it useful.

I can look at enabling height, but last time I tried I ended in a situation that was generating a lot of corner cases when enlarging : if you enlarge height with a ratio, width size may be out of the picture. Thats a lot of tests to run.
@Nilvus

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Sep 5, 2019

First of all, will it be merged into darktable? One of the maintainers
blocked my request and I do not want to switch to a forked version.

I don't know where you see that ? Questioning a feature and saying that he don't see what you want and why doesn't mean it is blocked. It's just about discussing, questioning to see if it's a good thing to add. It's just a good way too avoid adding too much usefulness things or things that will step by step make a software awful.

If it was a real block, I could say that this issue would be closed some days ago. It's good to see this feature discussed. We just have to be careful how it will be implemented.
@mdebooy

This comment has been minimized.

Sign in to view
Copy link
Author

commented Sep 6, 2019

@mdebooy

This comment has been minimized.

Sign in to view
Copy link
Author

commented Sep 6, 2019

@lesnake

This comment has been minimized.

Sign in to view
Copy link

commented Sep 6, 2019

This is the document that I use to log my tests:
keep crop center = no

do x<=0 result x=0, successful
do x>=image_width(5536) result x=image_width, successful
do y<=0 result y=0, successful
do y>image_height(3688) result y=image_height, successful
do w<=0 result w=1% of image_width = 554, successful
do w+x>image_width result w=image_width-x, successful
do h<=0 result h=1% of image_height = 369, successful
do y+h>=image_height, result h=image_height-y, successful
do Same after mouse mouvement, successful
do the same using +/- buttons, successful

keep crop center = yes

x and y changes expected to behave as "keep crop center=no", successful
*do w increase so that x would overshoot right edge (x=5336, change w from 1000 to 2000, shall not result in x +w > 5536) result x=6827, w=554 fail :'(
do w increase so that x would overshoot left edge (x=200, change w from 1000 to 2000 shall not result in x<0) result x=0, w=1400 (1000 + 200 * 2 ) successful

*do h increase so that y would overshoot bottom edge (y=3388, change h from 1000 to 2000, shall no result in y+h > 3688) result y=2338, h=1300 fail :'(
do h increase so that y would overshoot top edge (y=200, change h from 1000 to 2000, shall not result in y <0) result y=0, h=1400 (1000 + 200 * 2) successful

aspect = 16/9

"Test with keep crop center" = no and also check that h=ratio*w
x and y change tests are expected to be the same, successful
h change : h control inhibited not applicable by design choice, successful
*do w increase (x=1000, y=1000, change w from 1000 to 2000, expecting x=1000, y=1000, w=2000, h change from 562 to 1124) result x,w as expected h=1125 (acceptable rounding error) y = 719 fail :'(

aspect = 16/9 and keep crop center = yes

do x<=0 result x=0, successfull
do x>=image_width(5536) result x=image_width, successful
do y<=0 result y=0, successful
do y>=image_height(3688) not applicable by design choice, successful
do w<=0
x=1000, y = 1000, w = 1000, h = 562, center at (1500, 1281)
change w to -100
expecting w = 554, then x = 1223, y = 1125, h = 554*ratio = 312, center at (1500, 1281)
got w = 554, then x = 1223, y = 1125, h = 312, center at (1500, 1281)
successful
do w+x>image_width
x=4236, y = 1000, w = 1000, h = 562, center at (4736, 1281)
change w to 2000
expecting w = 1300, then x = 4386, y = 1135, h = 731, center at (4736, 1281)
got w = 1300, then x = 4086, y = 916, h = 731, center at (5036, 1281)
failed :'(

do h<0 result h=0,
do y+h>image_height, result h=image_height-y,

Test with w increase so that there is no overshoot on horizontal axis, but on vertical axis.
transpose

do Same after mouse mouvement,
do the same using +/- buttons,

Test with big image
Test with small image (so that 1 image pixel > 1 screen pixel)

Browse console output after all that
@lesnake

This comment has been minimized.

Sign in to view
Copy link

commented Sep 6, 2019

lol unexpected formatting !
@oxpa oxpa referenced this issue Oct 9, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
invalid
Projects
None yet
Milestone
No milestone
6 participants
@mdebooy @TurboGit @MStraeten @parafin @lesnake @Nilvus
You can’t perform that action at this time.