Use an enum for the effect in SubtitleContent.

4 files changed, 47 insertions, 43 deletions

diff --git a/src/lib/player.cc b/src/lib/player.cc
index 499aa2a38..8e56991f8 100644
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -186,8 +186,7 @@ Player::playlist_content_changed (weak_ptr<Content> w, int property, bool freque
 		property == DCPContentProperty::NEEDS_ASSETS ||
 		property == DCPContentProperty::NEEDS_KDM ||
 		property == SubtitleContentProperty::COLOUR ||
-		property == SubtitleContentProperty::OUTLINE ||
-		property == SubtitleContentProperty::SHADOW ||
+		property == SubtitleContentProperty::EFFECT ||
 		property == SubtitleContentProperty::EFFECT_COLOUR ||
 		property == FFmpegContentProperty::SUBTITLE_STREAM ||
 		property == FFmpegContentProperty::FILTERS ||
diff --git a/src/lib/subtitle_content.cc b/src/lib/subtitle_content.cc
index 0083ff8a2..df05b356a 100644
--- a/src/lib/subtitle_content.cc
+++ b/src/lib/subtitle_content.cc
@@ -49,13 +49,12 @@ int const SubtitleContentProperty::BURN = 505;
 int const SubtitleContentProperty::LANGUAGE = 506;
 int const SubtitleContentProperty::FONTS = 507;
 int const SubtitleContentProperty::COLOUR = 508;
-int const SubtitleContentProperty::OUTLINE = 509;
-int const SubtitleContentProperty::SHADOW = 510;
-int const SubtitleContentProperty::EFFECT_COLOUR = 511;
-int const SubtitleContentProperty::LINE_SPACING = 512;
-int const SubtitleContentProperty::FADE_IN = 513;
-int const SubtitleContentProperty::FADE_OUT = 514;
-int const SubtitleContentProperty::OUTLINE_WIDTH = 515;
+int const SubtitleContentProperty::EFFECT = 509;
+int const SubtitleContentProperty::EFFECT_COLOUR = 510;
+int const SubtitleContentProperty::LINE_SPACING = 511;
+int const SubtitleContentProperty::FADE_IN = 512;
+int const SubtitleContentProperty::FADE_OUT = 513;
+int const SubtitleContentProperty::OUTLINE_WIDTH = 514;
 
 SubtitleContent::SubtitleContent (Content* parent)
 	: ContentPart (parent)
@@ -65,8 +64,7 @@ SubtitleContent::SubtitleContent (Content* parent)
 	, _y_offset (0)
 	, _x_scale (1)
 	, _y_scale (1)
-	, _outline (false)
-	, _shadow (false)
+	, _effect (dcp::NONE)
 	, _line_spacing (1)
 	, _outline_width (2)
 {
@@ -102,8 +100,6 @@ SubtitleContent::SubtitleContent (Content* parent, cxml::ConstNodePtr node, int
 	, _y_offset (0)
 	, _x_scale (1)
 	, _y_scale (1)
-	, _outline (node->optional_bool_child("Outline").get_value_or(false))
-	, _shadow (node->optional_bool_child("Shadow").get_value_or(false))
 	, _line_spacing (node->optional_number_child<double>("LineSpacing").get_value_or (1))
 	, _fade_in (node->optional_number_child<Frame>("SubtitleFadeIn").get_value_or (0))
 	, _fade_out (node->optional_number_child<Frame>("SubtitleFadeOut").get_value_or (0))
@@ -121,6 +117,25 @@ SubtitleContent::SubtitleContent (Content* parent, cxml::ConstNodePtr node, int
 		_y_offset = node->number_child<double> ("SubtitleOffset");
 	}
 
+	if (node->optional_bool_child("Outline").get_value_or(false)) {
+		_effect = dcp::BORDER;
+	} else if (node->optional_bool_child("Shadow").get_value_or(false)) {
+		_effect = dcp::SHADOW;
+	} else {
+		_effect = dcp::NONE;
+	}
+
+	optional<string> effect = node->optional_string_child("Effect");
+	if (effect) {
+		if (*effect == "none") {
+			_effect = dcp::NONE;
+		} else if (*effect == "outline") {
+			_effect = dcp::BORDER;
+		} else if (*effect == "shadow") {
+			_effect = dcp::SHADOW;
+		}
+	}
+
 	if (version >= 10) {
 		_x_scale = node->number_child<double> ("SubtitleXScale");
 		_y_scale = node->number_child<double> ("SubtitleYScale");
@@ -256,8 +271,17 @@ SubtitleContent::as_xml (xmlpp::Node* root) const
 		root->add_child("Green")->add_child_text (raw_convert<string> (_colour->g));
 		root->add_child("Blue")->add_child_text (raw_convert<string> (_colour->b));
 	}
-	root->add_child("Outline")->add_child_text (_outline ? "1" : "0");
-	root->add_child("Shadow")->add_child_text (_shadow ? "1" : "0");
+	switch (_effect) {
+	case dcp::NONE:
+		root->add_child("none");
+		break;
+	case dcp::BORDER:
+		root->add_child("outline");
+		break;
+	case dcp::SHADOW:
+		root->add_child("shadow");
+		break;
+	}
 	if (_effect_colour) {
 		root->add_child("EffectRed")->add_child_text (raw_convert<string> (_effect_colour->r));
 		root->add_child("EffectGreen")->add_child_text (raw_convert<string> (_effect_colour->g));
@@ -341,15 +365,9 @@ SubtitleContent::unset_colour ()
 }
 
 void
-SubtitleContent::set_outline (bool o)
-{
-	maybe_set (_outline, o, SubtitleContentProperty::OUTLINE);
-}
-
-void
-SubtitleContent::set_shadow (bool s)
+SubtitleContent::set_effect (dcp::Effect e)
 {
-	maybe_set (_shadow, s, SubtitleContentProperty::SHADOW);
+	maybe_set (_effect, e, SubtitleContentProperty::EFFECT);
 }
 
 void
@@ -445,8 +463,7 @@ SubtitleContent::take_settings_from (shared_ptr<const SubtitleContent> c)
 	} else {
 		unset_colour ();
 	}
-	set_outline (c->_outline);
-	set_shadow (c->_shadow);
+	set_effect (c->_effect);
 	if (c->_effect_colour) {
 		set_effect_colour (*c->_effect_colour);
 	} else {
diff --git a/src/lib/subtitle_content.h b/src/lib/subtitle_content.h
index baf412bb6..73e53e446 100644
--- a/src/lib/subtitle_content.h
+++ b/src/lib/subtitle_content.h
@@ -40,8 +40,7 @@ public:
 	static int const LANGUAGE;
 	static int const FONTS;
 	static int const COLOUR;
-	static int const OUTLINE;
-	static int const SHADOW;
+	static int const EFFECT;
 	static int const EFFECT_COLOUR;
 	static int const LINE_SPACING;
 	static int const FADE_IN;
@@ -76,8 +75,7 @@ public:
 	void set_language (std::string language);
 	void set_colour (dcp::Colour);
 	void unset_colour ();
-	void set_outline (bool);
-	void set_shadow (bool);
+	void set_effect (dcp::Effect);
 	void set_effect_colour (dcp::Colour);
 	void unset_effect_colour ();
 	void set_line_spacing (double s);
@@ -130,14 +128,9 @@ public:
 		return _colour;
 	}
 
-	bool outline () const {
+	dcp::Effect effect () const {
 		boost::mutex::scoped_lock lm (_mutex);
-		return _outline;
-	}
-
-	bool shadow () const {
-		boost::mutex::scoped_lock lm (_mutex);
-		return _shadow;
+		return _effect;
 	}
 
 	boost::optional<dcp::Colour> effect_colour () const {
@@ -196,8 +189,7 @@ private:
 	double _y_scale;
 	std::list<boost::shared_ptr<Font> > _fonts;
 	boost::optional<dcp::Colour> _colour;
-	bool _outline;
-	bool _shadow;
+	dcp::Effect _effect;
 	boost::optional<dcp::Colour> _effect_colour;
 	/** scaling factor for line spacing; 1 is "standard", < 1 is closer together, > 1 is further apart */
 	double _line_spacing;
diff --git a/src/lib/subtitle_decoder.cc b/src/lib/subtitle_decoder.cc
index 8badf5183..90bedf0ce 100644
--- a/src/lib/subtitle_decoder.cc
+++ b/src/lib/subtitle_decoder.cc
@@ -82,11 +82,7 @@ SubtitleDecoder::emit_text_start (ContentTime from, list<dcp::SubtitleString> s)
 		if (content()->effect_colour()) {
 			i.set_effect_colour (*content()->effect_colour());
 		}
-		if (content()->outline()) {
-			i.set_effect (dcp::BORDER);
-		} else if (content()->shadow()) {
-			i.set_effect (dcp::SHADOW);
-		}
+		i.set_effect (content()->effect());
 		i.set_fade_up_time (dcp::Time(content()->fade_in().seconds(), 1000));
 		i.set_fade_down_time (dcp::Time(content()->fade_out().seconds(), 1000));
 	}