Skip to content

Clipping changes.#159

Closed
levitte wants to merge 1 commit intodarktable-org:masterfrom
levitte:clipping
Closed

Clipping changes.#159
levitte wants to merge 1 commit intodarktable-org:masterfrom
levitte:clipping

Conversation

@levitte
Copy link
Copy Markdown
Contributor

@levitte levitte commented Jan 16, 2013

  • Save the flipped state of the aspect ratio preset along with the preset
    itself. (plugins/darkroom/clipping/aspect_flipped)
  • Change the saved custom_aspect to be a string with the actual custom
    aspect. This is so the user will see what he/she entered instead of a
    computed value.
  • When the user enters an aspect ratio of his/her own, actually save it
    too. (plugins/darkroom/clipping/custom_aspect)
  • Handle the saved presets better when starting up.

Fixes #9151, #9143

…eset

  itself.  (plugins/darkroom/clipping/aspect_flipped)
* Change the saved custom_aspect to be a string with the actual custom
  aspect.  This is so the user will see what he/she entered instead of a
  computed value.
* When the user enters an aspect ratio of his/her own, actually save it
  too.  (plugins/darkroom/clipping/custom_aspect)
* Handle the saved presets better when starting up.
@TurboGit
Copy link
Copy Markdown
Member

I'm not sure what is supposed to do. Can you clarify what is changed? I have applied this patch and at the GUI point of view I do not see changes.

What are #9151 and #9143 numbers? I do not see them in Redmine.

@levitte
Copy link
Copy Markdown
Contributor Author

levitte commented Jan 16, 2013

For custom aspect ratios, it actually saves it (it didn't before, so you'd end up with 1.5:1 when coming back into develop mode because that is the installed default). It also saves it in the form that the user typed it in, so it can shown back the same way later on, instead of {fraction}:1.
Also, it saves the flipped state of aspect ratio presets, something it didn't do before (which means that when you came back into develop mode, your flip is suddenly unflipped).

#9151: http://darktable.org/redmine/issues/9151
#9143: http://darktable.org/redmine/issues/9143

@ghost ghost assigned TurboGit Jan 18, 2013
@boucman
Copy link
Copy Markdown
Member

boucman commented Jan 18, 2013

Pascal, I am assigning the PR to you since you worked in that area and said you would look at it with your changes...

if you don't want to look into it, please deassign yourself and i'll deal with it

thx

@TurboGit
Copy link
Copy Markdown
Member

Sure. Richard said on the mailing-list that he was working on an updated version IIRC. Richard?

@boucman
Copy link
Copy Markdown
Member

boucman commented Jan 18, 2013

oh, I must have missed that...

@levitte
Copy link
Copy Markdown
Contributor Author

levitte commented Jan 18, 2013

Yup. It'll be a couple of days before you see the rest happening, though.

@boucman
Copy link
Copy Markdown
Member

boucman commented Jan 25, 2013

still working on this ? should I wait for an update or is it good as is ?

@levitte
Copy link
Copy Markdown
Contributor Author

levitte commented Jan 26, 2013

[change of mind] please merge this pull if you see it fit, as it's a unit of its own, independent enough from what I'm working on now.

@boucman
Copy link
Copy Markdown
Member

boucman commented Jan 28, 2013

ok, I wasn't able to test the code itself because the aspect in c&r seems to be very broken in multiple ways

"image" ratio is not selected (3:2) on new images, though it seems tocome back randomly

the aspect ratio seems to be carried over from image to image when switching...

i'm sorry for your PR but i'd rather merge it on a solid basis and this is currently not the case. don't worry we didn't forget you, though

@boucman
Copy link
Copy Markdown
Member

boucman commented Feb 26, 2013

Ok, the C&R module was really buggy, and @AlicVB reimplemented a big part of it in his branch. Fixing the bugs meant implementing what this patch was doing but in a different way. So your feature request was fulfilled, but not using your patch

thx for the code anyway

@boucman boucman closed this Feb 26, 2013
@levitte
Copy link
Copy Markdown
Contributor Author

levitte commented Mar 1, 2013

Yup, I noticed his fixes, and I like. I'll simply delete this branch.

@levitte levitte deleted the clipping branch March 1, 2013 14:04
@LebedevRI LebedevRI modified the milestone: 1.1.x Oct 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants