diff --git a/src/Magnum/Text/AbstractFont.cpp b/src/Magnum/Text/AbstractFont.cpp index a7ba77b72..be01f267f 100644 --- a/src/Magnum/Text/AbstractFont.cpp +++ b/src/Magnum/Text/AbstractFont.cpp @@ -105,27 +105,33 @@ bool AbstractFont::openData(Containers::ArrayView data, const Float the check doesn't be done on the plugin side) because for some file formats it could be valid (MagnumFont in particular). */ close(); - const Metrics metrics = doOpenData(Containers::arrayCast(data), size); - _size = metrics.size; - _ascent = metrics.ascent; - _descent = metrics.descent; - _lineHeight = metrics.lineHeight; - CORRADE_INTERNAL_ASSERT(isOpened() || (!_size && !_ascent && !_descent && !_lineHeight)); - return isOpened(); + const Properties properties = doOpenData(Containers::arrayCast(data), size); + + /* If opening succeeded, save the returned values. If not, the values were + set to their default values by close() already. */ + if(isOpened()) { + _size = properties.size; + _ascent = properties.ascent; + _descent = properties.descent; + _lineHeight = properties.lineHeight; + return true; + } + + return false; } -auto AbstractFont::doOpenData(Containers::ArrayView, Float) -> Metrics { +auto AbstractFont::doOpenData(Containers::ArrayView, Float) -> Properties { CORRADE_ASSERT_UNREACHABLE("Text::AbstractFont::openData(): feature advertised but not implemented", {}); } bool AbstractFont::openFile(const Containers::StringView filename, const Float size) { close(); - Metrics metrics; + Properties properties; /* If file loading callbacks are not set or the font implementation supports handling them directly, call into the implementation */ if(!_fileCallback || (doFeatures() & FontFeature::FileCallback)) { - metrics = doOpenFile(filename, size); + properties = doOpenFile(filename, size); /* Otherwise, if loading from data is supported, use the callback and pass the data through to openData(). Mark the file as ready to be closed once @@ -146,24 +152,30 @@ bool AbstractFont::openFile(const Containers::StringView filename, const Float s Error() << "Text::AbstractFont::openFile(): cannot open file" << filename; return isOpened(); } - metrics = doOpenData(*data, size); + + properties = doOpenData(*data, size); _fileCallback(filename, InputFileCallbackPolicy::Close, _fileCallbackUserData); /* Shouldn't get here, the assert is fired already in setFileCallback() */ } else CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */ - _size = metrics.size; - _ascent = metrics.ascent; - _descent = metrics.descent; - _lineHeight = metrics.lineHeight; - CORRADE_INTERNAL_ASSERT(isOpened() || (!_size && !_ascent && !_descent && !_lineHeight)); - return isOpened(); + /* If opening succeeded, save the returned values. If not, the values were + set to their default values by close() already. */ + if(isOpened()) { + _size = properties.size; + _ascent = properties.ascent; + _descent = properties.descent; + _lineHeight = properties.lineHeight; + return true; + } + + return false; } -auto AbstractFont::doOpenFile(const Containers::StringView filename, const Float size) -> Metrics { +auto AbstractFont::doOpenFile(const Containers::StringView filename, const Float size) -> Properties { CORRADE_ASSERT(features() & FontFeature::OpenData, "Text::AbstractFont::openFile(): not implemented", {}); - Metrics metrics; + Properties properties; /* If callbacks are set, use them. This is the same implementation as in openFile(), see the comment there for details. */ @@ -173,7 +185,8 @@ auto AbstractFont::doOpenFile(const Containers::StringView filename, const Float Error() << "Text::AbstractFont::openFile(): cannot open file" << filename; return {}; } - metrics = doOpenData(*data, size); + + properties = doOpenData(*data, size); _fileCallback(filename, InputFileCallbackPolicy::Close, _fileCallbackUserData); /* Otherwise open the file directly */ @@ -184,19 +197,24 @@ auto AbstractFont::doOpenFile(const Containers::StringView filename, const Float return {}; } - metrics = doOpenData(*data, size); + properties = doOpenData(*data, size); } - return metrics; + return properties; } void AbstractFont::close() { - if(isOpened()) { - doClose(); - _size = 0.0f; - _lineHeight = 0.0f; - CORRADE_INTERNAL_ASSERT(!isOpened()); - } + if(!isOpened()) return; + + doClose(); + CORRADE_INTERNAL_ASSERT(!isOpened()); + + /* Clear the saved values to avoid accidental use of stale (state even + though their public access is guarded with isOpened()) */ + _size = {}; + _lineHeight = {}; + _descent = {}; + _lineHeight = {}; } Float AbstractFont::size() const { diff --git a/src/Magnum/Text/AbstractFont.h b/src/Magnum/Text/AbstractFont.h index 07b9c6ead..a6f7b15fb 100644 --- a/src/Magnum/Text/AbstractFont.h +++ b/src/Magnum/Text/AbstractFont.h @@ -496,15 +496,15 @@ class MAGNUM_TEXT_EXPORT AbstractFont: public PluginManager::AbstractPlugin { protected: /** - * @brief Font metrics + * @brief Font properties * - * @see @ref doOpenFile(), @ref doOpenData() + * Returned from @ref doOpenFile(), @ref doOpenData(). */ - struct Metrics { + struct Properties { #ifndef DOXYGEN_GENERATING_OUTPUT /* Otherwise GCC 4.8 loudly complains about missing initializers */ - constexpr /*implicit*/ Metrics() noexcept: size{}, ascent{}, descent{}, lineHeight{} {} - constexpr /*implicit*/ Metrics(Float size, Float ascent, Float descent, Float lineHeight) noexcept: size{size}, ascent{ascent}, descent{descent}, lineHeight{lineHeight} {} + constexpr /*implicit*/ Properties() noexcept: size{}, ascent{}, descent{}, lineHeight{} {} + constexpr /*implicit*/ Properties(Float size, Float ascent, Float descent, Float lineHeight) noexcept: size{size}, ascent{ascent}, descent{descent}, lineHeight{lineHeight} {} #endif /** @@ -536,8 +536,11 @@ class MAGNUM_TEXT_EXPORT AbstractFont: public PluginManager::AbstractPlugin { /** * @brief Implementation for @ref openFile() * - * Return metrics of opened font on successful opening, zeros - * otherwise. If @ref FontFeature::OpenData is supported, default + * If @ref doIsOpened() returns @cpp true @ce after calling this + * function, it's assumed that opening was successful and the + * @ref Properties are expected to contain valid values. If + * @ref doIsOpened() returns @cpp false @ce, the returned values are + * ignored. If @ref FontFeature::OpenData is supported, default * implementation opens the file and calls @ref doOpenData() with its * contents. It is allowed to call this function from your * @ref doOpenFile() implementation --- in particular, this @@ -549,7 +552,7 @@ class MAGNUM_TEXT_EXPORT AbstractFont: public PluginManager::AbstractPlugin { * supported --- instead, file is loaded though the callback and data * passed through to @ref doOpenData(). */ - virtual Metrics doOpenFile(Containers::StringView filename, Float size); + virtual Properties doOpenFile(Containers::StringView filename, Float size); private: /** @brief Implementation for @ref features() */ @@ -572,10 +575,13 @@ class MAGNUM_TEXT_EXPORT AbstractFont: public PluginManager::AbstractPlugin { /** * @brief Implementation for @ref openData() * - * Return metrics of opened font on successful opening, zeros - * otherwise. + * If @ref doIsOpened() returns @cpp true @ce after calling this + * function, it's assumed that opening was successful and the + * @ref Properties are expected to contain valid values. If + * @ref doIsOpened() returns @cpp false @ce, the returned values are + * ignored. */ - virtual Metrics doOpenData(Containers::ArrayView data, Float size); + virtual Properties doOpenData(Containers::ArrayView data, Float size); /** @brief Implementation for @ref close() */ virtual void doClose() = 0; diff --git a/src/Magnum/Text/Test/AbstractFontTest.cpp b/src/Magnum/Text/Test/AbstractFontTest.cpp index b8baf1471..0c69334c9 100644 --- a/src/Magnum/Text/Test/AbstractFontTest.cpp +++ b/src/Magnum/Text/Test/AbstractFontTest.cpp @@ -179,7 +179,7 @@ void AbstractFontTest::openData() { bool doIsOpened() const override { return _opened; } void doClose() override {} - Metrics doOpenData(const Containers::ArrayView data, Float size) override { + Properties doOpenData(const Containers::ArrayView data, Float size) override { _opened = (data.size() == 1 && data[0] == '\xa5'); return {size, 1.0f, 2.0f, 3.0f}; } @@ -209,7 +209,7 @@ void AbstractFontTest::openFileAsData() { bool doIsOpened() const override { return _opened; } void doClose() override {} - Metrics doOpenData(const Containers::ArrayView data, Float size) override { + Properties doOpenData(const Containers::ArrayView data, Float size) override { _opened = (data.size() == 1 && data[0] == '\xa5'); return {size, 1.0f, 2.0f, 3.0f}; } @@ -509,13 +509,13 @@ void AbstractFontTest::setFileCallbackOpenFileDirectly() { bool doIsOpened() const override { return _opened; } void doClose() override { _opened = false; } - Metrics doOpenFile(Containers::StringView filename, Float size) override { + Properties doOpenFile(Containers::StringView filename, Float size) override { /* Called because FileCallback is supported */ _opened = filename == "file.dat" && fileCallback() && fileCallbackUserData(); return {size, 1.0f, 2.0f, 3.0f}; } - Metrics doOpenData(Containers::ArrayView, Float) override { + Properties doOpenData(Containers::ArrayView, Float) override { /* Shouldn't be called because FileCallback is supported */ openDataCalledNotSureWhy = true; return {}; @@ -552,12 +552,12 @@ void AbstractFontTest::setFileCallbackOpenFileThroughBaseImplementation() { bool doIsOpened() const override { return _opened; } void doClose() override { _opened = false; } - Metrics doOpenFile(Containers::StringView filename, Float size) override { + Properties doOpenFile(Containers::StringView filename, Float size) override { openFileCalled = filename == "file.dat" && fileCallback() && fileCallbackUserData(); return AbstractFont::doOpenFile(filename, size); } - Metrics doOpenData(Containers::ArrayView data, Float size) override { + Properties doOpenData(Containers::ArrayView data, Float size) override { _opened = (data.size() == 1 && data[0] == '\xb0'); return {size, 1.0f, 2.0f, 3.0f}; } @@ -610,7 +610,7 @@ void AbstractFontTest::setFileCallbackOpenFileThroughBaseImplementationFailed() bool doIsOpened() const override { return false; } void doClose() override {} - Metrics doOpenFile(Containers::StringView filename, Float size) override { + Properties doOpenFile(Containers::StringView filename, Float size) override { openFileCalled = true; return AbstractFont::doOpenFile(filename, size); } @@ -642,12 +642,12 @@ void AbstractFontTest::setFileCallbackOpenFileAsData() { bool doIsOpened() const override { return _opened; } void doClose() override { _opened = false; } - Metrics doOpenFile(Containers::StringView, Float) override { + Properties doOpenFile(Containers::StringView, Float) override { openFileCalled = true; return {}; } - Metrics doOpenData(Containers::ArrayView data, Float size) override { + Properties doOpenData(Containers::ArrayView data, Float size) override { _opened = (data.size() == 1 && data[0] == '\xb0'); return {size, 1.0f, 2.0f, 3.0f}; } @@ -701,7 +701,7 @@ void AbstractFontTest::setFileCallbackOpenFileAsDataFailed() { bool doIsOpened() const override { return false; } void doClose() override {} - Metrics doOpenFile(Containers::StringView, Float) override { + Properties doOpenFile(Containers::StringView, Float) override { openFileCalled = true; return {}; } @@ -733,7 +733,7 @@ void AbstractFontTest::properties() { bool doIsOpened() const override { return _opened; } void doClose() override {} - Metrics doOpenData(const Containers::ArrayView, Float size) override { + Properties doOpenData(const Containers::ArrayView, Float size) override { _opened = true; return {size, 1.0f, 2.0f, 3.0f}; } diff --git a/src/Magnum/Text/Test/RendererGLTest.cpp b/src/Magnum/Text/Test/RendererGLTest.cpp index f04c0faf9..702d4afba 100644 --- a/src/Magnum/Text/Test/RendererGLTest.cpp +++ b/src/Magnum/Text/Test/RendererGLTest.cpp @@ -348,7 +348,7 @@ void RendererGLTest::multiline() { bool doIsOpened() const override { return _opened; } void doClose() override { _opened = false; } - Metrics doOpenFile(Containers::StringView, Float) override { + Properties doOpenFile(Containers::StringView, Float) override { _opened = true; return {0.5f, 0.45f, -0.25f, 0.75f}; } diff --git a/src/MagnumPlugins/MagnumFont/MagnumFont.cpp b/src/MagnumPlugins/MagnumFont/MagnumFont.cpp index cc7926ec9..61bf7387d 100644 --- a/src/MagnumPlugins/MagnumFont/MagnumFont.cpp +++ b/src/MagnumPlugins/MagnumFont/MagnumFont.cpp @@ -82,7 +82,7 @@ bool MagnumFont::doIsOpened() const { return _opened && _opened->image; } void MagnumFont::doClose() { _opened = nullptr; } -auto MagnumFont::doOpenData(const Containers::ArrayView data, const Float) -> Metrics { +auto MagnumFont::doOpenData(const Containers::ArrayView data, const Float) -> Properties { if(!_opened) _opened.emplace(); if(!_opened->filePath && !fileCallback()) { @@ -137,7 +137,7 @@ auto MagnumFont::doOpenData(const Containers::ArrayView data, const _opened->conf.value("lineHeight")}; } -auto MagnumFont::doOpenFile(const Containers::StringView filename, const Float size) -> Metrics { +auto MagnumFont::doOpenFile(const Containers::StringView filename, const Float size) -> Properties { _opened.emplace(); _opened->filePath.emplace(Utility::Path::split(filename).first()); diff --git a/src/MagnumPlugins/MagnumFont/MagnumFont.h b/src/MagnumPlugins/MagnumFont/MagnumFont.h index 0e6bad56d..1c1d56268 100644 --- a/src/MagnumPlugins/MagnumFont/MagnumFont.h +++ b/src/MagnumPlugins/MagnumFont/MagnumFont.h @@ -172,8 +172,8 @@ class MAGNUM_MAGNUMFONT_EXPORT MagnumFont: public AbstractFont { private: MAGNUM_MAGNUMFONT_LOCAL FontFeatures doFeatures() const override; MAGNUM_MAGNUMFONT_LOCAL bool doIsOpened() const override; - MAGNUM_MAGNUMFONT_LOCAL Metrics doOpenData(Containers::ArrayView data, Float) override; - MAGNUM_MAGNUMFONT_LOCAL Metrics doOpenFile(Containers::StringView filename, Float) override; + MAGNUM_MAGNUMFONT_LOCAL Properties doOpenData(Containers::ArrayView data, Float) override; + MAGNUM_MAGNUMFONT_LOCAL Properties doOpenFile(Containers::StringView filename, Float) override; MAGNUM_MAGNUMFONT_LOCAL void doClose() override; MAGNUM_MAGNUMFONT_LOCAL UnsignedInt doGlyphId(char32_t character) override; diff --git a/src/MagnumPlugins/MagnumFontConverter/Test/MagnumFontConverterTest.cpp b/src/MagnumPlugins/MagnumFontConverter/Test/MagnumFontConverterTest.cpp index 0211e8f1e..37ca2ca34 100644 --- a/src/MagnumPlugins/MagnumFontConverter/Test/MagnumFontConverterTest.cpp +++ b/src/MagnumPlugins/MagnumFontConverter/Test/MagnumFontConverterTest.cpp @@ -93,7 +93,7 @@ void MagnumFontConverterTest::exportFont() { private: void doClose() override { _opened = false; } bool doIsOpened() const override { return _opened; } - Metrics doOpenFile(Containers::StringView, Float) override { + Properties doOpenFile(Containers::StringView, Float) override { _opened = true; return {16.0f, 25.0f, -10.0f, 39.7333f}; }