Animation: Clamp V2D so keyframes cannot go offscreen #104516

Christoph Lendenfeld · 2023-02-09T14:48:09+01:00

2023-02-09 14:48:09 +01:00

In the Dope Sheet and the Timeline, it was possible to drag the view until the keyframes were completely out of view. (Important to drag in the region with the keyframes, dragging in the channel box already did clamping)

This patch adds a clamping mechanism matching that of the channel box. That means the last channel will stick to the bottom of the view.

before

after

archived patch comments: https://archive.blender.org/developer/D17182

Steps to reproduce:

Download the demo file (anim_test.blend)
In the dope sheet, drag up with the middle mouse button until all channels are out of view.

In the Dope Sheet and the Timeline, it was possible to drag the view until the keyframes were completely out of view. (Important to drag in the region with the keyframes, dragging in the channel box already did clamping) This patch adds a clamping mechanism matching that of the channel box. That means the last channel will stick to the bottom of the view. ----- before ![before](https://projects.blender.org/attachments/3bb25d0e-fef2-4886-b7a2-bbd57cc475c4) after ![channels clamped](/attachments/f6a2904f-bd59-40b4-9d8b-0a7fcd1d3e5d) archived patch comments: https://archive.blender.org/developer/D17182 **Steps to reproduce:** Download the demo file (anim_test.blend) In the dope sheet, drag up with the middle mouse button until all channels are out of view.

image.png

11 KiB

anim_test.blend

929 KiB

image.png

74 KiB

Christoph Lendenfeld added 7 commits 2023-02-09 14:48:11 +01:00

81c055b6fb implement clamping

b279059a82 Merge branch 'master' into vertical_scrolling_offscreen

64ecbf26cd Merge branch 'master' into vertical_scrolling_offscreen

de9be5132a remove unneeded import

5ffbf701f6 Merge branch 'master' into vertical_scrolling_offscreen

cd7cca6894 move function out

6301a6e284 Merge branch 'main' into vertical_scrolling_offscreen

Christoph Lendenfeld added the

Module

Animation & Rigging

label 2023-02-09 14:48:41 +01:00

Christoph Lendenfeld added 3 commits 2023-02-09 16:06:12 +01:00

4a3c442267 remove `bAnimContext` from action_clamp_scroll

3767c6e27e undo changes to the channel clamping logic

234886ce33 Merge branch 'main' into vertical_scrolling_offscreen

Christoph Lendenfeld requested review from Sybren A. Stüvel 2023-02-09 16:06:32 +01:00

Christoph Lendenfeld commented

2023-02-09 16:10:10 +01:00

@dr.sybren regarding your comments on phabricator

There is still a way to get "scrolled out of view" animation channels. In the default scene:

    Animate the default cube, expand the channels, scroll them all the way to the top so that only the last one is showing.
    Select the camera.

I tried adding the clamping logic in action_main_region_listener as well. However it doesn't do anything. My suspicion is that v2d->tot is only calculated on draw.
There are also other issues tangential to this patch. Anytime you open the N-Panel or resize the window, the channels jump around. Not sure if that should be addressed here though

@dr.sybren regarding your comments on phabricator ``` There is still a way to get "scrolled out of view" animation channels. In the default scene: Animate the default cube, expand the channels, scroll them all the way to the top so that only the last one is showing. Select the camera. ``` I tried adding the clamping logic in `action_main_region_listener` as well. However it doesn't do anything. My suspicion is that `v2d->tot` is only calculated on draw. There are also other issues tangential to this patch. Anytime you open the N-Panel or resize the window, the channels jump around. Not sure if that should be addressed here though

Christoph Lendenfeld referenced this pull request

2023-02-09 16:11:35 +01:00

Animation Editors Paper Cuts #103855

Brecht Van Lommel added this to the Animation & Rigging project 2023-02-13 09:13:57 +01:00

Sybren A. Stüvel approved these changes 2023-02-14 17:05:18 +01:00

Sybren A. Stüvel left a comment

I found the source of my confusion.

Middle-mouse drag the channel list: scrolling stops when the last channel is fully visible (i.e. the last channel is stuck to the bottom of the area).

Middle-mouse drag the keys: the channel list is allowed to scroll completely off-screen, which is indeed fixed in this PR.

I found the source of my confusion. Middle-mouse drag the *channel list*: scrolling stops when the last channel is fully visible (i.e. the last channel is stuck to the bottom of the area). Middle-mouse drag the *keys*: the channel list is allowed to scroll completely off-screen, which is indeed fixed in this PR.

Sybren A. Stüvel removed this from the Animation & Rigging project 2023-02-14 17:06:18 +01:00

Sybren A. Stüvel added this to the Animation & Rigging project 2023-02-14 17:06:26 +01:00

Christoph Lendenfeld commented

2023-02-16 11:54:22 +01:00

@dr.sybren that means the middle mouse drag behaviour is now inconsistent between the channel box and the keyframe view (I mean it was already inconsistent before the patch but anyway)
I think it would be better to make it consistent with the channel box.
Even if it means locking the scrolling in a way that makes the last element stick to the bottom of the view

@dr.sybren that means the middle mouse drag behaviour is now inconsistent between the channel box and the keyframe view (I mean it was already inconsistent before the patch but anyway) I think it would be better to make it consistent with the channel box. Even if it means locking the scrolling in a way that makes the last element stick to the bottom of the view

Sybren A. Stüvel commented

2023-02-16 12:07:55 +01:00

I think it would be better to make it consistent with the channel box.

Agreed.

Even if it means locking the scrolling in a way that makes the last element stick to the bottom of the view

Hmmmmmmmmmmmmm.... I'm a bit torn between "keep PRs small" and "make Blender work conveniently". Personally I don't like editors that have a "greedy bottom" like this; same with text editors, sometimes I just want to be able to look straight up at something and not have it stuck to the bottom of the window. Same with keys: maybe an animator wants to visually align the keys with the header, or for whatever other reason scroll to the top. I don't see the downside of that, and I think we should do that for the channel list as well.

So I think we have these choices:

A: also change the channel list behaviour in this patch (so that expands to other editors/modes as well), or
B: limit this PR to making the key scrolling the same as the channel list, and then do two more PRs (one non-functional change to have all those limits use the same code, and one functional change to change that limit to allow scrolling all the way to the top).

What would you prefer?

> I think it would be better to make it consistent with the channel box. Agreed. > Even if it means locking the scrolling in a way that makes the last element stick to the bottom of the view Hmmmmmmmmmmmmm.... I'm a bit torn between "keep PRs small" and "make Blender work conveniently". Personally I don't like editors that have a "greedy bottom" like this; same with text editors, sometimes I just want to be able to look straight up at something and not have it stuck to the bottom of the window. Same with keys: maybe an animator wants to visually align the keys with the header, or for whatever other reason scroll to the top. I don't see the downside of that, and I think we should do that for the channel list as well. So I think we have these choices: - `A`: also change the channel list behaviour in this patch (so that expands to other editors/modes as well), or - `B`: limit this PR to making the key scrolling the same as the channel list, and then do two more PRs (one non-functional change to have all those limits use the same code, and one functional change to change that limit to allow scrolling all the way to the top). What would you prefer?

Christoph Lendenfeld commented

2023-02-16 12:10:29 +01:00

I think B would be better.
Fix it first, then make it better

Also that way we don't have blender in an inconsistent state

I think `B` would be better. Fix it first, then make it better Also that way we don't have blender in an inconsistent state

👍 1

Christoph Lendenfeld added 3 commits 2023-02-16 16:57:21 +01:00

21d28a2d7d Merge branch 'main' into vertical_scrolling_offscreen

fcdedcc3fe change clamping to be consistent with channel box

d95aae1f4d Merge branch 'main' into vertical_scrolling_offscreen

Christoph Lendenfeld requested review from Sybren A. Stüvel 2023-02-16 17:02:37 +01:00

Sybren A. Stüvel approved these changes 2023-02-17 12:30:37 +01:00

Sybren A. Stüvel left a comment

Tiny thing, the rest looks good to me!
No need to re-review after adjusting.

Tiny thing, the rest looks good to me! No need to re-review after adjusting.

source/blender/editors/space_action/space_action.c

					
				@ -403,0 +410,4 @@

				    v2d->cur.ymax = 0;

				  }

				  else {

				    if (v2d->cur.ymin < v2d->tot.ymin) {

Sybren A. Stüvel commented

2023-02-17 12:09:46 +01:00

These two lines can be merged as else if (...) {. That reduces the nesting of the block, and makes it clearer this is "a list of alternatives" instead of "nested conditions".

These two lines can be merged as `else if (...) {`. That reduces the nesting of the block, and makes it clearer this is "a list of alternatives" instead of "nested conditions".

Christoph Lendenfeld added 2 commits 2023-02-17 12:46:08 +01:00

d6873a460b Merge branch 'main' into vertical_scrolling_offscreen

23ffae7b1e implement sybrens comment

Christoph Lendenfeld merged commit cb95f8aea7 into main

2023-02-17 12:47:58 +01:00

Christoph Lendenfeld deleted branch vertical_scrolling_offscreen

2023-02-17 12:47:59 +01:00

Christoph Lendenfeld referenced this issue from a commit

2023-02-17 12:47:59 +01:00

Animation: Clamp V2D so keyframes cannot go offscreen

RonanDucluzeau commented

2023-02-17 16:16:07 +01:00

First-time contributor

If there are markers in Timeline, in a narrow editor, last animation channel is hidden, not accessible.

Basically, user is forced to expand editor at a scale superior to the one when all channels are displayed or to use filter, in order to see last channel + markers.

In order to reach keyframes of last animation channel, an offset of this limit, equivalent to height of Makers Region would be welcomed.

If there are markers in Timeline, in a narrow editor, last animation channel is hidden, not accessible. Basically, user is forced to expand editor at a scale superior to the one when all channels are displayed or to use filter, in order to see last channel + markers. In order to reach keyframes of last animation channel, an offset of this limit, equivalent to height of Makers Region would be welcomed.

Christoph Lendenfeld commented

2023-02-17 16:35:16 +01:00

If there are markers in Timeline, in a narrow editor, last animation channel is hidden, not accessible.

Basically, user is forced to expand editor at a scale superior to the one when all channels are displayed, in order to see last channel + markers.

In order to reach keyframes of last animation channel, an offset of this limit, equivalent to height of Makers Region would be welcomed.

aah yes thanks for the comment. I'll make sure to fix it asap

> If there are markers in Timeline, in a narrow editor, last animation channel is hidden, not accessible. > > Basically, user is forced to expand editor at a scale superior to the one when all channels are displayed, in order to see last channel + markers. > > In order to reach keyframes of last animation channel, an offset of this limit, equivalent to height of Makers Region would be welcomed. aah yes thanks for the comment. I'll make sure to fix it asap

Christoph Lendenfeld referenced this pull request

2023-02-17 18:07:12 +01:00

Fix: Channel clamping when markers are used #104892

Christoph Lendenfeld referenced this issue from a commit

2023-02-21 09:24:11 +01:00

Fix: Channel clamping when markers are used

Julian Eisel referenced this issue from a commit

2023-02-21 15:22:37 +01:00

Fix: Channel clamping when markers are used

Sybren A. Stüvel removed this from the Animation & Rigging project 2023-03-02 17:00:20 +01:00

~~Pratik Borhade referenced this pull request 2023-04-05 13:01:27 +02:00~~

Exporting Geometry Nodes model with 'Apply Modifiers' option results in missing textures #106362