Rotation and Opacity files
Needs ReviewPublic

Authored by ashwins on Jan 29 2018, 2:33 PM.

Details

Reviewers
mardelle
Summary

Added Opacity.qml and Rotation.qml files for transform effect. The effects need modification in other files namely kdenlivemonitoreffectscene.qml, qmlmanager.cpp, monitor.cpp, animationwidget.cpp, these files are also added

Diff Detail

Repository
R158 Kdenlive
Branch
refactoring_timeline
Lint
No Linters Available
Unit
No Unit Test Coverage
ashwins requested review of this revision.Jan 29 2018, 2:33 PM
ashwins created this revision.

The animationwidget.cpp file has incomplete rotation update slot. Please guide for the same.

In animationWidget.cpp, you must enable the display of the opacity widget when necessary. Try something like:
at line 862, just after building the m_spinOpacity widget:
m_monitor->setEffectSceneProperty(QStringLiteral("showOpacity"), m_spinOpacity != nullptr);

Then try it to check if the opacity qml overlay shows up on monitor when adding a "Transform" effect to a clip and post a screenshot here if it succeeds.

Regarding rotation, that will need some changes to void AnimationWidget::buildSliderWidget.

You need to check if the paramTag value is "rotation". In that case, you should enable the Rotation qml overlay with something like:
m_monitor->setEffectSceneProperty(QStringLiteral("showRotation"), true);
You can then store the DoubleWidget *doubleparam in a private variable like m_rotationWidget, and use that m_rotationWidget in the slotUpdateRotation slot like:
m_rotationWidget->setValue(...)

Regards

src/uiresources.qrc
68

You should give the file path or it won't be found. Change to:
<file alias="Opacity.qml">monitor/view/Opacity.qml</file>

ashwins updated this revision to Diff 26314.Feb 1 2018, 2:16 PM
  1. Updating D10176: Rotation and Opacity files Changes in slotUpdateRotation, other mentioned changes performed
ashwins updated this revision to Diff 26316.Feb 1 2018, 2:22 PM
  1. Updating D10176: Rotation and Opacity files

    As angle has return type qreal, added accordingly in the slotUpdateRotation

As you mentioned to see if Opacity qml overlay is displayed on monitor, how to check that.?

As my dev package is in kdenlive_git I've tried building kdenlive CMakelists.txt in qt, it gives the following error - https://pastebin.com/jDczNBS8
am I following correct way to build.?

mardelle added a comment.EditedFeb 3 2018, 11:14 AM

The log states it cannot find QtSvg. Install the Qt5Svg devel package to solve problem, probably called libQt5svg-dev or something like that.

ashwins updated this revision to Diff 26651.Feb 6 2018, 2:45 PM
ashwins marked an inline comment as done.
  1. Updating D10176: Rotation and Opacity files #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment.

    Updated the files with mentioned change in uiresources.qrc
ashwins added a comment.EditedFeb 6 2018, 4:25 PM

As I mentioned earlier, when trying to build the kdenlive in qt with CMakelists.txt, it shows the Qt5svg5config.cmake error. Even after installing the libqt5svg5-dev package, the error exists.
Error - https://pastebin.com/tag8wbxE
Installed package - https://pastebin.com/hhuz4AFM

ashwins updated this revision to Diff 26691.Feb 7 2018, 10:10 AM

Updating D10176: Rotation and Opacity files

The signal used is angleChanged(), had used rotationChanged() at some places. Updates done.

Here are some comments on the qml. Please fix and try compiling.

src/monitor/view/kdenlivemonitoreffectscene.qml
540

The opacityChanged(real) should be defined in the root object (around line 38 of this file), and not as a child of the Opacity item.

Then, in the Opacity.qml file, you trigger the signal by calling:
root.opacityChanged(value)

548

There is no reason to trigger th signal when creating the object.
The root.opacityChanged signal should only be emitted when the slider value changed, so that means in the Opacity.qml file

553

Same remark as opacityChanged. Signal should be defined as root signal.

561

remove

ashwins updated this revision to Diff 26695.Feb 7 2018, 12:30 PM

Updating D10176: Rotation and Opacity files

Mentioned changes performed.

ashwins updated this revision to Diff 27147.Feb 14 2018, 1:52 PM
ashwins marked 4 inline comments as done.
  1. Updating D10176: Rotation and Opacity files added the changes in geometrywidget.cpp
ashwins updated this revision to Diff 27149.Feb 14 2018, 1:59 PM

changes from Animationwidget.cpp have been removed.

ashwins updated this revision to Diff 27265.Feb 15 2018, 2:37 PM

Some changes in the connect calls had to be done. Please have a look at it. The build has been successful, let me know for testing.

I don't see the change required to the src/uiresources.qrc file in the diff. They are necessary for the application to find your new qml files. Also, for some reason I don't understand, having the qml files called Opacity.qml and Rotation.qml does not work, maybe these names are internally reserved. Rename the files to OpacitySlider.qml and RotationSlider.qml and update the name in kdenlivemonitoreffectscene.qml.

After that (and removing the extra bracket) the sliders show up for me. They don't work thought because of a problem with the signals, I let you check that (it prints useful info in the console when trying to use the slider).

src/monitor/view/kdenlivemonitoreffectscene.qml
557

Extra bracket, remove

ashwins updated this revision to Diff 27362.Feb 16 2018, 4:59 PM
ashwins marked an inline comment as done.
  1. Updating D10176: Rotation and Opacity files

    Added uiresources.qrc and renamed the files.
ashwins updated this revision to Diff 27616.Feb 20 2018, 3:21 PM
  • New modifications
  • Merge branch 'refactoring_timeline' of git://anongit.kde.org/kdenlive into refactoring_timeline
  • GW modified
  • The sliders show up but doesn't work

Fixing my inline comments to the qml files should make it work.

src/monitor/view/OpacitySlider.qml
19

The signal has to be emitted from the monitorefffectscene root object where you define it. So you must change this line to:
root.opacityChanged(value)

Also the m_opacity widget in GeometryWidget expects a 0-100 value but your slider has 0-1 range so you must fix this also

src/monitor/view/RotationSlider.qml
19

The signal has to be emitted from the monitorefffectscene root object where you define it. So you must change this line to:
root.angleChanged(value)

ashwins updated this revision to Diff 27785.Feb 22 2018, 2:08 PM

The opacity slider works now. Rotation slider requires some work as it still doesn't give expected result.

ashwins updated this revision to Diff 28182.Feb 27 2018, 1:15 PM

keyframewidget file changes
Tried adding the m_rotation in keyframewidget.cpp file but couldn't get it working. Please do look into it.

ashwins updated this revision to Diff 28553.Mar 4 2018, 7:33 AM
  • Rotation value changes on moving the slider but effect does not persist.

The opacity slider required 'root.opacityChanged()' for the slider value change to persist with the clip frame. But although now with rotationslider, the rotation value changes on moving the slider but it does not appear on clip frame.

Hi!

So I finally managed to look at your diff. For rotation, we need to send a valueChanged(value) signal to trigger a monitor refresh. Also, since I made a few changes since your last diff, I attach her the working diff against latest git. Please test:

ashwins updated this revision to Diff 29511.Mar 14 2018, 5:21 PM
  • Changes in collapsibleeffectview
  • Both sliders are working now as expected