Page MenuHome
Differential D12806

Fix versioning for sequencer color balance modifier.
ClosedPublic
Actions

Authored by Josef Raschen (JosefR) on Oct 9 2021, 1:20 PM.

Details

Reviewers
Richard Antalik (ISS)
Commits
rBc383397d072f: Fix versioning for sequencer color balance modifier.
Summary

My last patch adding slope/offset/power controls to the sequencer color balance modifier (D12575) was not setting the defaults for old blend files correctly.

Before, if the old file is using a color balance modifier that is then changed to SOP mode:

With this patch:

Diff Detail

Event Timeline

Josef Raschen (JosefR) requested review of this revision.Oct 9 2021, 1:20 PM
Josef Raschen (JosefR) created this revision.
Pratik Borhade (PratikPB2123) edited subscribers, added: Richard Antalik (ISS), Pratik Borhade (PratikPB2123); removed: Video Sequencer.Oct 10 2021, 2:01 PM

Hi @Richard Antalik (ISS) , can you please take a look at this patch?

Richard Antalik (ISS) requested changes to this revision.Oct 11 2021, 10:58 AM

Seems to work Ok, got just minor requests.

source/blender/blenloader/intern/versioning_300.c
465–469

You can do

copy_v3_fl(cb->slope, 1.0f);
copy_v3_fl(cb->offset, 1.0f);
copy_v3_fl(cb->power, 1.0f);
1712–1713

you should include new versioning code at the end of do_versions function inside block

{
  /* Keep this block, even when empty. */
}

Since subversion number can increase often I rather do this manually when committing anyway.

This revision now requires changes to proceed.Oct 11 2021, 10:58 AM
Josef Raschen (JosefR) updated this revision to Diff 43277.Oct 13 2021, 9:36 PM

@Richard Antalik (ISS)
Changes as requested. Is it OK to keep the change within the v31 subversion block or shall it be moved out completely? I think this should match to when the original patch was submitted.

Richard Antalik (ISS) accepted this revision.Oct 13 2021, 10:09 PM
In D12806#337101, @Josef Raschen (JosefR) wrote:

@Richard Antalik (ISS)
Changes as requested. Is it OK to keep the change within the v31 subversion block or shall it be moved out completely? I think this should match to when the original patch was submitted.

I guess you are correct in that this should be added to older version when D12575 was committed. Otherwise you would have to check if the fields are uninitalized which is not done in this patch... And yes, BLENDER_FILE_SUBVERSION was set to 31 at the time so this is correct. Sorry for misleading you.

So should be OK to go. Thanks for patch!

This revision is now accepted and ready to land.Oct 13 2021, 10:09 PM
Josef Raschen (JosefR) marked 2 inline comments as done.Oct 13 2021, 11:09 PM
Josef Raschen (JosefR) added a subscriber: Peter Fog (tintwotin).
In D12806#337105, @Richard Antalik (ISS) wrote:

So should be OK to go. Thanks for patch!

Great, we have to thank @Peter Fog (tintwotin) for noticing that.

Closed by commit rBc383397d072f: Fix versioning for sequencer color balance modifier. (authored by Josef Raschen (JosefR), committed by Richard Antalik (ISS)). · Explain WhyOct 15 2021, 11:21 PM
This revision was automatically updated to reflect the committed changes.
Richard Antalik (ISS) added a commit: rBc383397d072f: Fix versioning for sequencer color balance modifier..

Revision Contents
Changeset List

PathSize
source/
blender/
blenloader/
intern/
23 lines

Diff 43277

View Options

source/blender/blenloader/intern/versioning_300.c

Loading...