Sequencer timeline UI repainting is 3x-4x faster now, for complex
timelines. On Sprite Fright Edit data set, with whole timeline visible
(2702 strips), repainting the timeline UI with all overlay options
(waveforms, offsets, thumbnails etc.):
- Windows (Ryzen 5950X, RTX 3080Ti, OpenGL): 62ms -> 18.6ms (16FPS -> 54FPS)
- Mac (M1 Max, Metal): 39.8ms -> 11.5ms (25FPS -> 86FPS)
This is achieved by:
- Avoiding tiny GPU draw calls (i.e. drawing one quad a time), instead
batch all the quads / lines needed by the timeline display into
series of about-1000 quads per draw.
- For retiming keys display, batch their keyframe point drawing too.
- For audio waveform overlay display, change it to draw batched quads
instead of alternating between line strips and triangle strips. This
actually changes how the waveform looks like (implements #115274)
and fixes some visual issues with waveforms too.
- For fcurve overlays, also draw them as batched quads.
While at it, this also fixes an issue where while dragging strips over
other strips, their text labels would look as if they are behind the
background strips.
Pull Request: https://projects.blender.org/blender/blender/pulls/115311
Since b1526dd2c6, viewport renderer sets `G.is_rendering`, but VSE scene
rendering function used this to decide whether to do offscreen opengl
render or use render API. This logic was quite weak.
Use `SeqRenderData::for_render` instead of `G.is_rendering`, since it
explicitly defines whether strip rendering is invoked by F12 render job.
Since offscreen gl rendering rely on `BLI_thread_is_main()` being
true, use this to initialize `do_seq_gl` variable for clarity.
The use of render job path was further conditioned on `G.is_rendering`,
with exception of running headless, but this condition was incorrect.
This condition was reformulated as precondition, which if true, returns
`nullptr` instead of crashing.
Pull Request: https://projects.blender.org/blender/blender/pulls/115079
The meta strip range was calculated before the effect strip range was
updated. This resulted in incorrect range. To the user this appeared
as the meta strip erratically jumping to another location in the
timeline when transforming it, when it contained effect strips.
Pull Request: https://projects.blender.org/blender/blender/pulls/114644
When VSE cache is invalidated (on most VSE edit operation e.g.), or
Blender quits, in some cases (complex and very long edits), the
prefetch job would block several seconds after being requested to stop.
This was because one codepath would keep looping over all frames without
checking for the `stop` flag.
Caused by 3fccfe0bc6 again, no idea why these two VertorSet were defined
as static variables in the functions generating them... But this was for
sure calling for _lots_ of problem. There are almost never good cases
for a function to return a static variable, and if it's done, it has to
be done extremely carefully.
BLF code is not threadsafe, yet font loading gets called over and over
by text strips when the font file is missing, including e.g. from
depsgraph evaluation code when duplicating the strip for evaluation.
WARNING: This is a quick fix for deblocking the Blender studio, proper
fix (and report) still needs to be worked on.
In `shuffle_seq_time_offset_get()` code tried to check whether
`strips_to_shuffle` would overlap with each other, but this was done
incorrectly. This check relied on result of `shuffle_seq_test_overlap()`
but that function assumes, that only 1 strip of its input is transformed
by `offset`. This means, that if 2 strips are moved by same offset and
overlap is checked against each other it could result in true return
value. However this is checked in the code already by
`strips_to_shuffle.contains(seq_other)`, in which case loop continues.
This resulted in emmision of warning which was incorrect.
The issue is fixed by continuing loop if `strips_to_shuffle` contains
`seq_other` as first precondition to signify, that this is expected and
in fact inevitable case. `shuffle_seq_test_overlap()` does not check
whether 2 passed strips are equal, rather `BLI_assert` is used to
signify, that this is not valid use-case.
Since the warning can not happen, the logging was removed.
Use `VectorSet`, `Vector` or `Span` instead of `SeqCollection` struct.
It is now possible to use native `for` loops and `SEQ_ITERATOR_FOREACH`
macro can be removed.
Another feature is, sets of strips no longer needs to be freed. However,
this poses a limitation, that query functions can not be used in case,
where these sets need to be available outside of scope where they are
created.
Pull Request: https://projects.blender.org/blender/blender/pulls/111909
Also remove const for non-pointer arguments which is meaningless in
the declaration, move a scene enum to the "enums.hh" header, and add
missing forward declaration of structs.
Adding strips or changing filepaths caused thatde movie files were loaded,
but memory was only released after new frame was rendered. Since FFmpeg
can take a lot of memory per strip, this can cause crash.
Free memory for each strip immediately after it is not needed after
these operations.
Pull Request: https://projects.blender.org/blender/blender/pulls/114381
Property `speed_factor` was used before retiming. It was kept for
potential versioning code for complex speed animation, that was
possible with even older `pitch` property.
Strips, where `speed_factor` is set to static value are already
correctly converted.
Such versioning code would be quite complex, possibly slow and maybe
could corrupt files. This is due to multiple factors:
- Sound seeking was broken, so conversion would have to ignore all
keyframes before strip starts to map frames properly.
- Because some animation was effectively ignored, it may cause
inconsistencies when doing conversion.
- It would have to integrate value of `speed_factor` to map keyframes
to strip space, but the animation is not limited to strip length.
- For each keyframe where speed gradually changes, at least one smooth
speed transition would be required, but there would be discrepancies
that would have to be accounted for. With simpler strategy it is
likely, that extent of ramps would be limited and thus animation would
be quite different from original.
Because of these reasons, I think it is best to not convert
`speed_factor` animation.
Pull Request: https://projects.blender.org/blender/blender/pulls/114295
The last good commit was 8474716abb.
After this commits from main were pushed to blender-v4.0-release. These are
being reverted.
Commits a4880576dc from to b26f176d1a that happend afterwards were meant for
4.0, and their contents is preserved.
Last key is drawn on the right edge of the strip, but that is end of
the frame, which it should be bound to. Because of this, drawing code
and operators must consider, that this key is always displaced.
This was not done in 86a0d0015a and caused issues like #113755 that was
fixed incorrectly.
In earlier design, retiming selection was managed outside of retiming
keys struct. This was changed to a flag of the retiming key struct.
Since moving of the transition deletes and re-creates keys, the
selection was lost. Selection is now re-established in
`seq_retiming_transition_offset()`.
After eda58d6419, multiply operation does not affect alpha channel, but
Some users do expect this feature present.
This adds option `multiply_alpha`, so that multiplication (in strip
color panel) will affect alpha channel as well.
Pull Request: https://projects.blender.org/blender/blender/pulls/113791
Conversion from timeline frame to frame index was done by casting to
integer, which followed logic of ffmpeg seeking. However this is not
best approach in some cases - for example when FPS of scene and movie
differs by a very small amount. In this case the first frame could be
duplicated and all other frames will appear as offset by one frame.
In particular this may happen scene is set to 29.97 fps and movie is
encoded at 30000/1001 fps. A frame will still have to be duplicated, but
it should be frame where decimal of frame index crosses 0.5 to keep
audio and video in sync as best as possible.
Pull Request: https://projects.blender.org/blender/blender/pulls/113870
Move the three current 'status variables' (stop, update and progress)
into a single 'WorkerStatus' struct. This is cleaner and will allow for
future workin this area without having to edit tens of 'startjob'
callbacks signatures all the time.
No functional change expected here.
Note: jobs' specific internal code has been modified as little as
possible, in many cases the job's own data still just store pointers to
these three values. Ideally in the future more refactor will be using a
single pointer to the shared `wmJobWorkerStatus` data instead.
Pull Request: https://projects.blender.org/blender/blender/pulls/113343
Aras Pranckevicius
Ray Molenkamp
Richard Antalik
Harley Acheson
ok_what
Hans Goudey
Bastien Montagne
Campbell Barton
Brecht Van Lommel
Sergey Sharybin