This is only visible for very low resolution image strips; the math was
operating on integers as image size but doing division by two to get the
outline. For non-even image sizes the outline could be off by a pixel due
to rounding.
Images in the PR.
Pull Request: https://projects.blender.org/blender/blender/pulls/116605
This also fixes crash when deleting keys.
Issue was caused by incorrect implementation of batch deleting with
`SEQ_retiming_remove_multiple_keys()` function. It tried to remove data
from different strips, when it should work with one strip at the time.
Also transitions and freeze frames were treated as normal keys, but they
do need special handling.
Pull Request: https://projects.blender.org/blender/blender/pulls/116722
Along with the 4.1 libraries upgrade, we are bumping the clang-format
version from 8-12 to 17. This affects quite a few files.
If not already the case, you may consider pointing your IDE to the
clang-format binary bundled with the Blender precompiled libraries.
When slipping meta strip, effect position was not updated. These updates
are limited to VSE core code, so function `SEQ_time_slip_strip()` was
added to resolve this issue.
This also simplifies operator code, so it does not have to deal with
recursion and doesn't need to hold quite detailed strip state for when
operator is cancelled.
Pull Request: https://projects.blender.org/blender/blender/pulls/113200
This was caused by 5c76c7bf84, which only updated sound when tagged by
`ID_RECALC_AUDIO`. On undo or rendering, this flag is missing.
Update equalizer when recalc is flagged by `ID_RECALC_COPY_ON_WRITE`.
Pull Request: https://projects.blender.org/blender/blender/pulls/116282
Split the code, use preconditions, use rather plain language for
function names and add comments where it is not totally obvious, what
the code is supposed to do.
The main problem is, that retiming key frame index is defined in strip
content space (0 <> seq->len -1), but API functions must rescale this
index by applying `SEQ_time_media_playback_rate_factor_get()` and return
value in timeline space.
This wasn't done properly, in many places, some had challenges:
- `SEQ_retiming_key_timeline_frame_get()` returned floats, but UI
expects integers. Otherwise keys may be drawn inbetween frames.
- Function `right_fake_key_frame_get()` must return exact frame of
keys, otherwise lookup by frame would fail. But `retime_key_draw()`
can not compensate position of last fake key, so this has to be done
in `fake_keys_draw()` and `try_to_realize_virtual_key()`.
- For transformation to work as expected, double precision value has
to be used for frame index.
- For UI either API would had to be extended to provide helper functions
to deal with FPS mismatch, or it needs to know the FPS difference.
I have opted to put `SEQ_time_media_playback_rate_factor_get()` in
"public" headers. Neither solution is great.
Now that the code is in C++, quite some duplication between "byte" and
"float" effect code paths can be reduced (easier than it was in C times).
So I did that, removing about 400 lines of code.
In that process I accidentally made Gaussian Blur faster, since while
reducing the amount of code I noticed it was doing some things
sub-optimally (calculated kernel tables for each job, etc.). Applying
100x100 gaussian blur on 4K UHD resolution image strip on Ryzen 5950X
went 630ms -> 450ms.
Pull Request: https://projects.blender.org/blender/blender/pulls/116089
Add "Preserve Current Retiming" option to set speed operator. This
option is enabled by default. When changing speed of retiming segment,
the strip changes length instead of changing next segment speed.
Ref: #112343
Content cut off by `anim_startofs` is as if it does not exist for
sequencer. But Audaspace seeking relies on having animation buffer
initialized for whole sequence. Initialize pitch of cut content to 1.
This is a smaller rewrite/refactor of the VSE copy paste code to use the file copy buffer logic that is used in other places of Blender.
This makes Blender able to copy paste between Blender processes.
It can also paste successfully after closing and reopening Blender.
Other than that, the functionally should remain the exact same as the current copy paste operator with one exception: Scene strips does not retain their scene pointer when pasting into the same file.
This is to make it consistent with how it was before when copy pasting between .blend files.
(The scene data for the scene strips were never copied in when doing that)
Logic for pulling in or linking scenes into new or current files are purposely left for later when a proper proposal of how this would work in a nice fashion is done.
Now the strip data gets copied either fully or partially besides for scene strips. There only the script data gets copied and not the scene data.
If there is a audio/video data block of the same name as in the paste data, it will be reused to reduce potential duplication of data.
Pull Request: https://projects.blender.org/blender/blender/pulls/114703
Wipe effect was completely single threaded. So multi-thread that.
Additionally, simplify math done in Clock wipe code; asin() + hypot()
+ some checks is just atan2() really.
Applying Wipe to 4K UHD sequencer output, on Windows Ryzen 5950X:
- Single/Double: 99.1 -> 9.3 ms (10.6x faster)
- Iris: 153.3 -> 12.3 ms (12.4x faster)
- Clock: 301.4 -> 14.5 ms (20.8x faster)
The same on Mac M1 Max:
- Single: 74.5 -> 13.4 ms (5.6x faster)
- Iris: 84.2 -> 14.3 ms (5.9x faster)
- Clock: 185.3 -> 18.8 ms (9.8x faster)
Pull Request: https://projects.blender.org/blender/blender/pulls/115837
Glow effect was doing the correct thing algorithmically (separable gaussian
blur), but it was 1) completely single-threaded, and 2) did operations in
several passes over the source images, instead of doing them in one go.
- Adds multi-threading to Glow effect.
- Combines some operations, e.g. instead of IMB_buffer_float_from_byte
followed by IMB_buffer_float_premultiply, do
IMB_colormanagement_transform_from_byte_threaded which achieves the same,
but more efficiently.
- Simplifies the code: removing separate loops around image boundaries is
both less code and slightly faster; use float4 vector type for more
compact code; use Array classes instead of manual memory allocation, etc.
- Removes IMB_buffer_float_unpremultiply and IMB_buffer_float_premultiply
since they are no longer used by anything whatsoever.
Applying Glow to 4K UHD sequencer output, on Windows Ryzen 5950X:
- Blur distance 4: 935ms -> 109ms (8.5x faster)
- Blur distance 20: 3526ms -> 336ms (10.5x faster)
Same on Mac M1 Max:
- Blur distance 4: 732ms -> 126ms (5.8x faster)
- Blur distance 20: 3047ms -> 528ms (5.7x faster)
Pull Request: https://projects.blender.org/blender/blender/pulls/115818
Gamma Cross code seems to be coming from year 2005 or earlier, with complex
table based machinery to approximate "raise to power" calculations. Which,
for Gamma Cross, have always been hardcoded to 2.0 "since forever". So
simplify all that, i.e. replace all the table lookup/interpolation things
with just `x*x` and `sqrt(x)`.
Applying Gamma Cross on 4K UHD resolution, Windows Ryzen 5950X machine:
36.2ms -> 8.1ms
Pull Request: https://projects.blender.org/blender/blender/pulls/115801
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
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