From 5a9d7fd8a6a6964082c6d502da3c7a0c4bca8128 Mon Sep 17 00:00:00 2001 From: bitsawer Date: Thu, 2 Mar 2023 16:46:06 +0200 Subject: [PATCH] Fix Image.convert() overwriting custom mipmaps --- core/io/image.cpp | 228 +++++++++++++++++++------------------ tests/core/io/test_image.h | 80 +++++++++++++ 2 files changed, 200 insertions(+), 108 deletions(-) diff --git a/core/io/image.cpp b/core/io/image.cpp index 736a3ec82e..a844eeff6f 100644 --- a/core/io/image.cpp +++ b/core/io/image.cpp @@ -516,21 +516,31 @@ void Image::convert(Format p_new_format) { return; } + // Includes the main image. + const int mipmap_count = get_mipmap_count() + 1; + if (format > FORMAT_RGBE9995 || p_new_format > FORMAT_RGBE9995) { ERR_FAIL_MSG("Cannot convert to <-> from compressed formats. Use compress() and decompress() instead."); } else if (format > FORMAT_RGBA8 || p_new_format > FORMAT_RGBA8) { //use put/set pixel which is slower but works with non byte formats - Image new_img(width, height, false, p_new_format); + Image new_img(width, height, mipmaps, p_new_format); - for (int i = 0; i < width; i++) { - for (int j = 0; j < height; j++) { - new_img.set_pixel(i, j, get_pixel(i, j)); + for (int mip = 0; mip < mipmap_count; mip++) { + Ref src_mip = get_image_from_mipmap(mip); + Ref new_mip = new_img.get_image_from_mipmap(mip); + + for (int y = 0; y < src_mip->height; y++) { + for (int x = 0; x < src_mip->width; x++) { + new_mip->set_pixel(x, y, src_mip->get_pixel(x, y)); + } } - } - if (has_mipmaps()) { - new_img.generate_mipmaps(); + int mip_offset = 0; + int mip_size = 0; + new_img.get_mipmap_offset_and_size(mip, mip_offset, mip_size); + + memcpy(new_img.data.ptrw() + mip_offset, new_mip->data.ptr(), mip_size); } _copy_internals_from(new_img); @@ -538,113 +548,115 @@ void Image::convert(Format p_new_format) { return; } - Image new_img(width, height, false, p_new_format); - - const uint8_t *rptr = data.ptr(); - uint8_t *wptr = new_img.data.ptrw(); + Image new_img(width, height, mipmaps, p_new_format); int conversion_type = format | p_new_format << 8; - switch (conversion_type) { - case FORMAT_L8 | (FORMAT_LA8 << 8): - _convert<1, false, 1, true, true, true>(width, height, rptr, wptr); - break; - case FORMAT_L8 | (FORMAT_R8 << 8): - _convert<1, false, 1, false, true, false>(width, height, rptr, wptr); - break; - case FORMAT_L8 | (FORMAT_RG8 << 8): - _convert<1, false, 2, false, true, false>(width, height, rptr, wptr); - break; - case FORMAT_L8 | (FORMAT_RGB8 << 8): - _convert<1, false, 3, false, true, false>(width, height, rptr, wptr); - break; - case FORMAT_L8 | (FORMAT_RGBA8 << 8): - _convert<1, false, 3, true, true, false>(width, height, rptr, wptr); - break; - case FORMAT_LA8 | (FORMAT_L8 << 8): - _convert<1, true, 1, false, true, true>(width, height, rptr, wptr); - break; - case FORMAT_LA8 | (FORMAT_R8 << 8): - _convert<1, true, 1, false, true, false>(width, height, rptr, wptr); - break; - case FORMAT_LA8 | (FORMAT_RG8 << 8): - _convert<1, true, 2, false, true, false>(width, height, rptr, wptr); - break; - case FORMAT_LA8 | (FORMAT_RGB8 << 8): - _convert<1, true, 3, false, true, false>(width, height, rptr, wptr); - break; - case FORMAT_LA8 | (FORMAT_RGBA8 << 8): - _convert<1, true, 3, true, true, false>(width, height, rptr, wptr); - break; - case FORMAT_R8 | (FORMAT_L8 << 8): - _convert<1, false, 1, false, false, true>(width, height, rptr, wptr); - break; - case FORMAT_R8 | (FORMAT_LA8 << 8): - _convert<1, false, 1, true, false, true>(width, height, rptr, wptr); - break; - case FORMAT_R8 | (FORMAT_RG8 << 8): - _convert<1, false, 2, false, false, false>(width, height, rptr, wptr); - break; - case FORMAT_R8 | (FORMAT_RGB8 << 8): - _convert<1, false, 3, false, false, false>(width, height, rptr, wptr); - break; - case FORMAT_R8 | (FORMAT_RGBA8 << 8): - _convert<1, false, 3, true, false, false>(width, height, rptr, wptr); - break; - case FORMAT_RG8 | (FORMAT_L8 << 8): - _convert<2, false, 1, false, false, true>(width, height, rptr, wptr); - break; - case FORMAT_RG8 | (FORMAT_LA8 << 8): - _convert<2, false, 1, true, false, true>(width, height, rptr, wptr); - break; - case FORMAT_RG8 | (FORMAT_R8 << 8): - _convert<2, false, 1, false, false, false>(width, height, rptr, wptr); - break; - case FORMAT_RG8 | (FORMAT_RGB8 << 8): - _convert<2, false, 3, false, false, false>(width, height, rptr, wptr); - break; - case FORMAT_RG8 | (FORMAT_RGBA8 << 8): - _convert<2, false, 3, true, false, false>(width, height, rptr, wptr); - break; - case FORMAT_RGB8 | (FORMAT_L8 << 8): - _convert<3, false, 1, false, false, true>(width, height, rptr, wptr); - break; - case FORMAT_RGB8 | (FORMAT_LA8 << 8): - _convert<3, false, 1, true, false, true>(width, height, rptr, wptr); - break; - case FORMAT_RGB8 | (FORMAT_R8 << 8): - _convert<3, false, 1, false, false, false>(width, height, rptr, wptr); - break; - case FORMAT_RGB8 | (FORMAT_RG8 << 8): - _convert<3, false, 2, false, false, false>(width, height, rptr, wptr); - break; - case FORMAT_RGB8 | (FORMAT_RGBA8 << 8): - _convert<3, false, 3, true, false, false>(width, height, rptr, wptr); - break; - case FORMAT_RGBA8 | (FORMAT_L8 << 8): - _convert<3, true, 1, false, false, true>(width, height, rptr, wptr); - break; - case FORMAT_RGBA8 | (FORMAT_LA8 << 8): - _convert<3, true, 1, true, false, true>(width, height, rptr, wptr); - break; - case FORMAT_RGBA8 | (FORMAT_R8 << 8): - _convert<3, true, 1, false, false, false>(width, height, rptr, wptr); - break; - case FORMAT_RGBA8 | (FORMAT_RG8 << 8): - _convert<3, true, 2, false, false, false>(width, height, rptr, wptr); - break; - case FORMAT_RGBA8 | (FORMAT_RGB8 << 8): - _convert<3, true, 3, false, false, false>(width, height, rptr, wptr); - break; - } + for (int mip = 0; mip < mipmap_count; mip++) { + int mip_offset = 0; + int mip_size = 0; + int mip_width = 0; + int mip_height = 0; + get_mipmap_offset_size_and_dimensions(mip, mip_offset, mip_size, mip_width, mip_height); - bool gen_mipmaps = mipmaps; + const uint8_t *rptr = data.ptr() + mip_offset; + uint8_t *wptr = new_img.data.ptrw() + new_img.get_mipmap_offset(mip); + + switch (conversion_type) { + case FORMAT_L8 | (FORMAT_LA8 << 8): + _convert<1, false, 1, true, true, true>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_L8 | (FORMAT_R8 << 8): + _convert<1, false, 1, false, true, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_L8 | (FORMAT_RG8 << 8): + _convert<1, false, 2, false, true, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_L8 | (FORMAT_RGB8 << 8): + _convert<1, false, 3, false, true, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_L8 | (FORMAT_RGBA8 << 8): + _convert<1, false, 3, true, true, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_LA8 | (FORMAT_L8 << 8): + _convert<1, true, 1, false, true, true>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_LA8 | (FORMAT_R8 << 8): + _convert<1, true, 1, false, true, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_LA8 | (FORMAT_RG8 << 8): + _convert<1, true, 2, false, true, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_LA8 | (FORMAT_RGB8 << 8): + _convert<1, true, 3, false, true, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_LA8 | (FORMAT_RGBA8 << 8): + _convert<1, true, 3, true, true, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_R8 | (FORMAT_L8 << 8): + _convert<1, false, 1, false, false, true>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_R8 | (FORMAT_LA8 << 8): + _convert<1, false, 1, true, false, true>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_R8 | (FORMAT_RG8 << 8): + _convert<1, false, 2, false, false, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_R8 | (FORMAT_RGB8 << 8): + _convert<1, false, 3, false, false, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_R8 | (FORMAT_RGBA8 << 8): + _convert<1, false, 3, true, false, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RG8 | (FORMAT_L8 << 8): + _convert<2, false, 1, false, false, true>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RG8 | (FORMAT_LA8 << 8): + _convert<2, false, 1, true, false, true>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RG8 | (FORMAT_R8 << 8): + _convert<2, false, 1, false, false, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RG8 | (FORMAT_RGB8 << 8): + _convert<2, false, 3, false, false, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RG8 | (FORMAT_RGBA8 << 8): + _convert<2, false, 3, true, false, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RGB8 | (FORMAT_L8 << 8): + _convert<3, false, 1, false, false, true>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RGB8 | (FORMAT_LA8 << 8): + _convert<3, false, 1, true, false, true>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RGB8 | (FORMAT_R8 << 8): + _convert<3, false, 1, false, false, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RGB8 | (FORMAT_RG8 << 8): + _convert<3, false, 2, false, false, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RGB8 | (FORMAT_RGBA8 << 8): + _convert<3, false, 3, true, false, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RGBA8 | (FORMAT_L8 << 8): + _convert<3, true, 1, false, false, true>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RGBA8 | (FORMAT_LA8 << 8): + _convert<3, true, 1, true, false, true>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RGBA8 | (FORMAT_R8 << 8): + _convert<3, true, 1, false, false, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RGBA8 | (FORMAT_RG8 << 8): + _convert<3, true, 2, false, false, false>(mip_width, mip_height, rptr, wptr); + break; + case FORMAT_RGBA8 | (FORMAT_RGB8 << 8): + _convert<3, true, 3, false, false, false>(mip_width, mip_height, rptr, wptr); + break; + } + } _copy_internals_from(new_img); - - if (gen_mipmaps) { - generate_mipmaps(); - } } Image::Format Image::get_format() const { diff --git a/tests/core/io/test_image.h b/tests/core/io/test_image.h index 763eaed2f8..79cee5f0df 100644 --- a/tests/core/io/test_image.h +++ b/tests/core/io/test_image.h @@ -304,6 +304,86 @@ TEST_CASE("[Image] Modifying pixels of an image") { image3->get_pixel(1, 0).is_equal_approx(Color(0, 0, 0, 0)), "flip_y() should not leave old pixels behind."); } + +TEST_CASE("[Image] Custom mipmaps") { + Ref image = memnew(Image(100, 100, false, Image::FORMAT_RGBA8)); + + REQUIRE(!image->has_mipmaps()); + image->generate_mipmaps(); + REQUIRE(image->has_mipmaps()); + + const int mipmaps = image->get_mipmap_count() + 1; + REQUIRE(mipmaps == 7); + + // Initialize reference mipmap data. + // Each byte is given value "mipmap_index * 5". + + { + PackedByteArray data = image->get_data(); + uint8_t *data_ptr = data.ptrw(); + + for (int mip = 0; mip < mipmaps; mip++) { + int mip_offset = 0; + int mip_size = 0; + image->get_mipmap_offset_and_size(mip, mip_offset, mip_size); + + for (int i = 0; i < mip_size; i++) { + data_ptr[mip_offset + i] = mip * 5; + } + } + image->set_data(image->get_width(), image->get_height(), image->has_mipmaps(), image->get_format(), data); + } + + // Byte format conversion. + + for (int format = Image::FORMAT_L8; format <= Image::FORMAT_RGBA8; format++) { + Ref image_bytes = memnew(Image()); + image_bytes->copy_internals_from(image); + image_bytes->convert((Image::Format)format); + REQUIRE(image_bytes->has_mipmaps()); + + PackedByteArray data = image_bytes->get_data(); + const uint8_t *data_ptr = data.ptr(); + + for (int mip = 0; mip < mipmaps; mip++) { + int mip_offset = 0; + int mip_size = 0; + image_bytes->get_mipmap_offset_and_size(mip, mip_offset, mip_size); + + for (int i = 0; i < mip_size; i++) { + if (data_ptr[mip_offset + i] != mip * 5) { + REQUIRE_MESSAGE(false, "Byte format conversion error."); + } + } + } + } + + // Floating point format conversion. + + for (int format = Image::FORMAT_RF; format <= Image::FORMAT_RGBAF; format++) { + Ref image_rgbaf = memnew(Image()); + image_rgbaf->copy_internals_from(image); + image_rgbaf->convert((Image::Format)format); + REQUIRE(image_rgbaf->has_mipmaps()); + + PackedByteArray data = image_rgbaf->get_data(); + const uint8_t *data_ptr = data.ptr(); + + for (int mip = 0; mip < mipmaps; mip++) { + int mip_offset = 0; + int mip_size = 0; + image_rgbaf->get_mipmap_offset_and_size(mip, mip_offset, mip_size); + + for (int i = 0; i < mip_size; i += 4) { + float value = *(float *)(data_ptr + mip_offset + i); + if (!Math::is_equal_approx(value * 255.0f, mip * 5)) { + REQUIRE_MESSAGE(false, "Floating point conversion error."); + } + } + } + } +} + } // namespace TestImage #endif // TEST_IMAGE_H