-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[core] Replace boost iostream with std::{i,s}stream #5417
Conversation
👍 |
🙇 |
@@ -54,8 +44,8 @@ struct png_struct_guard { | |||
}; | |||
|
|||
PremultipliedImage decodePNG(const uint8_t* data, size_t size) { | |||
source_type source(reinterpret_cast<const char*>(data), size); | |||
input_stream stream(source); | |||
std::istringstream source(std::string(reinterpret_cast<const char*>(data), size)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ creates a new std::string and copies data!
Better solution is to implement custom stream buffer : https://artofcode.wordpress.com/2010/12/12/deriving-from-stdstreambuf/ |
@brunoabinader This is what II did in mapnik mapnik/mapnik@2e8c0d3 pos_type seekoff( off_type off, ios_base::seekdir dir,
ios_base::openmode which = ios_base::in | ios_base::out ); http://en.cppreference.com/w/cpp/io/basic_streambuf/pubseekoff |
@artemp I was wondering about that, thanks for the links. |
@artemp and @jfirebaugh Is the concern here that duplicate copy of image data in std::string can cause performance issue? This may not be perceptible to the end user at all esp for Vector maps where this code will kick in only once when loading the sprite. #3048 is more likely to be a performance bottleneck arising due to copying data multiple times (But it has not been measured). When we switched from Curl to OkHttp for network access, it introduced multiple copies of tile data as it moves across JNI boundary. (Issue #3048). |
Thank you @artemp - I wonder if we could add |
Implements a custom std::streambuf to avoid creating temporary std::string objects and thus optimizing image decode. Suggested by @artemp in #5417 (comment).
Fixes #4915.
👀 @jfirebaugh @springmeyer