-
Notifications
You must be signed in to change notification settings - Fork 103
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
undo/redo extension #311
undo/redo extension #311
Changes from 2 commits
448aa8a
b6878ad
0442388
83ada77
658ead1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
#pragma once | ||
|
||
#include "../../plugin.h" | ||
#include "../../string-sizes.h" | ||
#include "../../stream.h" | ||
|
||
/// @page Undo/Redo | ||
/// | ||
/// For each action that causes a change of parameter values and non-parameter state | ||
/// (the 'plugin state'), the plugin provides an incremental change event object to the host. | ||
/// These incremental change event objects then can be applied in reverse order in order to | ||
/// undo those actions. If undone, the same objects can be used to redo the action again. | ||
Trinitou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// | ||
/// As opposed to the state extension, incremental change event objects must not contain the | ||
/// whole plugin state but rather only the necessary data for undoing/redoing a single action | ||
Trinitou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// on top of the current plugin state. | ||
|
||
static const char CLAP_EXT_UNDO_REDO[] = "clap.undo-redo.draft/0"; | ||
|
||
#ifdef __cplusplus | ||
extern "C" { | ||
#endif | ||
|
||
// Information about a single incremental change event. | ||
// The host can use context and action to embed a message into its own undo history. | ||
typedef struct clap_incremental_change_event_description { | ||
// A brief, human-readable description of the event context, for example: | ||
// - "Filter 1 > Cutoff" | ||
// - "Param Seq A" | ||
// It should not contain the name of the plugin. | ||
char context[CLAP_NAME_SIZE]; | ||
// A brief, human-readable description of what happened, for example: | ||
// - "Set to 100%" | ||
// - "Steps edited" | ||
char action[CLAP_NAME_SIZE]; | ||
} clap_incremental_change_event_description_t; | ||
|
||
typedef struct clap_plugin_undo_redo{ | ||
// Pulls an incremental change event object into stream. | ||
// The plugin must provide an event description together with the object. | ||
// If the host still holds a redo event object, it should compare the received | ||
// undo event object with the next redo object. If it is not equal, the host may | ||
Trinitou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// omit the next and all following redo event objects for that plugin. | ||
// before (besides it wants to support undo history branching). | ||
// Returns true if the incremental change event object was correctly saved. | ||
// [main-thread] | ||
bool(CLAP_ABI *pull_undo_event_object)(const clap_plugin_t *plugin, | ||
const clap_ostream_t *stream, | ||
clap_incremental_change_event_description_t* description); | ||
|
||
// Applies an incremental change event object from stream in order to undo the corresponding | ||
// plugin action. | ||
// The host should move the incremental change event object onto its redo stack. | ||
// Returns true if the incremental change event object was correctly applied. | ||
// The plugin must not call notify_undo as this is implied by returning true already. | ||
// [main-thread] | ||
bool(CLAP_ABI *perform_undo)(const clap_plugin_t *plugin, | ||
const clap_istream_t *stream); | ||
Trinitou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Applies an incremental change event object from stream in order to redo the corresponding | ||
// plugin action. | ||
// The host should move the incremental change event object onto its undo stack. | ||
// Returns true if the incremental change event object was correctly applied. | ||
// The plugin must not call mark_dirty_for_undo as this is implied by returning true already. | ||
// [main-thread] | ||
bool(CLAP_ABI *perform_redo)(const clap_plugin_t *plugin, | ||
const clap_istream_t *stream); | ||
} clap_plugin_undo_redo_t; | ||
|
||
typedef struct clap_host_undo_redo { | ||
// Tell the host that the plugin state has changed internally and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So basically the semantic is "when i have undo/redo gestures internally I call these two along with doing them" but then I think order matters a lot. So like if I undo, have I undone before I call these and what is the object on the stack for the next stream? If I undo redo undo before the host calls me back what happens? There's some tricky state management here which I think we need to document and consider. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I hope I could resolve some of those concerns with the 'pending objects' concept. This means, the plugin has to buffer pending objects and the host must sync all pending objects first before it can perform an undo/redo on the plugin. I think the buffering shouldn't be much of a problem for the plugin as it already has those objects for its internal undo/redo. So it just needs to store them a bit longer until the host has pulled them. |
||
// an incremental change event object is ready to be pulled from the plugin. | ||
// The host must then call pull_undo_event_object. | ||
// [main-thread] | ||
void(CLAP_ABI *mark_dirty_for_undo)(const clap_host_t *host); | ||
|
||
// Tell the host that the plugin has performed a single undo step internally. | ||
// The host then must roll back the last undo event for this plugin instance. | ||
// If multiple undo steps have been performed at once inside the plugin, | ||
// it must call this function multiple times as well. | ||
// [main-thread] | ||
void(CLAP_ABI *notify_undo)(const clap_host_t *host); | ||
} clap_host_undo_redo_t; | ||
|
||
#ifdef __cplusplus | ||
} | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few things that are missing
We show the undo/redo stack in our UI. Can we query the host undo/redo stack here?
The host will put other things on the undo redo stack. Do we want to be able to trigger them from the ui and interleave those stacks? I'm thinking especially of things like a setting a parameter from outside the plugin - what does that do as far as undo/redo? We really need to document that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My approach here would be that the plugin only knows about its internal undo/redo stacks and not the host's stacks. So if the plugin does internal undo/redo, the host might remove those steps from between own undo/redo steps. I think this would simplify things for the plugin devs and the host would be responsible for placing the plugin undo/redo objects in the appropriate place. You think that's OK?
Concerning parameter changes from outside, I would add a note to the documentation that the plugin is always responsible for creating undo steps for parameter changes, even if they come from the host and the host must not create undo steps for those parameter changes (?)