diff --git a/Makefile b/Makefile index e8957a80d..33de62e87 100755 --- a/Makefile +++ b/Makefile @@ -27,4 +27,7 @@ pep8: @pep8 -r --select=W293 -q --filename=*.py `pwd`/tests/ | xargs gsed -i 's/^[ \r\t]*$//' @pep8 -r --select=W391 -q --filename=*.py `pwd`/tests/ | xargs gsed -i -e :a -e '/^\n*$/{$d;N;ba' -e '}' +grind: + @valgrind --leak-check=full tests/cpp_tests/font_registration_test + .PHONY: clean reset uninstall test install diff --git a/SConstruct b/SConstruct index 60352732c..c014b9bb5 100644 --- a/SConstruct +++ b/SConstruct @@ -1716,7 +1716,7 @@ if not HELP_REQUESTED: # build C++ tests # not ready for release - #SConscript('tests/cpp_tests/build.py') + SConscript('tests/cpp_tests/build.py') # not ready for release #if env['SVG_RENDERER']: diff --git a/src/font_engine_freetype.cpp b/src/font_engine_freetype.cpp index 8e0746f29..cef4551d8 100644 --- a/src/font_engine_freetype.cpp +++ b/src/font_engine_freetype.cpp @@ -71,11 +71,10 @@ bool freetype_engine::is_font_file(std::string const& file_name) bool freetype_engine::register_font(std::string const& file_name) { - if (!boost::filesystem::is_regular_file(file_name) || !is_font_file(file_name)) return false; #ifdef MAPNIK_THREADSAFE mutex::scoped_lock lock(mutex_); #endif - FT_Library library; + FT_Library library = 0; FT_Error error = FT_Init_FreeType(&library); if (error) { @@ -84,6 +83,7 @@ bool freetype_engine::register_font(std::string const& file_name) FT_Face face = 0; int num_faces = 0; + bool success = false; // some font files have multiple fonts in a file // the count is in the 'root' face library[0] // see the FT_FaceRec in freetype.h @@ -92,31 +92,37 @@ bool freetype_engine::register_font(std::string const& file_name) error = FT_New_Face (library,file_name.c_str(),i,&face); if (error) { - FT_Done_FreeType(library); - return false; + break; } // store num_faces locally, after FT_Done_Face it can not be accessed any more if (!num_faces) num_faces = face->num_faces; // some fonts can lack names, skip them // http://www.freetype.org/freetype2/docs/reference/ft2-base_interface.html#FT_FaceRec - if (face->family_name && face->style_name) { + if (face->family_name && face->style_name) + { + success = true; std::string name = std::string(face->family_name) + " " + std::string(face->style_name); name2file_.insert(std::make_pair(name, std::make_pair(i,file_name))); - FT_Done_Face(face); - //FT_Done_FreeType(library); - //return true; - } else { - FT_Done_Face(face); - FT_Done_FreeType(library); + } + else + { std::ostringstream s; - s << "Error: unable to load invalid font file which lacks identifiable family and style name: '" - << file_name << "'"; - throw std::runtime_error(s.str()); + s << "Warning: unable to load font file '" << file_name << "' "; + if (!face->family_name && !face->style_name) + s << "which lacks both a family name and style name"; + else if (face->family_name) + s << "which reports a family name of '" << std::string(face->family_name) << "' and lacks a style name"; + else if (face->style_name) + s << "which reports a style name of '" << std::string(face->style_name) << "' and lacks a family name"; + std::clog << s.str() << std::endl; } } - FT_Done_FreeType(library); - return true; + if (face) + FT_Done_Face(face); + if (library) + FT_Done_FreeType(library); + return success; } bool freetype_engine::register_fonts(std::string const& dir, bool recurse) @@ -130,26 +136,24 @@ bool freetype_engine::register_fonts(std::string const& dir, bool recurse) return mapnik::freetype_engine::register_font(dir); boost::filesystem::directory_iterator end_itr; + bool success = false; for (boost::filesystem::directory_iterator itr(dir); itr != end_itr; ++itr) { +#if (BOOST_FILESYSTEM_VERSION == 3) + std::string const& file_name = itr->path().string(); +#else // v2 + std::string const& file_name = itr->string(); +#endif if (boost::filesystem::is_directory(*itr) && recurse) { -#if (BOOST_FILESYSTEM_VERSION == 3) - if (!register_fonts(itr->path().string(), true)) return false; -#else // v2 - if (!register_fonts(itr->string(), true)) return false; -#endif + success = register_fonts(file_name, true); } - else + else if (boost::filesystem::is_regular_file(file_name) && is_font_file(file_name)) { -#if (BOOST_FILESYSTEM_VERSION == 3) - mapnik::freetype_engine::register_font(itr->path().string()); -#else // v2 - mapnik::freetype_engine::register_font(itr->string()); -#endif + success = mapnik::freetype_engine::register_font(file_name); } } - return true; + return success; } diff --git a/src/load_map.cpp b/src/load_map.cpp index ee4313d91..2682015fd 100644 --- a/src/load_map.cpp +++ b/src/load_map.cpp @@ -242,7 +242,13 @@ void map_parser::parse_map(Map & map, xml_node const& pt, std::string const& bas if (font_directory) { extra_attr["font-directory"] = *font_directory; - freetype_engine::register_fonts(ensure_relative_to_xml(font_directory), false); + if (!freetype_engine::register_fonts(ensure_relative_to_xml(font_directory), false)) + { + if (strict_) + { + throw config_error(std::string("Failed to load fonts from: ") << *font_directory); + } + } } optional min_version_string = map_node.get_opt_attr("minimum-version"); diff --git a/tests/cpp_tests/build.py b/tests/cpp_tests/build.py index 11cd33f45..bcc31bc94 100644 --- a/tests/cpp_tests/build.py +++ b/tests/cpp_tests/build.py @@ -11,6 +11,8 @@ headers = env['CPPPATH'] libraries = copy(env['LIBMAPNIK_LIBS']) libraries.append('mapnik') +test_env.Append(CXXFLAGS='-g') + for cpp_test in glob.glob('*_test.cpp'): test_program = test_env.Program(cpp_test.replace('.cpp',''), [cpp_test], CPPPATH=headers, LIBS=libraries, LINKFLAGS=env['CUSTOM_LDFLAGS']) Depends(test_program, env.subst('../../src/%s' % env['MAPNIK_LIB_NAME'])) diff --git a/tests/cpp_tests/font_registration_test.cpp b/tests/cpp_tests/font_registration_test.cpp index 1906d896b..55715b9b1 100644 --- a/tests/cpp_tests/font_registration_test.cpp +++ b/tests/cpp_tests/font_registration_test.cpp @@ -6,8 +6,6 @@ using fs::path; namespace sys = boost::system; #include -//#include -//#include #include #include @@ -39,14 +37,23 @@ int main( int, char*[] ) // directories without fonts std::string src("src"); - // a legitimate directory will return true even it is does not - // successfully register a font... - BOOST_TEST( mapnik::freetype_engine::register_fonts(src , true ) ); - face_names = mapnik::freetype_engine::face_names(); - BOOST_TEST( face_names.size() == 0 ); - std::clog << "number of registered fonts: " << face_names.size() << std::endl; + // an empty directory will not return true + // we need to register at least one font and not fail on any + // to return true + BOOST_TEST( mapnik::freetype_engine::register_font(src) == false ); + BOOST_TEST( mapnik::freetype_engine::register_fonts(src, true) == false ); + BOOST_TEST( mapnik::freetype_engine::face_names().size() == 0 ); - // register unifont + // bogus, emtpy file that looks like font + BOOST_TEST( mapnik::freetype_engine::register_font("tests/data/fonts/fake.ttf") == false ); + BOOST_TEST( mapnik::freetype_engine::register_fonts("tests/data/fonts/fake.ttf") == false ); + BOOST_TEST( mapnik::freetype_engine::face_names().size() == 0 ); + + BOOST_TEST( mapnik::freetype_engine::register_font("tests/data/fonts/intentionally-broken.ttf") == false ); + BOOST_TEST( mapnik::freetype_engine::register_fonts("tests/data/fonts/intentionally-broken.ttf") == false ); + BOOST_TEST( mapnik::freetype_engine::face_names().size() == 0 ); + + // register unifont, since we know it sits in the root fonts/ dir BOOST_TEST( mapnik::freetype_engine::register_fonts(fontdir) ); face_names = mapnik::freetype_engine::face_names(); std::clog << "number of registered fonts: " << face_names.size() << std::endl; @@ -71,7 +78,7 @@ int main( int, char*[] ) BOOST_TEST( mapnik::freetype_engine::register_fonts(fontdir, true) ); face_names = mapnik::freetype_engine::face_names(); std::clog << "number of registered fonts: " << face_names.size() << std::endl; - BOOST_TEST( face_names.size() == 21 ); + BOOST_TEST( face_names.size() == 22 ); return ::boost::report_errors(); } diff --git a/tests/data/fonts/fake.ttf b/tests/data/fonts/fake.ttf new file mode 100644 index 000000000..e69de29bb diff --git a/tests/data/fonts/intentionally-broken.ttf b/tests/data/fonts/intentionally-broken.ttf new file mode 100644 index 000000000..887230d76 Binary files /dev/null and b/tests/data/fonts/intentionally-broken.ttf differ diff --git a/tests/data/good_maps/rtl_text_map.xml b/tests/data/good_maps/rtl_text_map.xml index 3892c69af..38f46aca1 100644 --- a/tests/data/good_maps/rtl_text_map.xml +++ b/tests/data/good_maps/rtl_text_map.xml @@ -1,5 +1,7 @@ - + + +