Do effects with an enum.

4 files changed, 31 insertions, 14 deletions

diff --git a/src/subtitle_asset.cc b/src/subtitle_asset.cc
index 2eeceab1..827fbf7d 100644
--- a/src/subtitle_asset.cc
+++ b/src/subtitle_asset.cc
@@ -66,7 +66,7 @@ SubtitleAsset::examine_font_node (shared_ptr<FontNode> font_node, list<shared_pt
 						(*j)->out,
 						(*k)->v_position,
 						(*k)->text,
-						effective.effect,
+						effective.effect.get(),
 						effective.effect_color.get()
 						)
 					)
@@ -88,7 +88,16 @@ FontNode::FontNode (xmlpp::Node const * node)
 	size = optional_int64_attribute ("Size");
 	italic = optional_bool_attribute ("Italic");
 	color = optional_color_attribute ("Color");
-	effect = optional_string_attribute ("Effect");
+	string const e = optional_string_attribute ("Effect");
+	if (e == "none") {
+		effect = NONE;
+	} else if (e == "border") {
+		effect = BORDER;
+	} else if (e == "shadow") {
+		effect = SHADOW;
+	} else if (!e.empty ()) {
+		throw DCPReadError ("unknown subtitle effect type");
+	}
 	effect_color = optional_color_attribute ("EffectColor");
 	subtitle_nodes = sub_nodes<SubtitleNode> ("Subtitle");
 	font_nodes = sub_nodes<FontNode> ("Font");
@@ -98,6 +107,7 @@ FontNode::FontNode (list<shared_ptr<FontNode> > const & font_nodes)
 	: size (0)
 	, italic (false)
 	, color ("FFFFFFFF")
+	, effect_color ("FFFFFFFF")
 {
 	for (list<shared_ptr<FontNode> >::const_iterator i = font_nodes.begin(); i != font_nodes.end(); ++i) {
 		if (!(*i)->id.empty ()) {
@@ -112,8 +122,8 @@ FontNode::FontNode (list<shared_ptr<FontNode> > const & font_nodes)
 		if ((*i)->color) {
 			color = (*i)->color.get ();
 		}
-		if (!(*i)->effect.empty ()) {
-			effect = (*i)->effect;
+		if ((*i)->effect) {
+			effect = (*i)->effect.get ();
 		}
 		if ((*i)->effect_color) {
 			effect_color = (*i)->effect_color.get ();
@@ -185,7 +195,7 @@ Subtitle::Subtitle (
 	Time out,
 	float v_position,
 	string text,
-	string effect,
+	Effect effect,
 	Color effect_color
 	)
 	: _font (font)
diff --git a/src/subtitle_asset.h b/src/subtitle_asset.h
index d3f9fa9b..aff3dae5 100644
--- a/src/subtitle_asset.h
+++ b/src/subtitle_asset.h
@@ -56,7 +56,7 @@ public:
 	int size;
 	boost::optional<bool> italic;
 	boost::optional<Color> color;
-	std::string effect;
+	boost::optional<Effect> effect;
 	boost::optional<Color> effect_color;
 	
 	std::list<boost::shared_ptr<SubtitleNode> > subtitle_nodes;
@@ -85,7 +85,7 @@ public:
 		Time out,
 		float v_position,
 		std::string text,
-		std::string effect,
+		Effect effect,
 		Color effect_color
 		);
 
@@ -117,7 +117,7 @@ public:
 		return _v_position;
 	}
 
-	std::string effect () const {
+	Effect effect () const {
 		return _effect;
 	}
 
@@ -136,7 +136,7 @@ private:
 	Time _out;
 	float _v_position;
 	std::string _text;
-	std::string _effect;
+	Effect _effect;
 	Color _effect_color;
 };
 
diff --git a/src/types.h b/src/types.h
index 3a50c5d3..c49c5f8f 100644
--- a/src/types.h
+++ b/src/types.h
@@ -51,6 +51,13 @@ enum ContentKind
 	ADVERTISEMENT
 };
 
+enum Effect
+{
+	NONE,
+	BORDER,
+	SHADOW
+};
+	
 class Fraction
 {
 public:
diff --git a/test/tests.cc b/test/tests.cc
index d503c00d..a03e2f68 100644
--- a/test/tests.cc
+++ b/test/tests.cc
@@ -126,7 +126,7 @@ BOOST_AUTO_TEST_CASE (subtitles)
 	BOOST_CHECK_EQUAL (s.front()->italic(), false);
 	BOOST_CHECK_EQUAL (s.front()->color(), libdcp::Color(255, 255, 255));
 	BOOST_CHECK_EQUAL (s.front()->size_in_pixels(1080), 53);
-	BOOST_CHECK_EQUAL (s.front()->effect(), "border");
+	BOOST_CHECK_EQUAL (s.front()->effect(), libdcp::BORDER);
 	BOOST_CHECK_EQUAL (s.front()->effect_color(), libdcp::Color(0, 0, 0));
 
 	s = subs.subtitles_at (libdcp::Time (0, 0, 7, 190));
@@ -138,7 +138,7 @@ BOOST_AUTO_TEST_CASE (subtitles)
 	BOOST_CHECK_EQUAL (s.front()->font(), "Arial");
 	BOOST_CHECK_EQUAL (s.front()->italic(), true);
 	BOOST_CHECK_EQUAL (s.front()->size_in_pixels(1080), 53);
-	BOOST_CHECK_EQUAL (s.front()->effect(), "border");
+	BOOST_CHECK_EQUAL (s.front()->effect(), libdcp::BORDER);
 	BOOST_CHECK_EQUAL (s.front()->effect_color(), libdcp::Color(0, 0, 0));
 	BOOST_CHECK_EQUAL (s.back()->text(), "My large wonderbra");
 	BOOST_CHECK_EQUAL (s.back()->v_position(), 15);
@@ -148,7 +148,7 @@ BOOST_AUTO_TEST_CASE (subtitles)
 	BOOST_CHECK_EQUAL (s.back()->italic(), true);
 	BOOST_CHECK_EQUAL (s.back()->color(), libdcp::Color(255, 255, 255));
 	BOOST_CHECK_EQUAL (s.back()->size_in_pixels(1080), 53);
-	BOOST_CHECK_EQUAL (s.back()->effect(), "border");
+	BOOST_CHECK_EQUAL (s.back()->effect(), libdcp::BORDER);
 	BOOST_CHECK_EQUAL (s.back()->effect_color(), libdcp::Color(0, 0, 0));
 	
 	s = subs.subtitles_at (libdcp::Time (0, 0, 11, 95));
@@ -161,7 +161,7 @@ BOOST_AUTO_TEST_CASE (subtitles)
 	BOOST_CHECK_EQUAL (s.front()->italic(), false);
 	BOOST_CHECK_EQUAL (s.front()->color(), libdcp::Color(255, 255, 255));
 	BOOST_CHECK_EQUAL (s.front()->size_in_pixels(1080), 53);
-	BOOST_CHECK_EQUAL (s.front()->effect(), "border");
+	BOOST_CHECK_EQUAL (s.front()->effect(), libdcp::BORDER);
 	BOOST_CHECK_EQUAL (s.front()->effect_color(), libdcp::Color(0, 0, 0));
 
 	s = subs.subtitles_at (libdcp::Time (0, 0, 14, 42));
@@ -174,7 +174,7 @@ BOOST_AUTO_TEST_CASE (subtitles)
 	BOOST_CHECK_EQUAL (s.front()->italic(), false);
 	BOOST_CHECK_EQUAL (s.front()->color(), libdcp::Color(255, 255, 255));
 	BOOST_CHECK_EQUAL (s.front()->size_in_pixels(1080), 53);
-	BOOST_CHECK_EQUAL (s.front()->effect(), "border");
+	BOOST_CHECK_EQUAL (s.front()->effect(), libdcp::BORDER);
 	BOOST_CHECK_EQUAL (s.front()->effect_color(), libdcp::Color(0, 0, 0));
 }