Change PNG functions to return std::vector.
Currently, PNG functions pass the std::vector containing the data an an
out parameters, with a separate bool return value to indicate success or
failure. Instead of doing this, just return the std::vector and check
whether it contains data or not.
For affected functions that return std::vector, use the same variable in
all return paths to make it obvious to the compiler that it can apply
RVO and not create a copy.
Change-Id: Ia1f84a6be29fe2d86df800607a258747999b311f
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/56653
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/samples/pdfium_test_write_helper.cc b/samples/pdfium_test_write_helper.cc
index f18a791..c6f630a 100644
--- a/samples/pdfium_test_write_helper.cc
+++ b/samples/pdfium_test_write_helper.cc
@@ -130,38 +130,33 @@
return "";
}
-bool EncodePng(const unsigned char* buffer,
- int width,
- int height,
- int stride,
- int format,
- std::vector<unsigned char>* png_encoding) {
- bool ret = false;
+std::vector<unsigned char> EncodePng(const unsigned char* buffer,
+ int width,
+ int height,
+ int stride,
+ int format) {
+ std::vector<unsigned char> png;
switch (format) {
case FPDFBitmap_Unknown:
- ret = false;
break;
case FPDFBitmap_Gray:
- ret = image_diff_png::EncodeGrayPNG(buffer, width, height, stride,
- png_encoding);
+ png = image_diff_png::EncodeGrayPNG(buffer, width, height, stride);
break;
case FPDFBitmap_BGR:
- ret = image_diff_png::EncodeBGRPNG(buffer, width, height, stride,
- png_encoding);
+ png = image_diff_png::EncodeBGRPNG(buffer, width, height, stride);
break;
case FPDFBitmap_BGRx:
- ret = image_diff_png::EncodeBGRAPNG(buffer, width, height, stride, true,
- png_encoding);
+ png = image_diff_png::EncodeBGRAPNG(buffer, width, height, stride,
+ /*discard_transparency=*/true);
break;
case FPDFBitmap_BGRA:
- ret = image_diff_png::EncodeBGRAPNG(buffer, width, height, stride, false,
- png_encoding);
+ png = image_diff_png::EncodeBGRAPNG(buffer, width, height, stride,
+ /*discard_transparency=*/false);
break;
default:
NOTREACHED();
}
-
- return ret;
+ return png;
}
#ifdef _WIN32
@@ -386,11 +381,11 @@
if (!CheckDimensions(stride, width, height))
return "";
- std::vector<unsigned char> png_encoding;
const auto* buffer = static_cast<const unsigned char*>(buffer_void);
- if (!EncodePng(buffer, width, height, stride, FPDFBitmap_BGRA,
- &png_encoding)) {
+ std::vector<unsigned char> png_encoding =
+ EncodePng(buffer, width, height, stride, FPDFBitmap_BGRA);
+ if (png_encoding.empty()) {
fprintf(stderr, "Failed to convert bitmap to PNG\n");
return "";
}
@@ -628,14 +623,15 @@
continue;
}
- std::vector<unsigned char> png_encoding;
const unsigned char* buffer =
static_cast<const unsigned char*>(FPDFBitmap_GetBuffer(bitmap.get()));
int width = FPDFBitmap_GetWidth(bitmap.get());
int height = FPDFBitmap_GetHeight(bitmap.get());
int stride = FPDFBitmap_GetStride(bitmap.get());
- if (!EncodePng(buffer, width, height, stride, format, &png_encoding)) {
+ std::vector<unsigned char> png_encoding =
+ EncodePng(buffer, width, height, stride, format);
+ if (png_encoding.empty()) {
fprintf(stderr,
"Failed to convert image object #%d on page #%d to png.\n", i + 1,
page_num + 1);
diff --git a/testing/image_diff/image_diff.cpp b/testing/image_diff/image_diff.cpp
index f9d70f3..3da138b 100644
--- a/testing/image_diff/image_diff.cpp
+++ b/testing/image_diff/image_diff.cpp
@@ -63,8 +63,9 @@
fclose(f);
- if (!image_diff_png::DecodePNG(compressed.data(), compressed.size(), &data_,
- &w_, &h_)) {
+ data_ = image_diff_png::DecodePNG(compressed.data(), compressed.size(), &w_,
+ &h_);
+ if (data_.empty()) {
Clear();
return false;
}
@@ -278,12 +279,10 @@
if (same)
return kStatusSame;
- std::vector<unsigned char> png_encoding;
- if (!image_diff_png::EncodeRGBAPNG(diff_image.data(), diff_image.w(),
- diff_image.h(), diff_image.w() * 4,
- &png_encoding)) {
+ std::vector<unsigned char> png_encoding = image_diff_png::EncodeRGBAPNG(
+ diff_image.data(), diff_image.w(), diff_image.h(), diff_image.w() * 4);
+ if (png_encoding.empty())
return kStatusError;
- }
FILE* f = fopen(out_file.c_str(), "wb");
if (!f)
diff --git a/testing/image_diff/image_diff_png.cpp b/testing/image_diff/image_diff_png.cpp
index 09fb34f..9825e28 100644
--- a/testing/image_diff/image_diff_png.cpp
+++ b/testing/image_diff/image_diff_png.cpp
@@ -362,26 +362,26 @@
} // namespace
// static
-bool Decode(const unsigned char* input,
- size_t input_size,
- ColorFormat format,
- std::vector<unsigned char>* output,
- int* w,
- int* h) {
+std::vector<unsigned char> Decode(const unsigned char* input,
+ size_t input_size,
+ ColorFormat format,
+ int* w,
+ int* h) {
+ std::vector<unsigned char> output;
png_struct* png_ptr = NULL;
png_info* info_ptr = NULL;
if (!BuildPNGStruct(input, input_size, &png_ptr, &info_ptr))
- return false;
+ return output;
PngReadStructDestroyer destroyer(&png_ptr, &info_ptr);
if (setjmp(png_jmpbuf(png_ptr))) {
// The destroyer will ensure that the structures are cleaned up in this
// case, even though we may get here as a jump from random parts of the
// PNG library called below.
- return false;
+ return output;
}
- PngDecoderState state(format, output);
+ PngDecoderState state(format, &output);
png_set_progressive_read_fn(png_ptr, &state, &DecodeInfoCallback,
&DecodeRowCallback, &DecodeEndCallback);
@@ -391,13 +391,13 @@
if (!state.done) {
// Fed it all the data but the library didn't think we got all the data, so
// this file must be truncated.
- output->clear();
- return false;
+ output.clear();
+ return output;
}
*w = state.width;
*h = state.height;
- return true;
+ return output;
}
// Encoder
@@ -569,20 +569,23 @@
} // namespace
// static
-bool EncodeWithCompressionLevel(const unsigned char* input,
- ColorFormat format,
- const int width,
- const int height,
- int row_byte_width,
- bool discard_transparency,
- const std::vector<Comment>& comments,
- int compression_level,
- std::vector<unsigned char>* output) {
+std::vector<unsigned char> EncodeWithCompressionLevel(
+ const unsigned char* input,
+ ColorFormat format,
+ const int width,
+ const int height,
+ int row_byte_width,
+ bool discard_transparency,
+ const std::vector<Comment>& comments,
+ int compression_level) {
+ std::vector<unsigned char> output;
+
// Run to convert an input row into the output row format, NULL means no
// conversion is necessary.
FormatConverter converter = nullptr;
- int input_color_components, output_color_components;
+ int input_color_components;
+ int output_color_components;
int png_output_color_type;
switch (format) {
case FORMAT_BGR:
@@ -629,95 +632,86 @@
default:
NOTREACHED();
- return false;
+ return output;
}
// Row stride should be at least as long as the length of the data.
if (row_byte_width < input_color_components * width)
- return false;
+ return output;
png_struct* png_ptr =
png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
if (!png_ptr)
- return false;
+ return output;
png_info* info_ptr = png_create_info_struct(png_ptr);
if (!info_ptr) {
png_destroy_write_struct(&png_ptr, NULL);
- return false;
+ return output;
}
- PngEncoderState state(output);
+ PngEncoderState state(&output);
bool success =
DoLibpngWrite(png_ptr, info_ptr, &state, width, height, row_byte_width,
input, compression_level, png_output_color_type,
output_color_components, converter, comments);
png_destroy_write_struct(&png_ptr, &info_ptr);
- return success;
+ if (!success)
+ output.clear();
+ return output;
}
// static
-bool Encode(const unsigned char* input,
- ColorFormat format,
- const int width,
- const int height,
- int row_byte_width,
- bool discard_transparency,
- const std::vector<Comment>& comments,
- std::vector<unsigned char>* output) {
+std::vector<unsigned char> Encode(const unsigned char* input,
+ ColorFormat format,
+ const int width,
+ const int height,
+ int row_byte_width,
+ bool discard_transparency,
+ const std::vector<Comment>& comments) {
return EncodeWithCompressionLevel(input, format, width, height,
row_byte_width, discard_transparency,
- comments, Z_DEFAULT_COMPRESSION, output);
+ comments, Z_DEFAULT_COMPRESSION);
}
-// Decode a PNG into an RGBA pixel array.
-bool DecodePNG(const unsigned char* input,
- size_t input_size,
- std::vector<unsigned char>* output,
- int* width,
- int* height) {
- return Decode(input, input_size, FORMAT_RGBA, output, width, height);
+std::vector<unsigned char> DecodePNG(const unsigned char* input,
+ size_t input_size,
+ int* width,
+ int* height) {
+ return Decode(input, input_size, FORMAT_RGBA, width, height);
}
-// Encode a BGR pixel array into a PNG.
-bool EncodeBGRPNG(const unsigned char* input,
- int width,
- int height,
- int row_byte_width,
- std::vector<unsigned char>* output) {
+std::vector<unsigned char> EncodeBGRPNG(const unsigned char* input,
+ int width,
+ int height,
+ int row_byte_width) {
return Encode(input, FORMAT_BGR, width, height, row_byte_width, false,
- std::vector<Comment>(), output);
+ std::vector<Comment>());
}
-// Encode an RGBA pixel array into a PNG.
-bool EncodeRGBAPNG(const unsigned char* input,
- int width,
- int height,
- int row_byte_width,
- std::vector<unsigned char>* output) {
+std::vector<unsigned char> EncodeRGBAPNG(const unsigned char* input,
+ int width,
+ int height,
+ int row_byte_width) {
return Encode(input, FORMAT_RGBA, width, height, row_byte_width, false,
- std::vector<Comment>(), output);
+ std::vector<Comment>());
}
-// Encode an BGRA pixel array into a PNG.
-bool EncodeBGRAPNG(const unsigned char* input,
- int width,
- int height,
- int row_byte_width,
- bool discard_transparency,
- std::vector<unsigned char>* output) {
+std::vector<unsigned char> EncodeBGRAPNG(const unsigned char* input,
+ int width,
+ int height,
+ int row_byte_width,
+ bool discard_transparency) {
return Encode(input, FORMAT_BGRA, width, height, row_byte_width,
- discard_transparency, std::vector<Comment>(), output);
+ discard_transparency, std::vector<Comment>());
}
-// Encode a grayscale pixel array into a PNG.
-bool EncodeGrayPNG(const unsigned char* input,
- int width,
- int height,
- int row_byte_width,
- std::vector<unsigned char>* output) {
+std::vector<unsigned char> EncodeGrayPNG(const unsigned char* input,
+ int width,
+ int height,
+ int row_byte_width) {
return Encode(input, FORMAT_GRAY, width, height, row_byte_width, false,
- std::vector<Comment>(), output);
+ std::vector<Comment>());
}
} // namespace image_diff_png
diff --git a/testing/image_diff/image_diff_png.h b/testing/image_diff/image_diff_png.h
index b334b20..380e6dc 100644
--- a/testing/image_diff/image_diff_png.h
+++ b/testing/image_diff/image_diff_png.h
@@ -12,40 +12,35 @@
namespace image_diff_png {
// Decode a PNG into an RGBA pixel array.
-bool DecodePNG(const unsigned char* input,
- size_t input_size,
- std::vector<unsigned char>* output,
- int* width,
- int* height);
+std::vector<unsigned char> DecodePNG(const unsigned char* input,
+ size_t input_size,
+ int* width,
+ int* height);
// Encode a BGR pixel array into a PNG.
-bool EncodeBGRPNG(const unsigned char* input,
- int width,
- int height,
- int row_byte_width,
- std::vector<unsigned char>* output);
+std::vector<unsigned char> EncodeBGRPNG(const unsigned char* input,
+ int width,
+ int height,
+ int row_byte_width);
// Encode an RGBA pixel array into a PNG.
-bool EncodeRGBAPNG(const unsigned char* input,
- int width,
- int height,
- int row_byte_width,
- std::vector<unsigned char>* output);
+std::vector<unsigned char> EncodeRGBAPNG(const unsigned char* input,
+ int width,
+ int height,
+ int row_byte_width);
// Encode an BGRA pixel array into a PNG.
-bool EncodeBGRAPNG(const unsigned char* input,
- int width,
- int height,
- int row_byte_width,
- bool discard_transparency,
- std::vector<unsigned char>* output);
+std::vector<unsigned char> EncodeBGRAPNG(const unsigned char* input,
+ int width,
+ int height,
+ int row_byte_width,
+ bool discard_transparency);
// Encode a grayscale pixel array into a PNG.
-bool EncodeGrayPNG(const unsigned char* input,
- int width,
- int height,
- int row_byte_width,
- std::vector<unsigned char>* output);
+std::vector<unsigned char> EncodeGrayPNG(const unsigned char* input,
+ int width,
+ int height,
+ int row_byte_width);
} // namespace image_diff_png
diff --git a/testing/utils/bitmap_saver.cpp b/testing/utils/bitmap_saver.cpp
index af13efc..bba5cd5 100644
--- a/testing/utils/bitmap_saver.cpp
+++ b/testing/utils/bitmap_saver.cpp
@@ -19,24 +19,20 @@
const auto* buffer =
static_cast<const unsigned char*>(FPDFBitmap_GetBuffer(bitmap));
- std::vector<unsigned char> png_encoding;
- bool encoded;
+ std::vector<unsigned char> png;
if (FPDFBitmap_GetFormat(bitmap) == FPDFBitmap_Gray) {
- encoded = image_diff_png::EncodeGrayPNG(buffer, width, height, stride,
- &png_encoding);
+ png = image_diff_png::EncodeGrayPNG(buffer, width, height, stride);
} else {
- encoded = image_diff_png::EncodeBGRAPNG(buffer, width, height, stride,
- /*discard_transparency=*/false,
- &png_encoding);
+ png = image_diff_png::EncodeBGRAPNG(buffer, width, height, stride,
+ /*discard_transparency=*/false);
}
- DCHECK(encoded);
+ DCHECK(!png.empty());
DCHECK(filename.size() < 256u);
std::ofstream png_file;
png_file.open(filename, std::ios_base::out | std::ios_base::binary);
- png_file.write(reinterpret_cast<char*>(&png_encoding.front()),
- png_encoding.size());
+ png_file.write(reinterpret_cast<char*>(&png.front()), png.size());
DCHECK(png_file.good());
png_file.close();
}
