Skip to content

Commit 1dfe600

Browse files
akien-mgaakien-mga
committed
Merge pull request godotengine#87628 from YuriSizov/assets-bigger-better-errors
Improve error reporting in the asset library and in related types
2 parents edbafe8 + 2dfa36b commit 1dfe600

7 files changed

Lines changed: 126 additions & 83 deletions

File tree

‎core/io/image.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2781,7 +2781,7 @@ void Image::_get_clipped_src_and_dest_rects(const Ref<Image> &p_src, const Rect2
27812781
}
27822782

27832783
void Image::blit_rect(const Ref<Image> &p_src, const Rect2i &p_src_rect, const Point2i &p_dest) {
2784-
ERR_FAIL_COND_MSG(p_src.is_null(), "It's not a reference to a valid Image object.");
2784+
ERR_FAIL_COND_MSG(p_src.is_null(), "Cannot blit_rect an image: invalid source Image object.");
27852785
int dsize = data.size();
27862786
int srcdsize = p_src->data.size();
27872787
ERR_FAIL_COND(dsize == 0);
@@ -2823,8 +2823,8 @@ void Image::blit_rect(const Ref<Image> &p_src, const Rect2i &p_src_rect, const P
28232823
}
28242824

28252825
void Image::blit_rect_mask(const Ref<Image> &p_src, const Ref<Image> &p_mask, const Rect2i &p_src_rect, const Point2i &p_dest) {
2826-
ERR_FAIL_COND_MSG(p_src.is_null(), "It's not a reference to a valid Image object.");
2827-
ERR_FAIL_COND_MSG(p_mask.is_null(), "It's not a reference to a valid Image object.");
2826+
ERR_FAIL_COND_MSG(p_src.is_null(), "Cannot blit_rect_mask an image: invalid source Image object.");
2827+
ERR_FAIL_COND_MSG(p_mask.is_null(), "Cannot blit_rect_mask an image: invalid mask Image object.");
28282828
int dsize = data.size();
28292829
int srcdsize = p_src->data.size();
28302830
int maskdsize = p_mask->data.size();
@@ -2873,7 +2873,7 @@ void Image::blit_rect_mask(const Ref<Image> &p_src, const Ref<Image> &p_mask, co
28732873
}
28742874

28752875
void Image::blend_rect(const Ref<Image> &p_src, const Rect2i &p_src_rect, const Point2i &p_dest) {
2876-
ERR_FAIL_COND_MSG(p_src.is_null(), "It's not a reference to a valid Image object.");
2876+
ERR_FAIL_COND_MSG(p_src.is_null(), "Cannot blend_rect an image: invalid source Image object.");
28772877
int dsize = data.size();
28782878
int srcdsize = p_src->data.size();
28792879
ERR_FAIL_COND(dsize == 0);
@@ -2908,8 +2908,8 @@ void Image::blend_rect(const Ref<Image> &p_src, const Rect2i &p_src_rect, const
29082908
}
29092909

29102910
void Image::blend_rect_mask(const Ref<Image> &p_src, const Ref<Image> &p_mask, const Rect2i &p_src_rect, const Point2i &p_dest) {
2911-
ERR_FAIL_COND_MSG(p_src.is_null(), "It's not a reference to a valid Image object.");
2912-
ERR_FAIL_COND_MSG(p_mask.is_null(), "It's not a reference to a valid Image object.");
2911+
ERR_FAIL_COND_MSG(p_src.is_null(), "Cannot blend_rect_mask an image: invalid source Image object.");
2912+
ERR_FAIL_COND_MSG(p_mask.is_null(), "Cannot blend_rect_mask an image: invalid mask Image object.");
29132913
int dsize = data.size();
29142914
int srcdsize = p_src->data.size();
29152915
int maskdsize = p_mask->data.size();

‎core/io/image.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ class Image : public Resource {
431431
void set_as_black();
432432

433433
void copy_internals_from(const Ref<Image> &p_image) {
434-
ERR_FAIL_COND_MSG(p_image.is_null(), "It's not a reference to a valid Image object.");
434+
ERR_FAIL_COND_MSG(p_image.is_null(), "Cannot copy image internals: invalid Image object.");
435435
format = p_image->format;
436436
width = p_image->width;
437437
height = p_image->height;

‎core/io/image_loader.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ void ImageFormatLoaderExtension::_bind_methods() {
8181
}
8282

8383
Error ImageLoader::load_image(String p_file, Ref<Image> p_image, Ref<FileAccess> p_custom, BitField<ImageFormatLoader::LoaderFlags> p_flags, float p_scale) {
84-
ERR_FAIL_COND_V_MSG(p_image.is_null(), ERR_INVALID_PARAMETER, "It's not a reference to a valid Image object.");
84+
ERR_FAIL_COND_V_MSG(p_image.is_null(), ERR_INVALID_PARAMETER, "Can't load an image: invalid Image object.");
8585

8686
Ref<FileAccess> f = p_custom;
8787
if (f.is_null()) {

‎editor/plugins/asset_library_editor_plugin.cpp‎

Lines changed: 110 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -758,86 +758,97 @@ void EditorAssetLibrary::_select_asset(int p_id) {
758758
_api_request("asset/" + itos(p_id), REQUESTING_ASSET);
759759
}
760760

761-
void EditorAssetLibrary::_image_update(bool use_cache, bool final, const PackedByteArray &p_data, int p_queue_id) {
761+
void EditorAssetLibrary::_image_update(bool p_use_cache, bool p_final, const PackedByteArray &p_data, int p_queue_id) {
762762
Object *obj = ObjectDB::get_instance(image_queue[p_queue_id].target);
763+
if (!obj) {
764+
return;
765+
}
763766

764-
if (obj) {
765-
bool image_set = false;
766-
PackedByteArray image_data = p_data;
767+
bool image_set = false;
768+
PackedByteArray image_data = p_data;
767769

768-
if (use_cache) {
769-
String cache_filename_base = EditorPaths::get_singleton()->get_cache_dir().path_join("assetimage_" + image_queue[p_queue_id].image_url.md5_text());
770+
if (p_use_cache) {
771+
String cache_filename_base = EditorPaths::get_singleton()->get_cache_dir().path_join("assetimage_" + image_queue[p_queue_id].image_url.md5_text());
770772

771-
Ref<FileAccess> file = FileAccess::open(cache_filename_base + ".data", FileAccess::READ);
772-
if (file.is_valid()) {
773-
PackedByteArray cached_data;
774-
int len = file->get_32();
775-
cached_data.resize(len);
773+
Ref<FileAccess> file = FileAccess::open(cache_filename_base + ".data", FileAccess::READ);
774+
if (file.is_valid()) {
775+
PackedByteArray cached_data;
776+
int len = file->get_32();
777+
cached_data.resize(len);
776778

777-
uint8_t *w = cached_data.ptrw();
778-
file->get_buffer(w, len);
779+
uint8_t *w = cached_data.ptrw();
780+
file->get_buffer(w, len);
779781

780-
image_data = cached_data;
781-
}
782+
image_data = cached_data;
783+
}
784+
}
785+
786+
int len = image_data.size();
787+
const uint8_t *r = image_data.ptr();
788+
Ref<Image> image = memnew(Image);
789+
790+
uint8_t png_signature[8] = { 137, 80, 78, 71, 13, 10, 26, 10 };
791+
uint8_t jpg_signature[3] = { 255, 216, 255 };
792+
uint8_t webp_signature[4] = { 82, 73, 70, 70 };
793+
uint8_t bmp_signature[2] = { 66, 77 };
794+
795+
if (r) {
796+
Ref<Image> parsed_image;
797+
798+
if ((memcmp(&r[0], &png_signature[0], 8) == 0) && Image::_png_mem_loader_func) {
799+
parsed_image = Image::_png_mem_loader_func(r, len);
800+
} else if ((memcmp(&r[0], &jpg_signature[0], 3) == 0) && Image::_jpg_mem_loader_func) {
801+
parsed_image = Image::_jpg_mem_loader_func(r, len);
802+
} else if ((memcmp(&r[0], &webp_signature[0], 4) == 0) && Image::_webp_mem_loader_func) {
803+
parsed_image = Image::_webp_mem_loader_func(r, len);
804+
} else if ((memcmp(&r[0], &bmp_signature[0], 2) == 0) && Image::_bmp_mem_loader_func) {
805+
parsed_image = Image::_bmp_mem_loader_func(r, len);
806+
} else if (Image::_svg_scalable_mem_loader_func) {
807+
parsed_image = Image::_svg_scalable_mem_loader_func(r, len, 1.0);
782808
}
783809

784-
int len = image_data.size();
785-
const uint8_t *r = image_data.ptr();
786-
Ref<Image> image = Ref<Image>(memnew(Image));
787-
788-
uint8_t png_signature[8] = { 137, 80, 78, 71, 13, 10, 26, 10 };
789-
uint8_t jpg_signature[3] = { 255, 216, 255 };
790-
uint8_t webp_signature[4] = { 82, 73, 70, 70 };
791-
uint8_t bmp_signature[2] = { 66, 77 };
792-
793-
if (r) {
794-
if ((memcmp(&r[0], &png_signature[0], 8) == 0) && Image::_png_mem_loader_func) {
795-
image->copy_internals_from(Image::_png_mem_loader_func(r, len));
796-
} else if ((memcmp(&r[0], &jpg_signature[0], 3) == 0) && Image::_jpg_mem_loader_func) {
797-
image->copy_internals_from(Image::_jpg_mem_loader_func(r, len));
798-
} else if ((memcmp(&r[0], &webp_signature[0], 4) == 0) && Image::_webp_mem_loader_func) {
799-
image->copy_internals_from(Image::_webp_mem_loader_func(r, len));
800-
} else if ((memcmp(&r[0], &bmp_signature[0], 2) == 0) && Image::_bmp_mem_loader_func) {
801-
image->copy_internals_from(Image::_bmp_mem_loader_func(r, len));
802-
} else if (Image::_svg_scalable_mem_loader_func) {
803-
image->copy_internals_from(Image::_svg_scalable_mem_loader_func(r, len, 1.0));
810+
if (parsed_image.is_null()) {
811+
if (is_print_verbose_enabled()) {
812+
ERR_PRINT(vformat("Asset Library: Invalid image downloaded from '%s' for asset # %d", image_queue[p_queue_id].image_url, image_queue[p_queue_id].asset_id));
804813
}
814+
} else {
815+
image->copy_internals_from(parsed_image);
805816
}
817+
}
806818

807-
if (!image->is_empty()) {
808-
switch (image_queue[p_queue_id].image_type) {
809-
case IMAGE_QUEUE_ICON:
819+
if (!image->is_empty()) {
820+
switch (image_queue[p_queue_id].image_type) {
821+
case IMAGE_QUEUE_ICON:
822+
image->resize(64 * EDSCALE, 64 * EDSCALE, Image::INTERPOLATE_LANCZOS);
823+
break;
810824

811-
image->resize(64 * EDSCALE, 64 * EDSCALE, Image::INTERPOLATE_LANCZOS);
825+
case IMAGE_QUEUE_THUMBNAIL: {
826+
float max_height = 85 * EDSCALE;
812827

813-
break;
814-
case IMAGE_QUEUE_THUMBNAIL: {
815-
float max_height = 85 * EDSCALE;
828+
float scale_ratio = max_height / (image->get_height() * EDSCALE);
829+
if (scale_ratio < 1) {
830+
image->resize(image->get_width() * EDSCALE * scale_ratio, image->get_height() * EDSCALE * scale_ratio, Image::INTERPOLATE_LANCZOS);
831+
}
832+
} break;
816833

817-
float scale_ratio = max_height / (image->get_height() * EDSCALE);
818-
if (scale_ratio < 1) {
819-
image->resize(image->get_width() * EDSCALE * scale_ratio, image->get_height() * EDSCALE * scale_ratio, Image::INTERPOLATE_LANCZOS);
820-
}
821-
} break;
822-
case IMAGE_QUEUE_SCREENSHOT: {
823-
float max_height = 397 * EDSCALE;
834+
case IMAGE_QUEUE_SCREENSHOT: {
835+
float max_height = 397 * EDSCALE;
824836

825-
float scale_ratio = max_height / (image->get_height() * EDSCALE);
826-
if (scale_ratio < 1) {
827-
image->resize(image->get_width() * EDSCALE * scale_ratio, image->get_height() * EDSCALE * scale_ratio, Image::INTERPOLATE_LANCZOS);
828-
}
829-
} break;
830-
}
837+
float scale_ratio = max_height / (image->get_height() * EDSCALE);
838+
if (scale_ratio < 1) {
839+
image->resize(image->get_width() * EDSCALE * scale_ratio, image->get_height() * EDSCALE * scale_ratio, Image::INTERPOLATE_LANCZOS);
840+
}
841+
} break;
842+
}
831843

832-
Ref<ImageTexture> tex = ImageTexture::create_from_image(image);
844+
Ref<ImageTexture> tex = ImageTexture::create_from_image(image);
833845

834-
obj->call("set_image", image_queue[p_queue_id].image_type, image_queue[p_queue_id].image_index, tex);
835-
image_set = true;
836-
}
846+
obj->call("set_image", image_queue[p_queue_id].image_type, image_queue[p_queue_id].image_index, tex);
847+
image_set = true;
848+
}
837849

838-
if (!image_set && final) {
839-
obj->call("set_image", image_queue[p_queue_id].image_type, image_queue[p_queue_id].image_index, get_editor_theme_icon(SNAME("FileBrokenBigThumb")));
840-
}
850+
if (!image_set && p_final) {
851+
obj->call("set_image", image_queue[p_queue_id].image_type, image_queue[p_queue_id].image_index, get_editor_theme_icon(SNAME("FileBrokenBigThumb")));
841852
}
842853
}
843854

@@ -870,7 +881,10 @@ void EditorAssetLibrary::_image_request_completed(int p_status, int p_code, cons
870881
_image_update(p_code == HTTPClient::RESPONSE_NOT_MODIFIED, true, p_data, p_queue_id);
871882

872883
} else {
873-
WARN_PRINT("Error getting image file from URL: " + image_queue[p_queue_id].image_url);
884+
if (is_print_verbose_enabled()) {
885+
WARN_PRINT(vformat("Asset Library: Error getting image from '%s' for asset # %d.", image_queue[p_queue_id].image_url, image_queue[p_queue_id].asset_id));
886+
}
887+
874888
Object *obj = ObjectDB::get_instance(image_queue[p_queue_id].target);
875889
if (obj) {
876890
obj->call("set_image", image_queue[p_queue_id].image_type, image_queue[p_queue_id].image_index, get_editor_theme_icon(SNAME("FileBrokenBigThumb")));
@@ -919,22 +933,48 @@ void EditorAssetLibrary::_update_image_queue() {
919933
}
920934
}
921935

922-
void EditorAssetLibrary::_request_image(ObjectID p_for, String p_image_url, ImageType p_type, int p_image_index) {
936+
void EditorAssetLibrary::_request_image(ObjectID p_for, int p_asset_id, String p_image_url, ImageType p_type, int p_image_index) {
937+
// Remove extra spaces around the URL. This isn't strictly valid, but recoverable.
938+
String trimmed_url = p_image_url.strip_edges();
939+
if (trimmed_url != p_image_url && is_print_verbose_enabled()) {
940+
WARN_PRINT(vformat("Asset Library: Badly formatted image URL '%s' for asset # %d.", p_image_url, p_asset_id));
941+
}
942+
943+
// Validate the image URL first.
944+
{
945+
String url_scheme;
946+
String url_host;
947+
int url_port;
948+
String url_path;
949+
Error err = trimmed_url.parse_url(url_scheme, url_host, url_port, url_path);
950+
if (err != OK) {
951+
if (is_print_verbose_enabled()) {
952+
ERR_PRINT(vformat("Asset Library: Invalid image URL '%s' for asset # %d.", trimmed_url, p_asset_id));
953+
}
954+
955+
Object *obj = ObjectDB::get_instance(p_for);
956+
if (obj) {
957+
obj->call("set_image", p_type, p_image_index, get_editor_theme_icon(SNAME("FileBrokenBigThumb")));
958+
}
959+
return;
960+
}
961+
}
962+
923963
ImageQueue iq;
924-
iq.image_url = p_image_url;
964+
iq.image_url = trimmed_url;
925965
iq.image_index = p_image_index;
926966
iq.image_type = p_type;
927967
iq.request = memnew(HTTPRequest);
928968
setup_http_request(iq.request);
929969

930970
iq.target = p_for;
971+
iq.asset_id = p_asset_id;
931972
iq.queue_id = ++last_queue_id;
932973
iq.active = false;
933974

934975
iq.request->connect("request_completed", callable_mp(this, &EditorAssetLibrary::_image_request_completed).bind(iq.queue_id));
935976

936977
image_queue[iq.queue_id] = iq;
937-
938978
add_child(iq.request);
939979

940980
_image_update(true, false, PackedByteArray(), iq.queue_id);
@@ -1311,7 +1351,7 @@ void EditorAssetLibrary::_http_request_completed(int p_status, int p_code, const
13111351
item->connect("category_selected", callable_mp(this, &EditorAssetLibrary::_select_category));
13121352

13131353
if (r.has("icon_url") && !r["icon_url"].operator String().is_empty()) {
1314-
_request_image(item->get_instance_id(), r["icon_url"], IMAGE_QUEUE_ICON, 0);
1354+
_request_image(item->get_instance_id(), r["asset_id"], r["icon_url"], IMAGE_QUEUE_ICON, 0);
13151355
}
13161356
}
13171357

@@ -1362,7 +1402,7 @@ void EditorAssetLibrary::_http_request_completed(int p_status, int p_code, const
13621402
}
13631403

13641404
if (r.has("icon_url") && !r["icon_url"].operator String().is_empty()) {
1365-
_request_image(description->get_instance_id(), r["icon_url"], IMAGE_QUEUE_ICON, 0);
1405+
_request_image(description->get_instance_id(), r["asset_id"], r["icon_url"], IMAGE_QUEUE_ICON, 0);
13661406
}
13671407

13681408
if (d.has("previews")) {
@@ -1383,11 +1423,11 @@ void EditorAssetLibrary::_http_request_completed(int p_status, int p_code, const
13831423
description->add_preview(i, is_video, video_url);
13841424

13851425
if (p.has("thumbnail")) {
1386-
_request_image(description->get_instance_id(), p["thumbnail"], IMAGE_QUEUE_THUMBNAIL, i);
1426+
_request_image(description->get_instance_id(), r["asset_id"], p["thumbnail"], IMAGE_QUEUE_THUMBNAIL, i);
13871427
}
13881428

13891429
if (!is_video) {
1390-
_request_image(description->get_instance_id(), p["link"], IMAGE_QUEUE_SCREENSHOT, i);
1430+
_request_image(description->get_instance_id(), r["asset_id"], p["link"], IMAGE_QUEUE_SCREENSHOT, i);
13911431
}
13921432
}
13931433
}

‎editor/plugins/asset_library_editor_plugin.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,14 +262,15 @@ class EditorAssetLibrary : public PanelContainer {
262262
String image_url;
263263
HTTPRequest *request = nullptr;
264264
ObjectID target;
265+
int asset_id = -1;
265266
};
266267

267268
int last_queue_id;
268269
HashMap<int, ImageQueue> image_queue;
269270

270-
void _image_update(bool use_cache, bool final, const PackedByteArray &p_data, int p_queue_id);
271+
void _image_update(bool p_use_cache, bool p_final, const PackedByteArray &p_data, int p_queue_id);
271272
void _image_request_completed(int p_status, int p_code, const PackedStringArray &headers, const PackedByteArray &p_data, int p_queue_id);
272-
void _request_image(ObjectID p_for, String p_image_url, ImageType p_type, int p_image_index);
273+
void _request_image(ObjectID p_for, int p_asset_id, String p_image_url, ImageType p_type, int p_image_index);
273274
void _update_image_queue();
274275

275276
HBoxContainer *_make_pages(int p_page, int p_page_count, int p_page_len, int p_total_items, int p_current_items);

‎modules/svg/image_loader_svg.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ Ref<Image> ImageLoaderSVG::load_mem_svg(const uint8_t *p_svg, int p_size, float
7272
img.instantiate();
7373

7474
Error err = create_image_from_utf8_buffer(img, p_svg, p_size, p_scale, false);
75-
ERR_FAIL_COND_V(err, Ref<Image>());
75+
ERR_FAIL_COND_V_MSG(err != OK, Ref<Image>(), vformat("ImageLoaderSVG: Failed to create SVG from buffer, error code %d.", err));
7676

7777
return img;
7878
}

‎scene/main/http_request.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,14 @@ Error HTTPRequest::_parse_url(const String &p_url) {
5050

5151
String scheme;
5252
Error err = p_url.parse_url(scheme, url, port, request_string);
53-
ERR_FAIL_COND_V_MSG(err != OK, err, "Error parsing URL: " + p_url + ".");
53+
ERR_FAIL_COND_V_MSG(err != OK, err, vformat("Error parsing URL: '%s'.", p_url));
54+
5455
if (scheme == "https://") {
5556
use_tls = true;
5657
} else if (scheme != "http://") {
57-
ERR_FAIL_V_MSG(ERR_INVALID_PARAMETER, "Invalid URL scheme: " + scheme + ".");
58+
ERR_FAIL_V_MSG(ERR_INVALID_PARAMETER, vformat("Invalid URL scheme: '%s'.", scheme));
5859
}
60+
5961
if (port == 0) {
6062
port = use_tls ? 443 : 80;
6163
}

0 commit comments

Comments
 (0)