Page MenuHome

Drag in empty space to box select does not work in animation editors
Closed, ResolvedPublic

Description

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)

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.

Details

Type
Bug

Event Timeline

Richard Antalik (ISS) triaged this task as Confirmed, High priority.May 14 2019, 10:45 AM

I was about to create this task.

@William Reynish (billreynish) 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

@Richard Antalik (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?

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.

@William Reynish (billreynish)
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.

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

@William Reynish (billreynish)

Ok found it T63994: 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?

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

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

Brecht Van Lommel (brecht) renamed this task from Selecting sequencer strips invlidates cache to Drag in empty space to box select does not work in animation editors.Mon, May 27, 12:38 PM
Brecht Van Lommel (brecht) removed Richard Antalik (ISS) as the assignee of this task.

I've reverted the commit that caused this in rB244c39f1abb8: Revert "Keymap: Drag in empty area to box select in animation editors". It was unusable for video editing and animation, we should first fix it and then enable the behavior again.

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.

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.

Implemented it for the sequencer using the approach Brecht mentioned: rB8e125f278c52b38cf3be53b2296f0dc30885bae4

@Jacques Lucke (JacquesLucke), regarding that commit:

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

@Jacques Lucke (JacquesLucke) @Brecht Van Lommel (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.

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

Thx guys for help here!

@Jacques Lucke (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

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.

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