Fix colour textures loading upside down and crashes on damaged PNGs

Colour height maps now displace the same way up as grayscale ones. A
truncated or corrupt PNG in the texture folder or a project file now
loads as an empty texture instead of aborting the app, and a 16-bit
colour PNG is converted on load rather than displacing as noise.
This commit is contained in:
SoftFever
2026-09-18 20:37:35 +08:00
parent 77bd987979
commit 3d7682e292
4 changed files with 94 additions and 20 deletions
+16 -3
View File
@@ -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<IStream *>(png_get_io_ptr(png_ptr));
if (!reader || !reader->is_ok()) return;
reader->read(static_cast<std::uint8_t *>(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<std::uint8_t *>(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<void *>(&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<void *>(&in_buf), png_read_callback);
// Tell that we have already read the first bytes to check the signature
+23 -13
View File
@@ -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<std::mutex> lock(g_decoded_texture_cache.mutex);
+4 -3
View File
@@ -153,8 +153,8 @@ std::vector<unsigned char> 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<unsigned char> &bytes)
{
if (bytes.empty())
@@ -167,7 +167,8 @@ bool is_supported_height_map(const std::vector<unsigned char> &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<TextureLibraryEntry> g_library;
+51 -1
View File
@@ -1,4 +1,6 @@
#ifndef NOMINMAX
#define NOMINMAX
#endif
#include <catch2/catch_all.hpp>
#include <algorithm>
@@ -779,11 +781,27 @@ static std::shared_ptr<std::vector<unsigned char>> make_rgb_png_2x2()
return std::make_shared<std::vector<unsigned char>>(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<std::vector<unsigned char>> 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::vector<unsigned char>>(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::vector<unsigned char>>(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<std::vector<unsigned char>>(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