diff --git a/src/libslic3r/PNGReadWrite.cpp b/src/libslic3r/PNGReadWrite.cpp index 32b9a8f79e..adc41a164f 100644 --- a/src/libslic3r/PNGReadWrite.cpp +++ b/src/libslic3r/PNGReadWrite.cpp @@ -54,9 +54,11 @@ static void png_read_callback(png_struct *png_ptr, // Retrieve our input buffer through the png_ptr auto reader = static_cast(png_get_io_ptr(png_ptr)); - if (!reader || !reader->is_ok()) return; - - reader->read(static_cast(outBytes), byteCountToRead); + // libpng expects a short read to be reported through png_error(); returning quietly would leave + // it decoding whatever happened to be in outBytes. + if (!reader || !reader->is_ok() || + reader->read(static_cast(outBytes), byteCountToRead) != byteCountToRead) + png_error(png_ptr, "PNG data is truncated"); } bool decode_png(IStream &in_buf, ImageGreyscale &out_img) @@ -77,6 +79,11 @@ bool decode_png(IStream &in_buf, ImageGreyscale &out_img) dsc.info = png_create_info_struct(dsc.png); if(!dsc.info) return false; + // libpng reports a corrupt or truncated image by longjmp()ing back here. Without a jump buffer + // it abort()s the whole process instead. + if (setjmp(png_jmpbuf(dsc.png))) + return false; + png_set_read_fn(dsc.png, static_cast(&in_buf), png_read_callback); // Tell that we have already read the first bytes to check the signature @@ -128,6 +135,12 @@ bool decode_colored_png(IStream &in_buf, ImageColorscale &out_img) return false; } + // See decode_png(). + if (setjmp(png_jmpbuf(dsc.png))) { + BOOST_LOG_TRIVIAL(error) << "decode_colored_png: corrupt or truncated PNG data"; + return false; + } + png_set_read_fn(dsc.png, static_cast(&in_buf), png_read_callback); // Tell that we have already read the first bytes to check the signature diff --git a/src/libslic3r/TextureDisplacement.cpp b/src/libslic3r/TextureDisplacement.cpp index cfc75c286f..0ab06e68b4 100644 --- a/src/libslic3r/TextureDisplacement.cpp +++ b/src/libslic3r/TextureDisplacement.cpp @@ -272,24 +272,34 @@ DecodedHeightTexture decode_height_texture(const TextureDisplacementLayer &layer // coefficients are wxImage::ConvertToGreyscale()'s, which is what the importer used to // apply on the way in - so a texture that used to be flattened to grey at import time // displaces identically now that its colour is preserved. + // The loop below steps bytes_per_pixel per texel, which only holds at 8 bits per channel: + // decode_colored_png() does not narrow a 16-bit image, so that would read as noise. png::ImageColorscale col; if (!png::decode_colored_png(rbuf, col) || col.cols == 0 || col.rows == 0 || - col.bytes_per_pixel < 3) + col.bytes_per_pixel < 3 || col.buf.size() != col.cols * col.rows * size_t(col.bytes_per_pixel)) return result; - const size_t n = size_t(col.cols) * size_t(col.rows); + const int w = int(col.cols); + const int h = int(col.rows); const size_t bpp = size_t(col.bytes_per_pixel); - result.width = int(col.cols); - result.height = int(col.rows); - result.pixels.resize(n); - result.rgb.resize(n * 3); - for (size_t i = 0; i < n; ++i) { - const uint8_t r = col.buf[i * bpp], g = col.buf[i * bpp + 1], b = col.buf[i * bpp + 2]; - result.rgb[i * 3] = r; - result.rgb[i * 3 + 1] = g; - result.rgb[i * 3 + 2] = b; - result.pixels[i] = uint8_t(std::lround(0.299 * r + 0.587 * g + 0.114 * b)); - } + result.width = w; + result.height = h; + result.pixels.resize(size_t(w) * size_t(h)); + result.rgb.resize(size_t(w) * size_t(h) * 3); + // decode_colored_png() hands its buffer back bottom-up - it is shared with the CLI's + // plate-thumbnail loader, which expects that - while this type, and decode_png()'s + // grayscale path above, are top-to-bottom. Reverse the rows on the way in so a colour + // height map displaces the same way up as a grayscale one. + for (int y = 0; y < h; ++y) + for (int x = 0; x < w; ++x) { + const uint8_t *src = col.buf.data() + (size_t(h - 1 - y) * size_t(w) + size_t(x)) * bpp; + const size_t dst = size_t(y) * size_t(w) + size_t(x); + const uint8_t r = src[0], g = src[1], b = src[2]; + result.rgb[dst * 3] = r; + result.rgb[dst * 3 + 1] = g; + result.rgb[dst * 3 + 2] = b; + result.pixels[dst] = uint8_t(std::lround(0.299 * r + 0.587 * g + 0.114 * b)); + } } std::lock_guard lock(g_decoded_texture_cache.mutex); diff --git a/src/slic3r/GUI/TextureLibrary.cpp b/src/slic3r/GUI/TextureLibrary.cpp index 30df4af22c..7cbd50a240 100644 --- a/src/slic3r/GUI/TextureLibrary.cpp +++ b/src/slic3r/GUI/TextureLibrary.cpp @@ -153,8 +153,8 @@ std::vector read_file_bytes(const std::string &path) } // True if these bytes are a PNG libslic3r can decode, i.e. can be stored on a layer as-is. Mirrors -// exactly what decode_height_texture() accepts: 8-bit grayscale, or colour (whose luminance is the -// height and whose RGB is available to colour the model). +// exactly what decode_height_texture() accepts: 8-bit grayscale, or 8-bit colour (whose luminance is +// the height and whose RGB is available to colour the model). Anything else is converted on load. bool is_supported_height_map(const std::vector &bytes) { if (bytes.empty()) @@ -167,7 +167,8 @@ bool is_supported_height_map(const std::vector &bytes) return true; png::ImageColorscale color; return png::decode_colored_png(rbuf, color) && color.cols > 0 && color.rows > 0 && - color.bytes_per_pixel >= 3; + color.bytes_per_pixel >= 3 && + color.buf.size() == color.cols * color.rows * size_t(color.bytes_per_pixel); } std::vector g_library; diff --git a/tests/libslic3r/test_texture_displacement.cpp b/tests/libslic3r/test_texture_displacement.cpp index e2f0147efb..efdfd32c44 100644 --- a/tests/libslic3r/test_texture_displacement.cpp +++ b/tests/libslic3r/test_texture_displacement.cpp @@ -1,4 +1,6 @@ +#ifndef NOMINMAX #define NOMINMAX +#endif #include #include @@ -779,11 +781,27 @@ static std::shared_ptr> make_rgb_png_2x2() return std::make_shared>(std::begin(bytes), std::end(bytes)); } +// The same image with an opaque alpha channel, so four bytes per pixel instead of three. +static std::shared_ptr> make_rgba_png_2x2() +{ + static const unsigned char bytes[] = { + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, + 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x02, + 0x08, 0x06, 0x00, 0x00, 0x00, 0x72, 0xb6, 0x0d, 0x24, 0x00, 0x00, 0x00, + 0x12, 0x49, 0x44, 0x41, 0x54, 0x78, 0xda, 0x63, 0xf8, 0xcf, 0xc0, 0xf0, + 0x1f, 0x0c, 0x81, 0x34, 0x18, 0x00, 0x00, 0x49, 0xc8, 0x09, 0xf7, 0x03, + 0xd9, 0x64, 0xf1, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, + 0x42, 0x60, 0x82, + }; + return std::make_shared>(std::begin(bytes), std::end(bytes)); +} + TEST_CASE("TextureDisplacement: a colour texture decodes to both colour and height", "[TextureDisplacement]") { TextureDisplacementLayer layer; layer.slot = 0; - layer.image_data = make_rgb_png_2x2(); + // Row stride differs between the two, and both must come out the same way up. + layer.image_data = GENERATE(make_rgb_png_2x2(), make_rgba_png_2x2()); const DecodedHeightTexture tex = decode_height_texture(layer); REQUIRE_FALSE(tex.empty()); @@ -806,6 +824,38 @@ TEST_CASE("TextureDisplacement: a colour texture decodes to both colour and heig CHECK(int(tex.pixels[3]) == 255); // white } +TEST_CASE("A 16-bit colour texture decodes to nothing rather than noise", "[TextureDisplacement]") +{ + // The 2x2 image above at 16 bits per channel. The texture library converts such a file to 8-bit + // on load, so it can only arrive here stored as-is, from a project file. + static const unsigned char bytes[] = { + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, + 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x02, + 0x10, 0x02, 0x00, 0x00, 0x00, 0xad, 0x44, 0x46, 0x30, 0x00, 0x00, 0x00, + 0x12, 0x49, 0x44, 0x41, 0x54, 0x78, 0xda, 0x63, 0xf8, 0xff, 0x9f, 0x01, + 0x0c, 0x60, 0x34, 0x90, 0x01, 0x01, 0x00, 0x75, 0xa4, 0x0b, 0xf5, 0x97, + 0xf4, 0x36, 0xa1, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, + 0x42, 0x60, 0x82, + }; + TextureDisplacementLayer layer; + layer.slot = 0; + layer.image_data = std::make_shared>(std::begin(bytes), std::end(bytes)); + + CHECK(decode_height_texture(layer).empty()); +} + +TEST_CASE("A truncated texture decodes to nothing instead of aborting", "[TextureDisplacement]") +{ + // A half-copied file in the texture folder, or a damaged project file. libpng reports this by + // longjmp, and aborts the process if the decoder has not set a jump buffer to land on. + const auto whole = GENERATE(make_flat_gray_png(128, 16, 16), make_rgb_png_2x2()); + TextureDisplacementLayer layer; + layer.slot = 0; + layer.image_data = std::make_shared>(whole->begin(), whole->begin() + whole->size() / 2); + + CHECK(decode_height_texture(layer).empty()); +} + TEST_CASE("TextureDisplacement: a grayscale texture reports no colour", "[TextureDisplacement]") { // The shipped library is all grayscale, and has_color() is what the whole colour feature keys