Improve "lift_gamma_gain" effect UI
Open, Needs TriagePublic

Description

Add

  • "restore default value" (for example by right click)
  • possibility to change values by typing e.g. into a text entry

After a little search I think the code to change is maybe found at:
https://invent.kde.org/kde/kdenlive/blob/master/data/effects/movit_lift_gamma_gain.xml
https://invent.kde.org/kde/kdenlive/blob/master/data/effects/lift_gamma_gain.xml
https://invent.kde.org/kde/kdenlive/blob/master/src/effectstack/widgets/lumaliftgain.cpp

kurtz created this task.Jan 8 2019, 9:31 PM
  • Restore default can normally be done with middle mouse click - although I understand it's not the best since unavailable to Mac/Windows/Laptop users. We should add another default

Just as an addon-info to this.
At the moment, resetting the color wheel does only that - the line values are not effected.
The other way around it's not like this: resetting the line will also reset the color wheel.
For what I saw on all color correction hardware, there are 2 seperate buttons to reset the wheel and the overall values.

I already implemented this in my local copy of the sourcecode.
If there's interest, I could commit it to the repo...

afarid added a subscriber: afarid.Jan 10 2019, 12:18 PM

Just as an addon-info to this.
At the moment, resetting the color wheel does only that - the line values are not effected.
The other way around it's not like this: resetting the line will also reset the color wheel.
For what I saw on all color correction hardware, there are 2 seperate buttons to reset the wheel and the overall values.

I already implemented this in my local copy of the sourcecode.
If there's interest, I could commit it to the repo...

I think it is a valid point. Submit the patch, right @mardelle? :)

kurtz added a comment.Jan 10 2019, 5:17 PM

Just as an addon-info to this.
At the moment, resetting the color wheel does only that - the line values are not effected.
The other way around it's not like this: resetting the line will also reset the color wheel.
For what I saw on all color correction hardware, there are 2 seperate buttons to reset the wheel and the overall values.

I already implemented this in my local copy of the sourcecode.
If there's interest, I could commit it to the repo...

Sounds good! I'm curious...

I just tried to git the actual sourcecode from the refactoring_timeline branch, but get a build error:
kdenlive/src/bin/bin.cpp:694: error: ‘renameFile’ is not a member of ‘KStandardAction’

m_renameAction = KStandardAction::renameFile(this, SLOT(slotRenameItem()), this);
                 ^

The fix ist quite easy to implement, just change line 176 from

c = NegQColor::fromRgbF(m_defaultValue/m_sizeFactor, m_defaultValue/m_sizeFactor, m_defaultValue/m_sizeFactor);
to
c = NegQColor::fromHsvF(m_color.hueF(), m_color.saturationF(), m_defaultValue/m_sizeFactor);

in src/assets/view/widgets/colorwheel.cpp

anyone interested in publishing this change?
(I guess it'll take a while, until I have a working refactoring version).

ok, I did "git clone", "git checkout", changed the line in colorwheel.cpp, "git add", "git commit", but "git push origin refactoring_timeline" throws an error "
fatal: remote error: service not enabled: /kdenlive.git"

what did I miss?

kurtz added a comment.Jan 10 2019, 8:22 PM
  • Restore default can normally be done with middle mouse click - although I understand it's not the best since unavailable to Mac/Windows/Laptop users. We should add another default

What would be a good (new) default?
My suggestions:

  • Right click
  • Right click opens context menu with "Restore default"-Entry (long way :( )
  • Reset buttons next to the widget (always visible)
kurtz added a comment.Jan 10 2019, 8:26 PM

ok, I did "git clone", "git checkout", changed the line in colorwheel.cpp, "git add", "git commit", but "git push origin refactoring_timeline" throws an error "
fatal: remote error: service not enabled: /kdenlive.git"

what did I miss?

Maybe this is interesting for you:
https://community.kde.org/Infrastructure/Github_Mirror

kurtz added a comment.Jan 12 2019, 7:56 PM

I've created a diff that changes the way to reset from middle button to right button: https://phabricator.kde.org/D18214