Page MenuHome

Add curve decimate in the graph editor
Needs ReviewPublic

Authored by Sebastian Parborg (zeddb) on May 10 2019, 5:56 PM.

Details

Summary

Added a decimate option in the graph editor per request by the animators at BI.

Diff Detail

Event Timeline

I didn't have time to implement it when I wrote this first draft, but I would like to add the same kind of logic as with "Relax pose".
IE when the operator is invoked, the cursor is grabbed and the location in the viewport determines the ratio of the decimation.
So the left side of the screen would be a ratio of 0 while the right would be a ratio of 1.

The 'ratio' parameter seems to influence the percentage of points to keep/remove. Is this really a useful metric? Wouldn't it be better to manipulate the allowed error instead?

source/blender/editors/animation/keyframes_general.c
336

Unused parameter ‘ac’.

341

I would always write full sentences, so "Check if there are any points." or something like that.

342

No need for parentheses around the equality checks.

source/blender/editors/space_graph/graph_edit.c
1330

Shouldn't this be done in a poll function?

1350

I don't quite understand what "the ratio keyframes" means.

1362

A ratio between 0 and FLT_MAX is weird. How can you remove more than 100% of the points?
It's also unclear what the ratio indicates. What would ratio=0.2 do? Remove 20% of the points? Or keep 20% of the points?

I've tried it with the default ratio, and it removed all the points except the first and last one, even though there were quite big jumps in the curve.

source/blender/editors/space_graph/graph_edit.c
1330

This is how it is done in the other exec functions in this file.
Should we change this?

Agree we could use a better metric here, especially if you're editing multiple curves at once a ratio is not useful.

source/blender/editors/space_graph/graph_edit.c
1330

The poll function already does it.

Strictly speaking, you could assume that this never returns 0. But still I think it's good practice to always check the return value of this function in case future refactoring breaks that assumption.

Agree we could use a better metric here, especially if you're editing multiple curves at once a ratio is not useful.

Sybren and I discussed this on thursday and came to the conclusion to ask the artists for input.
When I spoke to them, they wanted a ratio. So they could invoke the operator and move the mouse to the left (ratio=0) or the right (ratio=1).
This way they would have linear control of how much is decimated.

This is also how it works for the decimate operator for curve objects, it uses a ratio.

However, I do agree that also being able to use it with a error margin in addition to this would be nice.

If it was a ratio for all selected curves together that could make sense. If it's applied for each individual curve as it is now, I doubt that is what artists are expecting.

It's also not obvious to me that the ratio will be perceived as more linear compared to a good error metric.

The artists (Nacho) scenario only decimated one curve at the time. And it was also on baked key frames.

I don't think we will be able to have a measurement that that will work nicely on multiple curves at once.
Even if we normalize the error margin, the optimal value will not be the same for different curves.
This depends of course what the user want to get out of the decimation.

I don't know if a combined ratio would help in this sense.

For me at least, when I select multiple objects and run an operator on them, I expect the individual result to be the same as if I ran the operator on just one of the objects.

Well, I disagree about what the optimal behavior for multiple curves is here. But since this is a pain to implement over multiple curves I don't think this has to block the patch.