From 3eb630b02dc28175cfae387f207c2f3210e1ee09 Mon Sep 17 00:00:00 2001 From: Przemyslaw Brudny Date: Wed, 14 Apr 2021 20:24:11 +0200 Subject: [PATCH] [EGD-6534] TextFixedSize crash on char removal fix Fixed TextFixedSize character removal crash on single line space case. Added UT to cover it and other missing TextFixed sized drawLines cases. --- enabled_unittests | 2 + module-gui/gui/widgets/Lines.cpp | 33 ++- module-gui/gui/widgets/Text.cpp | 8 +- .../test/test-catch-text/CMakeLists.txt | 1 + .../test-gui-TextFixedSize.cpp | 197 ++++++++++++++++++ 5 files changed, 231 insertions(+), 10 deletions(-) create mode 100644 module-gui/test/test-catch-text/test-gui-TextFixedSize.cpp diff --git a/enabled_unittests b/enabled_unittests index 93d0f378f4145a366386ba1f8ed3d140be312f87..2ac232215a86e62f1fdc1507fb3d5c0566c8fcbb 100644 --- a/enabled_unittests +++ b/enabled_unittests @@ -225,6 +225,8 @@ TESTS_LIST["catch2-gui-text"]=" TextLineCursor - navigation without scroll; TextLineCursor - navigation with scroll; TextLineCursor - addition and deletion with scroll; + TextFixedSize drawLines; + TextFixedSize remove Char; " #--------- TESTS_LIST["catch2-PowerManager"]=" diff --git a/module-gui/gui/widgets/Lines.cpp b/module-gui/gui/widgets/Lines.cpp index e75687e8bf820ae3b851e758216a0ea12e67f849..dad69e2bafe46443ea26de22bbca55ec47997cd7 100644 --- a/module-gui/gui/widgets/Lines.cpp +++ b/module-gui/gui/widgets/Lines.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2020, Mudita Sp. z.o.o. All rights reserved. +// Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved. // For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md #include "Lines.hpp" @@ -80,20 +80,37 @@ namespace gui auto textLine = gui::TextLine(drawCursor, w, initHeight, underLine, UnderlineDrawMode::WholeLine, underLinePadding); - if (textLine.height() > 0 && initHeight != textLine.height()) { + if ((textLine.height() > 0) && initHeight != textLine.height()) { initHeight = textLine.height(); } - if (lineYPosition + initHeight > h) { - stopCondition = LinesDrawStop::OutOfSpace; - addToInvisibleLines(std::move(textLine)); + if (!previousLinesStart.empty() && (textLine.length() == 0) && textLine.getLineEnd()) { + stopCondition = LinesDrawStop::OutOfText; break; } + if (lineYPosition + initHeight > h) { + if ((textLine.length() == 0) && textLine.getLineEnd()) { + stopCondition = LinesDrawStop::OutOfText; + break; + } + else { + stopCondition = LinesDrawStop::OutOfSpace; + addToInvisibleLines(std::move(textLine)); + break; + } + } + if (lines.size() >= linesCount) { - stopCondition = LinesDrawStop::OutOfSpace; - addToInvisibleLines(std::move(textLine)); - break; + if ((textLine.length() == 0) && textLine.getLineEnd()) { + stopCondition = LinesDrawStop::OutOfText; + break; + } + else { + stopCondition = LinesDrawStop::OutOfSpace; + addToInvisibleLines(std::move(textLine)); + break; + } } emplace(std::move(textLine)); diff --git a/module-gui/gui/widgets/Text.cpp b/module-gui/gui/widgets/Text.cpp index df26cee04d884abbcdb5170dceb43c8bf58f0b96..2170204d7f3afbec72bbb576cef4021ade1fabee 100644 --- a/module-gui/gui/widgets/Text.cpp +++ b/module-gui/gui/widgets/Text.cpp @@ -206,8 +206,12 @@ namespace gui void Text::setFont(const UTF8 &fontName) { RawFont *newFont = FontManager::getInstance().getFont(fontName); - format.setFont(newFont); - buildCursor(); + + if (format.getFont() != newFont) { + format.setFont(newFont); + buildCursor(); + drawLines(); + } } void Text::setFont(RawFont *font) diff --git a/module-gui/test/test-catch-text/CMakeLists.txt b/module-gui/test/test-catch-text/CMakeLists.txt index 4d67c05282edf0d7714ab5155f4a68519178f923..deb88af2e6255b6ea5bae6ee814eab0985f4dc6b 100644 --- a/module-gui/test/test-catch-text/CMakeLists.txt +++ b/module-gui/test/test-catch-text/CMakeLists.txt @@ -8,6 +8,7 @@ add_catch2_executable( ../mock/multi-line-string.cpp ../mock/InitializedFontManager.cpp test-gui-Text.cpp + test-gui-TextFixedSize.cpp test-gui-TextBlock.cpp test-gui-TextBlockCursor.cpp test-gui-TextDocument.cpp diff --git a/module-gui/test/test-catch-text/test-gui-TextFixedSize.cpp b/module-gui/test/test-catch-text/test-gui-TextFixedSize.cpp new file mode 100644 index 0000000000000000000000000000000000000000..45382c198e395d5645a358db032e4412c4325934 --- /dev/null +++ b/module-gui/test/test-catch-text/test-gui-TextFixedSize.cpp @@ -0,0 +1,197 @@ +// Copyright (c) 2017-2021, Mudita Sp. z.o.o. All rights reserved. +// For licensing, see https://github.com/mudita/MuditaOS/LICENSE.md + +#include + +#include +#include + +namespace gui +{ + class TestTextFixedSize : public TextFixedSize + { + public: + TestTextFixedSize() : TextFixedSize(nullptr, 0, 0, 0, 0){}; + + [[nodiscard]] unsigned int linesSize() + { + return lines->get().size(); + } + + void drawLines() override + { + TextFixedSize::drawLines(); + } + + [[nodiscard]] auto lineGet(unsigned int nr) + { + auto line = lines->get().begin(); + + if (nr >= lines->size()) { + nr = lines->size() - 1; + } + + std::advance(line, nr); + return line; + } + + auto removeNCharacters(unsigned int n) + { + for (unsigned int i = 0; i < n; i++) { + removeChar(); + } + } + }; +} // namespace gui + +TEST_CASE("TextFixedSize drawLines") +{ + using namespace gui; + mockup::fontManager(); + + std::string testString1 = "Test \n"; + std::string testString2 = "Long \n"; + std::string testString3 = "Long \n"; + std::string testString4 = "Long \n"; + std::string testString5 = "Line \n"; + std::string testString6 = "End"; + + std::string testStringLongLine = testString1 + testString2 + testString3 + testString4 + testString5 + testString6; + + SECTION("Empty text for space to draw 4 lines") + { + auto text = TestTextFixedSize(); + text.setSize(20, 120); + + text.setFont(style::window::font::medium); + text.drawLines(); + + REQUIRE(text.linesSize() == 4); + } + + SECTION("Empty text for space to draw 4 lines but lines count limited to 2") + { + auto text = TestTextFixedSize(); + text.setSize(20, 120); + + text.setFont(style::window::font::medium); + text.setLines(2); + text.drawLines(); + + REQUIRE(text.linesSize() == 2); + } + + SECTION("Text for space to draw 4 lines - cursor moved to beginning") + { + auto text = TestTextFixedSize(); + text.setSize(200, 120); + + text.setCursorStartPosition(gui::CursorStartPosition::DocumentBegin); + text.setFont(style::window::font::medium); + text.setText(testStringLongLine); + + REQUIRE(text.lineGet(0)->getText(0) == testString1); + REQUIRE(text.lineGet(1)->getText(0) == testString2); + REQUIRE(text.lineGet(2)->getText(0) == testString3); + REQUIRE(text.lineGet(3)->getText(0) == testString4); + + // There should be added extra invisible line. + REQUIRE(text.linesSize() == 5); + + REQUIRE(text.lineGet(4)->getText(0) == testString5); + REQUIRE_FALSE(text.lineGet(4)->isVisible()); + } + + SECTION("Text for space to draw 4 lines - cursor moved to end") + { + auto text = TestTextFixedSize(); + text.setSize(200, 120); + + text.setFont(style::window::font::medium); + text.setText(testStringLongLine); + + REQUIRE(text.lineGet(0)->getText(0) == testString3); + REQUIRE(text.lineGet(1)->getText(0) == testString4); + REQUIRE(text.lineGet(2)->getText(0) == testString5); + REQUIRE(text.lineGet(3)->getText(0) == testString6); + REQUIRE(text.linesSize() == 4); + } + + SECTION("Text for space to draw 4 lines but lines count limited to 2 - cursor moved to beginning") + { + auto text = TestTextFixedSize(); + text.setSize(200, 120); + + text.setCursorStartPosition(gui::CursorStartPosition::DocumentBegin); + text.setFont(style::window::font::medium); + text.setLines(2); + text.setText(testStringLongLine); + + REQUIRE(text.lineGet(0)->getText(0) == testString1); + REQUIRE(text.lineGet(1)->getText(0) == testString2); + + // There should be added extra invisible line. + REQUIRE(text.linesSize() == 3); + + REQUIRE(text.lineGet(2)->getText(0) == testString3); + REQUIRE_FALSE(text.lineGet(4)->isVisible()); + } + + SECTION("Text for space to draw 4 lines but lines count limited to 2 - cursor moved to end") + { + auto text = TestTextFixedSize(); + text.setSize(200, 120); + + text.setFont(style::window::font::medium); + text.setLines(2); + text.setText(testStringLongLine); + + REQUIRE(text.lineGet(0)->getText(0) == testString5); + REQUIRE(text.lineGet(1)->getText(0) == testString6); + REQUIRE(text.linesSize() == 2); + } +} + +TEST_CASE("TextFixedSize remove Char") +{ + using namespace gui; + mockup::fontManager(); + + std::string testString1 = "Test Line Line Line"; + std::string testString2 = "End"; + + std::string testStringLongLine = testString1 + testString2; + + SECTION("Remove char from last line on size limited space") + { + auto text = TestTextFixedSize(); + text.setSize(210, 30); + + text.setFont(style::window::font::medium); + text.setText(testStringLongLine); + + REQUIRE(text.lineGet(0)->getText(0) == testString2); + REQUIRE(text.linesSize() == 1); + + text.removeNCharacters(testString2.size()); + REQUIRE(text.lineGet(0)->getText(0) == testString1); + REQUIRE(text.linesSize() == 1); + } + + SECTION("Remove char from last line on line limited space") + { + auto text = TestTextFixedSize(); + text.setSize(210, 120); + + text.setFont(style::window::font::medium); + text.setLines(1); + text.setText(testStringLongLine); + + REQUIRE(text.lineGet(0)->getText(0) == testString2); + REQUIRE(text.linesSize() == 1); + + text.removeNCharacters(testString2.size()); + REQUIRE(text.lineGet(0)->getText(0) == testString1); + REQUIRE(text.linesSize() == 1); + } +}