Page MenuHome

Gigantic VSE refactor
Confirmed, NormalPublicTO DO

Authored By
Richard Antalik (ISS)
Sun, Jun 7, 8:59 PM
Tokens
"Love" token, awarded by Xlindvain."Love" token, awarded by bnzs."Love" token, awarded by AndyCuccaro."Like" token, awarded by duarteframos."Love" token, awarded by Pipeliner."Love" token, awarded by pablovazquez."Like" token, awarded by pistolario."Love" token, awarded by jorsh."Love" token, awarded by vitos1k."Love" token, awarded by Andrea_Monzini."Love" token, awarded by Draise."Burninate" token, awarded by Gorro_Rojo."Love" token, awarded by Christopher_Anderssarian.

Description

Note: This is initial proposal aimed to gather feedback. It is subject to change.

This is task to coordinate restructurization of majority of VSE code.
Goal is to break existing code to smaller files with relatively narrow scope, create layer isolating low-level BKE code from functions close to user (operators, RNA, drawing). There should be minimum functional changes in first stages, followed by cleanups with minor functional changes if necessary (logic consolidation).

Motivation:

  • Maintainability in general
  • To allow work on T59540, where it is important for drawing, RNA and operator code to follow exact same logic to produce correct results.
  • Make it easier and cleaner to make changes in rendering code

Since this will be relatively large operation, it will have to be done in few stages:

  1. Move rendering code from BKE.
  2. Move rest of high level code from BKE.
  3. Move as much as possible lower-level code from operators and RNA.
  4. Refactor operator and RNA code moving remaining low-level logic. Aim for limiting logic to iteration and calling sets of functions from "libraries".

Because I am involved in triaging mostly, I think that these patches will be spread out over few months. Refactors will be likely fast, final cleanup not so much. I think that precondition for this task is to fix as much bugs as possible. I have marked them with #2.90 tag.

Structure proposal:

bf_blenkernel/
	sequencer.c							cleanup, primitive functions mostly to alloc/free stuff

space_sequencer/
	space_sequencer.c						no change, space code

	ops/
		sequencer_add.c					simplify logic / cleanup
		sequencer_buttons.c				simplify logic / cleanup
		sequencer_modifier.c				simplify logic / cleanup
		sequencer_ops.c					simplify logic / cleanup
		sequencer_preview.c				simplify logic / cleanup
		sequencer_select.c					simplify logic / cleanup
		sequencer_view.c					simplify logic / cleanup

	draw/
		sequencer_draw.c					I could break up to timeline and preview specific functions, otherwise little changes.
		sequencer_scopes.c					no change most likely

	render/
		sequencer_render.c					new file, general render pipeline
		sequencer_render_effects.c			no change likely, effect strips
		sequencer_render_modifier.c			no change likely, modifiers
		sequencer_render_cache.c			little changes, image cache
		sequencer_render_prefetch.c			little changes, prefetching

	strips/
		sequencer_strips_select.c			new file, handle selection
		sequencer_strips_add.c				new file, create strips
		sequencer_strips_transform.c			new file, move strips / content
		sequencer_strips_edit.c				new file, cutting strips, duplicating, changing properties
		sequencer_strips_hierarchy.c			new file, traversing, relations, checks
		sequencer_strips_util.c				new file, functions, that doesn't really fit anywhere else, this file should be kept small

Things to clarify:

  • Structure is not really finalized, if during refactoring process file still feels like mixed mag, I may split it to more categories.
  • I am not sure if proposed structure is OK - I don't see any subdirectories in bf_editor_xxx. I would like to have something at least resembling proposed structure. I guess sequencer_category_subcategory.c could be used?
  • Should functions in strips category have ED_ prefix? They are really meant to be a middle layer between BKE and operators.
  • Scale and scope of patches. for example 1 patch per file or 1 patch per "category"

Related Objects

Event Timeline

Generally is super nice to have sequencer better organized from the code point of view, and de-duplicate logic as much as possible.

As for the locations of the files I don't think they belong to editors. Editor API should not be used by render pipeline, for example. To me it seems that sequencer should become more top-level folder, like compositor is, and used by render pipeline, sequencer space and so on.

As for the approach of how to structure the changes:

  • Try to move files as-is to the neighbourhood of their destination
  • Split them up at their final destination

Moving files can happen as a bulk operation, since that'd be annoying to keep incrementally modify entire source code. Splitting up and other changes try to keep as isolated as possible, in a stream of commits.

For the planing, it seems like this is an involved change, and we have quite reasonable amount of reports which doesn't seem to be requiring such refactor. To me it seems for 2.90 is better to do stabilization pass, and aim this refactor for 2.91. Hence i'll place this to 2.91 column. Surely, some work can start early on, but finalized (with all the cleanup and de-duplication you've mentioned) is unlikely to happen for 2.90 due to time constraints.

This is my vision anyway. Since there is some source layout/etc involved, wouldn't mind if @Brecht Van Lommel (brecht) shares his strong opinion :)

Thanks for input, I agree with all points here, so I will change description with more discrete steps that can be checked out. Will have to look at compositor code organization, but I suspect that I will probably want to keep alloc/free functions in BKE if I wanted to make strip a ID at some point.

Dalai Felinto (dfelinto) changed the task status from Needs Triage to Confirmed.Wed, Jun 24, 5:14 PM