Frame editing operations are not correct

[Raffaella Traniello]

Cut, copy, paste operations frequently give odd results: frames are positioned randomly, even moved from scene to scene.
The most suspected bug triggering event is deleting frames (the most needed operation in stop-motion frame editing).

To replicate the bug:
1. Download the attached project.
    I created it on purpose, with 3 scenes (A, B and C) and 10 frames per scenes. Every frame is marked A1, A2, A3... B1, B2, ...
2. Open it in Stopmotion.
3. Duplicate all the frames of scene A, by dragging every frame on the preview area below the timeline.
4. Move to scene B, by clicking on the clapboard marked with 2.
5. Delete all the odd frames (5 frames: 1,3,5,7,9). To delete, select a frame and press the DEL key.
6. Move to scene C.
7. Duplicate all the frames of scene C.
8. Check scene A: what a mess!

I attach the result of my editing.
The first five couple of duplicated frames of scene A are OK.
The second five couples are a mess: they include the first 5 frames from scene C.
The list of images inside the .sto archive is missing 5 numbers (27,29, 31,33, 35) corresponding to the 5 replicas of the first 5 frames of scene C.

[Raffaella Traniello (raffaella-traniello)]

Here is the messed-up project I created with the procedure written above.

[Tim Band (tim-band)]

Remember that LSM uses different file names in projects that are started from scratch, compared to projects opened already populated with frames, so in testing fixes, it is important to test a project that is started from scratch instead of (or as well as) Raffaella's uploaded one.

[Tim Band (tim-band)]

I started looking at this a while ago and became convinced that LSM's file handling code was completely inadequate and needed a fairly major re-write. In preparing for this, I began to write a document. I got as far as working out some of the requirements, and proving to myself what LSM did to honour them, when I found out that LSM actually isn't far off doing what it's meant to.

So the fix is actually rather simple.

However, it would be nice if this work didn't entirely go to waste, so here is a document describing, briefly, how LSM deals with the files in its project directory.

Is there a good place this information should go?

[Tim Band (tim-band)]

So, here is the patch. It includes the removal of a few lines of dead code. If this code were to be made live (I have been tempted...) it would break duplication of frames that were loaded from a previously-saved project.

It would be worth finding out if, over time, projects can retain files that are no longer referred to. I cannot see how this can happen, but that's what the dead code is all about, so...

[Herman Robak (herman-skolelinux)]

The image file names should indeed be immutable, i.e. once stored, the file is never renamed. An ever increasing counter would be fine with me. Decimal, hex, base64, whatever, just as long as the filenames don't get too long, and wrap-around can't realistically happen.

[Tim Band (tim-band)]

With the code as I have patched it, the filename will change if it is deleted and the delete is undone: it will get the next fresh number when it is restored. I think it would be cleaner if we had garbage collection, and left the files in place in the tmp directory (bad name, that!).

However, as the fix for this is so simple, we should perhaps leave major restructuring for a new wishlist item: "duplicated frames should share a picture file in the project"?

[Herman Robak (herman-skolelinux)]

I agree that dup-ed frames ought to only be duplicated in the frame list (internally _and_ in saved projects), not copied on disk. Caching would benefit from that, too.

Speaking of caching; I'm in favour of immutable filenames, for a cleaner and saner design of pre-loading, caching and undo. An image should have a persistent in-memory ID and on-disk filename that NEVER changes. It's a design contract: The image is given a name at capture/import time, and that name sticks!

Exception: Ephemeral "preview" images that are captured several times per second. If they have a name at all, they will be temporary names as long as they are just thrown away after instant display. Once they become "for keeps", that is added to the project, they get a real name, and that never changes, ever.

[Herman Robak (herman-skolelinux)]

And, btw, foo/imgXYZ.jpg should be the same image as bar/imgXYZ.jpg. The directories are just convenient sorting bins, not separate namespaces.

[Herman Robak (herman-skolelinux)]

(About removing dupes) But there is a snag: Editing a frame with an external program. Once edited, it is no longer the same image.

If the user's intent was to edit the _image_, spanning multiple frames, then it's fine. But we can't really know that. What if the user only wanted to doodle on _one_ frame, and leave the other dupe(s) alone?
Suggestion: Implement doodling overlays, having "edit in Gimp" as a "full image" special case. That is, keep the animation images immutable, make a copy of the image to be edited in external program, and put that in the "foreground". The overlays may have immutable names and IDs pulled from the same pool/namespace as the images. The only relaxation is that their content is mutable; the user can edit them, and is expected to do so.

[Tim Band (tim-band)]

Yes, I mention this briefly in my doc. My opinion is that few users if any would welcome having all the duped images change. If you want that, you edit THEN duplicate, right? Therefore pressing the Edit button copies to a freshly-generated name (assuming we want to go back to the old one if Undo is pressed. That doesn't happen at the moment, though, does it?). As long as we have working garbage collection everything will be fine. Working garbage collection sorts out a lot of this stuff. I think it should be the first thing we attempt if we want to do it.

But as I say, applying this patch (assuming testers like it too) should happen first. Worse Is Better and all that...

[Tim Band (tim-band)]

released in 0.8.0