Merge pull request #2802 from shun-iwasawa/g/fix_macro_curvefx_crash

Fix Crash When Loading MacroFx Containing CurveFx

toonz/sources/common/tfx/tfx.cpp

@@ -703,11 +703,18 @@ void TFx::setNewIdentifier() { m_imp->m_id = ++m_imp->m_nextId; }
 void TFx::loadData(TIStream &is) {
   std::string tagName;
   VersionNumber tnzVersion = is.getVersion();
-
+  // Prevent to load "params" tag under "super" tag on saving macro fx.
+  // For now "params" tag is no longer saved under "super" tag.
+  // This is for keeping compatibility with older versions.
+  bool isInSuperTag = (is.getCurrentTagName() == "super");
   QList<int> groupIds;
   QList<std::wstring> groupNames;
   while (is.openChild(tagName)) {
     if (tagName == "params") {
+      if (isInSuperTag) {  // skip loading "params" tag under "super" tag
+        is.skipCurrentTag();
+        continue;
+      }
       while (!is.eos()) {
         std::string paramName;
         while (is.openChild(paramName)) {
@@ -829,29 +836,36 @@ void TFx::loadData(TIStream &is) {
 //--------------------------------------------------
 
 void TFx::saveData(TOStream &os) {
+  // Prevent to save "params" tag under "super" tag on saving macro fx.
+  // Parameters for macro fx are saved in "nodes" tag and corrected upon
+  // loading. Therefore, "params" tag is not needed and even causes crash if
+  // macrofx contains CurveFx (See the issue #2424)
+  bool isInSuperTag  = (os.getCurrentTagName() == "super");
   TFx *linkedSetRoot = this;
   if (m_imp->m_next != m_imp) {
     TFxImp *imp = m_imp->m_next;
     int guard   = 0;
     while (guard++ < 1000 && imp != m_imp) {
       if (imp->m_fx < linkedSetRoot) linkedSetRoot = imp->m_fx;
-      imp                                          = imp->m_next;
+      imp = imp->m_next;
     }
     assert(imp == m_imp);
     assert(linkedSetRoot);
   }
   if (linkedSetRoot == this) {
-    os.openChild("params");
+    if (!isInSuperTag) {  // skip saving "params" tag under "super" tag
-    for (int i = 0; i < getParams()->getParamCount(); i++) {
+      os.openChild("params");
-      std::string paramName     = getParams()->getParamName(i);
+      for (int i = 0; i < getParams()->getParamCount(); i++) {
-      const TParamVar *paramVar = getParams()->getParamVar(i);
+        std::string paramName     = getParams()->getParamName(i);
-      // skip saving for the obsolete parameters
+        const TParamVar *paramVar = getParams()->getParamVar(i);
-      if (paramVar->isObsolete()) continue;
+        // skip saving for the obsolete parameters
-      os.openChild(paramName);
+        if (paramVar->isObsolete()) continue;
-      paramVar->getParam()->saveData(os);
+        os.openChild(paramName);
+        paramVar->getParam()->saveData(os);
+        os.closeChild();
+      }
       os.closeChild();
     }
-    os.closeChild();
   } else {
     os.openChild("paramsLinkedTo");
     os << linkedSetRoot;

toonz/sources/common/tstream/tstream.cpp

@@ -24,7 +24,8 @@ namespace {
 string escape(string v) {
   int i = 0;
   for (;;) {
-// Removing escaping of apostrophe from Windows and OSX as it's not needed and causes problems
+// Removing escaping of apostrophe from Windows and OSX as it's not needed and
+// causes problems
 #ifdef LINUX
     i = v.find_first_of("\\\'\"", i);
 #else
@@ -353,10 +354,11 @@ TOStream &TOStream::operator<<(string v) {
   }
   int i;
   for (i = 0; i < len; i++)
-    if ((!iswalnum(v[i]) && v[i] != '_' && v[i] != '%')
+    if ((!iswalnum(v[i]) && v[i] != '_' && v[i] != '%') ||
-		|| v[i] < 32   // Less than ASCII for SPACE
+        v[i] < 32      // Less than ASCII for SPACE
-		|| v[i] > 126  // Greater than ASCII for ~
+        || v[i] > 126  // Greater than ASCII for ~
-		) break;
+    )
+      break;
   if (i == len)
     os << v << " ";
   else {
@@ -381,10 +383,11 @@ TOStream &TOStream::operator<<(QString _v) {
   }
   int i;
   for (i = 0; i < len; i++)
-	  if ((!iswalnum(v[i]) && v[i] != '_' && v[i] != '%')
+    if ((!iswalnum(v[i]) && v[i] != '_' && v[i] != '%') ||
-		  || v[i] < 32   // Less than ASCII for SPACE
+        v[i] < 32      // Less than ASCII for SPACE
-		  || v[i] > 126  // Greater than ASCII for ~
+        || v[i] > 126  // Greater than ASCII for ~
-		  ) break;
+    )
+      break;
   if (i == len)
     os << v << " ";
   else {
@@ -467,7 +470,7 @@ TOStream &TOStream::operator<<(const TPixel64 &v) {
 
 void TOStream::cr() {
   *(m_imp->m_os) << endl;
-  for (int i           = 0; i < m_imp->m_tab; i++) *(m_imp->m_os) << "  ";
+  for (int i = 0; i < m_imp->m_tab; i++) *(m_imp->m_os) << "  ";
   m_imp->m_justStarted = false;
 }
 
@@ -584,6 +587,10 @@ bool TOStream::checkStatus() const {
   return m_imp->m_os->rdstate() == ios_base::goodbit;
 }
 
+std::string TOStream::getCurrentTagName() {
+  return (m_imp->m_tagStack.empty()) ? "" : m_imp->m_tagStack.back();
+}
+
 //===============================================================
 /*!
         This class contains TIStream's attributes.
@@ -1109,8 +1116,8 @@ TIStream &TIStream::operator>>(TPersist *&v) {
   m_imp->m_currentTag = StreamTag();
   string tagName      = tag.m_name;
   std::map<std::string, string>::iterator it;
-  int id                               = -1;
+  int id = -1;
-  it                                   = tag.m_attributes.find("id");
+  it     = tag.m_attributes.find("id");
   if (it != tag.m_attributes.end()) id = atoi(it->second.c_str());
   // cout << "tagname = " << tagName << " id = " << id << endl;
 
@@ -1268,3 +1275,7 @@ void TIStream::setVersion(const VersionNumber &version) {
 //---------------------------------------------------------------
 
 void TIStream::skipCurrentTag() { m_imp->skipCurrentTag(); }
+
+//---------------------------------------------------------------
+
+std::string TIStream::getCurrentTagName() { return m_imp->m_tagStack.back(); }

toonz/sources/include/tstream.h

@@ -155,6 +155,8 @@ reimplementation of the TPersist::loadData() function.
   void skipCurrentTag();  //!< Silently ignores the content of currently opened
                           //! tag up to its end.
 
+  std::string getCurrentTagName();
+
 private:
   // Not copyable
   TIStream(const TIStream &);             //!< Not implemented
@@ -266,6 +268,8 @@ checking the status.
 */
   bool checkStatus() const;  //!< \b Flushes the stream and checks its validity.
 
+  std::string getCurrentTagName();
+
 private:
   // Not copyable
   TOStream(const TOStream &) = delete;             //!< Not implemented