Merge branch 'master' of ssh://git.carlh.net/home/carl/git/dcpomatic

10 files changed, 173 insertions, 35 deletions

diff --git a/src/lib/player.cc b/src/lib/player.cc
index 07bb097c2..10c92c073 100644
--- a/src/lib/player.cc
+++ b/src/lib/player.cc
@@ -183,6 +183,7 @@ Player::playlist_content_changed (weak_ptr<Content> w, int property, bool freque
 
 	} else if (
 		property == SubtitleContentProperty::LINE_SPACING ||
+		property == SubtitleContentProperty::OUTLINE_WIDTH ||
 		property == SubtitleContentProperty::Y_SCALE
 		) {
 
@@ -648,7 +649,7 @@ Player::get_subtitles (DCPTime time, DCPTime length, bool starting, bool burnt,
 				}
 				s.set_in (dcp::Time(content_subtitle_to_dcp (*j, ts.period().from).seconds(), 1000));
 				s.set_out (dcp::Time(content_subtitle_to_dcp (*j, ts.period().to).seconds(), 1000));
-				ps.text.push_back (s);
+				ps.text.push_back (SubtitleString (s, (*j)->content->subtitle->outline_width()));
 				ps.add_fonts ((*j)->content->subtitle->fonts ());
 			}
 		}
diff --git a/src/lib/player_subtitles.h b/src/lib/player_subtitles.h
index 9e50ea777..a202cf4f8 100644
--- a/src/lib/player_subtitles.h
+++ b/src/lib/player_subtitles.h
@@ -23,7 +23,7 @@
 
 #include "image_subtitle.h"
 #include "dcpomatic_time.h"
-#include <dcp/subtitle_string.h>
+#include "subtitle_string.h"
 
 class Font;
 
@@ -41,7 +41,7 @@ public:
 
 	/** ImageSubtitles, with their rectangles transformed as specified by their content */
 	std::list<ImageSubtitle> image;
-	std::list<dcp::SubtitleString> text;
+	std::list<SubtitleString> text;
 };
 
 #endif
diff --git a/src/lib/reel_writer.cc b/src/lib/reel_writer.cc
index 72e10e86b..d742818ae 100644
--- a/src/lib/reel_writer.cc
+++ b/src/lib/reel_writer.cc
@@ -491,7 +491,7 @@ ReelWriter::write (PlayerSubtitles subs)
 		}
 	}
 
-	BOOST_FOREACH (dcp::SubtitleString i, subs.text) {
+	BOOST_FOREACH (SubtitleString i, subs.text) {
 		i.set_in  (i.in()  - dcp::Time (_period.from.seconds(), i.in().tcr));
 		i.set_out (i.out() - dcp::Time (_period.from.seconds(), i.out().tcr));
 		_subtitle_asset->add (i);
diff --git a/src/lib/render_subtitles.cc b/src/lib/render_subtitles.cc
index 6e4dcf46a..c99827d10 100644
--- a/src/lib/render_subtitles.cc
+++ b/src/lib/render_subtitles.cc
@@ -45,13 +45,13 @@ static FcConfig* fc_config = 0;
 static list<pair<FontFiles, string> > fc_config_fonts;
 
 string
-marked_up (list<dcp::SubtitleString> subtitles)
+marked_up (list<SubtitleString> subtitles)
 {
 	string out;
 	bool italic = false;
 	bool bold = false;
 	bool underline = false;
-	BOOST_FOREACH (dcp::SubtitleString const & i, subtitles) {
+	BOOST_FOREACH (SubtitleString const & i, subtitles) {
 
 		if (i.italic() && !italic) {
 			out += "<i>";
@@ -96,7 +96,7 @@ marked_up (list<dcp::SubtitleString> subtitles)
  *  at the same time and with the same fade in/out.
  */
 static PositionImage
-render_line (list<dcp::SubtitleString> subtitles, list<shared_ptr<Font> > fonts, dcp::Size target, DCPTime time)
+render_line (list<SubtitleString> subtitles, list<shared_ptr<Font> > fonts, dcp::Size target, DCPTime time)
 {
 	/* XXX: this method can only handle italic / bold changes mid-line,
 	   nothing else yet.
@@ -285,8 +285,7 @@ render_line (list<dcp::SubtitleString> subtitles, list<shared_ptr<Font> > fonts,
 		/* Border effect; stroke the subtitle with a large (arbitrarily chosen) line width */
 		dcp::Colour ec = subtitles.front().effect_colour ();
 		context->set_source_rgba (float(ec.r) / 255, float(ec.g) / 255, float(ec.b) / 255, fade_factor);
-		/* This 300.0 is a magic number chosen to make the outline look good */
-		context->set_line_width (target.width / 300.0);
+		context->set_line_width (subtitles.front().outline_width * target.width / 2048.0);
 		context->set_line_join (Cairo::LINE_JOIN_ROUND);
 		context->move_to (x_offset, 0);
 		layout->add_to_cairo_context (context);
@@ -350,12 +349,12 @@ render_line (list<dcp::SubtitleString> subtitles, list<shared_ptr<Font> > fonts,
 
 /** @param time Time of the frame that these subtitles are going on */
 list<PositionImage>
-render_subtitles (list<dcp::SubtitleString> subtitles, list<shared_ptr<Font> > fonts, dcp::Size target, DCPTime time)
+render_subtitles (list<SubtitleString> subtitles, list<shared_ptr<Font> > fonts, dcp::Size target, DCPTime time)
 {
-	list<dcp::SubtitleString> pending;
+	list<SubtitleString> pending;
 	list<PositionImage> images;
 
-	BOOST_FOREACH (dcp::SubtitleString const & i, subtitles) {
+	BOOST_FOREACH (SubtitleString const & i, subtitles) {
 		if (!pending.empty() && fabs (i.v_position() - pending.back().v_position()) > 1e-4) {
 			images.push_back (render_line (pending, fonts, target, time));
 			pending.clear ();
diff --git a/src/lib/render_subtitles.h b/src/lib/render_subtitles.h
index 79194568a..f81685a28 100644
--- a/src/lib/render_subtitles.h
+++ b/src/lib/render_subtitles.h
@@ -20,12 +20,12 @@
 
 #include "position_image.h"
 #include "dcpomatic_time.h"
-#include <dcp/subtitle_string.h>
+#include "subtitle_string.h"
 #include <dcp/util.h>
 
 class Font;
 
-std::string marked_up (std::list<dcp::SubtitleString> subtitles);
+std::string marked_up (std::list<SubtitleString> subtitles);
 std::list<PositionImage> render_subtitles (
-	std::list<dcp::SubtitleString>, std::list<boost::shared_ptr<Font> > fonts, dcp::Size, DCPTime
+	std::list<SubtitleString>, std::list<boost::shared_ptr<Font> > fonts, dcp::Size, DCPTime
 	);
diff --git a/src/lib/subtitle_content.cc b/src/lib/subtitle_content.cc
index 34ffa3f4b..37d431e1a 100644
--- a/src/lib/subtitle_content.cc
+++ b/src/lib/subtitle_content.cc
@@ -54,6 +54,7 @@ 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;
 
 SubtitleContent::SubtitleContent (Content* parent)
 	: ContentPart (parent)
@@ -68,6 +69,7 @@ SubtitleContent::SubtitleContent (Content* parent)
 	, _shadow (false)
 	, _effect_colour (0, 0, 0)
 	, _line_spacing (1)
+	, _outline_width (2)
 {
 
 }
@@ -111,6 +113,7 @@ SubtitleContent::SubtitleContent (Content* parent, cxml::ConstNodePtr node, int
 	, _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))
+	, _outline_width (node->optional_number_child<int>("OutlineWidth").get_value_or (2))
 {
 	if (version >= 32) {
 		_use = node->bool_child ("UseSubtitles");
@@ -196,6 +199,10 @@ SubtitleContent::SubtitleContent (Content* parent, vector<shared_ptr<Content> >
 			throw JoinError (_("Content to be joined must have the same subtitle fades."));
 		}
 
+		if ((c[i]->subtitle->outline_width() != ref->outline_width())) {
+			throw JoinError (_("Content to be joined must have the same outline width."));
+		}
+
 		list<shared_ptr<Font> > fonts = c[i]->subtitle->fonts ();
 		if (fonts.size() != ref_fonts.size()) {
 			throw JoinError (_("Content to be joined must use the same fonts."));
@@ -224,6 +231,7 @@ SubtitleContent::SubtitleContent (Content* parent, vector<shared_ptr<Content> >
 	_line_spacing = ref->line_spacing ();
 	_fade_in = ref->fade_in ();
 	_fade_out = ref->fade_out ();
+	_outline_width = ref->outline_width ();
 
 	connect_to_fonts ();
 }
@@ -252,6 +260,7 @@ SubtitleContent::as_xml (xmlpp::Node* root) const
 	root->add_child("LineSpacing")->add_child_text (raw_convert<string> (_line_spacing));
 	root->add_child("SubtitleFadeIn")->add_child_text (raw_convert<string> (_fade_in.get()));
 	root->add_child("SubtitleFadeOut")->add_child_text (raw_convert<string> (_fade_out.get()));
+	root->add_child("OutlineWidth")->add_child_text (raw_convert<string> (_outline_width));
 
 	for (list<shared_ptr<Font> >::const_iterator i = _fonts.begin(); i != _fonts.end(); ++i) {
 		(*i)->as_xml (root->add_child("Font"));
@@ -267,7 +276,8 @@ SubtitleContent::identifier () const
 		+ "_" + raw_convert<string> (y_offset())
 		+ "_" + raw_convert<string> (line_spacing())
 		+ "_" + raw_convert<string> (fade_in().get())
-		+ "_" + raw_convert<string> (fade_out().get());
+		+ "_" + raw_convert<string> (fade_out().get())
+		+ "_" + raw_convert<string> (outline_width());
 
 	/* XXX: I suppose really _fonts shouldn't be in here, since not all
 	   types of subtitle content involve fonts.
@@ -397,6 +407,12 @@ SubtitleContent::set_fade_out (ContentTime t)
 }
 
 void
+SubtitleContent::set_outline_width (int w)
+{
+	maybe_set (_outline_width, w, SubtitleContentProperty::OUTLINE_WIDTH);
+}
+
+void
 SubtitleContent::use_template (shared_ptr<const SubtitleContent> c)
 {
 	_use = c->_use;
@@ -413,4 +429,5 @@ SubtitleContent::use_template (shared_ptr<const SubtitleContent> c)
 	_line_spacing = c->_line_spacing;
 	_fade_in = c->_fade_in;
 	_fade_out = c->_fade_out;
+	_outline_width = c->_outline_width;
 }
diff --git a/src/lib/subtitle_content.h b/src/lib/subtitle_content.h
index ddc97ce9f..7278b5408 100644
--- a/src/lib/subtitle_content.h
+++ b/src/lib/subtitle_content.h
@@ -46,6 +46,7 @@ public:
 	static int const LINE_SPACING;
 	static int const FADE_IN;
 	static int const FADE_OUT;
+	static int const OUTLINE_WIDTH;
 };
 
 /** @class SubtitleContent
@@ -80,6 +81,7 @@ public:
 	void set_line_spacing (double s);
 	void set_fade_in (ContentTime);
 	void set_fade_out (ContentTime);
+	void set_outline_width (int);
 
 	bool use () const {
 		boost::mutex::scoped_lock lm (_mutex);
@@ -156,6 +158,11 @@ public:
 		return _fade_out;
 	}
 
+	int outline_width () const {
+		boost::mutex::scoped_lock lm (_mutex);
+		return _outline_width;
+	}
+
 	static boost::shared_ptr<SubtitleContent> from_xml (Content* parent, cxml::ConstNodePtr, int version);
 
 protected:
@@ -194,6 +201,7 @@ private:
 	double _line_spacing;
 	ContentTime _fade_in;
 	ContentTime _fade_out;
+	int _outline_width;
 };
 
 #endif
diff --git a/src/lib/subtitle_string.h b/src/lib/subtitle_string.h
new file mode 100644
index 000000000..c1b0b14a8
--- /dev/null
+++ b/src/lib/subtitle_string.h
@@ -0,0 +1,46 @@
+/*
+    Copyright (C) 2016 Carl Hetherington <cth@carlh.net>
+
+    This file is part of DCP-o-matic.
+
+    DCP-o-matic is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    DCP-o-matic is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with DCP-o-matic.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+#ifndef DCPOMATIC_SUBTITLE_STRING_H
+#define DCPOMATIC_SUBTITLE_STRING_H
+
+#include <dcp/subtitle_string.h>
+
+/** A wrapper for SubtitleString which allows us to include settings that are not
+ *  applicable to true DCP subtitles.  For example, we can set outline width for burn-in
+ *  but this cannot be specified in DCP XML.
+ */
+class SubtitleString : public dcp::SubtitleString
+{
+public:
+	SubtitleString (dcp::SubtitleString dcp_)
+		: dcp::SubtitleString (dcp_)
+		, outline_width (2)
+	{}
+
+	SubtitleString (dcp::SubtitleString dcp_, int outline_width_)
+		: dcp::SubtitleString (dcp_)
+		, outline_width (outline_width_)
+	{}
+
+	int outline_width;
+};
+
+#endif
diff --git a/src/wx/text_subtitle_appearance_dialog.cc b/src/wx/text_subtitle_appearance_dialog.cc
index d00dd2cf1..6fc21bd6b 100644
--- a/src/wx/text_subtitle_appearance_dialog.cc
+++ b/src/wx/text_subtitle_appearance_dialog.cc
@@ -23,51 +23,91 @@
 #include "lib/subtitle_content.h"
 #include <wx/wx.h>
 #include <wx/clrpicker.h>
+#include <wx/spinctrl.h>
+#include <wx/gbsizer.h>
 
 using boost::shared_ptr;
+using boost::bind;
+
+int const TextSubtitleAppearanceDialog::NONE = 0;
+int const TextSubtitleAppearanceDialog::OUTLINE = 1;
+int const TextSubtitleAppearanceDialog::SHADOW = 2;
 
 TextSubtitleAppearanceDialog::TextSubtitleAppearanceDialog (wxWindow* parent, shared_ptr<Content> content)
-	: TableDialog (parent, _("Subtitle appearance"), 2, 1, true)
+	: wxDialog (parent, wxID_ANY, _("Subtitle appearance"))
 	, _content (content)
 {
-	add (_("Colour"), true);
+	wxSizer* overall_sizer = new wxBoxSizer (wxVERTICAL);
+	SetSizer (overall_sizer);
+
+	_table = new wxGridBagSizer (DCPOMATIC_SIZER_X_GAP, DCPOMATIC_SIZER_Y_GAP);
+
+	overall_sizer->Add (_table, 1, wxEXPAND | wxALL, DCPOMATIC_DIALOG_BORDER);
+
+	int r = 0;
+
+	add_label_to_sizer (_table, this, _("Colour"), true, wxGBPosition (r, 0));
 	_colour = new wxColourPickerCtrl (this, wxID_ANY);
-	add (_colour);
+	_table->Add (_colour, wxGBPosition (r, 1));
+	++r;
 
-	add (_("Effect"), true);
-	add (_effect = new wxChoice (this, wxID_ANY));
+	add_label_to_sizer (_table, this, _("Effect"), true, wxGBPosition (r, 0));
+	_effect = new wxChoice (this, wxID_ANY);
+	_table->Add (_effect, wxGBPosition (r, 1));
+	++r;
 
-	add (_("Outline / shadow colour"), true);
+	add_label_to_sizer (_table, this, _("Effect colour"), true, wxGBPosition (r, 0));
 	_effect_colour = new wxColourPickerCtrl (this, wxID_ANY);
-	add (_effect_colour);
+	_table->Add (_effect_colour, wxGBPosition (r, 1));
+	++r;
 
-	add (_("Fade in time"), true);
+	add_label_to_sizer (_table, this, _("Outline width"), true, wxGBPosition (r, 0));
+	_outline_width = new wxSpinCtrl (this, wxID_ANY);
+	_table->Add (_outline_width, wxGBPosition (r, 1));
+	++r;
+
+	add_label_to_sizer (_table, this, _("Fade in time"), true, wxGBPosition (r, 0));
 	_fade_in = new Timecode<ContentTime> (this);
-	add (_fade_in);
+	_table->Add (_fade_in, wxGBPosition (r, 1));
+	++r;
 
-	add (_("Fade out time"), true);
+	add_label_to_sizer (_table, this, _("Fade out time"), true, wxGBPosition (r, 0));
 	_fade_out = new Timecode<ContentTime> (this);
-	add (_fade_out);
+	_table->Add (_fade_out, wxGBPosition (r, 1));
+	++r;
+
+	wxSizer* buttons = CreateSeparatedButtonSizer (wxOK);
+	if (buttons) {
+		overall_sizer->Add (buttons, wxSizerFlags().Expand().DoubleBorder());
+	}
 
-	layout ();
+	overall_sizer->Layout ();
+	overall_sizer->SetSizeHints (this);
 
+	/* Keep these Appends() up to date with NONE/OUTLINE/SHADOW variables */
 	_effect->Append (_("None"));
 	_effect->Append (_("Outline"));
 	_effect->Append (_("Shadow"));;
 
 	_colour->SetColour (wxColour (_content->subtitle->colour().r, _content->subtitle->colour().g, _content->subtitle->colour().b));
 	if (_content->subtitle->outline()) {
-		_effect->SetSelection (1);
+		_effect->SetSelection (OUTLINE);
 	} else if (_content->subtitle->shadow()) {
-		_effect->SetSelection (2);
+		_effect->SetSelection (SHADOW);
 	} else {
-		_effect->SetSelection (0);
+		_effect->SetSelection (NONE);
 	}
 	_effect_colour->SetColour (
 		wxColour (_content->subtitle->effect_colour().r, _content->subtitle->effect_colour().g, _content->subtitle->effect_colour().b)
 		);
 	_fade_in->set (_content->subtitle->fade_in(), _content->active_video_frame_rate ());
 	_fade_out->set (_content->subtitle->fade_out(), _content->active_video_frame_rate ());
+	_outline_width->SetValue (_content->subtitle->outline_width ());
+
+	_effect->Bind (wxEVT_COMMAND_CHOICE_SELECTED, bind (&TextSubtitleAppearanceDialog::setup_sensitivity, this));
+	_content_connection = _content->Changed.connect (bind (&TextSubtitleAppearanceDialog::setup_sensitivity, this));
+
+	setup_sensitivity ();
 }
 
 void
@@ -75,10 +115,25 @@ TextSubtitleAppearanceDialog::apply ()
 {
 	wxColour const c = _colour->GetColour ();
 	_content->subtitle->set_colour (dcp::Colour (c.Red(), c.Green(), c.Blue()));
-	_content->subtitle->set_outline (_effect->GetSelection() == 1);
-	_content->subtitle->set_shadow (_effect->GetSelection() == 2);
+	_content->subtitle->set_outline (_effect->GetSelection() == OUTLINE);
+	_content->subtitle->set_shadow (_effect->GetSelection() == SHADOW);
 	wxColour const ec = _effect_colour->GetColour ();
 	_content->subtitle->set_effect_colour (dcp::Colour (ec.Red(), ec.Green(), ec.Blue()));
 	_content->subtitle->set_fade_in (_fade_in->get (_content->active_video_frame_rate ()));
 	_content->subtitle->set_fade_out (_fade_out->get (_content->active_video_frame_rate ()));
+	_content->subtitle->set_outline_width (_outline_width->GetValue ());
+}
+
+void
+TextSubtitleAppearanceDialog::setup_sensitivity ()
+{
+	_effect_colour->Enable (_effect->GetSelection() != NONE);
+
+	bool const can_outline_width = _effect->GetSelection() == OUTLINE && _content->subtitle->burn ();
+	_outline_width->Enable (can_outline_width);
+	if (can_outline_width) {
+		_outline_width->UnsetToolTip ();
+	} else {
+		_outline_width->SetToolTip (_("Outline width cannot be set unless you are burning in subtitles"));
+	}
 }
diff --git a/src/wx/text_subtitle_appearance_dialog.h b/src/wx/text_subtitle_appearance_dialog.h
index 381b36ec7..10169fb50 100644
--- a/src/wx/text_subtitle_appearance_dialog.h
+++ b/src/wx/text_subtitle_appearance_dialog.h
@@ -18,15 +18,17 @@
 
 */
 
-#include "table_dialog.h"
 #include "timecode.h"
+#include <wx/wx.h>
 #include <boost/shared_ptr.hpp>
+#include <boost/signals2.hpp>
 
 class wxRadioButton;
 class wxColourPickerCtrl;
+class wxGridBagSizer;
 class Content;
 
-class TextSubtitleAppearanceDialog : public TableDialog
+class TextSubtitleAppearanceDialog : public wxDialog
 {
 public:
 	TextSubtitleAppearanceDialog (wxWindow* parent, boost::shared_ptr<Content> content);
@@ -34,11 +36,21 @@ public:
 	void apply ();
 
 private:
+	void setup_sensitivity ();
+
 	wxColourPickerCtrl* _colour;
 	wxChoice* _effect;
 	wxColourPickerCtrl* _effect_colour;
 	Timecode<ContentTime>* _fade_in;
 	Timecode<ContentTime>* _fade_out;
+	wxSpinCtrl* _outline_width;
+	wxGridBagSizer* _table;
 
 	boost::shared_ptr<Content> _content;
+
+	boost::signals2::scoped_connection _content_connection;
+
+	static int const NONE;
+	static int const OUTLINE;
+	static int const SHADOW;
 };