Skip to content

Commit

Permalink
Merge pull request #94243 from Calinou/image-fix-large-cowdata-size-c…
Browse files Browse the repository at this point in the history 
…rash

Fix Image CowData crash when baking large lightmaps
  • Loading branch information
@akien-mga
akien-mga committed Jul 19, 2024
2 parents cca2201 + 0445ccf commit a0943ac
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 76 deletions.
96 changes: 39 additions & 57 deletions core/io/image.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,10 @@ int Image::get_format_block_size(Format p_format) {
return 1;
}

void Image::_get_mipmap_offset_and_size(int p_mipmap, int &r_offset, int &r_width, int &r_height) const {
void Image::_get_mipmap_offset_and_size(int p_mipmap, int64_t &r_offset, int &r_width, int &r_height) const {
int w = width;
int h = height;
int ofs = 0;
int64_t ofs = 0;

int pixel_size = get_format_pixel_size(format);
int pixel_rshift = get_format_pixel_rshift(format);
Expand All @@ -315,7 +315,7 @@ void Image::_get_mipmap_offset_and_size(int p_mipmap, int &r_offset, int &r_widt
int bw = w % block != 0 ? w + (block - w % block) : w;
int bh = h % block != 0 ? h + (block - h % block) : h;

int s = bw * bh;
int64_t s = bw * bh;

s *= pixel_size;
s >>= pixel_rshift;
Expand All @@ -329,37 +329,30 @@ void Image::_get_mipmap_offset_and_size(int p_mipmap, int &r_offset, int &r_widt
r_height = h;
}

int Image::get_mipmap_offset(int p_mipmap) const {
int64_t Image::get_mipmap_offset(int p_mipmap) const {
ERR_FAIL_INDEX_V(p_mipmap, get_mipmap_count() + 1, -1);

int ofs, w, h;
int64_t ofs;
int w, h;
_get_mipmap_offset_and_size(p_mipmap, ofs, w, h);
return ofs;
}

int Image::get_mipmap_byte_size(int p_mipmap) const {
ERR_FAIL_INDEX_V(p_mipmap, get_mipmap_count() + 1, -1);

int ofs, w, h;
_get_mipmap_offset_and_size(p_mipmap, ofs, w, h);
int ofs2;
_get_mipmap_offset_and_size(p_mipmap + 1, ofs2, w, h);
return ofs2 - ofs;
}

void Image::get_mipmap_offset_and_size(int p_mipmap, int &r_ofs, int &r_size) const {
int ofs, w, h;
void Image::get_mipmap_offset_and_size(int p_mipmap, int64_t &r_ofs, int64_t &r_size) const {
int64_t ofs;
int w, h;
_get_mipmap_offset_and_size(p_mipmap, ofs, w, h);
int ofs2;
int64_t ofs2;
_get_mipmap_offset_and_size(p_mipmap + 1, ofs2, w, h);
r_ofs = ofs;
r_size = ofs2 - ofs;
}

void Image::get_mipmap_offset_size_and_dimensions(int p_mipmap, int &r_ofs, int &r_size, int &w, int &h) const {
int ofs;
void Image::get_mipmap_offset_size_and_dimensions(int p_mipmap, int64_t &r_ofs, int64_t &r_size, int &w, int &h) const {
int64_t ofs;
_get_mipmap_offset_and_size(p_mipmap, ofs, w, h);
int ofs2, w2, h2;
int64_t ofs2;
int w2, h2;
_get_mipmap_offset_and_size(p_mipmap + 1, ofs2, w2, h2);
r_ofs = ofs;
r_size = ofs2 - ofs;
Expand Down Expand Up @@ -538,8 +531,8 @@ void Image::convert(Format p_new_format) {
}
}

int mip_offset = 0;
int mip_size = 0;
int64_t mip_offset = 0;
int64_t 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);
Expand All @@ -555,8 +548,8 @@ void Image::convert(Format p_new_format) {
int conversion_type = format | p_new_format << 8;

for (int mip = 0; mip < mipmap_count; mip++) {
int mip_offset = 0;
int mip_size = 0;
int64_t mip_offset = 0;
int64_t 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);
Expand Down Expand Up @@ -1151,15 +1144,15 @@ void Image::resize(int p_width, int p_height, Interpolation p_interpolation) {
if (i == 0) {
// Read from the first mipmap that will be interpolated
// (if both levels are the same, we will not interpolate, but at least we'll sample from the right level)
int offs;
int64_t offs;
_get_mipmap_offset_and_size(mip1, offs, src_width, src_height);
src_ptr = r_ptr + offs;
} else if (!interpolate_mipmaps) {
// No need generate a second image
break;
} else {
// Switch to read from the second mipmap that will be interpolated
int offs;
int64_t offs;
_get_mipmap_offset_and_size(mip2, offs, src_width, src_height);
src_ptr = r_ptr + offs;
// Switch to write to the second destination image
Expand Down Expand Up @@ -1599,9 +1592,9 @@ void Image::flip_x() {
}

/// Get mipmap size and offset.
int Image::_get_dst_image_size(int p_width, int p_height, Format p_format, int &r_mipmaps, int p_mipmaps, int *r_mm_width, int *r_mm_height) {
int64_t Image::_get_dst_image_size(int p_width, int p_height, Format p_format, int &r_mipmaps, int p_mipmaps, int *r_mm_width, int *r_mm_height) {
// Data offset in mipmaps (including the original texture).
int size = 0;
int64_t size = 0;

int w = p_width;
int h = p_height;
Expand All @@ -1623,7 +1616,7 @@ int Image::_get_dst_image_size(int p_width, int p_height, Format p_format, int &
int bw = w % block != 0 ? w + (block - w % block) : w;
int bh = h % block != 0 ? h + (block - h % block) : h;

int s = bw * bh;
int64_t s = bw * bh;

s *= pixsize;
s >>= pixshift;
Expand Down Expand Up @@ -1837,7 +1830,8 @@ Error Image::generate_mipmaps(bool p_renormalize) {
int prev_w = width;

for (int i = 1; i <= mmcount; i++) {
int ofs, w, h;
int64_t ofs;
int w, h;
_get_mipmap_offset_and_size(i, ofs, w, h);

switch (format) {
Expand Down Expand Up @@ -1993,7 +1987,8 @@ Error Image::generate_mipmap_roughness(RoughnessChannel p_roughness_channel, con
uint8_t *base_ptr = data.ptrw();

for (int i = 1; i <= mmcount; i++) {
int ofs, w, h;
int64_t ofs;
int w, h;
_get_mipmap_offset_and_size(i, ofs, w, h);
uint8_t *ptr = &base_ptr[ofs];

Expand Down Expand Up @@ -2102,21 +2097,6 @@ Error Image::generate_mipmap_roughness(RoughnessChannel p_roughness_channel, con
_set_color_at_ofs(ptr, pixel_ofs, c);
}
}
#if 0
{
int size = get_mipmap_byte_size(i);
print_line("size for mimpap " + itos(i) + ": " + itos(size));
Vector<uint8_t> imgdata;
imgdata.resize(size);


uint8_t* wr = imgdata.ptrw();
memcpy(wr.ptr(), ptr, size);
wr = uint8_t*();
Ref<Image> im = Image::create_from_data(w, h, false, format, imgdata);
im->save_png("res://mipmap_" + itos(i) + ".png");
}
#endif
}

return OK;
Expand All @@ -2131,7 +2111,8 @@ void Image::clear_mipmaps() {
return;
}

int ofs, w, h;
int64_t ofs;
int w, h;
_get_mipmap_offset_and_size(1, ofs, w, h);
data.resize(ofs);

Expand Down Expand Up @@ -2176,7 +2157,7 @@ void Image::initialize_data(int p_width, int p_height, bool p_use_mipmaps, Forma
ERR_FAIL_INDEX_MSG(p_format, FORMAT_MAX, "The Image format specified (" + itos(p_format) + ") is out of range. See Image's Format enum.");

int mm = 0;
int size = _get_dst_image_size(p_width, p_height, p_format, mm, p_use_mipmaps ? -1 : 0);
int64_t size = _get_dst_image_size(p_width, p_height, p_format, mm, p_use_mipmaps ? -1 : 0);
data.resize(size);

{
Expand All @@ -2202,7 +2183,7 @@ void Image::initialize_data(int p_width, int p_height, bool p_use_mipmaps, Forma
ERR_FAIL_INDEX_MSG(p_format, FORMAT_MAX, "The Image format specified (" + itos(p_format) + ") is out of range. See Image's Format enum.");

int mm;
int size = _get_dst_image_size(p_width, p_height, p_format, mm, p_use_mipmaps ? -1 : 0);
int64_t size = _get_dst_image_size(p_width, p_height, p_format, mm, p_use_mipmaps ? -1 : 0);

if (unlikely(p_data.size() != size)) {
String description_mipmaps = get_format_name(p_format) + " ";
Expand Down Expand Up @@ -2405,7 +2386,7 @@ bool Image::is_invisible() const {
return false;
}

int len = data.size();
int64_t len = data.size();

if (len == 0) {
return true;
Expand Down Expand Up @@ -2445,7 +2426,7 @@ bool Image::is_invisible() const {
}

Image::AlphaMode Image::detect_alpha() const {
int len = data.size();
int64_t len = data.size();

if (len == 0) {
return ALPHA_NONE;
Expand Down Expand Up @@ -2597,15 +2578,15 @@ Size2i Image::get_image_mipmap_size(int p_width, int p_height, Format p_format,
return ret;
}

int Image::get_image_mipmap_offset(int p_width, int p_height, Format p_format, int p_mipmap) {
int64_t Image::get_image_mipmap_offset(int p_width, int p_height, Format p_format, int p_mipmap) {
if (p_mipmap <= 0) {
return 0;
}
int mm;
return _get_dst_image_size(p_width, p_height, p_format, mm, p_mipmap - 1);
}

int Image::get_image_mipmap_offset_and_dimensions(int p_width, int p_height, Format p_format, int p_mipmap, int &r_w, int &r_h) {
int64_t Image::get_image_mipmap_offset_and_dimensions(int p_width, int p_height, Format p_format, int p_mipmap, int &r_w, int &r_h) {
if (p_mipmap <= 0) {
r_w = p_width;
r_h = p_height;
Expand Down Expand Up @@ -3642,9 +3623,10 @@ Ref<Image> Image::rgbe_to_srgb() {
return new_image;
}

Ref<Image> Image::get_image_from_mipmap(int p_mipamp) const {
int ofs, size, w, h;
get_mipmap_offset_size_and_dimensions(p_mipamp, ofs, size, w, h);
Ref<Image> Image::get_image_from_mipmap(int p_mipmap) const {
int64_t ofs, size;
int w, h;
get_mipmap_offset_size_and_dimensions(p_mipmap, ofs, size, w, h);

Vector<uint8_t> new_data;
new_data.resize(size);
Expand Down
20 changes: 11 additions & 9 deletions core/io/image.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,9 @@ class Image : public Resource {
data = p_image.data;
}

_FORCE_INLINE_ void _get_mipmap_offset_and_size(int p_mipmap, int &r_offset, int &r_width, int &r_height) const; //get where the mipmap begins in data
_FORCE_INLINE_ void _get_mipmap_offset_and_size(int p_mipmap, int64_t &r_offset, int &r_width, int &r_height) const; //get where the mipmap begins in data

static int _get_dst_image_size(int p_width, int p_height, Format p_format, int &r_mipmaps, int p_mipmaps = -1, int *r_mm_width = nullptr, int *r_mm_height = nullptr);
static int64_t _get_dst_image_size(int p_width, int p_height, Format p_format, int &r_mipmaps, int p_mipmaps = -1, int *r_mm_width = nullptr, int *r_mm_height = nullptr);
bool _can_modify(Format p_format) const;

_FORCE_INLINE_ void _get_clipped_src_and_dest_rects(const Ref<Image> &p_src, const Rect2i &p_src_rect, const Point2i &p_dest, Rect2i &r_clipped_src_rect, Rect2i &r_clipped_dest_rect) const;
Expand Down Expand Up @@ -238,10 +238,12 @@ class Image : public Resource {
*/
Format get_format() const;

int get_mipmap_byte_size(int p_mipmap) const; //get where the mipmap begins in data
int get_mipmap_offset(int p_mipmap) const; //get where the mipmap begins in data
void get_mipmap_offset_and_size(int p_mipmap, int &r_ofs, int &r_size) const; //get where the mipmap begins in data
void get_mipmap_offset_size_and_dimensions(int p_mipmap, int &r_ofs, int &r_size, int &w, int &h) const; //get where the mipmap begins in data
/**
* Get where the mipmap begins in data.
*/
int64_t get_mipmap_offset(int p_mipmap) const;
void get_mipmap_offset_and_size(int p_mipmap, int64_t &r_ofs, int64_t &r_size) const;
void get_mipmap_offset_size_and_dimensions(int p_mipmap, int64_t &r_ofs, int64_t &r_size, int &w, int &h) const;

enum Image3DValidateError {
VALIDATE_3D_OK,
Expand Down Expand Up @@ -354,8 +356,8 @@ class Image : public Resource {
static int get_image_data_size(int p_width, int p_height, Format p_format, bool p_mipmaps = false);
static int get_image_required_mipmaps(int p_width, int p_height, Format p_format);
static Size2i get_image_mipmap_size(int p_width, int p_height, Format p_format, int p_mipmap);
static int get_image_mipmap_offset(int p_width, int p_height, Format p_format, int p_mipmap);
static int get_image_mipmap_offset_and_dimensions(int p_width, int p_height, Format p_format, int p_mipmap, int &r_w, int &r_h);
static int64_t get_image_mipmap_offset(int p_width, int p_height, Format p_format, int p_mipmap);
static int64_t get_image_mipmap_offset_and_dimensions(int p_width, int p_height, Format p_format, int p_mipmap, int &r_w, int &r_h);

enum CompressMode {
COMPRESS_S3TC,
Expand Down Expand Up @@ -383,7 +385,7 @@ class Image : public Resource {
void srgb_to_linear();
void normal_map_to_xy();
Ref<Image> rgbe_to_srgb();
Ref<Image> get_image_from_mipmap(int p_mipamp) const;
Ref<Image> get_image_from_mipmap(int p_mipmap) const;
void bump_map_to_normal_map(float bump_scale = 1.0);

void blit_rect(const Ref<Image> &p_src, const Rect2i &p_src_rect, const Point2i &p_dest);
Expand Down
2 changes: 1 addition & 1 deletion drivers/gles3/storage/texture_storage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1491,7 +1491,7 @@ void TextureStorage::_texture_set_data(RID p_texture, const Ref<Image> &p_image,
int tsize = 0;

for (int i = 0; i < mipmaps; i++) {
int size, ofs;
int64_t size, ofs;
img->get_mipmap_offset_and_size(i, ofs, size);
if (compressed) {
glPixelStorei(GL_UNPACK_ALIGNMENT, 4);
Expand Down
8 changes: 8 additions & 0 deletions misc/extension_api_validation/4.2-stable.expected
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,3 +372,11 @@ GH-93982
Validate extension JSON: Error: Field 'classes/Sprite3D/properties/frame_coords': type changed value in new API, from "Vector2" to "Vector2i".

The type was wrong to begin with and has been corrected. Vector2 and Vector2i are convertible, so it should be compatible.


GH-94243
--------
Validate extension JSON: Error: Field 'classes/Image/methods/get_mipmap_offset/return_value': meta changed value in new API, from "int32" to "int64".

Type changed to int64_t to support baking large lightmaps.
No compatibility method needed, both GDExtension and C# generate it as int64_t anyway.
3 changes: 2 additions & 1 deletion modules/basis_universal/image_compress_basisu.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ Vector<uint8_t> basis_universal_packer(const Ref<Image> &p_image, Image::UsedCha
Vector<uint32_t> mip_data_padded;

for (int32_t i = 0; i <= image->get_mipmap_count(); i++) {
int ofs, size, width, height;
int64_t ofs, size;
int width, height;
image->get_mipmap_offset_size_and_dimensions(i, ofs, size, width, height);

const uint8_t *image_mip_data = image_data.ptr() + ofs;
Expand Down
5 changes: 3 additions & 2 deletions modules/squish/image_decompress_squish.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,11 @@ void image_decompress_squish(Image *p_image) {
}

for (int i = 0; i <= mm_count; i++) {
int src_ofs = 0, mipmap_size = 0, mipmap_w = 0, mipmap_h = 0;
int64_t src_ofs = 0, mipmap_size = 0;
int mipmap_w = 0, mipmap_h = 0;
p_image->get_mipmap_offset_size_and_dimensions(i, src_ofs, mipmap_size, mipmap_w, mipmap_h);

int dst_ofs = Image::get_image_mipmap_offset(p_image->get_width(), p_image->get_height(), target_format, i);
int64_t dst_ofs = Image::get_image_mipmap_offset(p_image->get_width(), p_image->get_height(), target_format, i);
squish::DecompressImage(&wb[dst_ofs], w, h, &rb[src_ofs], squish_flags);

w >>= 1;
Expand Down
12 changes: 6 additions & 6 deletions tests/core/io/test_image.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,8 @@ TEST_CASE("[Image] Custom mipmaps") {
uint8_t *data_ptr = data.ptrw();

for (int mip = 0; mip < mipmaps; mip++) {
int mip_offset = 0;
int mip_size = 0;
int64_t mip_offset = 0;
int64_t mip_size = 0;
image->get_mipmap_offset_and_size(mip, mip_offset, mip_size);

for (int i = 0; i < mip_size; i++) {
Expand All @@ -378,8 +378,8 @@ TEST_CASE("[Image] Custom mipmaps") {
const uint8_t *data_ptr = data.ptr();

for (int mip = 0; mip < mipmaps; mip++) {
int mip_offset = 0;
int mip_size = 0;
int64_t mip_offset = 0;
int64_t mip_size = 0;
image_bytes->get_mipmap_offset_and_size(mip, mip_offset, mip_size);

for (int i = 0; i < mip_size; i++) {
Expand All @@ -402,8 +402,8 @@ TEST_CASE("[Image] Custom mipmaps") {
const uint8_t *data_ptr = data.ptr();

for (int mip = 0; mip < mipmaps; mip++) {
int mip_offset = 0;
int mip_size = 0;
int64_t mip_offset = 0;
int64_t mip_size = 0;
image_rgbaf->get_mipmap_offset_and_size(mip, mip_offset, mip_size);

for (int i = 0; i < mip_size; i += 4) {
Expand Down

0 comments on commit a0943ac

Please sign in to comment.