From 3003cbea2e8a8f240d68432d6199d07d09af41e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Fri, 8 Nov 2013 21:43:05 +0100 Subject: [PATCH] Text: moved common glyph render code into AbstractLayouter::renderGlyph(). The virtual method is moved to doRenderGlyph(), for plugin implementers it means that the function only needs to be renamed (and moved to private section) and plugin version interface needs to be updated. The common layouting code allows to remove many redundant code from Renderer and also ability to test cursor position/bounding rectangle updated properly. Rectangle updating now treats rectangle with zero size as invalid and replaces it with glyph quad instead of merging the two. It means that the returned rectangle now wraps the text more tightly and does not always contain origin. --- src/Plugins/MagnumFont/MagnumFont.cpp | 28 +++--- .../MagnumFont/Test/MagnumFontTest.cpp | 19 +++-- .../pluginRegistrationMagnumFont.cpp | 2 +- src/Text/AbstractFont.cpp | 27 +++++- src/Text/AbstractFont.h | 52 +++++++++--- src/Text/Renderer.cpp | 48 +---------- src/Text/Test/AbstractLayouterTest.cpp | 85 +++++++++++++++++++ src/Text/Test/CMakeLists.txt | 1 + src/Text/Test/RendererGLTest.cpp | 8 +- 9 files changed, 182 insertions(+), 88 deletions(-) create mode 100644 src/Text/Test/AbstractLayouterTest.cpp diff --git a/src/Plugins/MagnumFont/MagnumFont.cpp b/src/Plugins/MagnumFont/MagnumFont.cpp index 6ab867287..aca408387 100644 --- a/src/Plugins/MagnumFont/MagnumFont.cpp +++ b/src/Plugins/MagnumFont/MagnumFont.cpp @@ -45,15 +45,15 @@ struct MagnumFont::Data { namespace { class MagnumFontLayouter: public AbstractLayouter { public: - explicit MagnumFontLayouter(const std::unordered_map& glyphId, const std::vector& glyphAdvance, const GlyphCache& cache, Float fontSize, Float textSize, const std::string& text); - - std::tuple renderGlyph(UnsignedInt i) override; + explicit MagnumFontLayouter(const std::vector& glyphAdvance, const GlyphCache& cache, Float fontSize, Float textSize, std::vector&& glyphs); private: + std::tuple doRenderGlyph(UnsignedInt i) override; + const std::vector& glyphAdvance; const GlyphCache& cache; const Float fontSize, textSize; - std::vector glyphs; + const std::vector glyphs; }; } @@ -194,24 +194,24 @@ std::unique_ptr MagnumFont::doCreateGlyphCache() { } std::unique_ptr MagnumFont::doLayout(const GlyphCache& cache, Float size, const std::string& text) { - return std::unique_ptr(new MagnumFontLayouter(_opened->glyphId, _opened->glyphAdvance, cache, this->size(), size, text)); -} - -namespace { - -MagnumFontLayouter::MagnumFontLayouter(const std::unordered_map& glyphId, const std::vector& glyphAdvance, const GlyphCache& cache, Float fontSize, Float textSize, const std::string& text): glyphAdvance(glyphAdvance), cache(cache), fontSize(fontSize), textSize(textSize) { /* Get glyph codes from characters */ + std::vector glyphs; glyphs.reserve(text.size()); for(std::size_t i = 0; i != text.size(); ) { UnsignedInt codepoint; std::tie(codepoint, i) = Utility::Unicode::nextChar(text, i); - const auto it = glyphId.find(codepoint); - glyphs.push_back(it == glyphId.end() ? 0 : it->second); + const auto it = _opened->glyphId.find(codepoint); + glyphs.push_back(it == _opened->glyphId.end() ? 0 : it->second); } - _glyphCount = glyphs.size(); + + return std::unique_ptr(new MagnumFontLayouter(_opened->glyphAdvance, cache, this->size(), size, std::move(glyphs))); } -std::tuple MagnumFontLayouter::renderGlyph(UnsignedInt i) { +namespace { + +MagnumFontLayouter::MagnumFontLayouter(const std::vector& glyphAdvance, const GlyphCache& cache, const Float fontSize, const Float textSize, std::vector&& glyphs): AbstractLayouter(glyphs.size()), glyphAdvance(glyphAdvance), cache(cache), fontSize(fontSize), textSize(textSize), glyphs(std::move(glyphs)) {} + +std::tuple MagnumFontLayouter::doRenderGlyph(const UnsignedInt i) { /* Position of the texture in the resulting glyph, texture coordinates */ Vector2i position; Rectanglei rectangle; diff --git a/src/Plugins/MagnumFont/Test/MagnumFontTest.cpp b/src/Plugins/MagnumFont/Test/MagnumFontTest.cpp index d22468073..c46d5d293 100644 --- a/src/Plugins/MagnumFont/Test/MagnumFontTest.cpp +++ b/src/Plugins/MagnumFont/Test/MagnumFontTest.cpp @@ -68,33 +68,34 @@ void MagnumFontTest::layout() { CORRADE_VERIFY(layouter); CORRADE_COMPARE(layouter->glyphCount(), 4); + Rectangle rectangle; Rectangle position; Rectangle textureCoordinates; - Vector2 advance; /* 'W' */ - std::tie(position, textureCoordinates, advance) = layouter->renderGlyph(0); + Vector2 cursorPosition; + std::tie(position, textureCoordinates) = layouter->renderGlyph(0, cursorPosition = {}, rectangle); CORRADE_COMPARE(position, Rectangle({0.78125f, 1.0625f}, {1.28125f, 4.8125f})); CORRADE_COMPARE(textureCoordinates, Rectangle({0, 0.03125f}, {0.0625f, 0.5f})); - CORRADE_COMPARE(advance, Vector2(0.71875f, 0.0f)); + CORRADE_COMPARE(cursorPosition, Vector2(0.71875f, 0.0f)); /* 'a' (not found) */ - std::tie(position, textureCoordinates, advance) = layouter->renderGlyph(1); + std::tie(position, textureCoordinates) = layouter->renderGlyph(1, cursorPosition = {}, rectangle); CORRADE_COMPARE(position, Rectangle()); CORRADE_COMPARE(textureCoordinates, Rectangle()); - CORRADE_COMPARE(advance, Vector2(0.25f, 0.0f)); + CORRADE_COMPARE(cursorPosition, Vector2(0.25f, 0.0f)); /* 'v' (not found) */ - std::tie(position, textureCoordinates, advance) = layouter->renderGlyph(2); + std::tie(position, textureCoordinates) = layouter->renderGlyph(2, cursorPosition = {}, rectangle); CORRADE_COMPARE(position, Rectangle()); CORRADE_COMPARE(textureCoordinates, Rectangle()); - CORRADE_COMPARE(advance, Vector2(0.25f, 0.0f)); + CORRADE_COMPARE(cursorPosition, Vector2(0.25f, 0.0f)); /* 'e' */ - std::tie(position, textureCoordinates, advance) = layouter->renderGlyph(3); + std::tie(position, textureCoordinates) = layouter->renderGlyph(3, cursorPosition = {}, rectangle); CORRADE_COMPARE(position, Rectangle({0.78125f, 0.375f}, {2.28125f, 1.25f})); CORRADE_COMPARE(textureCoordinates, Rectangle({0.0625f, 0.015625f}, {0.25f, 0.125f})); - CORRADE_COMPARE(advance, Vector2(0.375f, 0.0f)); + CORRADE_COMPARE(cursorPosition, Vector2(0.375f, 0.0f)); } void MagnumFontTest::createGlyphCache() { diff --git a/src/Plugins/MagnumFont/pluginRegistrationMagnumFont.cpp b/src/Plugins/MagnumFont/pluginRegistrationMagnumFont.cpp index 9438143d0..374ca3657 100644 --- a/src/Plugins/MagnumFont/pluginRegistrationMagnumFont.cpp +++ b/src/Plugins/MagnumFont/pluginRegistrationMagnumFont.cpp @@ -25,4 +25,4 @@ #include "MagnumFont/MagnumFont.h" CORRADE_PLUGIN_REGISTER(MagnumFont, Magnum::Text::MagnumFont, - "cz.mosra.magnum.Text.AbstractFont/0.2.2") + "cz.mosra.magnum.Text.AbstractFont/0.2.3") diff --git a/src/Text/AbstractFont.cpp b/src/Text/AbstractFont.cpp index a45c9ff43..0ebd9e2b3 100644 --- a/src/Text/AbstractFont.cpp +++ b/src/Text/AbstractFont.cpp @@ -162,8 +162,33 @@ std::unique_ptr AbstractFont::layout(const GlyphCache& cache, return doLayout(cache, size, text); } -AbstractLayouter::AbstractLayouter(): _glyphCount(0) {} +AbstractLayouter::AbstractLayouter(UnsignedInt glyphCount): _glyphCount(glyphCount) {} AbstractLayouter::~AbstractLayouter() {} +std::pair AbstractLayouter::renderGlyph(const UnsignedInt i, Vector2& cursorPosition, Rectangle& rectangle) { + CORRADE_ASSERT(i < glyphCount(), "Text::AbstractLayouter::renderGlyph(): glyph index out of bounds", {}); + + /* Render the glyph */ + Rectangle quadPosition, textureCoordinates; + Vector2 advance; + std::tie(quadPosition, textureCoordinates, advance) = doRenderGlyph(i); + + /* Move the quad to cursor */ + quadPosition.bottomLeft() += cursorPosition; + quadPosition.topRight() += cursorPosition; + + /* Extend rectangle with current quad bounds. If zero size, replace it. */ + if(!rectangle.size().isZero()) { + rectangle.bottomLeft() = Math::min(rectangle.bottomLeft(), quadPosition.bottomLeft()); + rectangle.topRight() = Math::max(rectangle.topRight(), quadPosition.topRight()); + } else rectangle = quadPosition; + + /* Advance cursor position to next character */ + cursorPosition += advance; + + /* Return moved quad and unchanged texture coordinates */ + return {quadPosition, textureCoordinates}; +} + }} diff --git a/src/Text/AbstractFont.h b/src/Text/AbstractFont.h index 57e04c753..69820d1fd 100644 --- a/src/Text/AbstractFont.h +++ b/src/Text/AbstractFont.h @@ -52,8 +52,9 @@ text rendering later, see @ref GlyphCache for more information. See @section AbstractFont-subclassing Subclassing -Plugin implements @ref doFeatures(), @ref doClose(), @ref doCreateGlyphCache(), -@ref doLayout() and one or more of `doOpen*()` functions. +Plugin implements @ref doFeatures(), @ref doClose(), @ref doLayout(), either +@ref doCreateGlyphCache() or @ref doFillGlyphCache() and one or more of +`doOpen*()` functions. See also @ref AbstractLayouter for more information. You don't need to do most of the redundant sanity checks, these things are checked by the implementation: @@ -67,7 +68,7 @@ checked by the implementation: there is any file opened. */ class MAGNUM_TEXT_EXPORT AbstractFont: public PluginManager::AbstractPlugin { - CORRADE_PLUGIN_INTERFACE("cz.mosra.magnum.Text.AbstractFont/0.2.2") + CORRADE_PLUGIN_INTERFACE("cz.mosra.magnum.Text.AbstractFont/0.2.3") public: /** @@ -282,6 +283,12 @@ CORRADE_ENUMSET_OPERATORS(AbstractFont::Features) @brief Base for text layouters Returned by @ref AbstractFont::layout(). + +@section TextAbstractLayouter-subclassing Subclassing + +Plugin creates private subclass (no need to expose it to end users) and +implements @ref doRenderGlyph(). Bounds checking on @p i is done automatically +in the wrapping @ref renderGlyph() function. */ class MAGNUM_TEXT_EXPORT AbstractLayouter { AbstractLayouter(const AbstractLayouter&) = delete; @@ -290,27 +297,46 @@ class MAGNUM_TEXT_EXPORT AbstractLayouter { AbstractLayouter& operator=(const AbstractLayouter&&) = delete; public: - explicit AbstractLayouter(); - virtual ~AbstractLayouter() = 0; + ~AbstractLayouter(); /** @brief Count of glyphs in laid out text */ - UnsignedInt glyphCount() const { - return _glyphCount; - } + UnsignedInt glyphCount() const { return _glyphCount; } /** * @brief Render glyph * @param i Glyph index + * @param cursorPosition Cursor position + * @param rectangle Bounding rectangle * - * Returns quad position, texture coordinates and advance to next - * glyph. + * The function returns pair of quad position and texture coordinates, + * advances @p cursorPosition to next character and updates @p rectangle + * with extended bounds. */ - virtual std::tuple renderGlyph(UnsignedInt i) = 0; + std::pair renderGlyph(UnsignedInt i, Vector2& cursorPosition, Rectangle& rectangle); + + protected: + /** + * @brief Constructor + * @param glyphCount Count of glyphs in laid out text + */ + explicit AbstractLayouter(UnsignedInt glyphCount); #ifdef DOXYGEN_GENERATING_OUTPUT - private: - #else protected: + #else + private: + #endif + /** + * @brief Implementation for @ref renderGlyph() + * @param i Glyph index + * + * Return quad position (relative to current cursor position), texture + * coordinates and advance to next glyph. + */ + virtual std::tuple doRenderGlyph(UnsignedInt i) = 0; + + #ifdef DOXYGEN_GENERATING_OUTPUT + private: #endif UnsignedInt _glyphCount; }; diff --git a/src/Text/Renderer.cpp b/src/Text/Renderer.cpp index 1ab54576e..bb7fdacd4 100644 --- a/src/Text/Renderer.cpp +++ b/src/Text/Renderer.cpp @@ -32,9 +32,6 @@ namespace Magnum { namespace Text { -/** @todo Move duplicate code to layouter::renderGlyph(), the implementation - should be in doRenderGlyph() then */ - namespace { template void createIndices(void* output, const UnsignedInt glyphCount) { @@ -103,14 +100,8 @@ std::tuple, std::vector, std::vector, /* Render all glyphs */ Vector2 cursorPosition; for(UnsignedInt i = 0; i != layouter->glyphCount(); ++i) { - /* Position of the texture in the resulting glyph, texture coordinates */ Rectangle quadPosition, textureCoordinates; - Vector2 advance; - std::tie(quadPosition, textureCoordinates, advance) = layouter->renderGlyph(i); - - /* Move the quad to cursor */ - quadPosition.bottomLeft() += cursorPosition; - quadPosition.topRight() += cursorPosition; + std::tie(quadPosition, textureCoordinates) = layouter->renderGlyph(i, cursorPosition, rectangle); /* 0---2 | | @@ -130,13 +121,6 @@ std::tuple, std::vector, std::vector, textureCoordinates.topRight(), textureCoordinates.bottomRight() }); - - /* Extend rectangle with current quad bounds */ - rectangle.bottomLeft() = Math::min(rectangle.bottomLeft(), quadPosition.bottomLeft()); - rectangle.topRight() = Math::max(rectangle.topRight(), quadPosition.topRight()); - - /* Advance cursor position to next character */ - cursorPosition += advance; } /* Respect the alignment */ @@ -166,14 +150,8 @@ std::tuple AbstractRenderer::render(AbstractFont& font, const G /* Render all glyphs */ Vector2 cursorPosition; for(UnsignedInt i = 0; i != layouter->glyphCount(); ++i) { - /* Position of the texture in the resulting glyph, texture coordinates */ Rectangle quadPosition, textureCoordinates; - Vector2 advance; - std::tie(quadPosition, textureCoordinates, advance) = layouter->renderGlyph(i); - - /* Move the quad to cursor */ - quadPosition.bottomLeft() += cursorPosition; - quadPosition.topRight() += cursorPosition; + std::tie(quadPosition, textureCoordinates) = layouter->renderGlyph(i, cursorPosition, rectangle); vertices.insert(vertices.end(), { {quadPosition.topLeft(), textureCoordinates.topLeft()}, @@ -181,13 +159,6 @@ std::tuple AbstractRenderer::render(AbstractFont& font, const G {quadPosition.topRight(), textureCoordinates.topRight()}, {quadPosition.bottomRight(), textureCoordinates.bottomRight()} }); - - /* Extend rectangle with current quad bounds */ - rectangle.bottomLeft() = Math::min(rectangle.bottomLeft(), quadPosition.bottomLeft()); - rectangle.topRight() = Math::max(rectangle.topRight(), quadPosition.topRight()); - - /* Advance cursor position to next character */ - cursorPosition += advance; } /* Respect the alignment */ @@ -379,27 +350,14 @@ void AbstractRenderer::render(const std::string& text) { /* Render all glyphs */ Vector2 cursorPosition; for(UnsignedInt i = 0; i != layouter->glyphCount(); ++i) { - /* Position of the texture in the resulting glyph, texture coordinates */ Rectangle quadPosition, textureCoordinates; - Vector2 advance; - std::tie(quadPosition, textureCoordinates, advance) = layouter->renderGlyph(i); - - /* Move the quad to cursor */ - quadPosition.bottomLeft() += cursorPosition; - quadPosition.topRight() += cursorPosition; - - /* Extend rectangle with current quad bounds */ - _rectangle.bottomLeft() = Math::min(_rectangle.bottomLeft(), quadPosition.bottomLeft()); - _rectangle.topRight() = Math::max(_rectangle.topRight(), quadPosition.topRight()); + std::tie(quadPosition, textureCoordinates) = layouter->renderGlyph(i, cursorPosition, _rectangle); const std::size_t vertex = i*4; vertices[vertex] = {quadPosition.topLeft(), textureCoordinates.topLeft()}; vertices[vertex+1] = {quadPosition.bottomLeft(), textureCoordinates.bottomLeft()}; vertices[vertex+2] = {quadPosition.topRight(), textureCoordinates.topRight()}; vertices[vertex+3] = {quadPosition.bottomRight(), textureCoordinates.bottomRight()}; - - /* Advance cursor position to next character */ - cursorPosition += advance; } /* Respect the alignment */ diff --git a/src/Text/Test/AbstractLayouterTest.cpp b/src/Text/Test/AbstractLayouterTest.cpp new file mode 100644 index 000000000..5da5b0584 --- /dev/null +++ b/src/Text/Test/AbstractLayouterTest.cpp @@ -0,0 +1,85 @@ +/* + This file is part of Magnum. + + Copyright © 2010, 2011, 2012, 2013 Vladimír Vondruš + + Permission is hereby granted, free of charge, to any person obtaining a + copy of this software and associated documentation files (the "Software"), + to deal in the Software without restriction, including without limitation + the rights to use, copy, modify, merge, publish, distribute, sublicense, + and/or sell copies of the Software, and to permit persons to whom the + Software is furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included + in all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + DEALINGS IN THE SOFTWARE. +*/ + +#include + +#include "Math/Geometry/Rectangle.h" +#include "Text/AbstractFont.h" + +namespace Magnum { namespace Text { namespace Test { + +class AbstractLayouterTest: public TestSuite::Tester { + public: + explicit AbstractLayouterTest(); + + void renderGlyph(); +}; + +AbstractLayouterTest::AbstractLayouterTest() { + addTests({&AbstractLayouterTest::renderGlyph}); +} + +void AbstractLayouterTest::renderGlyph() { + class Layouter: public AbstractLayouter { + public: + explicit Layouter(): AbstractLayouter(3) {} + + private: + std::tuple doRenderGlyph(UnsignedInt) override { + return std::make_tuple(Rectangle({1.0f, 0.5f}, {1.1f, 1.0f}), + Rectangle({0.3f, 1.1f}, {-0.5f, 0.7f}), + Vector2(2.0f, -1.0f)); + } + }; + + /* Rectangle of zero size shouldn't be merged, but replaced */ + Rectangle rectangle({-1.0f, -1.0f}, {-1.0f, -1.0f}); + Vector2 cursorPosition(1.0f, 2.0f); + + Layouter l; + Rectangle quadPosition; + Rectangle textureCoords; + + std::tie(quadPosition, textureCoords) = l.renderGlyph(0, cursorPosition, rectangle); + CORRADE_COMPARE(quadPosition, Rectangle({2.0f, 2.5f}, {2.1f, 3.0f})); + CORRADE_COMPARE(textureCoords, Rectangle({0.3f, 1.1f}, {-0.5f, 0.7f})); + CORRADE_COMPARE(cursorPosition, Vector2(3.0f, 1.0f)); + CORRADE_COMPARE(rectangle, Rectangle({2.0f, 2.5f}, {2.1f, 3.0f})); + + std::tie(quadPosition, textureCoords) = l.renderGlyph(1, cursorPosition, rectangle); + CORRADE_COMPARE(quadPosition, Rectangle({4.0f, 1.5f}, {4.1f, 2.0f})); + CORRADE_COMPARE(textureCoords, Rectangle({0.3f, 1.1f}, {-0.5f, 0.7f})); + CORRADE_COMPARE(cursorPosition, Vector2(5.0f, 0.0f)); + CORRADE_COMPARE(rectangle, Rectangle({2.0f, 1.5f}, {4.1f, 3.0f})); + + std::tie(quadPosition, textureCoords) = l.renderGlyph(2, cursorPosition, rectangle); + CORRADE_COMPARE(quadPosition, Rectangle({6.0f, 0.5f}, {6.1f, 1.0f})); + CORRADE_COMPARE(textureCoords, Rectangle({0.3f, 1.1f}, {-0.5f, 0.7f})); + CORRADE_COMPARE(cursorPosition, Vector2(7.0f, -1.0f)); + CORRADE_COMPARE(rectangle, Rectangle({2.0f, 0.5f}, {6.1f, 3.0f})); +} + +}}} + +CORRADE_TEST_MAIN(Magnum::Text::Test::AbstractLayouterTest) diff --git a/src/Text/Test/CMakeLists.txt b/src/Text/Test/CMakeLists.txt index 3f133a008..e48529eaa 100644 --- a/src/Text/Test/CMakeLists.txt +++ b/src/Text/Test/CMakeLists.txt @@ -29,6 +29,7 @@ include_directories(${CMAKE_CURRENT_BINARY_DIR}) corrade_add_test(TextAbstractFontTest AbstractFontTest.cpp LIBRARIES Magnum MagnumText) corrade_add_test(TextAbstractFontConverterTest AbstractFontConverterTest.cpp LIBRARIES Magnum MagnumText) +corrade_add_test(TextAbstractLayouterTest AbstractLayouterTest.cpp LIBRARIES Magnum MagnumText) if(BUILD_GL_TESTS) corrade_add_test(TextGlyphCacheGLTest GlyphCacheGLTest.cpp LIBRARIES MagnumText ${GL_TEST_LIBRARIES}) diff --git a/src/Text/Test/RendererGLTest.cpp b/src/Text/Test/RendererGLTest.cpp index ebc47d7d7..8727fb9b1 100644 --- a/src/Text/Test/RendererGLTest.cpp +++ b/src/Text/Test/RendererGLTest.cpp @@ -47,11 +47,10 @@ namespace { class TestLayouter: public Text::AbstractLayouter { public: - explicit TestLayouter(Float size, std::size_t glyphCount): _size(size) { - _glyphCount = glyphCount; - } + explicit TestLayouter(Float size, std::size_t glyphCount): AbstractLayouter(glyphCount), _size(size) {} - std::tuple renderGlyph(UnsignedInt i) override { + private: + std::tuple doRenderGlyph(UnsignedInt i) override { return std::make_tuple( Rectangle({}, Vector2(3.0f, 2.0f)*((i+1)*_size)), Rectangle::fromSize({i*6.0f, 0.0f}, {6.0f, 10.0f}), @@ -59,7 +58,6 @@ class TestLayouter: public Text::AbstractLayouter { ); } - private: Float _size; };