Sequencer: Don't create undo step when click-select does nothing #104453
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#104453
Loading…
Reference in New Issue
No description provided.
Delete Branch "Yup_Lucas/blender:T94080/sequencer-empty-undo"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
I had originally sent this patch in https://developer.blender.org/D17212.
Ref #94080
Summary
When the sequencer is empty (i.e., there are no sequences), we would have
the
deselect_all
variable set totrue
calledED_sequencer_deselect_all
to select any existing sequences.The code just assumed that because the
deselect_all
flag was true and wecalled
ED_sequencer_deselect_all
that something changed. That's not truewhen the sequencer is empty.
Changes made
I've changed
ED_sequencer_deselect_all
to return a boolean valueindicating whether or not it has mark any sequences for deselection.
The
changed
variable is set to the value returned byED_sequencer_deselect_all
.When there are no sequences present,
changed
is false.Notes
I noticed that if I click and drag (Box selection) around the editor even
when there are no sequences an empty Box selection undo item gets created.
We can either update the parent task to get that fixed or create a new
task.
I'm gonna deal with that after this diff.
Test plan:
WIP [Sequencer] Don't create empty undo items when clickedto WIP: [Sequencer] Don't create empty undo items when clickedWIP: [Sequencer] Don't create empty undo items when clickedto [Sequencer] Don't create empty undo items when clicked@iss I still need to help understanding your comment in https://developer.blender.org/D17212 :)
[Sequencer] Don't create empty undo items when clickedto Sequencer: Don't create empty undo items when clicked@ -1 +1 @@
Subproject commit b3f0ffc587d197b37eac9a1566d1d24b7bee7d9a
Subproject commit 48320b2d9f3d829c46ae4a6a6aab0bd94e80c830
This needs change should not be included.
any idea how to do that?
tried
git submodule foreach --recursive git submodule update --init
but that doesn't seem to do anythingNot super familiar with new workflow,
git submodule foreach --recursive git checkout origin/main
worked with synthetic test. I think it would be good to runmake update
before. But here you change commit to newer one, which I am not sure how it can happen. Also not sure if when submitting PR it shows changes in submodules hash to your branch or to origin/main.For future, it's best to avoid including submodules in commit by specifying changed files manually.
To avoid this use
git commit -a
instead ofgit commit .
.ack, thanks @iss and @ideasman42
Yeah I tried @iss 's command and it didn't make any change for me here. Same for make update.
I don't think it would make a different for trunk/master since the hashes in the
.gitmodules
are unchanged (all of the point to master)@ -2496,3 +2496,3 @@
{
Editing *ed = SEQ_editing_get(scene);
bool any_deselected = false;
Many operators track changes, the convention is to name this
changed
.@ -2502,3 +2503,4 @@
LISTBASE_FOREACH (Sequence *, seq, SEQ_active_seqbase_get(ed)) {
seq->flag &= ~SEQ_ALLSEL;
any_deselected = true;
Since this is intended to track change, this should only be set if the flags changed.
done
@ -976,3 +976,2 @@
if (deselect_all || (seq && (extend == false && deselect == false && toggle == false))) {
ED_sequencer_deselect_all(scene);
changed = true;
// 'changed' will be true if any sequences were deselected. Otherwise, it'll be false.
Use C-style comments for comment blocks.
Also,
'changed' will be true if any sequences were deselected. Otherwise, it'll be false.
isn't necessary, this is already part of the functions doc-string and quite clear as well.done
I think whole comment is redundant, variable
changed
is in general used to decide whether operator has been finished or cancelled and therefore generates undo step.removed
picky prefer
changed |= ED_sequencer_deselect_all(scene);
ensures in the future, this assignment never clears the changed state.done
Thanks for re-posting this to new site, what I meant is suggested by @ideasman42 in inline #104453 (comment)
Merged origin/main into this branch to make sure it was up to date.
Sequencer: Don't create empty undo items when clickedto Sequencer: Don't create undo step when click-select does nothingThanks, committed
5d30c3994e
, closing.Pull request closed