Marker RGB colors

Signed-off-by: Spencer Magnusson <spencer.magnusson@dreamworks.com>

Author: semagnum

docs/tutorials/otio-serialized-schema-only-fields.md

@@ -31,15 +31,6 @@ parameters:
 
 ## Module: opentimelineio.core
 
-### Color.1
-
-parameters:
-- *a*
-- *b*
-- *g*
-- *name*
-- *r*
-
 ### Composable.1
 
 parameters:
@@ -152,6 +143,15 @@ parameters:
 - *name*
 - *source_range*
 
+### Color.1
+
+parameters:
+- *a*
+- *b*
+- *g*
+- *name*
+- *r*
+
 ### Effect.1
 
 parameters:
@@ -224,7 +224,7 @@ parameters:
 - *name*
 - *time_scalar*
 
-### Marker.2
+### Marker.3
 
 parameters:
 - *color*

docs/tutorials/otio-serialized-schema.md

@@ -55,27 +55,6 @@ parameters:
 
 ## Module: opentimelineio.core
 
-### Color.1
-
-*full module path*: `opentimelineio.core.Color`
-
-*documentation*:
-
-```
-:class:`Color` is a definition of red, green, blue, and alpha double floating point values, allowing
- conversion between different formats. To be considered interoperable, the sRGB transfer function 
-encoded values, ranging between zero and one, are expected to be accurate to within 1/255 of the 
-intended value. Round-trip conversions may not be guaranteed outside that. This Color class is meant
- for use in user interface elements, like marker or clip coloring, NOT for image pixel content.
-```
-
-parameters:
-- *a*: 
-- *b*: 
-- *g*: 
-- *name*: 
-- *r*: 
-
 ### Composable.1
 
 *full module path*: `opentimelineio.core.Composable`
@@ -309,6 +288,27 @@ parameters:
 - *name*: 
 - *source_range*: 
 
+### Color.1
+
+*full module path*: `opentimelineio.schema.Color`
+
+*documentation*:
+
+```
+:class:`Color` is a definition of red, green, blue, and alpha double floating point values, allowing
+ conversion between different formats. To be considered interoperable, the sRGB transfer function 
+encoded values, ranging between zero and one, are expected to be accurate to within 1/255 of the 
+intended value. Round-trip conversions may not be guaranteed outside that. This Color class is meant
+ for use in user interface elements, like marker or clip coloring, NOT for image pixel content.
+```
+
+parameters:
+- *a*: 
+- *b*: 
+- *g*: 
+- *name*: 
+- *r*: 
+
 ### Effect.1
 
 *full module path*: `opentimelineio.schema.Effect`
@@ -519,7 +519,7 @@ parameters:
 Note that adjusting the time_scalar of a :class:`~LinearTimeWarp` does not affect the duration of the item this effect is attached to.
 Instead it affects the speed of the media displayed within that item.
 
-### Marker.2
+### Marker.3
 
 *full module path*: `opentimelineio.schema.Marker`
 

src/opentimelineio/CORE_VERSION_MAP.cpp

@@ -221,7 +221,7 @@ const label_to_schema_version_map CORE_VERSION_MAP{
           { "ImageSequenceReference", 1 },
           { "Item", 1 },
           { "LinearTimeWarp", 1 },
-          { "Marker", 2 },
+          { "Marker", 3 },
           { "MediaLinker", 1 },
           { "MediaReference", 1 },
           { "MissingReference", 1 },

src/opentimelineio/marker.cpp

@@ -7,11 +7,11 @@
 namespace opentimelineio { namespace OPENTIMELINEIO_VERSION_NS {
 
 Marker::Marker(
-    std::string const&   name,
-    TimeRange const&     marked_range,
-    std::string const&   color,
-    AnyDictionary const& metadata,
-    std::string const&   comment)
+    std::string const&          name,
+    TimeRange const&            marked_range,
+    std::optional<Color> const& color,
+    AnyDictionary const&        metadata,
+    std::string const&          comment)
     : Parent(name, metadata)
     , _color(color)
     , _marked_range(marked_range)

src/opentimelineio/marker.h

@@ -5,6 +5,7 @@
 
 #include "opentimelineio/serializableObjectWithMetadata.h"
 #include "opentimelineio/version.h"
+#include "opentimelineio/color.h"
 
 namespace opentimelineio { namespace OPENTIMELINEIO_VERSION_NS {
 
@@ -16,27 +17,12 @@ namespace opentimelineio { namespace OPENTIMELINEIO_VERSION_NS {
 class OTIO_API_TYPE Marker : public SerializableObjectWithMetadata
 {
 public:
-    /// @brief This struct provides the base set of colors.
-    struct Color
-    {
-        static auto constexpr pink    = "PINK";
-        static auto constexpr red     = "RED";
-        static auto constexpr orange  = "ORANGE";
-        static auto constexpr yellow  = "YELLOW";
-        static auto constexpr green   = "GREEN";
-        static auto constexpr cyan    = "CYAN";
-        static auto constexpr blue    = "BLUE";
-        static auto constexpr purple  = "PURPLE";
-        static auto constexpr magenta = "MAGENTA";
-        static auto constexpr black   = "BLACK";
-        static auto constexpr white   = "WHITE";
-    };
 
     /// @brief This struct provides the Marker schema.
     struct Schema
     {
         static auto constexpr name   = "Marker";
-        static int constexpr version = 2;
+        static int constexpr version = 3;
     };
 
     using Parent = SerializableObjectWithMetadata;
@@ -51,15 +37,15 @@ class OTIO_API_TYPE Marker : public SerializableObjectWithMetadata
     OTIO_API Marker(
         std::string const&   name         = std::string(),
         TimeRange const&     marked_range = TimeRange(),
-        std::string const&   color        = Color::green,
+        std::optional<Color> const& color = Color::green,
         AnyDictionary const& metadata     = AnyDictionary(),
         std::string const&   comment      = std::string());
 
     /// @brief Return the marker color.
-    std::string color() const noexcept { return _color; }
+    std::optional<Color> color() const noexcept { return _color; }
 
     /// @brief Set the marker color.
-    void set_color(std::string const& color) { _color = color; }
+    void set_color(std::optional<Color> const& color) { _color = color; }
 
     /// @brief Return the marker time range.
     TimeRange marked_range() const noexcept { return _marked_range; }
@@ -83,7 +69,7 @@ class OTIO_API_TYPE Marker : public SerializableObjectWithMetadata
     void write_to(Writer&) const override;
 
 private:
-    std::string _color;
+    std::optional<Color> _color;
     TimeRange   _marked_range;
     std::string _comment;
 };

src/opentimelineio/typeRegistry.cpp

@@ -29,7 +29,11 @@
 #include "opentimelineio/unknownSchema.h"
 #include "stringUtils.h"
 
+#include <algorithm>
 #include <assert.h>
+#include <cctype>
+#include <map>
+#include <set>
 #include <vector>
 
 namespace opentimelineio { namespace OPENTIMELINEIO_VERSION_NS {
@@ -96,6 +100,62 @@ TypeRegistry::TypeRegistry()
         d->erase("range");
     });
 
+    // 2 - 3
+    register_upgrade_function(Marker::Schema::name, 3, [](AnyDictionary* d) {
+        // get color name
+        std::string color_name_v2;
+        if (d->get_if_set("color", &color_name_v2))
+        {
+            static const std::map<std::string, Color> color_map = {
+                { "PINK", Color::pink },
+                { "RED", Color::red },
+                { "ORANGE", Color::orange },
+                { "YELLOW", Color::yellow },
+                { "GREEN", Color::green },
+                { "CYAN", Color::cyan },
+                { "BLUE", Color::blue },
+                { "MAGENTA", Color::magenta },
+                { "PURPLE", Color::purple },
+                { "BLACK", Color::black },
+                { "WHITE", Color::white },
+                { "TRANSPARENT", Color::transparent }
+            };
+
+            // make copy of color_name_v2, for uppercase change to be separate
+            std::string color_name_v2_upper = color_name_v2;
+
+            // force color name to uppercase for lookup
+            // since v2 color names were case-insensitive
+            std::transform(
+                color_name_v2_upper.begin(),
+                color_name_v2_upper.end(),
+                color_name_v2_upper.begin(),
+                [](unsigned char c) { return std::toupper(c); }
+            );
+
+            // if all-caps name matches a known color, convert to color with r,g,b,a, and name fields
+            auto it = color_map.find(color_name_v2_upper);
+            Color color_match = Color::white;
+            std::string color_match_name = "";
+            if (it != color_map.end()) {  // match found
+                color_match = it->second;
+                color_match_name = color_match.name();
+            }
+            else {  // no match, default to white but keep original name
+                color_match = Color::white;
+                color_match_name = color_name_v2;
+            }
+
+            (*d)["color"] = Color(
+                color_match.r(),
+                color_match.g(),
+                color_match.b(),
+                color_match.a(),
+                color_match_name
+            );
+        }
+    });
+
     register_upgrade_function(Clip::Schema::name, 2, [](AnyDictionary* d) {
         auto media_ref = (*d)["media_reference"];
 
@@ -116,6 +176,46 @@ TypeRegistry::TypeRegistry()
         d->erase("media_reference");
     });
 
+    // 3->2
+    register_downgrade_function(Marker::Schema::name, 3, [](AnyDictionary* d) {
+        AnyDictionary color_dict;
+
+        if (d->get_if_set("color", &color_dict))
+        {
+            std::string color_name = "";
+            color_dict.get_if_set("name", &color_name);
+
+            // if the name matches case-insensitive to a known color, 
+            // set the color an all-caps version of that name
+            if (!color_name.empty())
+            {
+                // Convert to uppercase for comparison and storage
+                std::string upper_name = color_name;
+                std::transform(
+                    upper_name.begin(),
+                    upper_name.end(),
+                    upper_name.begin(),
+                    [](unsigned char c) { return std::toupper(c); }
+                );
+
+                // Known color names - these are the valid color enum values
+                static const std::set<std::string> known_colors = {
+                    "RED", "GREEN", "BLUE", "YELLOW", "CYAN", "MAGENTA",
+                    "PINK", "ORANGE", "PURPLE", "BLACK", "WHITE", "TRANSPARENT"
+                };
+
+                if (known_colors.find(upper_name) != known_colors.end())
+                {
+                    // remove color object and replace with color name string
+                    (*d)["color"] = upper_name;
+                }
+                else {  // otherwise, keep name as-is
+                    (*d)["color"] = color_name;
+                }
+            }
+        }
+    });
+
     // 2->1
     register_downgrade_function(Clip::Schema::name, 2, [](AnyDictionary* d) {
         AnyDictionary mrefs;

src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp

@@ -433,11 +433,11 @@ A marker indicates a marked range of time on an item in a timeline, usually with
 The marked range may have a zero duration. The marked range is in the owning item's time coordinate system.
 )docstring")
             .def(
-                py::init([](std::string        name,
-                            TimeRange          marked_range,
-                            std::string const& color,
-                            py::object         metadata,
-                            std::string const& comment) {
+                py::init([](std::string          name,
+                            TimeRange            marked_range,
+                            std::optional<Color> color,
+                            py::object           metadata,
+                            std::string const&   comment) {
                     return new Marker(
                         name,
                         marked_range,
@@ -447,14 +447,14 @@ The marked range may have a zero duration. The marked range is in the owning ite
                 }),
                 py::arg_v("name"_a = std::string()),
                 "marked_range"_a = TimeRange(),
-                "color"_a        = std::string(Marker::Color::red),
+                "color"_a        = Color::red,
                 py::arg_v("metadata"_a = py::none()),
                 "comment"_a = std::string())
             .def_property(
                 "color",
                 &Marker::color,
                 &Marker::set_color,
-                "Color string for this marker (for example: 'RED'), based on the :class:`~Color` enum.")
+                "Color object assigned to this marker")
             .def_property(
                 "marked_range",
                 &Marker::marked_range,
@@ -466,41 +466,6 @@ The marked range may have a zero duration. The marked range is in the owning ite
                 &Marker::set_comment,
                 "Optional comment for this marker.");
 
-    py::class_<Marker::Color>(marker_class, "Color")
-        .def_property_readonly_static(
-            "PINK",
-            [](py::object /* self */) { return Marker::Color::pink; })
-        .def_property_readonly_static(
-            "RED",
-            [](py::object /* self */) { return Marker::Color::red; })
-        .def_property_readonly_static(
-            "ORANGE",
-            [](py::object /* self */) { return Marker::Color::orange; })
-        .def_property_readonly_static(
-            "YELLOW",
-            [](py::object /* self */) { return Marker::Color::yellow; })
-        .def_property_readonly_static(
-            "GREEN",
-            [](py::object /* self */) { return Marker::Color::green; })
-        .def_property_readonly_static(
-            "CYAN",
-            [](py::object /* self */) { return Marker::Color::cyan; })
-        .def_property_readonly_static(
-            "BLUE",
-            [](py::object /* self */) { return Marker::Color::blue; })
-        .def_property_readonly_static(
-            "PURPLE",
-            [](py::object /* self */) { return Marker::Color::purple; })
-        .def_property_readonly_static(
-            "MAGENTA",
-            [](py::object /* self */) { return Marker::Color::magenta; })
-        .def_property_readonly_static(
-            "BLACK",
-            [](py::object /* self */) { return Marker::Color::black; })
-        .def_property_readonly_static("WHITE", [](py::object /* self */) {
-            return Marker::Color::white;
-        });
-
     using SerializableCollectionIterator =
         ContainerIterator<SerializableCollection, SerializableObject*>;
     py::class_<SerializableCollectionIterator>(