Drag in empty space to box select does not work in animation editors #64565

New Issue

Josef Raschen · 2019-05-13T20:58:48+02:00

Josef Raschen commented

2019-05-13 20:58:48 +02:00

System Information
Operating system: Ubuntu 18.10
Graphics card: Radeon RX 560 Series (POLARIS11, DRM 3.31.0, 4.18.0-18-generic, LLVM 7.0.0)example_vse_cache_invalidated.webm

Blender Version
Broken: 2.80, ebc44aae98, master, 2018-05-11

Short description of error
Cached frames become invalidated as soon as another video strip is selected.

Exact steps for others to reproduce the error
Add two video strips, one above the other. Select one of them and play, to get some frames cached. Then select the other strip and parts of the cache becomes invalidated.

**System Information** Operating system: Ubuntu 18.10 Graphics card: Radeon RX 560 Series (POLARIS11, DRM 3.31.0, 4.18.0-18-generic, LLVM 7.0.0)[example_vse_cache_invalidated.webm](https://archive.blender.org/developer/F7034212/example_vse_cache_invalidated.webm) **Blender Version** Broken: 2.80, ebc44aae9897, master, 2018-05-11 **Short description of error** Cached frames become invalidated as soon as another video strip is selected. **Exact steps for others to reproduce the error** Add two video strips, one above the other. Select one of them and play, to get some frames cached. Then select the other strip and parts of the cache becomes invalidated.

Josef Raschen commented

2019-05-13 20:58:48 +02:00

Added subscriber: @JosefR

blender-admin commented

2019-05-13 20:58:48 +02:00

#65063 was marked as duplicate of this issue

blender-admin commented

2019-05-13 20:58:48 +02:00

#64961 was marked as duplicate of this issue

Richard Antalik was assigned by Brecht Van Lommel

2019-05-13 21:40:16 +02:00

Richard Antalik commented

2019-05-14 10:45:53 +02:00

Added subscriber: @WilliamReynish

Richard Antalik commented

2019-05-14 10:45:53 +02:00

I was about to create this task.

@WilliamReynish is it OK to change sequence move operator to LMB + tweak?

Minor issue also is, when you box select strips and try to move them by mouse - only the one you move will be moved, because others will be deselected

I was about to create this task. @WilliamReynish is it OK to change sequence move operator to LMB + tweak? Minor issue also is, when you box select strips and try to move them by mouse - only the one you move will be moved, because others will be deselected

William Reynish commented

2019-05-15 15:43:30 +02:00

@iss

is it OK to change sequence move operator to LMB + tweak?

Afaik that won't work, as tweak is used to box select. Ideally this could work so that if you tweak ON a clip, it moves, but if you tweak outside, it box selects. But the keymap cannot currently make this distinction in that way.

when you box select strips and try to move them by mouse - only the one you move will be moved

Yes, this is a known issue. It was just solved for the Node editor. This should become generic so we can use it in places like this too.

As for the cache, I guess it really should not be discarded if the clip was never actually moved?

@iss > is it OK to change sequence move operator to LMB + tweak? Afaik that won't work, as tweak is used to box select. Ideally this could work so that if you tweak ON a clip, it moves, but if you tweak outside, it box selects. But the keymap cannot currently make this distinction in that way. > when you box select strips and try to move them by mouse - only the one you move will be moved Yes, this is a known issue. It was just solved for the Node editor. This should become generic so we can use it in places like this too. As for the cache, I guess it really should not be discarded if the clip was never actually moved?

Richard Antalik commented

2019-05-15 16:46:02 +02:00

As for the cache, I guess it really should not be discarded if the clip was never actually moved?

Technically this would solve this, but often I actually move it, then it returns back to position, because it can not move.

Ok so I will do ideally both checks and run invalidation accordingly.

> As for the cache, I guess it really should not be discarded if the clip was never actually moved? Technically this would solve this, but often I actually move it, then it returns back to position, because it can not move. Ok so I will do ideally both checks and run invalidation accordingly.

Richard Antalik commented

2019-05-16 02:11:42 +02:00

@WilliamReynish
Haven't got too far with this today, but I have noticed, that when you have effects applied on top of a strip, move operator will select whole chain.
This makes it hard to actually select a strip in such case.

So again I would rather make distinction at what I am pointing to when doing LMB+tweak and decide between box select / move.
Not quite sure how exactly I would do this though.

@WilliamReynish Haven't got too far with this today, but I have noticed, that when you have effects applied on top of a strip, move operator will select whole chain. This makes it hard to actually select a strip in such case. So again I would rather make distinction at what I am pointing to when doing LMB+tweak and decide between box select / move. Not quite sure how exactly I would do this though.

William Reynish commented

2019-05-16 09:46:53 +02:00

You should be able to check the Node editor which I think does it.

Richard Antalik commented

2019-05-16 12:51:54 +02:00

@WilliamReynish

Ok found it #63994 (Node Editor: Move All Selected Nodes when dragging)

To sum up / confirm behavior:

on click:

item not selected: deselect all, select item
item already selected: do nothing

on click release:

item not selected: do nothing
item already selected: select item

on tweak:

select OP passthrough, move selected items.

is that correct?

@WilliamReynish Ok found it #63994 (Node Editor: Move All Selected Nodes when dragging) To sum up / confirm behavior: on click: - item not selected: deselect all, select item - item already selected: do nothing on click release: - item not selected: do nothing - item already selected: select item on tweak: - select OP passthrough, move selected items. is that correct?

William Reynish commented

2019-05-16 13:26:33 +02:00

Added subscriber: @mont29

William Reynish commented

2019-05-16 13:26:33 +02:00

I think that's correct - @mont29 coded this so should be able to confirm.

Bastien Montagne commented

2019-05-16 14:07:55 +02:00

yes, that’s basically it. But I would advise you wait a bit here, going modal raised some annoying issue (#64660 and #64693 are actually same fundamental problem), that needs to be addressed before generalizing that approach.

Brecht Van Lommel commented

2019-05-22 00:05:11 +02:00

Added subscriber: @snuq

Brecht Van Lommel commented

2019-05-24 10:26:45 +02:00

Added subscriber: @WayneDixon

Brecht Van Lommel changed title from ~~Selecting sequencer strips invlidates cache~~ to Drag in empty space to box select does not work in animation editors

2019-05-27 12:38:11 +02:00

Richard Antalik was unassigned by Brecht Van Lommel

2019-05-27 12:38:11 +02:00

Brecht Van Lommel commented

2019-05-27 12:38:11 +02:00

Added subscriber: @iss

Brecht Van Lommel commented

2019-05-27 12:45:16 +02:00

Added subscriber: @brecht

Brecht Van Lommel commented

2019-05-27 12:45:16 +02:00

I've reverted the commit that caused this in 244c39f1ab. It was unusable for video editing and animation, we should first fix it and then enable the behavior again.

I've reverted the commit that caused this in 244c39f1ab. It was unusable for video editing and animation, we should first fix it and then enable the behavior again.

Brecht Van Lommel commented

2019-05-27 12:46:19 +02:00

As far as I know, this bug is not related to "Move All Selected Nodes when dragging" specifically. Even in Blender 2.7 could you box select from empty space in the node editor, without it causing issues like this.

Brecht Van Lommel commented

2019-05-27 12:51:27 +02:00

The way this is handled in the node editor is like this:

static int node_box_select_invoke(bContext *C, wmOperator *op, const wmEvent *event)
{
  const bool tweak = RNA_boolean_get(op->ptr, "tweak");

  if (tweak && is_event_over_node_or_socket(C, event)) {
    return OPERATOR_CANCELLED | OPERATOR_PASS_THROUGH;
  }

  return WM_gesture_box_invoke(C, op, event);

I think the same fix could work in the sequencer and animation editors.

The way this is handled in the node editor is like this: ``` static int node_box_select_invoke(bContext *C, wmOperator *op, const wmEvent *event) { const bool tweak = RNA_boolean_get(op->ptr, "tweak"); if (tweak && is_event_over_node_or_socket(C, event)) { return OPERATOR_CANCELLED | OPERATOR_PASS_THROUGH; } return WM_gesture_box_invoke(C, op, event); ``` I think the same fix could work in the sequencer and animation editors.

Jacques Lucke commented

2019-05-28 17:51:08 +02:00

Added subscriber: @JacquesLucke

Jacques Lucke commented

2019-05-28 17:51:08 +02:00

Implemented it for the sequencer using the approach Brecht mentioned: 8e125f278c

Implemented it for the sequencer using the approach Brecht mentioned: 8e125f278c

Brecht Van Lommel commented

2019-05-28 18:38:26 +02:00

@JacquesLucke, regarding that commit:

It should perhaps check for markers as well?
We should bring back all the sequencer editor keymap changes reverted in 244c39f1ab, assuming they are compatible with this now.

@JacquesLucke, regarding that commit: * It should perhaps check for markers as well? * We should bring back all the sequencer editor keymap changes reverted in 244c39f1ab, assuming they are compatible with this now.

William Reynish commented

2019-05-28 21:10:15 +02:00

@JacquesLucke @brecht Now box select indeed works again, but should we also allow dragging multiple strips, just like we do for nodes? It seems like if we use the Node Editor method we could allow that too.

Brecht Van Lommel commented

2019-05-28 22:13:48 +02:00

No, we should focus on stabilizing now, further improvements are for 2.81.

Richard Antalik commented

2019-05-31 03:01:27 +02:00

Thx guys for help here!

In #64565#689814, @brecht wrote:
@JacquesLucke, regarding that commit:

It should perhaps check for markers as well?

Do you want to do that or should this be closed?

If I understand this correctly, marker.select operator would have to be defined in sequencer region. But then ED_markers_find_nearest_marker will always return marker if markers are used, which would prevent box select from working, so that would have to be modified

Thx guys for help here! > In #64565#689814, @brecht wrote: > @JacquesLucke, regarding that commit: > * It should perhaps check for markers as well? Do you want to do that or should this be closed? If I understand this correctly, `marker.select` operator would have to be defined in sequencer region. But then `ED_markers_find_nearest_marker` will always return marker if markers are used, which would prevent box select from working, so that would have to be modified

Brecht Van Lommel commented

2019-05-31 23:53:02 +02:00

We still have to bring box selection for graph/dopesheet/NLA editors as well, they have the same issue.

For markers, what I meant is that sequencer_box_select_invoke should also check if the mouse is over any markers before allowing box select to start, to be sure it's really over empty space.

We still have to bring box selection for graph/dopesheet/NLA editors as well, they have the same issue. For markers, what I meant is that `sequencer_box_select_invoke` should also check if the mouse is over any markers before allowing box select to start, to be sure it's really over empty space.

Abid Maqbool commented

2019-06-01 06:49:50 +02:00

Added subscriber: @AbidMaqbool

Richard Antalik commented

2019-06-01 23:36:50 +02:00

In #64565#692683, @brecht wrote:
We still have to bring box selection for graph/dopesheet/NLA editors as well, they have the same issue.

For markers, what I meant is that sequencer_box_select_invoke should also check if the mouse is over any markers before allowing box select to start, to be sure it's really over empty space.

AFAIK markers are selected in their own area by own operator, so sequencer_box_select_invoke shouldn't have to do such check.

> In #64565#692683, @brecht wrote: > We still have to bring box selection for graph/dopesheet/NLA editors as well, they have the same issue. > > For markers, what I meant is that `sequencer_box_select_invoke` should also check if the mouse is over any markers before allowing box select to start, to be sure it's really over empty space. AFAIK markers are selected in their own area by own operator, so `sequencer_box_select_invoke` shouldn't have to do such check.

William Reynish commented

2019-06-05 15:56:58 +02:00

Changed status from 'Open' to: 'Resolved'

William Reynish closed this issue

2019-06-05 15:56:58 +02:00

William Reynish self-assigned this 2019-06-05 15:56:58 +02:00

Jacques Lucke commented

2019-06-05 15:57:43 +02:00

b79976a9bd, 8b1ca3ec53, 04bac38731, ab5219af34, 8e125f278c

b79976a9bd, 8b1ca3ec53, 04bac38731, ab5219af34, 8e125f278c

Sign in to join this conversation.