From 6fff7711182b289d3714c522dedc16e6dead9f1c Mon Sep 17 00:00:00 2001 From: David Capello Date: Thu, 1 Aug 2019 20:20:02 -0300 Subject: [PATCH] Fix crash using mismatch of FormatOptions (fix #2130) --- src/app/doc.cpp | 2 +- src/app/doc.h | 6 ++--- src/app/file/bmp_format.cpp | 2 +- src/app/file/file.cpp | 9 ++------ src/app/file/file.h | 42 ++++++++++++++++++++++++++++++----- src/app/file/file_format.h | 9 ++++---- src/app/file/format_options.h | 5 +++++ src/app/file/gif_format.cpp | 29 +++++++++--------------- src/app/file/jpeg_format.cpp | 23 +++++++------------ src/app/file/svg_format.cpp | 22 +++++++----------- src/app/file/webp_format.cpp | 17 +++++--------- 11 files changed, 86 insertions(+), 80 deletions(-) diff --git a/src/app/doc.cpp b/src/app/doc.cpp index 902a61031..1a4944eec 100644 --- a/src/app/doc.cpp +++ b/src/app/doc.cpp @@ -247,7 +247,7 @@ bool Doc::isFullyBackedUp() const ////////////////////////////////////////////////////////////////////// // Loaded options from file -void Doc::setFormatOptions(const std::shared_ptr& format_options) +void Doc::setFormatOptions(const FormatOptionsPtr& format_options) { m_format_options = format_options; } diff --git a/src/app/doc.h b/src/app/doc.h index 23bd4b09a..5d73fa5b4 100644 --- a/src/app/doc.h +++ b/src/app/doc.h @@ -129,8 +129,8 @@ namespace app { ////////////////////////////////////////////////////////////////////// // Loaded options from file - void setFormatOptions(const std::shared_ptr& format_options); - std::shared_ptr getFormatOptions() const { return m_format_options; } + void setFormatOptions(const FormatOptionsPtr& format_options); + FormatOptionsPtr formatOptions() const { return m_format_options; } ////////////////////////////////////////////////////////////////////// // Boundaries @@ -210,7 +210,7 @@ namespace app { std::unique_ptr m_maskBoundaries; // Data to save the file in the same format that it was loaded - std::shared_ptr m_format_options; + FormatOptionsPtr m_format_options; // Extra cel used to draw extra stuff (e.g. editor's pen preview, pixels in movement, etc.) ExtraCelRef m_extraCel; diff --git a/src/app/file/bmp_format.cpp b/src/app/file/bmp_format.cpp index 47b189722..176e42df1 100644 --- a/src/app/file/bmp_format.cpp +++ b/src/app/file/bmp_format.cpp @@ -713,7 +713,7 @@ bool BmpFormat::onLoad(FileOp *fop) bmp_options->green_mask = gmask; bmp_options->blue_mask = bmask; - fop->setFormatOptions(bmp_options); + fop->setLoadedFormatOptions(bmp_options); } return true; diff --git a/src/app/file/file.cpp b/src/app/file/file.cpp index 32d5dde6a..e4c26351b 100644 --- a/src/app/file/file.cpp +++ b/src/app/file/file.cpp @@ -545,7 +545,7 @@ FileOp* FileOp::createSaveDocumentOperation(const Context* context, // Configure output format? if (fop->m_format->support(FILE_SUPPORT_GET_FORMAT_OPTIONS)) { - auto opts = fop->m_format->getFormatOptions(fop.get()); + auto opts = fop->m_format->askUserForFormatOptions(fop.get()); // Does the user cancelled the operation? if (!opts) @@ -1002,12 +1002,7 @@ void FileOp::postLoad() m_document->markAsSaved(); } -std::shared_ptr FileOp::formatOptions() const -{ - return m_formatOptions; -} - -void FileOp::setFormatOptions(const std::shared_ptr& opts) +void FileOp::setLoadedFormatOptions(const FormatOptionsPtr& opts) { ASSERT(!m_formatOptions); m_formatOptions = opts; diff --git a/src/app/file/file.h b/src/app/file/file.h index 6af73b98c..93ddaeace 100644 --- a/src/app/file/file.h +++ b/src/app/file/file.h @@ -10,7 +10,9 @@ #pragma once #include "app/color.h" +#include "app/doc.h" #include "app/file/file_op_config.h" +#include "app/file/format_options.h" #include "app/pref/preferences.h" #include "base/mutex.h" #include "base/paths.h" @@ -49,9 +51,7 @@ namespace doc { namespace app { class Context; - class Doc; class FileFormat; - class FormatOptions; using namespace doc; @@ -143,8 +143,39 @@ namespace app { void postLoad(); // Special options specific to the file format. - std::shared_ptr formatOptions() const; - void setFormatOptions(const std::shared_ptr& opts); + FormatOptionsPtr formatOptions() const { + return m_formatOptions; + } + + // Options to save the document. This function doesn't return + // nullptr. + template + std::shared_ptr formatOptionsForSaving() const { + auto opts = std::dynamic_pointer_cast(m_formatOptions); + if (!opts) + opts = std::make_shared(); + ASSERT(opts); + return opts; + } + + // Options from the document when it was loaded. This function + // doesn't return nullptr. + template + std::shared_ptr formatOptionsOfDocument() const { + // We use the dynamic cast because the document format options + // could be an instance of another class than T. + auto opts = std::dynamic_pointer_cast(m_document->formatOptions()); + if (!opts) { + // If the document doesn't have format options (or the type + // doesn't matches T), we create default format options for + // this file. + opts = std::make_shared(); + } + ASSERT(opts); + return opts; + } + + void setLoadedFormatOptions(const FormatOptionsPtr& opts); // Helpers for file decoder/encoder (FileFormat) with // FILE_SUPPORT_SEQUENCES flag. @@ -214,7 +245,8 @@ namespace app { FileOpConfig m_config; - std::shared_ptr m_formatOptions; + // Options + FormatOptionsPtr m_formatOptions; // Data for sequences. struct { diff --git a/src/app/file/file_format.h b/src/app/file/file_format.h index c8aeda5d3..cb1387a38 100644 --- a/src/app/file/file_format.h +++ b/src/app/file/file_format.h @@ -9,6 +9,7 @@ #define APP_FILE_FILE_FORMAT_H_INCLUDED #pragma once +#include "app/file/format_options.h" #include "base/paths.h" #include "dio/file_format.h" @@ -65,8 +66,8 @@ namespace app { // Returns extra options for this format. It can return != NULL // only if flags() returns FILE_SUPPORT_GET_FORMAT_OPTIONS. - std::shared_ptr getFormatOptions(FileOp* fop) { - return onGetFormatOptions(fop); + FormatOptionsPtr askUserForFormatOptions(FileOp* fop) { + return onAskUserForFormatOptions(fop); } // Returns true if this file format supports the given flag. @@ -87,8 +88,8 @@ namespace app { #endif virtual void onDestroyData(FileOp* fop) { } - virtual std::shared_ptr onGetFormatOptions(FileOp* fop) { - return std::shared_ptr(nullptr); + virtual FormatOptionsPtr onAskUserForFormatOptions(FileOp* fop) { + return FormatOptionsPtr(nullptr); } }; diff --git a/src/app/file/format_options.h b/src/app/file/format_options.h index d210a3a61..c71ea61a1 100644 --- a/src/app/file/format_options.h +++ b/src/app/file/format_options.h @@ -1,4 +1,5 @@ // Aseprite +// Copyright (C) 2019 Igara Studio S.A. // Copyright (C) 2001-2015 David Capello // // This program is distributed under the terms of @@ -8,6 +9,8 @@ #define APP_FILE_FORMAT_OPTIONS_H_INCLUDED #pragma once +#include + namespace app { // Extra options loaded from a file that can be useful to save the @@ -18,6 +21,8 @@ namespace app { virtual ~FormatOptions() { } }; + typedef std::shared_ptr FormatOptionsPtr; + } // namespace app #endif diff --git a/src/app/file/gif_format.cpp b/src/app/file/gif_format.cpp index 21b9bb0f4..11fc14193 100644 --- a/src/app/file/gif_format.cpp +++ b/src/app/file/gif_format.cpp @@ -96,7 +96,7 @@ class GifFormat : public FileFormat { #ifdef ENABLE_SAVE bool onSave(FileOp* fop) override; #endif - std::shared_ptr onGetFormatOptions(FileOp* fop) override; + FormatOptionsPtr onAskUserForFormatOptions(FileOp* fop) override; }; FileFormat* CreateGifFormat() @@ -1412,29 +1412,23 @@ bool GifFormat::onSave(FileOp* fop) #endif // ENABLE_SAVE -std::shared_ptr GifFormat::onGetFormatOptions(FileOp* fop) +FormatOptionsPtr GifFormat::onAskUserForFormatOptions(FileOp* fop) { - std::shared_ptr gif_options; - if (fop->document()->getFormatOptions()) - gif_options = std::static_pointer_cast(fop->document()->getFormatOptions()); - - if (!gif_options) - gif_options = std::make_shared(); - + auto opts = fop->formatOptionsOfDocument(); #ifdef ENABLE_UI if (fop->context() && fop->context()->isUIAvailable()) { try { auto& pref = Preferences::instance(); if (pref.isSet(pref.gif.interlaced)) - gif_options->setInterlaced(pref.gif.interlaced()); + opts->setInterlaced(pref.gif.interlaced()); if (pref.isSet(pref.gif.loop)) - gif_options->setLoop(pref.gif.loop()); + opts->setLoop(pref.gif.loop()); if (pref.gif.showAlert()) { app::gen::GifOptions win; - win.interlaced()->setSelected(gif_options->interlaced()); - win.loop()->setSelected(gif_options->loop()); + win.interlaced()->setSelected(opts->interlaced()); + win.loop()->setSelected(opts->loop()); win.openWindowInForeground(); @@ -1443,11 +1437,11 @@ std::shared_ptr GifFormat::onGetFormatOptions(FileOp* fop) pref.gif.loop(win.loop()->isSelected()); pref.gif.showAlert(!win.dontShow()->isSelected()); - gif_options->setInterlaced(pref.gif.interlaced()); - gif_options->setLoop(pref.gif.loop()); + opts->setInterlaced(pref.gif.interlaced()); + opts->setLoop(pref.gif.loop()); } else { - gif_options.reset(); + opts.reset(); } } } @@ -1457,8 +1451,7 @@ std::shared_ptr GifFormat::onGetFormatOptions(FileOp* fop) } } #endif // ENABLE_UI - - return gif_options; + return opts; } } // namespace app diff --git a/src/app/file/jpeg_format.cpp b/src/app/file/jpeg_format.cpp index b77414cff..9887fb90d 100644 --- a/src/app/file/jpeg_format.cpp +++ b/src/app/file/jpeg_format.cpp @@ -75,7 +75,7 @@ class JpegFormat : public FileFormat { const gfx::ColorSpace* colorSpace); #endif - std::shared_ptr onGetFormatOptions(FileOp* fop) override; + FormatOptionsPtr onAskUserForFormatOptions(FileOp* fop) override; }; FileFormat* CreateJpegFormat() @@ -521,36 +521,30 @@ void JpegFormat::saveColorSpace(FileOp* fop, jpeg_compress_struct* cinfo, #endif // ENABLE_SAVE // Shows the JPEG configuration dialog. -std::shared_ptr JpegFormat::onGetFormatOptions(FileOp* fop) +FormatOptionsPtr JpegFormat::onAskUserForFormatOptions(FileOp* fop) { - std::shared_ptr jpeg_options; - if (fop->document()->getFormatOptions()) - jpeg_options = std::static_pointer_cast(fop->document()->getFormatOptions()); - - if (!jpeg_options) - jpeg_options = std::make_shared(); - + auto opts = fop->formatOptionsOfDocument(); #ifdef ENABLE_UI if (fop->context() && fop->context()->isUIAvailable()) { try { auto& pref = Preferences::instance(); if (pref.isSet(pref.jpeg.quality)) - jpeg_options->quality = pref.jpeg.quality(); + opts->quality = pref.jpeg.quality(); if (pref.jpeg.showAlert()) { app::gen::JpegOptions win; - win.quality()->setValue(int(jpeg_options->quality * 10.0f)); + win.quality()->setValue(int(opts->quality * 10.0f)); win.openWindowInForeground(); if (win.closer() == win.ok()) { pref.jpeg.quality(float(win.quality()->getValue()) / 10.0f); pref.jpeg.showAlert(!win.dontShow()->isSelected()); - jpeg_options->quality = pref.jpeg.quality(); + opts->quality = pref.jpeg.quality(); } else { - jpeg_options.reset(); + opts.reset(); } } } @@ -560,8 +554,7 @@ std::shared_ptr JpegFormat::onGetFormatOptions(FileOp* fop) } } #endif // ENABLE_UI - - return jpeg_options; + return opts; } } // namespace app diff --git a/src/app/file/svg_format.cpp b/src/app/file/svg_format.cpp index 31a248ee2..f4b5c5589 100644 --- a/src/app/file/svg_format.cpp +++ b/src/app/file/svg_format.cpp @@ -64,7 +64,7 @@ class SvgFormat : public FileFormat { #ifdef ENABLE_SAVE bool onSave(FileOp* fop) override; #endif - std::shared_ptr onGetFormatOptions(FileOp* fop) override; + FormatOptionsPtr onAskUserForFormatOptions(FileOp* fop) override; }; FileFormat* CreateSvgFormat() @@ -164,36 +164,30 @@ bool SvgFormat::onSave(FileOp* fop) #endif // Shows the SVG configuration dialog. -std::shared_ptr SvgFormat::onGetFormatOptions(FileOp* fop) +FormatOptionsPtr SvgFormat::onAskUserForFormatOptions(FileOp* fop) { - std::shared_ptr svg_options; - if (fop->document()->getFormatOptions()) - svg_options = std::static_pointer_cast(fop->document()->getFormatOptions()); - - if (!svg_options) - svg_options = std::make_shared(); - + auto opts = fop->formatOptionsOfDocument(); #ifdef ENABLE_UI if (fop->context() && fop->context()->isUIAvailable()) { try { auto& pref = Preferences::instance(); if (pref.isSet(pref.svg.pixelScale)) - svg_options->pixelScale = pref.svg.pixelScale(); + opts->pixelScale = pref.svg.pixelScale(); if (pref.svg.showAlert()) { app::gen::SvgOptions win; - win.pxsc()->setTextf("%d", svg_options->pixelScale); + win.pxsc()->setTextf("%d", opts->pixelScale); win.openWindowInForeground(); if (win.closer() == win.ok()) { pref.svg.pixelScale((int)win.pxsc()->textInt()); pref.svg.showAlert(!win.dontShow()->isSelected()); - svg_options->pixelScale = pref.svg.pixelScale(); + opts->pixelScale = pref.svg.pixelScale(); } else { - svg_options.reset(); + opts.reset(); } } } @@ -203,7 +197,7 @@ std::shared_ptr SvgFormat::onGetFormatOptions(FileOp* fop) } } #endif - return svg_options; + return opts; } } // namespace app diff --git a/src/app/file/webp_format.cpp b/src/app/file/webp_format.cpp index 63b3073ca..79e802df6 100644 --- a/src/app/file/webp_format.cpp +++ b/src/app/file/webp_format.cpp @@ -69,7 +69,7 @@ class WebPFormat : public FileFormat { #ifdef ENABLE_SAVE bool onSave(FileOp* fop) override; #endif - std::shared_ptr onGetFormatOptions(FileOp* fop) override; + FormatOptionsPtr onAskUserForFormatOptions(FileOp* fop) override; }; FileFormat* CreateWebPFormat() @@ -147,7 +147,7 @@ bool WebPFormat::onLoad(FileOp* fop) case 2: type = WebPOptions::Lossless; break; } opts->setType(type); - fop->setFormatOptions(opts); + fop->setLoadedFormatOptions(opts); } } else { @@ -268,7 +268,7 @@ bool WebPFormat::onSave(FileOp* fop) return false; } - auto opts = std::static_pointer_cast(fop->formatOptions()); + auto opts = fop->formatOptionsForSaving(); WebPConfig config; WebPConfigInit(&config); @@ -367,15 +367,9 @@ bool WebPFormat::onSave(FileOp* fop) #endif // ENABLE_SAVE // Shows the WebP configuration dialog. -std::shared_ptr WebPFormat::onGetFormatOptions(FileOp* fop) +FormatOptionsPtr WebPFormat::onAskUserForFormatOptions(FileOp* fop) { - std::shared_ptr opts; - if (fop->document()->getFormatOptions()) - opts = std::static_pointer_cast(fop->document()->getFormatOptions()); - - if (!opts) - opts = std::make_shared(); - + auto opts = fop->formatOptionsOfDocument(); #ifdef ENABLE_UI if (fop->context() && fop->context()->isUIAvailable()) { try { @@ -462,7 +456,6 @@ std::shared_ptr WebPFormat::onGetFormatOptions(FileOp* fop) } } #endif // ENABLE_UI - return opts; }