From d3b8faac746b9ecffbec1f905c9eceef27c27ad7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Fri, 18 Nov 2022 22:47:55 +0100 Subject: [PATCH] ImageLoaderSVG: Improve error reporting --- editor/editor_themes.cpp | 3 +- modules/svg/image_loader_svg.cpp | 41 +++++++++++++------ modules/svg/image_loader_svg.h | 2 +- .../resources/default_theme/default_theme.cpp | 3 +- 4 files changed, 34 insertions(+), 15 deletions(-) diff --git a/editor/editor_themes.cpp b/editor/editor_themes.cpp index 692deb3beb46..df28b2e6ab2b 100644 --- a/editor/editor_themes.cpp +++ b/editor/editor_themes.cpp @@ -240,7 +240,8 @@ static Ref editor_generate_icon(int p_index, float p_scale, float // with integer editor scales. const bool upsample = !Math::is_equal_approx(Math::round(p_scale), p_scale); ImageLoaderSVG img_loader; - img_loader.create_image_from_string(img, editor_icons_sources[p_index], p_scale, upsample, p_convert_colors); + Error err = img_loader.create_image_from_string(img, editor_icons_sources[p_index], p_scale, upsample, p_convert_colors); + ERR_FAIL_COND_V_MSG(err != OK, Ref(), "Failed generating icon, unsupported or invalid SVG data in editor theme."); if (p_saturation != 1.0) { img->adjust_bcs(1.0, 1.0, p_saturation); } diff --git a/modules/svg/image_loader_svg.cpp b/modules/svg/image_loader_svg.cpp index b8c412a2017c..2dba4916a038 100644 --- a/modules/svg/image_loader_svg.cpp +++ b/modules/svg/image_loader_svg.cpp @@ -67,8 +67,8 @@ void ImageLoaderSVG::_replace_color_property(const HashMap &p_colo } } -void ImageLoaderSVG::create_image_from_string(Ref p_image, String p_string, float p_scale, bool p_upsample, const HashMap &p_color_map) { - ERR_FAIL_COND(Math::is_zero_approx(p_scale)); +Error ImageLoaderSVG::create_image_from_string(Ref p_image, String p_string, float p_scale, bool p_upsample, const HashMap &p_color_map) { + ERR_FAIL_COND_V_MSG(Math::is_zero_approx(p_scale), ERR_INVALID_PARAMETER, "ImageLoaderSVG: Can't load SVG with a scale of 0."); if (p_color_map.size()) { _replace_color_property(p_color_map, "stop-color=\"", p_string); @@ -81,13 +81,23 @@ void ImageLoaderSVG::create_image_from_string(Ref p_image, String p_strin tvg::Result result = picture->load((const char *)bytes.ptr(), bytes.size(), "svg", true); if (result != tvg::Result::Success) { - return; + return ERR_INVALID_DATA; } float fw, fh; picture->size(&fw, &fh); - uint32_t width = MIN(round(fw * p_scale), 16 * 1024); - uint32_t height = MIN(round(fh * p_scale), 16 * 1024); + uint32_t width = round(fw * p_scale); + uint32_t height = round(fh * p_scale); + + const uint32_t max_dimension = 16384; + if (width > max_dimension || height > max_dimension) { + WARN_PRINT(vformat( + String::utf8("ImageLoaderSVG: Target canvas dimensions %d×%d (with scale %.2f) exceed the max supported dimensions %d×%d. The target canvas will be scaled down."), + width, height, p_scale, max_dimension, max_dimension)); + width = MIN(width, max_dimension); + height = MIN(height, max_dimension); + } + picture->size(width, height); std::unique_ptr sw_canvas = tvg::SwCanvas::gen(); @@ -97,25 +107,25 @@ void ImageLoaderSVG::create_image_from_string(Ref p_image, String p_strin tvg::Result res = sw_canvas->target(buffer, width, width, height, tvg::SwCanvas::ARGB8888_STRAIGHT); if (res != tvg::Result::Success) { memfree(buffer); - ERR_FAIL_MSG("ImageLoaderSVG can't create image."); + ERR_FAIL_V_MSG(FAILED, "ImageLoaderSVG: Couldn't set target on ThorVG canvas."); } res = sw_canvas->push(std::move(picture)); if (res != tvg::Result::Success) { memfree(buffer); - ERR_FAIL_MSG("ImageLoaderSVG can't create image."); + ERR_FAIL_V_MSG(FAILED, "ImageLoaderSVG: Couldn't insert ThorVG picture on canvas."); } res = sw_canvas->draw(); if (res != tvg::Result::Success) { memfree(buffer); - ERR_FAIL_MSG("ImageLoaderSVG can't create image."); + ERR_FAIL_V_MSG(FAILED, "ImageLoaderSVG: Couldn't draw ThorVG pictures on canvas."); } res = sw_canvas->sync(); if (res != tvg::Result::Success) { memfree(buffer); - ERR_FAIL_MSG("ImageLoaderSVG can't create image."); + ERR_FAIL_V_MSG(FAILED, "ImageLoaderSVG: Couldn't sync ThorVG canvas."); } Vector image; @@ -136,6 +146,7 @@ void ImageLoaderSVG::create_image_from_string(Ref p_image, String p_strin memfree(buffer); p_image->set_data(width, height, false, Image::FORMAT_RGBA8, image); + return OK; } void ImageLoaderSVG::get_recognized_extensions(List *p_extensions) const { @@ -145,13 +156,19 @@ void ImageLoaderSVG::get_recognized_extensions(List *p_extensions) const Error ImageLoaderSVG::load_image(Ref p_image, Ref p_fileaccess, BitField p_flags, float p_scale) { String svg = p_fileaccess->get_as_utf8_string(); + Error err; if (p_flags & FLAG_CONVERT_COLORS) { - create_image_from_string(p_image, svg, p_scale, false, forced_color_map); + err = create_image_from_string(p_image, svg, p_scale, false, forced_color_map); } else { - create_image_from_string(p_image, svg, p_scale, false, HashMap()); + err = create_image_from_string(p_image, svg, p_scale, false, HashMap()); + } + + if (err != OK) { + return err; + } else if (p_image->is_empty()) { + return ERR_INVALID_DATA; } - ERR_FAIL_COND_V(p_image->is_empty(), FAILED); if (p_flags & FLAG_FORCE_LINEAR) { p_image->srgb_to_linear(); } diff --git a/modules/svg/image_loader_svg.h b/modules/svg/image_loader_svg.h index b0b0963c15e5..84511f170861 100644 --- a/modules/svg/image_loader_svg.h +++ b/modules/svg/image_loader_svg.h @@ -41,7 +41,7 @@ class ImageLoaderSVG : public ImageFormatLoader { public: static void set_forced_color_map(const HashMap &p_color_map); - void create_image_from_string(Ref p_image, String p_string, float p_scale, bool p_upsample, const HashMap &p_color_map); + Error create_image_from_string(Ref p_image, String p_string, float p_scale, bool p_upsample, const HashMap &p_color_map); virtual Error load_image(Ref p_image, Ref p_fileaccess, BitField p_flags, float p_scale) override; virtual void get_recognized_extensions(List *p_extensions) const override; diff --git a/scene/resources/default_theme/default_theme.cpp b/scene/resources/default_theme/default_theme.cpp index 894936acd7ff..f179b4b81895 100644 --- a/scene/resources/default_theme/default_theme.cpp +++ b/scene/resources/default_theme/default_theme.cpp @@ -84,7 +84,8 @@ static Ref generate_icon(int p_index) { // with integer scales. const bool upsample = !Math::is_equal_approx(Math::round(scale), scale); ImageLoaderSVG img_loader; - img_loader.create_image_from_string(img, default_theme_icons_sources[p_index], scale, upsample, HashMap()); + Error err = img_loader.create_image_from_string(img, default_theme_icons_sources[p_index], scale, upsample, HashMap()); + ERR_FAIL_COND_V_MSG(err != OK, Ref(), "Failed generating icon, unsupported or invalid SVG data in default theme."); #endif return ImageTexture::create_from_image(img);