Maniphest T101996

Snap Selection to Current Frame does not replace keyframes on same frame
Closed, ResolvedBUG
Actions

Assigned To
Philipp Oeser (lichtwerk)
Authored By
Julien Kaspar (JulienKaspar)
Oct 22 2022, 12:59 PM
Tags
  • BF Blender
  • Animation & Rigging
Subscribers
Germano Cavalcante (mano-wii)
Julien Kaspar (JulienKaspar)
Philipp Oeser (lichtwerk)
Sybren A. Stüvel (sybren)

Description

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: rB5814e35af115
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.

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

Revisions and Commits

rB Blender
ClosedD16664 Fix T101996: merge fcurve keyframes on the same frame after snapping
D16663rBa179246e1fdf Move fcurve cleanup from transform system to BKE
rBb5ebc9bb24ae Fix T101996: merge fcurve keyframes on the same frame after snapping

Related Objects

Event Timeline

Julien Kaspar (JulienKaspar) created this task.Oct 22 2022, 12:59 PM
Germano Cavalcante (mano-wii) changed the task status from Needs Triage to Confirmed.Oct 22 2022, 10:26 PM
Germano Cavalcante (mano-wii) changed the subtype of this task from "Report" to "Bug".
Germano Cavalcante (mano-wii) added a project: Animation & Rigging.
Germano Cavalcante (mano-wii) added a subscriber: Germano Cavalcante (mano-wii).
Philipp Oeser (lichtwerk) added subscribers: Sybren A. Stüvel (sybren), Philipp Oeser (lichtwerk).Oct 24 2022, 2:12 PM

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.

@Sybren A. Stüvel (sybren) : objections?

Sybren A. Stüvel (sybren) added a comment.Nov 8 2022, 1:05 PM
In T101996#1436639, @Philipp Oeser (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 (lichtwerk) claimed this task.Nov 8 2022, 1:38 PM

OK, will put on my desk

Philipp Oeser (lichtwerk) added a revision: D16663: Move fcurve cleanup from transform system to BKE.Dec 1 2022, 2:01 PM
Philipp Oeser (lichtwerk) mentioned this in D16663: Move fcurve cleanup from transform system to BKE.
Philipp Oeser (lichtwerk) added a revision: D16664: Fix T101996: merge fcurve keyframes on the same frame after snapping.Dec 1 2022, 2:10 PM
Philipp Oeser (lichtwerk) closed this task as Resolved by committing rBb5ebc9bb24ae: Fix T101996: merge fcurve keyframes on the same frame after snapping.Dec 1 2022, 3:44 PM
Philipp Oeser (lichtwerk) added a commit: rBb5ebc9bb24ae: Fix T101996: merge fcurve keyframes on the same frame after snapping.
Philipp Oeser (lichtwerk) added a commit: rBa179246e1fdf: Move fcurve cleanup from transform system to BKE.