Animation: time offset slider #106520
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Code Documentation
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#106520
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "AresDeveaux/blender:time_offset_slider"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
New operator for the slider tool in the Graph Editor.
It shifts the value in time of selected keys. It achieves the same as moving the selected keys in time with the added advantage of keeping the keys in the same frame.
It uses the structure already created by @ChrisLend. The main functionality chancge is in 'keyframes_general.c', the rest for the most part just follows what hi did.
WIP: Animation&Rigging: time_offset_sliderto Animation: time offset sliderneeds a memory allocation. Currently it doesn't compile for me.
Also the slider currently allows an overshoot in the positive axis, but doesn't allow to go below 0.
When overshooting on positive x it doesn't loop infinitely as you'd expect
@ -494,0 +504,4 @@
float delta_y;
/* If we operate directly on the fcurve there will be a feedback loop
* so we need to capture the "y" values on an array to then apply them on a second loop*/
forgot the . at the end of the comment
@ -494,0 +505,4 @@
/* If we operate directly on the fcurve there will be a feedback loop
* so we need to capture the "y" values on an array to then apply them on a second loop*/
float y_values[segment->length];
this doesn't compile for me. C doesn't allow variable length arrays.
you'd need to do
MEM_callocN(sizeof(float) * segment->length, "Time Offset Samples");
and at the end of the function
MEM_freeN(y_values);
so to not have a memory leakWhy can I compile it? do I need to set something I haven't for
make
?When you say C doesn't allow variable length array, I'm not sure I follow. I thought by declaring an array with a fixed value, C could handle it. Isn't
segment->length
an unchanged value for that segment?Didn't know we could overshoot. Where is that set in the code, and how can I test it on the UI?
In regard to the memory leak, should I be adding those types of lines to the other sliders too?
variable length arrays are a C99 feature, but I think Blender runs on a different standard. But honestly I can't find in the wiki where that is.
A lot of the slider features are controller by the slider itself. You can see the shortcuts in the Status Bar
ED_slider_allow_overshoot_set
allows you to change the behaviourInteresting! I didn't see those before.
I tried overshoot in
time offset slider
and as you say, to the right it works as expected, but to the left stops at 0. I then tried theease slider
, theneighbor slider
and thedefault slider
, and all of them stop at 0. I could try to find the problem, but it seems the issue is not in my function but in the slider itself and that might be beyond my understanding at this point.I added the lines you suggested to avoid memory leaks, and when compiling I get this message in regard to one of the lines:
When I add
MEM_freeN(y_values);
at the end of the function, Blender crashes any time I try to use the slider.sorry might have been unclear
the allocation needs to be like this
float *y_values = MEM_callocN(sizeof(float) * segment->length, "Time Offset Samples");
I added the lines to avoid memory leaks.
Also, as stated previously I tried overshoot in
time offset slider
and stops at 0 as you said, but also all the other sliders (ease
,blend to neighbor
,blend to default
). It seems to be an issue with the overshot tool of the slider or a setting I don't know about.Good news!... The 0 limit when using overshoot in negative values is gone now.
Since the bidirectional slider worked so well addressing Sybren's note, I decided to use it on every slider because most of them are bidirectional (with the exception of
push pull
andblend to frame
now that which only uses the current frame). In doing that I realized the 0 limit of theovershoot
goes away when the slider is bidirectional. So maybe we need to changeease slider
,blend to neighbor
andblend to default
.It actually makes sense that when a slider is not bidirectional would not overshoot less than 0. For example, on
push pull
on 0 the curve becomes linear and you don't want it to go beyond that, but on the other direction, you do want to overshoot.one more improvement, in order for the overshoot to wrap endlessly we can modify the code to this
the if statement in there annoys me but I couldn't think of a way to get rid of it.
@ -494,0 +503,4 @@
/* If we operate directly on the fcurve there will be a feedback loop
* so we need to capture the "y" values on an array to then apply them on a second loop. */
float *y_values = MEM_callocN(sizeof(float) * segment->length, "Time Offset Samples");
y_values[segment->length];
you don't need this line
While trying to understand what you are doing in those lines, I went back to one comment you had a while back and could not understand what you meant.
Everything was working just fine on my end, and then I realized it was working because I have a cycle modifier (just to better see what the curve is suppose to do before and after) apparently the modifier was helping the operator. I now deleted the modifier and I see what you mean.
no more complaints :)
A few minor remarks. Most importantly, the PR description and the
time_offset_fcurve_segment
function name only mention 'offset', but the code seems to wrap as well. Please make sure the PR description covers the functionality entirely, as it'll also be used as commit message, and to fill out the release notes, user manual, etc.@ -491,6 +491,50 @@ void ease_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const float factor
/* ---------------- */
void time_offset_fcurve_segment(FCurve *fcu, FCurveSegment *segment, const float factor)
An offset usually is just, mathematically speaking, an addition of a constant value. So shifting all the keys into a certain direction. A comment below mentions "the wrapping point", which suggests that more is going on than just applying an offset.
This should be reflected in the name,. If that's not possible, at least it should be documented.
@ -494,0 +497,4 @@
const BezTriple *last_key = &fcu->bezt[fcu->totvert - 1];
const BezTriple *first_key = &fcu->bezt[0];
const int fcu_x_range = last_key->vec[1][0] - first_key->vec[1][0];
The X-coordinate of keys is also a
float
, as keys can be placed on sub-frames as well (to control things like motion blur).@ -494,0 +516,4 @@
if (time - first_key_x < 0) {
/* Offset negative values since the modulo at fmod(-0.1, 1) would produce -0.1 instead of
* 0.9, as we'd need it to. */
wrapped_time += fcu_x_range;
(please read on to the end of the comment, I'm asking less of you than apparent at first sight)
This code (adjusting the behaviour of
fmod
to match what you need) shouldn't be here. The fact that you need to write it means that you need another modulo function that behaves correctly.Another way of making the same point: 'computing the modulo in an always-positive manner' is not the same job as as 'offsetting an fcurve segment', and thus it belongs in a different function.
I think it would be best to add a function declaration to
source/blender/blenlib/BLI_math_base.h
, underneathmod_i()
:The implementation can then be added to
source/blender/blenlib/intern/math_base_inline.c
, again underneathmod_i()
. Don't forget to make the parametersconst
in the function implementation (the surrounding code is old, and doesn't follow this new-ish style rule yet). Something like this likely works:Note that I've also used
fmodf()
here instead offmod()
. The latter works fine, but converts thefloat
paramters todouble
, computes thedouble
result, which is then put back into afloat
again.fmodf()
just operates onfloat
directly.TL;DR: And yeah, then I figured that adding such a function would also require writing a unit test for it, and that's getting too much for this PR. So I just went ahead and implemented that. It's now in the
main
branch and you can replace the call tofmod()
withmod_f_positive()
.@ -494,0 +517,4 @@
/* Offset negative values since the modulo at fmod(-0.1, 1) would produce -0.1 instead of
* 0.9, as we'd need it to. */
wrapped_time += fcu_x_range;
/* Similarly, (int)-0.1 produces 0 while we need it to be -1. */
Use
floor(x)
if you want to always round down (instead of to zero). That way you don't have to correct the result of the computation.this has been landed here: #110540: Anim: Time Offset Slider
Pull request closed