elements: Select MultipleKeyframeCurve keyframes.
ClosedPublic

Authored by stefanzzz on Jul 18 2017, 5:04 PM.

Details

Summary

Added the possibility to select MultipleKeyframeCurve keyframes by clicking
on them.

Depends on D1771

Diff Detail

Repository
rPTV Pitivi
stefanzzz created this revision.Jul 18 2017, 5:04 PM
thiblahute requested changes to this revision.Jul 18 2017, 6:43 PM

Almost good, I just do not like so much the way you force the seek on selection.

pitivi/timeline/elements.py
529

What guarantees there will be a selected clip?

531

I think you should be able to do the seeking yourself here and not propagate the event down.

578

What does that mean? (never seen that synthax!)

pitivi/timeline/timeline.py
380

See my previous comment about seeking.

This revision now requires changes to proceed.Jul 18 2017, 6:43 PM
stefanzzz added inline comments.Jul 19 2017, 1:40 PM
pitivi/timeline/elements.py
529

The fact that there was a visible keyframe to click on. That should only happen when exactly one clip is selected.

578

It's numpy.ndarray indexing. It selects the first column. https://stackoverflow.com/questions/8386675/extracting-specific-columns-in-numpy-array

We use it in other places, as well.

thiblahute added inline comments.Jul 19 2017, 2:03 PM
pitivi/timeline/elements.py
529

OK, L570 you handle that case, which lead to my question.

578

Nice, ok :-)

stefanzzz marked 8 inline comments as done.Jul 19 2017, 2:26 PM
stefanzzz updated this revision to Diff 6074.Jul 19 2017, 2:26 PM
stefanzzz edited the summary of this revision. (Show Details)

Renamed timeline forced_next_position to __next_seek_position and added a
method to set it.

aleb added inline comments.Jul 19 2017, 2:29 PM
pitivi/timeline/timeline.py
676

could be a pydoc. Make it look consistent with the others.

stefanzzz updated this revision to Diff 6075.Jul 19 2017, 2:52 PM

Made sure keyframes can be selected (i.e. the playhead seeks to them)
even if 'left click also seeks' is disabled. Also, a pydoc was added.

stefanzzz marked an inline comment as done.Jul 19 2017, 2:53 PM
aleb added inline comments.Jul 19 2017, 2:56 PM
pitivi/timeline/timeline.py
675

Add a dot at the end please :) Must look like a sentence.

stefanzzz marked an inline comment as done.Jul 19 2017, 2:59 PM
thiblahute accepted this revision.Jul 19 2017, 3:54 PM
This revision is now accepted and ready to land.Jul 19 2017, 3:54 PM
aleb added inline comments.Jul 19 2017, 8:11 PM
pitivi/timeline/elements.py
136–137

indentation

stefanzzz updated this revision to Diff 6079.Jul 20 2017, 12:16 PM
stefanzzz marked an inline comment as done.

Fixed indentation.

aleb added inline comments.Jul 21 2017, 8:24 AM
pitivi/timeline/elements.py
524

Can you distinguish here between single-click and double-click, so the seek is performed only when single-clicking?

stefanzzz added inline comments.Jul 21 2017, 1:05 PM
pitivi/timeline/elements.py
524

Not really. When double-clicking, the order of events is:
BUTTON_PRESS
BUTTON_RELEASE
BUTTON_PRESS
2BUTTON_PRESS
BUTTON_RELEASE

I can't distinguish the first two ones from a single click. The only thing that I could do is save the last position and restore it after double-clicking, but the visual effect wouldn't be too nice.

This revision was automatically updated to reflect the committed changes.