darktable-org / dtdocs

darktable user manual
GNU General Public License v3.0
74 stars 74 forks source link

Custom crop settings aspect "remove duplicates"-feature not documented #352

Closed pahlers closed 3 years ago

pahlers commented 3 years ago

I was adding an extra "crop aspect" for Instagram (4:5 and 1.91:1) according the manual.

If you want to add an aspect ratio to the pre-defined drop-down list you can do this by including a line of the form “plugins/darkroom/clipping/extra_aspect_ratios/foo=x:y” in darktable’s configuration file $HOME/.config/darktable/darktablerc. Here “foo” defines the name of the new aspect ratio and “x” and “y” the corresponding numerical values (x and y must be integers).

So I added: plugins/darkroom/clipping/extra_aspect_ratios/instagram=4:5. But it didn't work! I didn't saw my aspect ration in the list regardless of my efforts. No extra ratio, but also no error. Not in the console or application. Strange isn't it?

It turns out the code is has an undocumented "feature" called "remove duplicates from the aspect ratio list" (https://github.com/darktable-org/darktable/blob/release-3.6.1/src/iop/clipping.c#L2263-L2285). This code prevents double aspect ratios, which makes sense if it would cause performance issues.

It is very hard for a common user to understand this feature and I would like to suggest a change for this feature.

My suggestion would be one of the following options:

  1. Remove the "remove duplicates from the aspect ratio list"-code. It would be a handy feature that a user can add ratios using his own labels. And there will be 22 less code, yeah! Kill your darlings @houz :wink:. (bonuspoints when you can do this within the application);
  2. Expand the existing label with the custom label. The user can still customise and the list stays short;
  3. Throw an error when the custom ratio is deleted. This way the user can learn and call himself stupid;
  4. Update the documentation with a description of this "feature". This would be also an option.

Fix this "feature", please.

edit: provide correct link to the code

elstoc commented 3 years ago

(4) is the only option I can do from a dtdocs issue. If you would prefer one of the other solutions you should raise an issue against the darktable repository.

pahlers commented 3 years ago

Big thanks! I will raise a new issue.

pahlers commented 3 years ago

(sorry I edited the wrong issue :wink: )