Fix T84057: Delete operator removes more strips
Needs ReviewPublic
Actions

Authored by Richard Antalik (ISS) on Mar 11 2022, 5:26 PM.

Details

Reviewers

Sergey Sharybin (sergey)

Maniphest Tasks

T84057: Unwanted deletion of non selected VSE strips.

Summary

Because removing strips is 2 step process where strips are flagged first
and finally removed at once, a bug can cause that strip won't be removed,
but still flagged.

Clear SEQ_FLAG_DELETE flag from all strips before deleting selected
strips. Add assertion statement, so in case bug exists, it can be
discovered more easily.

I have realized just later, that this is quite old report, so it's
likely, that this bug is already fixed. I could have done this in versioning
code, but we have similar "fix" for seq->tmp field already.

Diff Detail

Repository

rB Blender

Branch

T84057 (branched from master)

Build Status

Buildable 20990
Build 20990: arc lint + arc unit

Event Timeline

Richard Antalik (ISS) requested review of this revision.Mar 11 2022, 5:26 PM

Richard Antalik (ISS) created this revision.

Harbormaster completed remote builds in B20990: Diff 49173.Mar 11 2022, 5:26 PM

so it's likely, that this bug is already fixed

Did you test that? Is this patch still relevant?

In D14314#383143, @Sergey Sharybin (sergey) wrote:

so it's likely, that this bug is already fixed

Did you test that? Is this patch still relevant?

I don't have steps to reproduce, only broken file. If there are more broken files, than patch is relevant.

If the new code is following policy that flags are clear after use (which means, input of operators is expected to be in a good state) then better idea would be to clear unexpected flags as part of do-versions.
If the flags policy is such that operators are not guaranteed to clear the flag after they've run, then the input is to be considered as dirty, and the flag is to be cleared without an assert.

In either case fix is simple enough, so it will be good to fix it for the file from the report.

Right, the code should leave flags in correct state, but I can't technically guarantee, that similar bug won't happen in future. But I have no issue cleaning this up in do versions.

Bugs are always possible, but we can not write code in a way which will accomodate for possibility of bug in the future. It will make code very hard to write and follow.

Revision Contents
Changeset List

Path

Size

source/

blender/

editors/

space_sequencer/

sequencer_edit.c

15 lines

Diff 49173

View Options

source/blender/editors/space_sequencer/sequencer_edit.c

Fix T84057: Delete operator removes more stripsNeeds ReviewPublicActions