From 9496a95af4ad139e9f099489e99f9008ace0e924 Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 23 Jun 2014 15:14:54 +0100 Subject: [PATCH 01/10] use C-style file I/O --- src/font_engine_freetype.cpp | 37 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/src/font_engine_freetype.cpp b/src/font_engine_freetype.cpp index 3d60fa25cd..fe958cb567 100644 --- a/src/font_engine_freetype.cpp +++ b/src/font_engine_freetype.cpp @@ -102,17 +102,17 @@ bool freetype_engine::is_font_file(std::string const& file_name) boost::algorithm::ends_with(fn,std::string(".dfont")); } -unsigned long ft_read_cb(FT_Stream stream, unsigned long offset, unsigned char *buffer, unsigned long count) { - if (count <= 0) return count; - std::ifstream * file = static_cast(stream->descriptor.pointer); - file->seekg(offset, std::ios::beg); - file->read((char*)buffer, count); - return file->gcount(); +unsigned long ft_read_cb(FT_Stream stream, unsigned long offset, unsigned char *buffer, unsigned long count) +{ + if (count <= 0) return 0; + FILE * file = static_cast(stream->descriptor.pointer); + fseek (file , offset , SEEK_SET); + return fread ((char*)buffer, 1, count, file); } -void ft_close_cb(FT_Stream stream) { - std::ifstream * file = static_cast(stream->descriptor.pointer); - file->close(); +void ft_close_cb(FT_Stream stream) +{ + fclose (static_cast(stream->descriptor.pointer)); } bool freetype_engine::register_font(std::string const& file_name) @@ -132,28 +132,23 @@ bool freetype_engine::register_font(std::string const& file_name) bool freetype_engine::register_font_impl(std::string const& file_name, FT_LibraryRec_ * library) { #ifdef _WINDOWS - std::ifstream file(mapnik::utf8_to_utf16(file_name), std::ios::binary); + FILE * file = fopen(mapnik::utf8_to_utf16(file_name).c_str(),"rb"); #else - std::ifstream file(file_name.c_str(), std::ios::binary); + FILE * file = fopen(file_name.c_str(),"rb"); #endif - if (!file.good()) { - return false; - } + if (file == nullptr) return false; FT_Face face = 0; FT_Open_Args args; FT_StreamRec streamRec; memset(&args, 0, sizeof(args)); memset(&streamRec, 0, sizeof(streamRec)); - std::streampos beg = file.tellg(); - file.seekg (0, std::ios::end); - std::streampos end = file.tellg(); - std::size_t file_size = end - beg; - file.seekg (0, std::ios::beg); - + fseek (file , 0 , SEEK_END); + std::size_t file_size = ftell(file); + rewind(file); streamRec.base = 0; streamRec.pos = 0; streamRec.size = file_size; - streamRec.descriptor.pointer = &file; + streamRec.descriptor.pointer = file; streamRec.read = ft_read_cb; streamRec.close = ft_close_cb; args.flags = FT_OPEN_STREAM; From 4acba303a5e51e483781c32885f26bad65bb6b80 Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 23 Jun 2014 16:56:59 +0100 Subject: [PATCH 02/10] + use C-style I/O to read font file into memory --- src/font_engine_freetype.cpp | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/font_engine_freetype.cpp b/src/font_engine_freetype.cpp index fe958cb567..9758d0c58d 100644 --- a/src/font_engine_freetype.cpp +++ b/src/font_engine_freetype.cpp @@ -320,21 +320,27 @@ face_ptr freetype_engine::create_face(std::string const& family_name) #ifdef MAPNIK_THREADSAFE mapnik::scoped_lock lock(mutex_); #endif - std::ifstream is(itr->second.second.c_str() , std::ios::binary); - std::string buffer((std::istreambuf_iterator(is)), - std::istreambuf_iterator()); - auto result = memory_fonts_.insert(std::make_pair(itr->second.second, buffer)); - - FT_Error error = FT_New_Memory_Face (library_, - reinterpret_cast(result.first->second.c_str()), - static_cast(buffer.size()), - itr->second.first, - &face); - if (!error) return std::make_shared(face); - else + FILE * file = fopen(itr->second.second.c_str(),"rb"); + if (file != nullptr) { - // we can't load font, erase it. - memory_fonts_.erase(result.first); + fseek(file, 0, SEEK_END); + unsigned long file_size = ftell(file); + fseek(file, 0, SEEK_SET); + boost::scoped_array buffer(new char[file_size]); + fread(buffer.get(), 1, file_size, file); + auto result = memory_fonts_.insert(std::make_pair(itr->second.second, std::string(buffer.get()))); + + FT_Error error = FT_New_Memory_Face (library_, + reinterpret_cast(result.first->second.c_str()), + static_cast(result.first->second.size()), + itr->second.first, + &face); + if (!error) return std::make_shared(face); + else + { + // we can't load font, erase it. + memory_fonts_.erase(result.first); + } } } } From 1c25c34dec826de0e403cfcc40d2606c0b8f2796 Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 23 Jun 2014 16:59:30 +0100 Subject: [PATCH 03/10] no sure if 'rewind' is a standard - use fseek instead --- src/font_engine_freetype.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/font_engine_freetype.cpp b/src/font_engine_freetype.cpp index 9758d0c58d..5dbf1bec9f 100644 --- a/src/font_engine_freetype.cpp +++ b/src/font_engine_freetype.cpp @@ -142,9 +142,9 @@ bool freetype_engine::register_font_impl(std::string const& file_name, FT_Librar FT_StreamRec streamRec; memset(&args, 0, sizeof(args)); memset(&streamRec, 0, sizeof(streamRec)); - fseek (file , 0 , SEEK_END); + fseek(file, 0, SEEK_END); std::size_t file_size = ftell(file); - rewind(file); + fseek(file, 0, SEEK_SET); streamRec.base = 0; streamRec.pos = 0; streamRec.size = file_size; From b1d8e06032b094fa23584912e62e4e8500149656 Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 23 Jun 2014 17:08:00 +0100 Subject: [PATCH 04/10] oops buffer.get() returns raw bytes (not `\0` terminated C-string) --- src/font_engine_freetype.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/font_engine_freetype.cpp b/src/font_engine_freetype.cpp index 5dbf1bec9f..e3d37cacea 100644 --- a/src/font_engine_freetype.cpp +++ b/src/font_engine_freetype.cpp @@ -328,7 +328,7 @@ face_ptr freetype_engine::create_face(std::string const& family_name) fseek(file, 0, SEEK_SET); boost::scoped_array buffer(new char[file_size]); fread(buffer.get(), 1, file_size, file); - auto result = memory_fonts_.insert(std::make_pair(itr->second.second, std::string(buffer.get()))); + auto result = memory_fonts_.insert(std::make_pair(itr->second.second, std::string(buffer.get(),file_size))); FT_Error error = FT_New_Memory_Face (library_, reinterpret_cast(result.first->second.c_str()), From 4d42df8f3c6b2c66567f0df7ddecc2743933aba7 Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 23 Jun 2014 17:11:53 +0100 Subject: [PATCH 05/10] ensure we read the whole file at once --- src/font_engine_freetype.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/font_engine_freetype.cpp b/src/font_engine_freetype.cpp index e3d37cacea..c5bf040897 100644 --- a/src/font_engine_freetype.cpp +++ b/src/font_engine_freetype.cpp @@ -327,7 +327,7 @@ face_ptr freetype_engine::create_face(std::string const& family_name) unsigned long file_size = ftell(file); fseek(file, 0, SEEK_SET); boost::scoped_array buffer(new char[file_size]); - fread(buffer.get(), 1, file_size, file); + fread(buffer.get(), file_size, 1, file); auto result = memory_fonts_.insert(std::make_pair(itr->second.second, std::string(buffer.get(),file_size))); FT_Error error = FT_New_Memory_Face (library_, From 284786ec16ee0b93b06c0924d7f21d30228d89b5 Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 23 Jun 2014 17:23:28 +0100 Subject: [PATCH 06/10] wrap FILE* in std::unique_ptr (RAII) pull C-i/o from std:: namespace --- src/font_engine_freetype.cpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/font_engine_freetype.cpp b/src/font_engine_freetype.cpp index c5bf040897..3dc5475985 100644 --- a/src/font_engine_freetype.cpp +++ b/src/font_engine_freetype.cpp @@ -106,13 +106,13 @@ unsigned long ft_read_cb(FT_Stream stream, unsigned long offset, unsigned char * { if (count <= 0) return 0; FILE * file = static_cast(stream->descriptor.pointer); - fseek (file , offset , SEEK_SET); - return fread ((char*)buffer, 1, count, file); + std::fseek (file , offset , SEEK_SET); + return std::fread ((char*)buffer, 1, count, file); } void ft_close_cb(FT_Stream stream) { - fclose (static_cast(stream->descriptor.pointer)); + std::fclose (static_cast(stream->descriptor.pointer)); } bool freetype_engine::register_font(std::string const& file_name) @@ -143,7 +143,7 @@ bool freetype_engine::register_font_impl(std::string const& file_name, FT_Librar memset(&args, 0, sizeof(args)); memset(&streamRec, 0, sizeof(streamRec)); fseek(file, 0, SEEK_END); - std::size_t file_size = ftell(file); + std::size_t file_size = std::ftell(file); fseek(file, 0, SEEK_SET); streamRec.base = 0; streamRec.pos = 0; @@ -320,16 +320,15 @@ face_ptr freetype_engine::create_face(std::string const& family_name) #ifdef MAPNIK_THREADSAFE mapnik::scoped_lock lock(mutex_); #endif - FILE * file = fopen(itr->second.second.c_str(),"rb"); + std::unique_ptr file(std::fopen(itr->second.second.c_str(),"rb"), std::fclose); if (file != nullptr) { - fseek(file, 0, SEEK_END); - unsigned long file_size = ftell(file); - fseek(file, 0, SEEK_SET); + std::fseek(file.get(), 0, SEEK_END); + unsigned long file_size = std::ftell(file.get()); + std::fseek(file.get(), 0, SEEK_SET); boost::scoped_array buffer(new char[file_size]); - fread(buffer.get(), file_size, 1, file); + std::fread(buffer.get(), file_size, 1, file.get()); auto result = memory_fonts_.insert(std::make_pair(itr->second.second, std::string(buffer.get(),file_size))); - FT_Error error = FT_New_Memory_Face (library_, reinterpret_cast(result.first->second.c_str()), static_cast(result.first->second.size()), From f4acd06f84496440d29eedfce9c32035dd4fb181 Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 23 Jun 2014 17:31:47 +0100 Subject: [PATCH 07/10] c++11 - use std::unique_ptr instead of boost::scoped_array (TODO - consider storing unique_ptr's in memory_fonts_ and avoid constructing std::string) --- src/font_engine_freetype.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/font_engine_freetype.cpp b/src/font_engine_freetype.cpp index 3dc5475985..9dc8310c48 100644 --- a/src/font_engine_freetype.cpp +++ b/src/font_engine_freetype.cpp @@ -324,9 +324,9 @@ face_ptr freetype_engine::create_face(std::string const& family_name) if (file != nullptr) { std::fseek(file.get(), 0, SEEK_END); - unsigned long file_size = std::ftell(file.get()); + std::size_t file_size = std::ftell(file.get()); std::fseek(file.get(), 0, SEEK_SET); - boost::scoped_array buffer(new char[file_size]); + std::unique_ptr buffer(new char[file_size]); std::fread(buffer.get(), file_size, 1, file.get()); auto result = memory_fonts_.insert(std::make_pair(itr->second.second, std::string(buffer.get(),file_size))); FT_Error error = FT_New_Memory_Face (library_, From 68775c52ffa8aceb5099cbd61d4ac30e81f5a9e0 Mon Sep 17 00:00:00 2001 From: artemp Date: Tue, 24 Jun 2014 11:21:01 +0100 Subject: [PATCH 08/10] call FT_Done_Face for matching FT_Open_Face --- src/font_engine_freetype.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/font_engine_freetype.cpp b/src/font_engine_freetype.cpp index 9dc8310c48..cefff19eb2 100644 --- a/src/font_engine_freetype.cpp +++ b/src/font_engine_freetype.cpp @@ -158,13 +158,11 @@ bool freetype_engine::register_font_impl(std::string const& file_name, FT_Librar // 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 - for ( int i = 0; face == 0 || i < num_faces; i++ ) { + for ( int i = 0; face == 0 || i < num_faces; ++i ) + { // if face is null then this is the first face FT_Error error = FT_Open_Face(library, &args, i, &face); - if (error) - { - break; - } + if (error) break; // store num_faces locally, after FT_Done_Face it can not be accessed any more if (!num_faces) num_faces = face->num_faces; @@ -193,10 +191,8 @@ bool freetype_engine::register_font_impl(std::string const& file_name, FT_Librar MAPNIK_LOG_ERROR(font_engine_freetype) << "register_font: " << s.str(); } + if (face) FT_Done_Face(face); } - - if (face) FT_Done_Face(face); - return success; } From 8255163945198da4455736df676196dc81180687 Mon Sep 17 00:00:00 2001 From: artemp Date: Tue, 24 Jun 2014 11:23:13 +0100 Subject: [PATCH 09/10] style --- src/font_engine_freetype.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/font_engine_freetype.cpp b/src/font_engine_freetype.cpp index cefff19eb2..ba5b68eba2 100644 --- a/src/font_engine_freetype.cpp +++ b/src/font_engine_freetype.cpp @@ -164,7 +164,7 @@ bool freetype_engine::register_font_impl(std::string const& file_name, FT_Librar FT_Error error = FT_Open_Face(library, &args, i, &face); if (error) break; // store num_faces locally, after FT_Done_Face it can not be accessed any more - if (!num_faces) + if (num_faces == 0) 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 From 45ba10e701360ae931c9162c8b166ed0899d9d7a Mon Sep 17 00:00:00 2001 From: artemp Date: Tue, 24 Jun 2014 11:40:04 +0100 Subject: [PATCH 10/10] use _wfopen in windows --- src/font_engine_freetype.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/font_engine_freetype.cpp b/src/font_engine_freetype.cpp index ba5b68eba2..e7b5aa1a52 100644 --- a/src/font_engine_freetype.cpp +++ b/src/font_engine_freetype.cpp @@ -132,9 +132,9 @@ bool freetype_engine::register_font(std::string const& file_name) bool freetype_engine::register_font_impl(std::string const& file_name, FT_LibraryRec_ * library) { #ifdef _WINDOWS - FILE * file = fopen(mapnik::utf8_to_utf16(file_name).c_str(),"rb"); + FILE * file = _wfopen(mapnik::utf8_to_utf16(file_name).c_str(),"rb"); #else - FILE * file = fopen(file_name.c_str(),"rb"); + FILE * file = std::fopen(file_name.c_str(),"rb"); #endif if (file == nullptr) return false; FT_Face face = 0; @@ -316,7 +316,12 @@ face_ptr freetype_engine::create_face(std::string const& family_name) #ifdef MAPNIK_THREADSAFE mapnik::scoped_lock lock(mutex_); #endif + +#ifdef _WINDOWS + std::unique_ptr file(_wfopen(mapnik::utf8_to_utf16(itr->second.second).c_str(),"rb"), fclose); +#else std::unique_ptr file(std::fopen(itr->second.second.c_str(),"rb"), std::fclose); +#endif if (file != nullptr) { std::fseek(file.get(), 0, SEEK_END);