From 0e5c2b56057072abacc59f5403ed7f768c0ae793 Mon Sep 17 00:00:00 2001 From: Alexandre Bique Date: Wed, 18 Sep 2024 11:31:46 +0200 Subject: [PATCH 1/7] undo: split the plugin interface --- include/clap/ext/draft/undo.h | 61 ++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 8 deletions(-) diff --git a/include/clap/ext/draft/undo.h b/include/clap/ext/draft/undo.h index 7932c176..4f03ce61 100644 --- a/include/clap/ext/draft/undo.h +++ b/include/clap/ext/draft/undo.h @@ -1,8 +1,12 @@ #pragma once #include "../../plugin.h" +#include "../../stream.h" static CLAP_CONSTEXPR const char CLAP_EXT_UNDO[] = "clap.undo/3"; +static CLAP_CONSTEXPR const char CLAP_EXT_UNDO_CONTEXT[] = "clap.undo_context/3"; +static CLAP_CONSTEXPR const char CLAP_EXT_UNDO_DELTA[] = "clap.undo_delta/3"; +static CLAP_CONSTEXPR const char CLAP_EXT_UNDO_STATE[] = "clap.undo_state/3"; #ifdef __cplusplus extern "C" { @@ -40,6 +44,13 @@ extern "C" { /// history. This simplifies the host implementation, leading to less bugs, a more robust design /// and maybe an easier experience for the user because there's a single undo context versus one /// for the host and one for each plugin instance. +/// +/// The goal for this extension is to make it as easy as possible for the plugin to hook into +/// the host undo and make it efficient when possible by using deltas. +/// +/// The plugin interfaces are all optional, and the plugin can for a minimal implementation, +/// just use the host interface and call host->change_made() without providing a delta. +/// This is enough for the host to know that it can capture a plugin state for the undo step. typedef struct clap_undo_delta_properties { // If false, then all clap_undo_delta_properties's attributes become irrelevant. @@ -54,7 +65,9 @@ typedef struct clap_undo_delta_properties { uint32_t format_version; } clap_undo_delta_properties_t; -typedef struct clap_plugin_undo { +// Use CLAP_EXT_UNDO_DELTA +// This is an optional interface, using deltas is an optimization versus making state snapshot. +typedef struct clap_plugin_undo_delta { // Asks the plugin the delta properties. // [main-thread] void(CLAP_ABI *get_delta_properties)(const clap_plugin_t *plugin, @@ -83,21 +96,47 @@ typedef struct clap_plugin_undo { clap_id format_version, const void *delta, size_t delta_size); +} clap_plugin_undo_delta_t; + +// Use CLAP_EXT_UNDO_STATE +// This is an optional interface. +// When the plugin didn't provide a delta, then the host will perform a plugin state snapshot. +// This interface should only be implemented if the plugin can provide a lighter state for the undo +// history. +// A state saved using this interface must be restored using this interface. +// +// TODO: give precise assumptions that can be made by the plugin in order to make its state smaller. +// TODO: give an example +typedef struct clap_plugin_undo_state { + // Saves the plugin state into stream. + // Returns true if the state was correctly saved. + // [main-thread] + bool(CLAP_ABI *save)(const clap_plugin_t *plugin, const clap_ostream_t *stream); + + // Loads the plugin state from stream. + // Returns true if the state was correctly restored. + // [main-thread] + bool(CLAP_ABI *load)(const clap_plugin_t *plugin, const clap_istream_t *stream); +} clap_plugin_state_t; +// Use CLAP_EXT_UNDO_CONTEXT +// This is an optional interface, that the plugin can implement in order to know about +// the current undo context. +typedef struct clap_plugin_undo_context { // Indicate if it is currently possible to perform a redo or undo operation. // if can_* is false then it invalidates the corresponding name. // [main-thread] - void (CLAP_ABI *set_can_undo)(const clap_plugin_t *plugin, bool can_undo); - void (CLAP_ABI *set_can_redo)(const clap_plugin_t *plugin, bool can_redo); + void(CLAP_ABI *set_can_undo)(const clap_plugin_t *plugin, bool can_undo); + void(CLAP_ABI *set_can_redo)(const clap_plugin_t *plugin, bool can_redo); // Sets the name of the next undo or redo step. // name: null terminated string if an redo/undo step exists, null otherwise. // [main-thread] - void (CLAP_ABI *set_undo_name)(const clap_plugin_t *plugin, const char *name); - void (CLAP_ABI *set_redo_name)(const clap_plugin_t *plugin, const char *name); - -} clap_plugin_undo_t; + void(CLAP_ABI *set_undo_name)(const clap_plugin_t *plugin, const char *name); + void(CLAP_ABI *set_redo_name)(const clap_plugin_t *plugin, const char *name); +} clap_plugin_undo_context_t; +// Use CLAP_EXT_UNDO typedef struct clap_host_undo { // Begins a long running change. // The plugin must not call this twice: there must be either a call to cancel_change() or @@ -124,12 +163,16 @@ typedef struct clap_host_undo { // plugin can indicate a format version id and the validity lifetime for the binary blobs. // The host can use these to verify the compatibility before applying the delta. // If the plugin is unable to use a delta, a notification should be provided to the user and - // the crash recovery should perform a best effort job, at least restoring the latest saved state. + // the crash recovery should perform a best effort job, at least restoring the latest saved + // state. // // Special case: for objects with shared and synchronized state, changes shouldn't be reported // as the host already knows about it. // For example, plugin parameter changes shouldn't produce a call to change_made(). // + // Note: if the plugin asked for this interface, then host_state->mark_dirty() will not create an + // implicit undo step. + // // [main-thread] void(CLAP_ABI *change_made)(const clap_host_t *host, const char *name, @@ -154,6 +197,8 @@ typedef struct clap_host_undo { // // is_subscribed: set to true to receive context info // + // It is mandatory for the plugin to implement CLAP_EXT_UNDO_CONTEXT when using this method. + // // [main-thread] void(CLAP_ABI *set_wants_context_updates)(const clap_host_t *host, bool is_subscribed); } clap_host_undo_t; From 6c75329acf6f9db2b612b6e1b43d011f3cd76e03 Mon Sep 17 00:00:00 2001 From: Alexandre Bique Date: Wed, 18 Sep 2024 11:34:39 +0200 Subject: [PATCH 2/7] Fix compilation --- include/clap/ext/draft/undo.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/clap/ext/draft/undo.h b/include/clap/ext/draft/undo.h index 4f03ce61..56a3f4cd 100644 --- a/include/clap/ext/draft/undo.h +++ b/include/clap/ext/draft/undo.h @@ -117,7 +117,7 @@ typedef struct clap_plugin_undo_state { // Returns true if the state was correctly restored. // [main-thread] bool(CLAP_ABI *load)(const clap_plugin_t *plugin, const clap_istream_t *stream); -} clap_plugin_state_t; +} clap_plugin_undo_state_t; // Use CLAP_EXT_UNDO_CONTEXT // This is an optional interface, that the plugin can implement in order to know about From 0278396c7cb273ee2faadaf669e5589d4467120c Mon Sep 17 00:00:00 2001 From: Alexandre Bique Date: Wed, 18 Sep 2024 11:58:51 +0200 Subject: [PATCH 3/7] remove undo_state, it was unclear --- include/clap/ext/draft/undo.h | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/include/clap/ext/draft/undo.h b/include/clap/ext/draft/undo.h index 56a3f4cd..588d62d8 100644 --- a/include/clap/ext/draft/undo.h +++ b/include/clap/ext/draft/undo.h @@ -6,7 +6,6 @@ static CLAP_CONSTEXPR const char CLAP_EXT_UNDO[] = "clap.undo/3"; static CLAP_CONSTEXPR const char CLAP_EXT_UNDO_CONTEXT[] = "clap.undo_context/3"; static CLAP_CONSTEXPR const char CLAP_EXT_UNDO_DELTA[] = "clap.undo_delta/3"; -static CLAP_CONSTEXPR const char CLAP_EXT_UNDO_STATE[] = "clap.undo_state/3"; #ifdef __cplusplus extern "C" { @@ -98,27 +97,6 @@ typedef struct clap_plugin_undo_delta { size_t delta_size); } clap_plugin_undo_delta_t; -// Use CLAP_EXT_UNDO_STATE -// This is an optional interface. -// When the plugin didn't provide a delta, then the host will perform a plugin state snapshot. -// This interface should only be implemented if the plugin can provide a lighter state for the undo -// history. -// A state saved using this interface must be restored using this interface. -// -// TODO: give precise assumptions that can be made by the plugin in order to make its state smaller. -// TODO: give an example -typedef struct clap_plugin_undo_state { - // Saves the plugin state into stream. - // Returns true if the state was correctly saved. - // [main-thread] - bool(CLAP_ABI *save)(const clap_plugin_t *plugin, const clap_ostream_t *stream); - - // Loads the plugin state from stream. - // Returns true if the state was correctly restored. - // [main-thread] - bool(CLAP_ABI *load)(const clap_plugin_t *plugin, const clap_istream_t *stream); -} clap_plugin_undo_state_t; - // Use CLAP_EXT_UNDO_CONTEXT // This is an optional interface, that the plugin can implement in order to know about // the current undo context. From 189708518db58fa0988545ab5912254d8c0c3197 Mon Sep 17 00:00:00 2001 From: Alexandre Bique Date: Wed, 18 Sep 2024 12:00:25 +0200 Subject: [PATCH 4/7] Doc. --- include/clap/ext/draft/undo.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/clap/ext/draft/undo.h b/include/clap/ext/draft/undo.h index 588d62d8..61db88f1 100644 --- a/include/clap/ext/draft/undo.h +++ b/include/clap/ext/draft/undo.h @@ -108,7 +108,7 @@ typedef struct clap_plugin_undo_context { void(CLAP_ABI *set_can_redo)(const clap_plugin_t *plugin, bool can_redo); // Sets the name of the next undo or redo step. - // name: null terminated string if an redo/undo step exists, null otherwise. + // name: null terminated string. // [main-thread] void(CLAP_ABI *set_undo_name)(const clap_plugin_t *plugin, const char *name); void(CLAP_ABI *set_redo_name)(const clap_plugin_t *plugin, const char *name); From ca273d7b96edf670cff04be409b279e64f94a98a Mon Sep 17 00:00:00 2001 From: Alexandre Bique Date: Wed, 18 Sep 2024 12:05:14 +0200 Subject: [PATCH 5/7] Doc. --- include/clap/ext/draft/undo.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/include/clap/ext/draft/undo.h b/include/clap/ext/draft/undo.h index 61db88f1..eed32337 100644 --- a/include/clap/ext/draft/undo.h +++ b/include/clap/ext/draft/undo.h @@ -50,6 +50,9 @@ extern "C" { /// The plugin interfaces are all optional, and the plugin can for a minimal implementation, /// just use the host interface and call host->change_made() without providing a delta. /// This is enough for the host to know that it can capture a plugin state for the undo step. +/// +/// Note: if a plugin is producing a lot of changes within a small amount of time, the host +/// may merge them into a single undo step. typedef struct clap_undo_delta_properties { // If false, then all clap_undo_delta_properties's attributes become irrelevant. @@ -151,6 +154,10 @@ typedef struct clap_host_undo { // Note: if the plugin asked for this interface, then host_state->mark_dirty() will not create an // implicit undo step. // + // Note: if the plugin did load a preset or did something that leads to a large delta, + // it may consider not producing a delta (pass null) and let the host make a state snapshot + // instead. + // // [main-thread] void(CLAP_ABI *change_made)(const clap_host_t *host, const char *name, From 300e6b19da33c3f441ca97dd23a151114a5e838f Mon Sep 17 00:00:00 2001 From: Alexandre Bique Date: Wed, 18 Sep 2024 12:05:42 +0200 Subject: [PATCH 6/7] undo: set the rev to /4 --- include/clap/ext/draft/undo.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/clap/ext/draft/undo.h b/include/clap/ext/draft/undo.h index eed32337..34f66bb7 100644 --- a/include/clap/ext/draft/undo.h +++ b/include/clap/ext/draft/undo.h @@ -3,9 +3,9 @@ #include "../../plugin.h" #include "../../stream.h" -static CLAP_CONSTEXPR const char CLAP_EXT_UNDO[] = "clap.undo/3"; -static CLAP_CONSTEXPR const char CLAP_EXT_UNDO_CONTEXT[] = "clap.undo_context/3"; -static CLAP_CONSTEXPR const char CLAP_EXT_UNDO_DELTA[] = "clap.undo_delta/3"; +static CLAP_CONSTEXPR const char CLAP_EXT_UNDO[] = "clap.undo/4"; +static CLAP_CONSTEXPR const char CLAP_EXT_UNDO_CONTEXT[] = "clap.undo_context/4"; +static CLAP_CONSTEXPR const char CLAP_EXT_UNDO_DELTA[] = "clap.undo_delta/4"; #ifdef __cplusplus extern "C" { From 329a696889ee876eae1e6f5e7c02422d35657464 Mon Sep 17 00:00:00 2001 From: Alexandre Bique Date: Wed, 18 Sep 2024 12:09:54 +0200 Subject: [PATCH 7/7] Doc. --- include/clap/ext/draft/undo.h | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/include/clap/ext/draft/undo.h b/include/clap/ext/draft/undo.h index 34f66bb7..0dedcdca 100644 --- a/include/clap/ext/draft/undo.h +++ b/include/clap/ext/draft/undo.h @@ -18,7 +18,7 @@ extern "C" { /// /// Calling host->undo() or host->redo() is equivalent to clicking undo/redo within the host's GUI. /// -/// If the plugin implements this interface then its undo and redo should be entirely delegated to +/// If the plugin uses this interface then its undo and redo should be entirely delegated to /// the host; clicking in the plugin's UI undo or redo is equivalent to clicking undo or redo in the /// host's UI. /// @@ -44,15 +44,11 @@ extern "C" { /// and maybe an easier experience for the user because there's a single undo context versus one /// for the host and one for each plugin instance. /// -/// The goal for this extension is to make it as easy as possible for the plugin to hook into -/// the host undo and make it efficient when possible by using deltas. -/// -/// The plugin interfaces are all optional, and the plugin can for a minimal implementation, -/// just use the host interface and call host->change_made() without providing a delta. -/// This is enough for the host to know that it can capture a plugin state for the undo step. -/// -/// Note: if a plugin is producing a lot of changes within a small amount of time, the host -/// may merge them into a single undo step. +/// This extension tries to make it as easy as possible for the plugin to hook into the host undo +/// and make it efficient when possible by using deltas. The plugin interfaces are all optional, and +/// the plugin can for a minimal implementation, just use the host interface and call +/// host->change_made() without providing a delta. This is enough for the host to know that it can +/// capture a plugin state for the undo step. typedef struct clap_undo_delta_properties { // If false, then all clap_undo_delta_properties's attributes become irrelevant. @@ -158,6 +154,9 @@ typedef struct clap_host_undo { // it may consider not producing a delta (pass null) and let the host make a state snapshot // instead. // + // Note: if a plugin is producing a lot of changes within a small amount of time, the host + // may merge them into a single undo step. + // // [main-thread] void(CLAP_ABI *change_made)(const clap_host_t *host, const char *name,