documented EffectUI

effects/effectfield.cpp

@@ -93,11 +93,6 @@ QString EffectField::ConvertValueToString(const QVariant &v)
   return v.toString();
 }
 
-QWidget *EffectField::CreateWidget(QWidget *existing)
-{
-  Q_ASSERT(false);
-}
-
 void EffectField::UpdateWidgetValue(QWidget *, double) {}
 
 QVariant EffectField::GetValueAt(double timecode)
@@ -106,7 +101,7 @@ QVariant EffectField::GetValueAt(double timecode)
     int before_keyframe;
     int after_keyframe;
     double progress;
-    get_keyframe_data(timecode, before_keyframe, after_keyframe, progress);
+    GetKeyframeData(timecode, before_keyframe, after_keyframe, progress);
 
     const QVariant& before_data = keyframes.at(before_keyframe).data;
     switch (type_) {
@@ -133,7 +128,7 @@ QVariant EffectField::GetValueAt(double timecode)
           if (before_key.type == EFFECT_KEYFRAME_BEZIER && after_key.type == EFFECT_KEYFRAME_BEZIER) {
 
             // cubic bezier
-            double t = cubic_t_from_x(timecodeToFrame(timecode),
+            double t = cubic_t_from_x(SecondsToFrame(timecode),
                                       before_key.time,
                                       before_key.time+GetValidKeyframeHandlePosition(before_keyframe, true),
                                       after_key.time+GetValidKeyframeHandlePosition(after_keyframe, false),
@@ -148,7 +143,7 @@ QVariant EffectField::GetValueAt(double timecode)
           } else if (after_key.type == EFFECT_KEYFRAME_LINEAR) { // quadratic bezier
 
             // last keyframe is the bezier one
-            double t = quad_t_from_x(timecodeToFrame(timecode),
+            double t = quad_t_from_x(SecondsToFrame(timecode),
                                      before_key.time,
                                      before_key.time+GetValidKeyframeHandlePosition(before_keyframe, true),
                                      after_key.time);
@@ -160,7 +155,7 @@ QVariant EffectField::GetValueAt(double timecode)
 
           } else {
             // this keyframe is the bezier one
-            double t = quad_t_from_x(timecodeToFrame(timecode),
+            double t = quad_t_from_x(SecondsToFrame(timecode),
                                      before_key.time,
                                      after_key.time+GetValidKeyframeHandlePosition(after_keyframe, false),
                                      after_key.time);
@@ -215,7 +210,7 @@ void EffectField::SetValueAt(double time, const QVariant &value)
     // Create keyframe here
 
     // Convert seconds timecode to frame
-    long frame_timecode = timecodeToFrame(time);
+    long frame_timecode = SecondsToFrame(time);
 
     // Check array if a keyframe at this time already exists
     int keyframe_index = -1;
@@ -345,20 +340,20 @@ double EffectField::GetValidKeyframeHandlePosition(int key, bool post) {
   return adjusted_key;
 }
 
-double EffectField::frameToTimecode(long frame) {
+double EffectField::FrameToSeconds(long frame) {
   return (double(frame) / GetParentRow()->GetParentEffect()->parent_clip->sequence->frame_rate);
 }
 
-long EffectField::timecodeToFrame(double timecode) {
-  return qRound(timecode * GetParentRow()->GetParentEffect()->parent_clip->sequence->frame_rate);
+long EffectField::SecondsToFrame(double seconds) {
+  return qRound(seconds * GetParentRow()->GetParentEffect()->parent_clip->sequence->frame_rate);
 }
 
-void EffectField::get_keyframe_data(double timecode, int &before, int &after, double &progress) {
+void EffectField::GetKeyframeData(double timecode, int &before, int &after, double &progress) {
   int before_keyframe_index = -1;
   int after_keyframe_index = -1;
   long before_keyframe_time = LONG_MIN;
   long after_keyframe_time = LONG_MAX;
-  long frame = timecodeToFrame(timecode);
+  long frame = SecondsToFrame(timecode);
 
   for (int i=0;i<keyframes.size();i++) {
     long eval_keyframe_time = keyframes.at(i).time;
@@ -375,11 +370,12 @@ void EffectField::get_keyframe_data(double timecode, int &before, int &after, do
     }
   }
 
-  if ((type_ == EFFECT_FIELD_DOUBLE || type_ == EFFECT_FIELD_COLOR) && (before_keyframe_index > -1 && after_keyframe_index > -1)) {
+  if ((type_ == EFFECT_FIELD_DOUBLE || type_ == EFFECT_FIELD_COLOR)
+      && (before_keyframe_index > -1 && after_keyframe_index > -1)) {
     // interpolate
     before = before_keyframe_index;
     after = after_keyframe_index;
-    progress = (timecode-frameToTimecode(before_keyframe_time))/(frameToTimecode(after_keyframe_time)-frameToTimecode(before_keyframe_time));
+    progress = (timecode-FrameToSeconds(before_keyframe_time))/(FrameToSeconds(after_keyframe_time)-FrameToSeconds(before_keyframe_time));
   } else if (before_keyframe_index > -1) {
     before = before_keyframe_index;
     after = before_keyframe_index;

effects/effectrow.cpp

@@ -60,6 +60,11 @@ bool EffectRow::IsKeyframing() {
 }
 
 void EffectRow::SetKeyframingInternal(bool b) {
+  // No need to run this function if the keyframing state isn't actually changing.
+  if (b == keyframing_) {
+    return;
+  }
+
   if (GetParentEffect()->meta->type != EFFECT_TYPE_TRANSITION) {
     keyframing_ = b;
     emit KeyframingSetChanged(keyframing_);
@@ -77,6 +82,10 @@ bool EffectRow::IsKeyframable()
 }
 
 void EffectRow::SetKeyframingEnabled(bool enabled) {
+  if (enabled == keyframing_) {
+    return;
+  }
+
   if (enabled) {
 
     ComboAction* ca = new ComboAction();
@@ -257,18 +266,6 @@ void EffectRow::SetKeyframeOnAllFields(ComboAction* ca) {
   panel_effect_controls->update_keyframes();
 }
 
-void EffectRow::delete_keyframe_at_time(ComboAction* ca, long time) {
-  for (int j=0;j<FieldCount();j++) {
-    EffectField* f = Field(j);
-    for (int i=0;i<f->keyframes.size();i++) {
-      if (f->keyframes.at(i).time == time) {
-        ca->append(new KeyframeDelete(f, i));
-        break;
-      }
-    }
-  }
-}
-
 Effect *EffectRow::GetParentEffect()
 {
   return static_cast<Effect*>(parent());

effects/effectrow.h

@@ -36,43 +36,226 @@ class ClickableLabel;
 
 #include "effectfields.h"
 
+/**
+ * @brief The EffectRow class
+ *
+ * Primarily a way of grouping EffectField objects together. As UI objects, Effects are largely formatted as a table
+ * and you can think of EffectRows as the rows of the table and EffectFields as the columns.
+ *
+ * Within Olive, keyframing is enabled on the EffectRow (rather than individual EffectFields) so all attached fields
+ * will be keyframed together.
+ *
+ * Unlike EffectField, there's no reason to derive from EffectRow as it's simply a container of fields and a few
+ * keyframe functions.
+ */
 class EffectRow : public QObject {
   Q_OBJECT
 public:
+  /**
+   * @brief EffectRow Constructor
+   *
+   * @param parent
+   *
+   * Every EffectRow object must be attached to a valid Effect object. The Effect object takes ownership of the
+   * EffectRow and automatically frees it through the QObject parent/child system. EffectRows are never intended to
+   * change parents throughout their lifetimes.
+   *
+   * @param n
+   *
+   * Row name. This is not used as an internal identifier, it's just for the user interface, so it can be translated
+   * with no issue.
+   *
+   * @param savable
+   *
+   * Whether the fields in this row should be saved to the project file. This is true by default. If the row contains
+   * non-value UI widgets, setting this to false is recommended.
+   *
+   * @param keyframable
+   *
+   * Whether keyframing can be enabled on this row or not. This is true by default. Some values you may want to prevent
+   * the user from keyframing (e.g. the filename of a VST plugin), which can be done by setting this to false.
+   */
   EffectRow(Effect* parent, const QString& n, bool savable = true, bool keyframable = true);
 
+  /**
+   * @brief Add a field to this row
+   *
+   * Ownership of the EffectField is transferred to this row and the row will free its memory. In the Effect's UI, this
+   * will add the field to an additional column.
+   *
+   * @param Field
+   *
+   * The field to add to this row.
+   */
   void AddField(EffectField* Field);
 
+  /**
+   * @brief Retrieve the EffectField at this index. Must be less than FieldCount().
+   *
+   * @param i
+   *
+   * Index to retrieve the EffectField at.
+   *
+   * @return
+   *
+   * EffectField at the provided index.
+   */
   EffectField* Field(int i);
-  int FieldCount();
-  void SetKeyframeOnAllFields(ComboAction *ca);
-  void delete_keyframe_at_time(ComboAction *ca, long time);
 
+  /**
+   * @brief Number of fields currently contained in this row.
+   *
+   * @return The number of fields currently contained in this row as an integer. Any field index (retrieved with
+   * Field()) is guaranteed to be valid >= 0 and < FieldCount().
+   */
+  int FieldCount();
+
+  /**
+   * @brief Set a keyframe at the current playhead on all fields contained within this row
+   *
+   * @param ca
+   *
+   * The ComboAction to add this action to. This may not be nullptr.
+   */
+  void SetKeyframeOnAllFields(ComboAction *ca);
+
+  /**
+   * @brief Get parent Effect
+   *
+   * Equivalent to `static_cast<Effect*>(parent())`.
+   *
+   * @return The parent Effect object that this row is attached to.
+   */
   Effect* GetParentEffect();
 
+  /**
+   * @brief Return the row's name
+   *
+   * @return The name specified in the constructor as a QString
+   */
   const QString& name();
 
+  /**
+   * @brief Get whether this row is keyframing or not
+   *
+   * @return True if this row is keyframing.
+   */
   bool IsKeyframing();
+
+  /**
+   * @brief Set whether this row is keyframing or not.
+   *
+   * It's not recommended to use this function for any user-initiated keyframe setting change as it doesn't create any
+   * undoable actions. Use SetKeyframingEnabled() instead for a user-friendly variant.
+   */
   void SetKeyframingInternal(bool);
 
+  /**
+   * @brief Get whether this row should be saved into a project file or not
+   *
+   * @return True if this row should be saved. This value is set in the constructor.
+   */
   bool IsSavable();
+
+  /**
+   * @brief Get whether this row can be keyframed or not.
+   *
+   * @return True if this row can be keyframed. This value is set in the constructor.
+   */
   bool IsKeyframable();
 public slots:
+  /**
+   * @brief Go to previous keyframe
+   *
+   * Gets the closest keyframe prior to the current playhead and seeks to it.
+   *
+   * Attach to KeyframeNavigator::goto_previous_key() signal.
+   */
   void GoToPreviousKeyframe();
+
+  /**
+   * @brief Toggle a keyframe at this point in time
+   *
+   * Either deletes (if any child EffectFields have any keyframes here) or creates (if none do) a keyframe on all
+   * EffectField children at the current time.
+   *
+   * Attach to KeyframeNavigator::toggle_key() signal.
+   */
   void ToggleKeyframe();
+
+  /**
+   * @brief Go to next keyframe
+   *
+   * Gets the closest keyframe after the current playhead and seeks to it.
+   *
+   * Attach to KeyframeNavigator::goto_next_key() signal.
+   */
   void GoToNextKeyframe();
+
+  /**
+   * @brief Slot for whenever this EffectRow is focused.
+   *
+   * Connect UI objects gaining focus to this slot. Automatically updates the Graph Editor to attach to this row.
+   */
   void FocusRow();
 signals:
+  /**
+   * @brief Keyframing setting changed signal
+   *
+   * Emitted whenever keyframing is enabled or disabled.
+   *
+   * @param
+   *
+   * True if keyframing was enabled, false if keyframing was disabled.
+   */
   void KeyframingSetChanged(bool);
 private slots:
+  /**
+   * @brief Set keyframing enabled state
+   *
+   * A user-friendly function for enabling or disabling keyframes on this row. Preferred to SetKeyframingInternal()
+   * for any user-initiated change. Automatically creates an undoable action so users can undo the enabling/disabling.
+   * Also confirms with the user when disabling keyframing whether they wish to continue and remove all the current
+   * keyframes.
+   *
+   * Attach to KeyframeNavigator::keyframe_enabled_changed() signal.
+   */
   void SetKeyframingEnabled(bool);
 private:
+  /**
+   * @brief Internal variable for the row's name
+   *
+   * Set in the constructor, retrieved with name().
+   */
   QString name_;
 
+  /**
+   * @brief Internal variable for whether this row can be keyframed.
+   *
+   * Set in the constructor, retrieved with IsKeyframable().
+   */
   bool keyframable_;
+
+  /**
+   * @brief Internal variable for whether this row is currently keyframing.
+   *
+   * Set by SetKeyframingInternal() and retrieved with IsKeyframing().
+   */
   bool keyframing_;
+
+  /**
+   * @brief Internal variable for whether this row should be saved.
+   *
+   * Set in the constructor, retrieved with IsSavable().
+   */
   bool savable_;
 
+  /**
+   * @brief Internal array of EffectField objects.
+   *
+   * It is not necessary to delete the elements in this array as they're already children of this QObject, so they'll
+   * get freed automatically.
+   */
   QVector<EffectField*> fields_;
 };
 

panels/effectcontrols.cpp

@@ -155,17 +155,21 @@ void EffectControls::copy(bool del) {
     if (open_effects_.at(i)->IsSelected()) {
       Effect* e = open_effects_.at(i)->GetEffect();
 
-      if (!cleared) {
-        clear_clipboard();
-        cleared = true;
-        clipboard_type = CLIPBOARD_TYPE_EFFECT;
-      }
+      if (e->meta->type == EFFECT_TYPE_EFFECT) {
 
-      clipboard.append(e->copy(nullptr));
+        if (!cleared) {
+          clear_clipboard();
+          cleared = true;
+          clipboard_type = CLIPBOARD_TYPE_EFFECT;
+        }
 
-      if (del) {
+        clipboard.append(e->copy(nullptr));
 
-        DeleteEffect(ca, e);
+        if (del) {
+
+          DeleteEffect(ca, e);
+
+        }
 
       }
     }

ui/effectui.cpp

@@ -10,8 +10,7 @@
 #include "panels/panels.h"
 
 EffectUI::EffectUI(Effect* e) :
-  effect_(e),
-  multiple_(false)
+  effect_(e)
 {
   Q_ASSERT(e != nullptr);
 
@@ -75,7 +74,7 @@ EffectUI::EffectUI(Effect* e) :
   connect(title_bar,
           SIGNAL(customContextMenuRequested(const QPoint&)),
           this,
-          SIGNAL(TitleBarContextMenuRequested(const QPoint&)));
+          SLOT(show_context_menu(const QPoint&)));
 
   int maximum_column = 0;
 
@@ -136,7 +135,6 @@ EffectUI::EffectUI(Effect* e) :
 
     }
 
-
     keyframe_navigators_[i] = nav;
 
   }
@@ -152,12 +150,10 @@ void EffectUI::AddAdditionalEffect(Effect *e)
   Q_ASSERT(e->meta == effect_->meta);
 
   // Add multiple modifer to header label (but only once)
-  if (!multiple_) {
+  if (additional_effects_.isEmpty()) {
     QString new_title = tr("%1 (multiple)").arg(Title());
 
     SetTitle(new_title);
-
-    multiple_ = true;
   }
 
   // Add effect to list

ui/effectui.h

@@ -4,39 +4,195 @@
 #include "collapsiblewidget.h"
 #include "effects/effect.h"
 
+/**
+ * @brief The EffectUI class
+ *
+ * EffectUI is a complete QWidget-based representation of an Effect that can be added to any Qt layout. It overrides
+ * CollapsibleWidget (meaning the Effect can be collapsed to just a titlebar to save space). The titlebar is
+ * automatically set to the Effect's name and the contents are composed of a grid layout (QGridLayout) corresponding
+ * to the Effect's EffectRow and EffectField children.
+ *
+ * Many EffectUIs can be created from a single Effect, and many Effects can be attached to a single EffectUI (provided
+ * the Effects are all the same type). Neither gains ownership of each other and deleting an EffectUI without any other
+ * work is perfectly safe (deleting an Effect with an open EffectUI however, is not).
+ */
 class EffectUI : public CollapsibleWidget {
   Q_OBJECT
 public:
+  /**
+   * @brief EffectUI Constructor
+   *
+   * Creates a QWidget-based UI representation of an Effect.
+   *
+   * @param e
+   *
+   * The Effect to make a UI of. It must be a valid object.
+   */
   EffectUI(Effect* e);
 
+  /**
+   * @brief Attach additional effects to this UI
+   *
+   * Olive allows users to modify several effects (of the same type) with one UI representation. To do this, you can
+   * add any amount of extra Effect objects using this function and the UI will attach all of its UI functions to that
+   * Effect as well without creating any new QWidgets.
+   *
+   * @param e
+   *
+   * The Effect to add to this UI object.
+   */
   void AddAdditionalEffect(Effect* e);
 
+  /**
+   * @brief Get the primary Effect that this UI object was created for
+   *
+   * @return
+   *
+   * The Effect object passed to the constrcutor whe creating this EffectUI.
+   */
   Effect* GetEffect();
 
+  /**
+   * @brief Get the Y position of a given row
+   *
+   * Retrieve the on-screen Y position of the attached Effect's EffectRow at a given index. This is primarily used for
+   * displaying UI elements that align with the the row's on-screen widgets (e.g. keyframes in the EffectControls
+   * panel).
+   *
+   * The Y value provided is specifically the center point of the row's name label. It gets mapped to a provided
+   * QWidget object so it can be used locally by that QWidget without further modification.
+   *
+   * @param row
+   *
+   * The index of the EffectRow to retrieve the Y position of.
+   *
+   * @param mapToWidget
+   *
+   * The widget to map the Y value to.
+   *
+   * @return
+   *
+   * The row's Y position.
+   */
   int GetRowY(int row, QWidget *mapToWidget);
 
+  /**
+   * @brief Update widgets with the current Effect's values.
+   *
+   * When the Timeline playhead moves, the current values in the Effect might change if its fields are keyframed.
+   * In order to visually update these values on the UI, this function should be called. It will loop through all
+   * fields of all attached effects and update them to the value at the current Timeline playhead.
+   *
+   * Currently this function is called by update_ui() which is also responsible for updating other parts of the UI
+   * like the Timeline and Viewer so they all get updated together.
+   */
   void UpdateFromEffect();
 
+  /**
+   * @brief Check if a given Clip has an Effect referenced by this EffectUI
+   *
+   * Olive allows users to modify several effects (of the same type) with one UI representation. The behavior is if
+   * multiple clips are selected that have effects of the same type, all those Effects can be modified by the same
+   * EffectUI object. However this behavior is undesirable if a single Clip has more than one of the same type of Effect
+   * (e.g. two or more blurs). In this scenario, the user will most likely expect two separate UI objects for each of
+   * these effects individually, rather than consolidating them into one UI object.
+   *
+   * To address this, EffectControls will check this function to determine if this EffectUI already references
+   * an Effect of this type from this Clip. If it does, it's assumed a new EffectUI should be made rather than
+   * consolidating that Effect into the same EffectUI.
+   *
+   * @param c
+   *
+   * The Clip to determine whether an Effect of this type is already referenced by this EffectUI.
+   *
+   * @return
+   *
+   * True is an Effect from this Clip is already attached to this EffectUI.
+   */
   bool IsAttachedToClip(Clip* c);
 
 signals:
+  /**
+   * @brief Cut signal
+   *
+   * Emitted when the user selects Cut from the right-click context menu.
+   */
   void CutRequested();
+
+  /**
+   * @brief Copy signal
+   *
+   * Emitted when the user selects Copy from the right-click context menu.
+   */
   void CopyRequested();
-  void TitleBarContextMenuRequested(const QPoint&);
 private:
+  /**
+   * @brief Retrieve the QWidget corresponding a specific EffectField
+   *
+   * Convenience function equivalent to widgets_.at(row).at(field).
+   *
+   * @param row
+   *
+   * EffectRow index to retrieve field QWidget from
+   *
+   * @param field
+   *
+   * EffectField index to retrieve QWidget from
+   *
+   * @return
+   *
+   * The QWidget at this row and field index.
+   */
   QWidget* Widget(int row, int field);
 
+  /**
+   * @brief Internal reference to the Effect this object was constructed around.
+   */
   Effect* effect_;
+
+  /**
+   * @brief Internal array of additional Effect objects attached to this UI.
+   */
   QVector<Effect*> additional_effects_;
+
+  /**
+   * @brief Layout for UI widgets
+   */
   QGridLayout* layout_;
-  QVector<QLabel*> labels_;
+
+  /**
+   * @brief Grid array of QWidgets corresponding to the Effect's rows and fields
+   */
   QVector< QVector<QWidget*> > widgets_;
+
+  /**
+   * @brief Array of QLabel objects corresponding to each row's name().
+   */
+  QVector<QLabel*> labels_;
+
+  /**
+   * @brief Array of KeyframeNavigator objects corresponding to each row.
+   */
   QVector<KeyframeNavigator*> keyframe_navigators_;
 
-  bool multiple_;
-
+  /**
+   * @brief Attach a KeyframeNavigator object to an EffectRow.
+   *
+   * Internal function for connecting a KeyframeNavigator UI object to an EffectRow.
+   *
+   * @param row
+   *
+   * The EffectRow object.
+   *
+   * @param nav
+   *
+   * The KeyframeNavigator object.
+   */
   void AttachKeyframeNavigationToRow(EffectRow* row, KeyframeNavigator* nav);
 private slots:
+  /**
+   * @brief Slot for titlebar's right-click signal to show a context menu for extra Effect functions.
+   */
   void show_context_menu(const QPoint&);
 };