Snap Selection to Current Frame does not replace keyframes on same frame #101996

New Issue

Julien Kaspar · 2022-10-22T12:59:27+02:00

Julien Kaspar commented

2022-10-22 12:59:27 +02:00

System Information
Operating system: Linux-5.15.0-52-generic-x86_64-with-glibc2.35 64 Bits
Graphics card: Quadro GP100/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 510.85.02

Blender Version
Broken: version: 3.4.0 Alpha, branch: master, commit date: 2022-10-21 06:51, hash: 5814e35af1
Worked: (newest version of Blender that worked as expected)

Short description of error
Typically when moving keyframes onto already exisiting keyframes, they will be replaced.
Unfortunately when using the snapping operators this is not the case, whcih results in multiple keyframes on the same frame.

2022-10-22 12-52-19.mp4

With the graph editor this is very visible when it happens, but I encountered this often when using the dope sheet editor, where this effect is invisible.

Exact steps for others to reproduce the error

Add a simple animation on the default cube
Show the graph editor
Select keyframes on one frame and move them onto another keyframe
Try the same by selecting the keyframes and using the Snap option from the title

**System Information** Operating system: Linux-5.15.0-52-generic-x86_64-with-glibc2.35 64 Bits Graphics card: Quadro GP100/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 510.85.02 **Blender Version** Broken: version: 3.4.0 Alpha, branch: master, commit date: 2022-10-21 06:51, hash: `5814e35af1` Worked: (newest version of Blender that worked as expected) **Short description of error** Typically when moving keyframes onto already exisiting keyframes, they will be replaced. Unfortunately when using the snapping operators this is not the case, whcih results in multiple keyframes on the same frame. [2022-10-22 12-52-19.mp4](https://archive.blender.org/developer/F13748799/2022-10-22_12-52-19.mp4) With the graph editor this is very visible when it happens, but I encountered this often when using the dope sheet editor, where this effect is invisible. **Exact steps for others to reproduce the error** - Add a simple animation on the default cube - Show the graph editor - Select keyframes on one frame and move them onto another keyframe - Try the same by selecting the keyframes and using the Snap option from the title

Julien Kaspar commented

2022-10-22 12:59:27 +02:00

Added subscriber: @JulienKaspar

Germano Cavalcante commented

2022-10-22 22:26:10 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Germano Cavalcante commented

2022-10-22 22:26:10 +02:00

Added subscriber: @mano-wii

Philipp Oeser commented

2022-10-24 14:12:10 +02:00

Added subscribers: @dr.sybren, @lichtwerk

Philipp Oeser commented

2022-10-24 14:12:10 +02:00

posttrans_fcurve_clean is a feature of the transform system.
Could be exposed elsewhere though and used in snap_graph_keys / snap_action_keys though.

@dr.sybren : objections?

`posttrans_fcurve_clean` is a feature of the transform system. Could be exposed elsewhere though and used in `snap_graph_keys` / `snap_action_keys` though. @dr.sybren : objections?

Sybren A. Stüvel commented

2022-11-08 13:05:26 +01:00

In #101996#1436639, @lichtwerk wrote:
posttrans_fcurve_clean is a feature of the transform system.
Could be exposed elsewhere though and used in snap_graph_keys / snap_action_keys though.

I think that having a state, after snapping, that cannot be reproduced in any way is a bad idea. So yeah, I see this as a bug, and I think applying posttrans_fcurve_clean after snapping would be a good one.

Would probably be two patches:

Non-functional refactor to move the posttrans_fcurve_clean function to BKE. It could be named something like BKE_fcurve_merge_duplicate_keys so that it's named after what it does, and not after when it's called.
Functional patch to use this function in other places, like the snapping functions.

> In #101996#1436639, @lichtwerk wrote: > `posttrans_fcurve_clean` is a feature of the transform system. > Could be exposed elsewhere though and used in `snap_graph_keys` / `snap_action_keys` though. I think that having a state, after snapping, that cannot be reproduced in any way is a bad idea. So yeah, I see this as a bug, and I think applying `posttrans_fcurve_clean` after snapping would be a good one. Would probably be two patches: 1. Non-functional refactor to move the `posttrans_fcurve_clean` function to BKE. It could be named something like `BKE_fcurve_merge_duplicate_keys` so that it's named after what it does, and not after when it's called. 2. Functional patch to use this function in other places, like the snapping functions.

Philipp Oeser self-assigned this 2022-11-08 13:38:12 +01:00

Philipp Oeser commented

2022-11-08 13:38:12 +01:00

OK, will put on my desk