The bug is still present in 21.08.2. In fact, it proved to be the fatal one-click crasher: if you work with Spacer tool and have groups of items in your Timeline, any click on a grouped element that is not the first (leftmost) in the group will crash the app--at least in Windows 10.

It is obviously due to some conflict while making a temporary group for Placer to move--Placer works fine when clicked on the first (leftmost) element of the group, but generates some fatal internal error if the element is somewhere in the middle.

Thank you for reporting. We fixed a lot of crashes lately. Please check with the latest daily build to see if the crash still happens. https://binary-factory.kde.org/job/Kdenlive_Nightly_mingw64/lastSuccessfulBuild/artifact/

(In reply to emohr from comment #2)
> Thank you for reporting. We fixed a lot of crashes lately. Please check with
> the latest daily build to see if the crash still happens.
> https://binary-factory.kde.org/job/Kdenlive_Nightly_mingw64/
> lastSuccessfulBuild/artifact/

Thanks. I have just repeated the test with the latest build. The result is almost the same, except that now Kdenlive closes automatically in a couple of secs. I had to call Task Manager to End Task in last released versions (21.08.1-3).

I am afraid two important points still are to be taken into consideration by the devs.

First, it seems obvious for me (I have some developer experience) that there is an inner Error in Spacer Tool algorithms, namely in how it makes/defines a group of moving parts. I.e. the tool works perfectly fine if no grouped items involved, and it works fine if you click (and hold) on the leftmost element of a group. But it reliably crashes the app if you click on any other element in the group. So it doesn't look like it a 'general crash'—the problem seems quite specific.

Second, maybe even more important. The nature of this bug seems not just annoying (any app has bugs, and any of them is annoying), but catastrophic: a user makes some groups (not many, the effect doesn't depend on the project size) which is a basic feature, then them use Spacer (basic tool), and all the project is under risk of crash all of sudden: just one click on a (wrong, though—why wrong?) part of a group consistently makes a crash. I am sure such a problem could/should be of higher priority than it is.

Though, I still regard Kdenlive as outstanding piece of software.

I can confirm the crash following your steps (tried on master). After clicking on the rigth grouped clip I get only this output of gdb:

warning: === REG FOCUS:  false
warning: qml: ENDING DRAG!!!!!!!!!!!!!!!!!!!!!!
warning: Invalid parameter passed to C runtime function.
warning: Invalid parameter passed to C runtime function.

Git commit 504154d1ecd773b3f4854436138cc21082265d0c by Jean-Baptiste Mardelle.
Committed on 23/11/2021 at 15:51.
Pushed by mardelle into branch 'release/21.12'.

Fix crash using spacer tool on grouped clips with a clip in the group positioned before spacer start operation.

M  +2    -3    src/timeline2/model/groupsmodel.cpp
M  +1    -1    src/timeline2/model/groupsmodel.hpp
M  +41   -5    src/timeline2/model/timelinefunctions.cpp
M  +0    -1    src/timeline2/model/timelinemodel.cpp
M  +1    -1    src/timeline2/view/qml/timeline.qml

https://invent.kde.org/multimedia/kdenlive/commit/504154d1ecd773b3f4854436138cc21082265d0c

(In reply to Jean-Baptiste Mardelle from comment #5)
> Git commit 504154d1ecd773b3f4854436138cc21082265d0c by Jean-Baptiste
> Mardelle.
> Committed on 23/11/2021 at 15:51.
> Pushed by mardelle into branch 'release/21.12'.
> 
> Fix crash using spacer tool on grouped clips with a clip in the group
> positioned before spacer start operation.
> 
> M  +2    -3    src/timeline2/model/groupsmodel.cpp
> M  +1    -1    src/timeline2/model/groupsmodel.hpp
> M  +41   -5    src/timeline2/model/timelinefunctions.cpp
> M  +0    -1    src/timeline2/model/timelinemodel.cpp
> M  +1    -1    src/timeline2/view/qml/timeline.qml
> 
> https://invent.kde.org/multimedia/kdenlive/commit/
> 504154d1ecd773b3f4854436138cc21082265d0c

Great! Thank you!

(In reply to Jean-Baptiste Mardelle from comment #5)
> Git commit 504154d1ecd773b3f4854436138cc21082265d0c by Jean-Baptiste
> Mardelle.
> Committed on 23/11/2021 at 15:51.
> Pushed by mardelle into branch 'release/21.12'.
> 
> Fix crash using spacer tool on grouped clips with a clip in the group
> positioned before spacer start operation.
> 
> M  +2    -3    src/timeline2/model/groupsmodel.cpp
> M  +1    -1    src/timeline2/model/groupsmodel.hpp
> M  +41   -5    src/timeline2/model/timelinefunctions.cpp
> M  +0    -1    src/timeline2/model/timelinemodel.cpp
> M  +1    -1    src/timeline2/view/qml/timeline.qml
> 
> https://invent.kde.org/multimedia/kdenlive/commit/
> 504154d1ecd773b3f4854436138cc21082265d0c

Just tested the latest night build (kdenlive-master-955-windows-mingw_64-gcc.exe). Two points:

(1) Yes, no more crashes on Spacer Tool clicking ‘wrong’ element in a group.

(2) The new problem is, Spacer clicking on a 'not leftmost' element of the group breaks the group: the group moves fine, but the leftmost element doesn't. Looks like we are a just next door to perfection. ))

Question to your point 2): we have 3 clips on a track: A, B, C. A and C are grouped. You click with spacer on clip C. Should A and B also move in the spacer operation?

(In reply to emohr from comment #8)
> Question to your point 2): we have 3 clips on a track: A, B, C. A and C are
> grouped. You click with spacer on clip C. Should A and B also move in the
> spacer operation?

$One-million question! Strangely, my reply is: A should, and B shouldn't.

Maybe, it is to be discussed with experts (I am not), but in my general and formal view, as you Spacer click on an element, and if the element is a part of a group, what should move is the respective group as a whole (treated as a single element), and all the elements to the right of the group. In our case, A & C are to move as a group, and B shouldn't because B is not right to the group! In my understanding, the group, if it is a group, should be treated as one element consistently.

Following the above: (1) I suppose the Spacer algorithm could reliably work like it did in case of clicking the leftmost element of a group (no crashes in that case were detected), the only correction expected is that the first moving element should be the leftmost element of the clicked group. And, by the way, the group should not treated as temporary like other moving elements to the right of it. ))

(2) Maybe, the whole practice of grouping non-adjacent clips in one track should be considered weird. ))

The practice of grouping not adjacent

P.S. Just formality: right to the group == right to the rightmost element of the group.

Maybe, this could help. Short and simple summary of how Spacer should work if clicked on a group, IMHO:

1. Each element of the clicked group should move with Spacer (no matter if there are non-adjacent elements—and I cannot imagine any, other reason). The essential point is, the group (as an entity previously created by the user) should stay intact after Spacer operation is finished. (Now it doesn't.)

2. All clips (and groups of clips) to the right of the rightmost clip of the clicked group are to form a temporary group to move along the clicked group. After Spacer operation ends, the temporary group should be properly ungrouped (without creating new groups nor breaking existing ones made by the user).

The problem with "move all grouped elements together" is that this might result in spacer tool appearing broken.
If you take again my example in a track:

[ CLIP A][               CLIP B               ][ CLIP C  ]

Clicking with spacer tool on Clip C to drag right will not work - nothing will happen until you move the cursor to an offset equivalent to the length of clips A and B. This is also confusing as the spacer tool could cause a reordering of the clips, and in case of clips grouped on multiple tracks could lead to some unexpected results... That's why I took the current approach which does not break groups (grouped elements stay grouped) but can create a space between grouped elements... I would like input from other users here before making a final decision...

(In reply to Jean-Baptiste Mardelle from comment #12)
> The problem with "move all grouped elements together" is that this might
> result in spacer tool appearing broken.
> If you take again my example in a track:
> 
> [ CLIP A][               CLIP B               ][ CLIP C  ]
> 
> Clicking with spacer tool on Clip C to drag right will not work - nothing
> will happen until you move the cursor to an offset equivalent to the length
> of clips A and B. This is also confusing as the spacer tool could cause a
> reordering of the clips, and in case of clips grouped on multiple tracks
> could lead to some unexpected results... That's why I took the current
> approach which does not break groups (grouped elements stay grouped) but can
> create a space between grouped elements... I would like input from other
> users here before making a final decision...

Yes, exactly, this is the conceptual point that demands some broader discussion. In the meantime, just a couple of important conceptual objections:

1. ‘Spacer tool appearing broken’ — is not a problem at all, so far as it works AS EXPECTED. If a user decided to group the non-adjacent A & C, he/she expects that (a) they will always moves together, and, yes, will not move ‘until the cursor moves to an offset equivalent to the length of clips A and B’. At least, this is true when Selection tool is used, so why would it change with Spacer tool? 

2. The same is true concerning ‘the spacer tool could cause a reordering of the clips’. — Well, basic Selection tool reoders clips if there is any ungrouped B between non-adjacent A & C. I would not regard this as a problem either.

3. In fact, in the last version I tested (kdenlive-master-955-windows-mingw_64-gcc.exe) current approach somehow breaks the group if A & B are grouped and Spacer cursor is on B. I just tested again. What is more, this breaks the group during Spacer operation, and A & B stay ungrouped after switch to Selection tool: the group is gone. Which is hardly what the user expects.

Again, my appoach may seem a little formalistic, but it looks like the most natural at the same time. There is an entity (i.e. group) that user has made. This is the highest priority: I want some clips to behave like one, and why any tool should interpret my will? )) If you worry for some possible confusions, you can forbid non-adjacent groups (in one track) or something.

And the definition of Spacer tool, so far as I see, is simple and natural: to make temporal group to the right of the clicked element, and move all this group with this element. It works perfectly without groups. It works perfectly when the leftmost element of the group is selected. The only important point to clarify and implement here is to regard the group of the clicked (grouped) element as a whole, i.e. one element, without any conditions and exceptions.

Sorry if this is too long. All best, Andrei

Git commit c1b0f27582bbb97932d5a400aa189042739582db by Jean-Baptiste Mardelle.
Committed on 25/11/2021 at 17:17.
Pushed by mardelle into branch 'release/21.12'.

Spacer tool: Don't allow independant move of grouped items.

M  +11   -10   src/timeline2/model/timelinefunctions.cpp

https://invent.kde.org/multimedia/kdenlive/commit/c1b0f27582bbb97932d5a400aa189042739582db

Okay, so after discussing this further, I just pushed a change that should make it work as you describe. Please test & let me know if it now works as you expect.

(In reply to Jean-Baptiste Mardelle from comment #15)
> Okay, so after discussing this further, I just pushed a change that should
> make it work as you describe. Please test & let me know if it now works as
> you expect.

My first impression is really very positive. In general, the tool works fine, predictable, and doesn't cause crashes, which is just great. There are some questions yet, and situations to be tested thoroughly (namely, some more complicated groups; clips on the parallel tracks; plus a special things: moving Spacer left; plus dependency of the Spacer operation on the cursor position—it seems that sometimes the tool works one way when click is on the left side of a long clip, and other way if click is at the opposite side of the clip—not sure yet if this is a bug or good feature). I am going to submit some more details today or tomorrow. Again—very good job, thanks!

(In reply to Jean-Baptiste Mardelle from comment #15)
> Please test & let me know if it now works as you expect.

Here is after testing some one track cases.

Case one: (*) (A) (B) (.)
where (A), (B) stand for grouped clips, and (.), (*) are for non-grouped ones.

Works as expected. My only question is, just in case: the whole temporary group under Spacer tool can move both left and right, but it never goes to the left of its nearest left neighbour (*), even if there is enough empty space in the Timeline. I suppose it is by design, but is it?

Case two: (*) (A) (-) (B) (.)

Works fine, but there is a doubtful point here. Namely: if Spacer tool clicks on (A), the moving group is (A)(-)(B)(.), but if clicked on (B), only (A)(B)(.) will move. This means that the temporary moving group to the right of the user-created one, (A)(B), is in fact defined as ‘to the right of the clicked element of the group’. By no means I’m sure, but I think this is at least discussible. Two points: (1) I would rather expect that a Spacer click on any element of the group give the same behaviour; (2) I would expect that if Spacer tool treats a clicked group as a whole, it defines ‘right to it’ by the whole group’s borders which is ‘to the right of the group's rightmost element’ (i.e. no matter which of the grouped elements was clicked). On the other hand, the current behaviour seems a bit more flexible. So this is just a notion.

Some tests on parallel tracks and with more complex groups are—hopefully—coming. In any case, there was no crash during the described above. Which is great.

As this is no crash anymore, I change it to TASK.