Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase visibility of active layer keyframes #1324



Copy link

@MrStevns MrStevns commented Apr 2, 2020


  • current keyframe is highlighted with white
  • All layer keyframes are more visible


closes #1322

+ current keyframe is highlighted with white
+ All layer keyframes are more visible
@MrStevns MrStevns changed the title #1322: Increase visibility of active layer keyframes Increase visibility of active layer keyframes Apr 2, 2020
@davidlamhauge davidlamhauge self-requested a review Apr 3, 2020
Copy link

@davidlamhauge davidlamhauge left a comment

I can't see any downsides in this PR. The tracks in general and the selected frame(-s) in particular, are more visible than ever, and still appealing.

@MrStevns MrStevns added the 🔹 Minor PR (only one reviewer required) label Apr 5, 2020
@MrStevns MrStevns merged commit 3828433 into pencil2d:master Apr 5, 2020
@Jose-Moreno Jose-Moreno added this to the 0.6.5 milestone Apr 6, 2020
Copy link

scribblemaniac commented Apr 9, 2020

Something that was overlooked in this PR is overlapping sound frames. They are now nearly impossible to distinguish.



In my opinion, the second sound frame here is too difficult to see.

Copy link

Jose-Moreno commented Apr 10, 2020

@scribblemaniac Ouch. I haven't yet tested these as I was waiting for the other sound PR's (but then I got busy sorry). Thanks for looking into it! I agree it's not easy to see now, although I do wonder if the playback header lands on a sound keyframe, does the whole container lit up? or only the frame, or it doesn't work for sound frames? 🤔 (Well I guess i'll know soon enough)

In this specific case I think the overlapped keyframe container should become darker, right now it seems to be slightly lighter actually 🤔

Copy link
Member Author

MrStevns commented Apr 10, 2020

MMmm... I've personally never been a fan of the ability to overlap sound keyframes on the same track, when that's said, there is indeed an issue now. I'll make a PR to fix it but we should consider whether this is something we want to continue. Overlapping keyframes is the one thing that's inconsistent about the keyframe logic on the timeline, why should we allow this special behavior just for sound?

If you need overlapping soundtracks, then you should create more tracks, just like in audacity or most other audio editing applications.

Copy link

davidlamhauge commented Apr 10, 2020

I agree with @candyface . Overlapping sounds on the same track is not supported in any other applications I use, kdenlive, shotcut, audacity, to mention some.
I also think that it will solve some of the audio problems we struggle with. My sound scrub feature would be perfect, if sounds were not allowed to overlap.
I also recognize that there would be problems if we changed it.
First there is backward compatibility, because there must be many files where sounds overlap.
Secondly you could argue that you'll need many sound layers, if we didn't allow overlapping sounds. My argument here is that you should never have any sounds but lip sync sounds, in your animation file. All other sounds (music, effects, background noise etc) should be added in the video editor.
EDIT: Overlapping keyframes on sound layers, is not the only inconsistency in keyframe logic on the timeline.
I sincerely think the camera layer should have keyframes for every key between a camera movements start-and end-position. These keyframes should be readonly, and should hold information on position, transform etc. I have gotten used to the way things are now, but I miss a more visible indication of the camera movements.

Copy link

Jose-Moreno commented Apr 10, 2020

@candyface @davidlamhauge Not disagreeing with not wanting the overlapping sounds, but additionally to using more sound layers, this should be also corrected by having the ability to clip the sound containers frame length, which is a very old part of the roadmap and part of the original Pencil / Pencil2D vision.

Just as in video editors, Ideally we want to keep the ability to have multiple sounds on a single track, however those should not be allowed to be overlapped in a single track, just clipped manually, as it makes no sense otherwise (even though it may feel intuitive for a beginner to stack up sounds like lego blocks 😋 ).

Well that's wishful thinking for the future.

On the comment of not having any sounds other than lipsync, I respectfully disagree due to the following: As mentioned briefly, part of the vision for the software is to have limited video editing capabilities, but animating to pre-recorded foley or soundtracks is not strange to animators as you all know.

The old masters would surely need to have prior musical knowledge and they'd pre-time the animation while using the musical score to map the beats on the x-sheet, but kids or even entry-level animators might not have those talents or assumed knowledge in advance, so having the soundtrack in order to animate to the musical beats is a helpful tool for animating music videos and rhythmic scenes (like a dance showdown),and I feel this is valid example of why all manner of sounds should be allowed, not just lip-sync.

It's one thing to finalize sound timing in the video editor as a director or author, and another to use pre-existing soundtracks (and of course dialogue) as guidelines to animate the action, as such I believe we shouldn't close those possibilities to our users.

As for the camera, I agree there needs to be a better indication of the camera motion, but this and having keyframes for every frame is a different conversation, as well as unrelated to the sound layer visibility.

Let's make a new issue to discuss this (i'll add it later today if that's ok)

Copy link

davidlamhauge commented Apr 10, 2020

I have already opened a new issue for the discussion @Jose-Moreno .
As for which sounds should be added to the Sound layer, it is entirely up to the user, and when I talk about lip sync sounds, I think of any sound that you need to know the position for, to do your animation. Sounds like coughs, clapping, hammering, rain and so forth will most likely be ok to add in editing, but if they are a part of something that you need to animate in sync, then you should of course add it to the sound layer.
I use lipsync, for things that you have to have in sync while animating. Is there a better word?

Copy link

Jose-Moreno commented Apr 11, 2020

@davidlamhauge I see I use lipsync in relation to dialogue only mainly because the word is self explanatory (lip - synchronization) Didn't know you used it in a broader context.

However terminology aside, summarizing the main point of my reply, scrubbing should work for ALL the existing sound tracks, like a video editor or sound editor does. This is the simplest (for the user) and most effective way to go about it because people also expect this.

If you want to probe a single sound you should be prompted to use the "isolate" / "solo" visibility mode and only hear that particular sound, and in CandyFace's timeline mockup there is a mute icon included so you'll be able to selectively "mute" any sound layer as well (right now we should be able to do this by turning the visibility off)

I mean we could consider ways to categorize sounds per "clip", like placing sound tags to differentiate dialogue from foley and what not, but why make this more difficult for us too?

What I was thinking is that the different sound layer data streams need to be combined and read, but actually doesn't the playback already does this? why can't we "scrub" on top of that resulting "preview" sound when moving the playback header manually? For the latter maybe @scribblemaniac and @candyface may have a better idea regarding this assumption of mine 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
🔹 Minor PR (only one reviewer required)
None yet

Successfully merging this pull request may close these issues.

Increase visibility of active layer keyframes
4 participants