From f82c274444f86f3c70a7406cc4a8442f61e34cac Mon Sep 17 00:00:00 2001 From: Hermet Park Date: Wed, 3 May 2023 21:51:54 +0900 Subject: [PATCH] svg_loader: fix memory violation. LoadModule data is designed to be returned to the user's call. and should not be writable in Task::run() @Issue: https://github.com/thorvg/thorvg/issues/1409 --- src/loaders/svg/tvgSvgLoader.cpp | 90 ++++++++++++++++---------- src/loaders/svg/tvgSvgLoaderCommon.h | 20 ++++-- src/loaders/svg/tvgSvgSceneBuilder.cpp | 62 +++++++++--------- src/loaders/svg/tvgSvgSceneBuilder.h | 2 +- 4 files changed, 103 insertions(+), 71 deletions(-) diff --git a/src/loaders/svg/tvgSvgLoader.cpp b/src/loaders/svg/tvgSvgLoader.cpp index 19e52c93..f78bd564 100644 --- a/src/loaders/svg/tvgSvgLoader.cpp +++ b/src/loaders/svg/tvgSvgLoader.cpp @@ -862,14 +862,16 @@ static bool _attrParseSvgNode(void* data, const char* key, const char* value) if (!strcmp(key, "width")) { doc->w = _toFloat(loader->svgParse, value, SvgParserLengthType::Horizontal); if (strstr(value, "%")) { - doc->sw = svgUtilStrtof(value, nullptr) / 100.0f; + doc->viewFlag = (doc->viewFlag | SvgViewFlag::WidthInPercent); + doc->w /= 100.0f; } else { doc->viewFlag = (doc->viewFlag | SvgViewFlag::Width); } } else if (!strcmp(key, "height")) { doc->h = _toFloat(loader->svgParse, value, SvgParserLengthType::Vertical); if (strstr(value, "%")) { - doc->sh = svgUtilStrtof(value, nullptr) / 100.0f; + doc->viewFlag = (doc->viewFlag | SvgViewFlag::HeightInPercent); + doc->h /= 100.0f; } else { doc->viewFlag = (doc->viewFlag | SvgViewFlag::Height); } @@ -1355,8 +1357,6 @@ static SvgNode* _createSvgNode(SvgLoaderData* loader, SvgNode* parent, const cha doc->align = AspectRatioAlign::XMidYMid; doc->meetOrSlice = AspectRatioMeetOrSlice::Meet; doc->viewFlag = SvgViewFlag::None; - doc->sw = 1.0f; - doc->sh = 1.0f; func(buf, bufLength, _attrParseSvgNode, loader); if (!(doc->viewFlag & SvgViewFlag::Viewbox)) { @@ -3502,7 +3502,7 @@ void SvgLoader::run(unsigned tid) if (loaderData.gradients.count > 0) _updateGradient(&loaderData, loaderData.doc, &loaderData.gradients); if (defs) _updateGradient(&loaderData, loaderData.doc, &defs->node.defs.gradients); } - root = svgSceneBuild(loaderData.doc, vx, vy, vw, vh, w, h, align, meetOrSlice, svgPath, viewFlag); + root = svgSceneBuild(loaderData, {vx, vy, vw, vh}, w, h, align, meetOrSlice, svgPath, viewFlag); } @@ -3523,43 +3523,70 @@ bool SvgLoader::header() viewFlag = loaderData.doc->node.doc.viewFlag; align = loaderData.doc->node.doc.align; meetOrSlice = loaderData.doc->node.doc.meetOrSlice; - w = 1.0f; - h = 1.0f; - //Return the brief resource info such as viewbox: - if (viewFlag & SvgViewFlag::Width) { - w = loaderData.doc->node.doc.w; - } - if (viewFlag & SvgViewFlag::Height) { - h = loaderData.doc->node.doc.h; - } - //Override size if (viewFlag & SvgViewFlag::Viewbox) { vx = loaderData.doc->node.doc.vx; vy = loaderData.doc->node.doc.vy; vw = loaderData.doc->node.doc.vw; vh = loaderData.doc->node.doc.vh; - if (!(viewFlag & SvgViewFlag::Width)) { - w = vw * loaderData.doc->node.doc.sw; - loaderData.doc->node.doc.sw = 1.0f; + if (viewFlag & SvgViewFlag::Width) w = loaderData.doc->node.doc.w; + else { + w = loaderData.doc->node.doc.vw; + if (viewFlag & SvgViewFlag::WidthInPercent) { + w *= loaderData.doc->node.doc.w; + viewFlag = (viewFlag ^ SvgViewFlag::WidthInPercent); + } viewFlag = (viewFlag | SvgViewFlag::Width); } - if (!(viewFlag & SvgViewFlag::Height)) { - h = vh * loaderData.doc->node.doc.sh; - loaderData.doc->node.doc.sh = 1.0f; + if (viewFlag & SvgViewFlag::Height) h = loaderData.doc->node.doc.h; + else { + h = loaderData.doc->node.doc.vh; + if (viewFlag & SvgViewFlag::HeightInPercent) { + h *= loaderData.doc->node.doc.h; + viewFlag = (viewFlag ^ SvgViewFlag::HeightInPercent); + } viewFlag = (viewFlag | SvgViewFlag::Height); } + //In case no viewbox and width/height data is provided the completion of loading + //has to be forced, in order to establish this data based on the whole picture. } else { - vw = w; - vh = h; + //Before loading, set default viewbox & size if they are empty + vx = vy = 0.0f; + if (viewFlag & SvgViewFlag::Width) { + vw = w = loaderData.doc->node.doc.w; + } else { + vw = 1.0f; + if (viewFlag & SvgViewFlag::WidthInPercent) { + w = loaderData.doc->node.doc.w; + } else w = 1.0f; + } + + if (viewFlag & SvgViewFlag::Height) { + vh = h = loaderData.doc->node.doc.h; + } else { + vh = 1.0f; + if (viewFlag & SvgViewFlag::HeightInPercent) { + h = loaderData.doc->node.doc.h; + } else h = 1.0f; + } + + run(0); + + //Override viewbox & size again after svg loading. + vx = loaderData.doc->node.doc.vx; + vy = loaderData.doc->node.doc.vy; + vw = loaderData.doc->node.doc.vw; + vh = loaderData.doc->node.doc.vh; + w = loaderData.doc->node.doc.w; + h = loaderData.doc->node.doc.h; } - } else { - TVGLOG("SVG", "No SVG File. There is no "); - return false; + + return true; } - return true; + TVGLOG("SVG", "No SVG File. There is no "); + return false; } @@ -3624,13 +3651,10 @@ bool SvgLoader::read() renderingDisabled = true; } - TaskScheduler::request(this); + //the loading has been already completed in header() + if (root) return true; - //In case no viewbox and width/height data is provided the completion of loading - //has to be forced, in order to establish this data based on the whole picture bounding box. - if (!(viewFlag & SvgViewFlag::Viewbox) && (!(viewFlag & SvgViewFlag::Width) || !(viewFlag & SvgViewFlag::Height))) { - this->done(); - } + TaskScheduler::request(this); return true; } diff --git a/src/loaders/svg/tvgSvgLoaderCommon.h b/src/loaders/svg/tvgSvgLoaderCommon.h index 3e4ff033..dec9fade 100644 --- a/src/loaders/svg/tvgSvgLoaderCommon.h +++ b/src/loaders/svg/tvgSvgLoaderCommon.h @@ -219,7 +219,9 @@ enum class SvgViewFlag None = 0x0, Width = 0x01, //viewPort width Height = 0x02, //viewPort height - Viewbox = 0x04 //viewBox x,y,w,h - used only if all 4 are correctly set + Viewbox = 0x04, //viewBox x,y,w,h - used only if all 4 are correctly set + WidthInPercent = 0x08, + HeightInPercent = 0x10 }; constexpr bool operator &(SvgViewFlag a, SvgViewFlag b) @@ -232,6 +234,11 @@ constexpr SvgViewFlag operator |(SvgViewFlag a, SvgViewFlag b) return SvgViewFlag(int(a) | int(b)); } +constexpr SvgViewFlag operator ^(SvgViewFlag a, SvgViewFlag b) +{ + return SvgViewFlag(int(a) ^ int(b)); +} + enum class AspectRatioAlign { None, @@ -254,14 +261,12 @@ enum class AspectRatioMeetOrSlice struct SvgDocNode { - float w; - float h; + float w; //unit: point or in percentage see: SvgViewFlag + float h; //unit: point or in percentage see: SvgViewFlag float vx; float vy; float vw; float vh; - float sw; - float sh; SvgViewFlag viewFlag; SvgNode* defs; SvgNode* style; @@ -548,6 +553,11 @@ struct SvgLoaderData bool style = false; }; +struct Box +{ + float x, y, w, h; +}; + /* * https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/strtof-strtod-l-wcstod-wcstod-l?view=vs-2017 * diff --git a/src/loaders/svg/tvgSvgSceneBuilder.cpp b/src/loaders/svg/tvgSvgSceneBuilder.cpp index e57c9f4f..c5e5df89 100644 --- a/src/loaders/svg/tvgSvgSceneBuilder.cpp +++ b/src/loaders/svg/tvgSvgSceneBuilder.cpp @@ -61,11 +61,6 @@ /* Internal Class Implementation */ /************************************************************************/ -struct Box -{ - float x, y, w, h; -}; - static bool _appendShape(SvgNode* node, Shape* shape, const Box& vBox, const string& svgPath); static unique_ptr _sceneBuildHelper(const SvgNode* node, const Box& vBox, const string& svgPath, bool mask, int depth, bool* isMaskWhite = nullptr); @@ -763,49 +758,45 @@ static unique_ptr _sceneBuildHelper(const SvgNode* node, const Box& vBox, } -static void _applySvgViewFlag(const Scene* scene, float& vx, float& vy, float& vw, float& vh, float& w, float& h, SvgViewFlag viewFlag) +static void _updateInvalidViewSize(const Scene* scene, Box& vBox, float& w, float& h, SvgViewFlag viewFlag) { - bool noViewbox = !(viewFlag & SvgViewFlag::Viewbox); - bool noWidth = !(viewFlag & SvgViewFlag::Width); - bool noHeight = !(viewFlag & SvgViewFlag::Height); + bool validWidth = (viewFlag & SvgViewFlag::Width); + bool validHeight = (viewFlag & SvgViewFlag::Height); - if (noViewbox) { - float x, y; - scene->bounds(&x, &y, &vw, &vh, false); - if (noWidth && noHeight) { - vx = x; - vy = y; - } else { - vw = noWidth ? vw : w; - vh = noHeight ? vh : h; - } + float x, y; + scene->bounds(&x, &y, &vBox.w, &vBox.h, false); + if (!validWidth && !validHeight) { + vBox.x = x; + vBox.y = y; + } else { + if (validWidth) vBox.w = w; + if (validHeight) vBox.h = h; } - w = noWidth ? vw : w; - h = noHeight ? vh : h; + + //the size would have 1x1 or percentage values. + if (!validWidth) w *= vBox.w; + if (!validHeight) h *= vBox.h; } /************************************************************************/ /* External Class Implementation */ /************************************************************************/ -unique_ptr svgSceneBuild(SvgNode* node, float& vx, float& vy, float& vw, float& vh, float& w, float& h, AspectRatioAlign align, AspectRatioMeetOrSlice meetOrSlice, const string& svgPath, SvgViewFlag viewFlag) +unique_ptr svgSceneBuild(SvgLoaderData& loaderData, Box vBox, float w, float h, AspectRatioAlign align, AspectRatioMeetOrSlice meetOrSlice, const string& svgPath, SvgViewFlag viewFlag) { //TODO: aspect ratio is valid only if viewBox was set - if (!node || (node->type != SvgNodeType::Doc)) return nullptr; + if (!loaderData.doc || (loaderData.doc->type != SvgNodeType::Doc)) return nullptr; - Box vBox = {vx, vy, vw, vh}; - auto docNode = _sceneBuildHelper(node, vBox, svgPath, false, 0); + auto docNode = _sceneBuildHelper(loaderData.doc, vBox, svgPath, false, 0); - _applySvgViewFlag(docNode.get(), vx, vy, vw, vh, w, h, viewFlag); - w *= node->node.doc.sw; - h *= node->node.doc.sh; + if (!(viewFlag & SvgViewFlag::Viewbox)) _updateInvalidViewSize(docNode.get(), vBox, w, h, viewFlag); - if (!mathEqual(w, vw) || !mathEqual(h, vh)) { - Matrix m = _calculateAspectRatioMatrix(align, meetOrSlice, w, h, {vx, vy, vw, vh}); + if (!mathEqual(w, vBox.w) || !mathEqual(h, vBox.h)) { + Matrix m = _calculateAspectRatioMatrix(align, meetOrSlice, w, h, vBox); docNode->transform(m); - } else if (!mathZero(vx) || !mathZero(vy)) { - docNode->translate(-vx, -vy); + } else if (!mathZero(vBox.x) || !mathZero(vBox.y)) { + docNode->translate(-vBox.x, -vBox.y); } auto viewBoxClip = Shape::gen(); @@ -819,5 +810,12 @@ unique_ptr svgSceneBuild(SvgNode* node, float& vx, float& vy, float& vw, auto root = Scene::gen(); root->push(move(compositeLayer)); + loaderData.doc->node.doc.vx = vBox.x; + loaderData.doc->node.doc.vy = vBox.y; + loaderData.doc->node.doc.vw = vBox.w; + loaderData.doc->node.doc.vh = vBox.h; + loaderData.doc->node.doc.w = w; + loaderData.doc->node.doc.h = h; + return root; } diff --git a/src/loaders/svg/tvgSvgSceneBuilder.h b/src/loaders/svg/tvgSvgSceneBuilder.h index 0de8c9a7..f6a60f85 100644 --- a/src/loaders/svg/tvgSvgSceneBuilder.h +++ b/src/loaders/svg/tvgSvgSceneBuilder.h @@ -25,6 +25,6 @@ #include "tvgCommon.h" -unique_ptr svgSceneBuild(SvgNode* node, float& vx, float& vy, float& vw, float& vh, float& w, float& h, AspectRatioAlign align, AspectRatioMeetOrSlice meetOrSlice, const string& svgPath, SvgViewFlag viewFlag); +unique_ptr svgSceneBuild(SvgLoaderData& loaderData, Box vBox, float w, float h, AspectRatioAlign align, AspectRatioMeetOrSlice meetOrSlice, const string& svgPath, SvgViewFlag viewFlag); #endif //_TVG_SVG_SCENE_BUILDER_H_