diff --git a/src/Magnum/Text/AbstractShaper.cpp b/src/Magnum/Text/AbstractShaper.cpp index 0712787a6..024d129ee 100644 --- a/src/Magnum/Text/AbstractShaper.cpp +++ b/src/Magnum/Text/AbstractShaper.cpp @@ -80,8 +80,6 @@ UnsignedInt AbstractShaper::shape(const Containers::StringView text, const Unsig CORRADE_ASSERT((end == ~UnsignedInt{} && begin <= text.size()) || (begin <= end && end <= text.size()), "Text::AbstractShaper::shape(): begin" << begin << "and end" << end << "out of range for a text of" << text.size() << "bytes", {}); - CORRADE_ASSERT(begin < text.size() && begin != end, - "Text::AbstractShaper::shape(): shaped text at begin" << begin << "is empty", {}); #ifndef CORRADE_NO_ASSERT for(std::size_t i = 0; i != features.size(); ++i) { const FeatureRange& feature = features[i]; @@ -89,7 +87,6 @@ UnsignedInt AbstractShaper::shape(const Containers::StringView text, const Unsig (feature._end == ~UnsignedInt{} && feature._begin <= text.size()) || (feature._begin <= feature._end && feature._end <= text.size()), "Text::AbstractShaper::shape(): feature" << i << "begin" << feature._begin << "and end" << feature._end << "out of range for a text of" << text.size() << "bytes", {}); - /** @todo catch empty feature ranges too? or not important */ } #endif return _glyphCount = doShape(text, begin, end, features); @@ -116,19 +113,19 @@ UnsignedInt AbstractShaper::shape(const Containers::StringView text) { } Script AbstractShaper::script() const { - return _glyphCount ? doScript() : Script::Unspecified; + return doScript(); } Script AbstractShaper::doScript() const { return Script::Unspecified; } Containers::StringView AbstractShaper::language() const { - return _glyphCount ? doLanguage() : Containers::StringView{}; + return doLanguage(); } Containers::StringView AbstractShaper::doLanguage() const { return {}; } Direction AbstractShaper::direction() const { - return _glyphCount ? doDirection() : Direction::Unspecified; + return doDirection(); } Direction AbstractShaper::doDirection() const { return Direction::Unspecified; } diff --git a/src/Magnum/Text/AbstractShaper.h b/src/Magnum/Text/AbstractShaper.h index 779158956..f332b732c 100644 --- a/src/Magnum/Text/AbstractShaper.h +++ b/src/Magnum/Text/AbstractShaper.h @@ -344,18 +344,13 @@ class MAGNUM_TEXT_EXPORT AbstractShaper { * @param features OpenType features to apply for the whole text or * its subranges * - * Expects that @p text is non-empty, that both @p begin and all - * @ref FeatureRange::begin() are contained within @p text, and that - * @p end and all @ref FeatureRange::end() are either contained within - * @p text or have a value of @cpp 0xffffffffu @ce. On success returns - * the number of shaped glyphs (which is also subsequently available - * through @ref glyphCount() const) and updates the @ref script() const, - * @ref language() const and @ref direction() const values. - * - * On failure, such as when the input is not valid UTF-8 or when - * specified combination or script, language and direction is - * unsupported, prints a message to @relativeref{Magnum,Error} and - * returns @cpp 0 @ce. + * Expects that both @p begin and all @ref FeatureRange::begin() are + * contained within @p text, and that @p end and all + * @ref FeatureRange::end() are either contained within @p text or have + * a value of @cpp 0xffffffffu @ce. Returns the number of shaped glyphs + * (which is also subsequently available through @ref glyphCount() const) + * and updates the @ref script() const, @ref language() const and + * @ref direction() const values. * * Whether @p features are used depends on a particular * @ref AbstractFont plugin implementation and the font file itself as @@ -415,9 +410,9 @@ class MAGNUM_TEXT_EXPORT AbstractShaper { /** * @brief Script used for the last @ref shape() call * - * If the last @ref shape() call failed, it hasn't been called yet or - * if the @ref AbstractFont doesn't implement any script-specific - * behavior, returns @ref Script::Unspecified. + * May return @ref Script::Unspecified if @ref shape() hasn't been + * called yet or if the @ref AbstractFont doesn't implement any + * script-specific behavior. * @see @ref setScript(), @ref language(), @ref direction() */ Script script() const; @@ -425,9 +420,9 @@ class MAGNUM_TEXT_EXPORT AbstractShaper { /** * @brief Language used for the last @ref shape() call * - * If the last @ref shape() call failed, it hasn't been called yet or + * May return an empty string if @ref shape() hasn't been called yet or * if the @ref AbstractFont doesn't implement any language-specific - * behavior, returns an empty string. + * behavior. * * The returned view is generally neither * @relativeref{Corrade,Containers::StringViewFlag::Global} nor @@ -443,9 +438,9 @@ class MAGNUM_TEXT_EXPORT AbstractShaper { /** * @brief Language used for the last @ref shape() call * - * If the last @ref shape() call failed, it hasn't been called yet or - * if the @ref AbstractFont doesn't implement any direction-specific - * behavior, returns @ref Direction::Unspecified. + * May return @ref Direction::Unspecified if @ref shape() hasn't been + * called yet or if the @ref AbstractFont doesn't implement any + * script-specific behavior. * @see @ref setDirection(), @ref script(), @ref language() */ Direction direction() const; diff --git a/src/Magnum/Text/Test/AbstractShaperTest.cpp b/src/Magnum/Text/Test/AbstractShaperTest.cpp index 7a0eef195..5c6a616bd 100644 --- a/src/Magnum/Text/Test/AbstractShaperTest.cpp +++ b/src/Magnum/Text/Test/AbstractShaperTest.cpp @@ -64,9 +64,8 @@ struct AbstractShaperTest: TestSuite::Tester { void shapeNoBeginEnd(); void shapeNoBeginEndFeatures(); void shapeScriptLanguageDirectionNotImplemented(); - void shapeFailed(); + void shapeZeroGlyphs(); void shapeBeginEndOutOfRange(); - void shapeEmptyText(); /* glyphsInto() tested in shape() already */ void glyphsIntoEmpty(); @@ -97,9 +96,8 @@ AbstractShaperTest::AbstractShaperTest() { &AbstractShaperTest::shapeNoBeginEnd, &AbstractShaperTest::shapeNoBeginEndFeatures, &AbstractShaperTest::shapeScriptLanguageDirectionNotImplemented, - &AbstractShaperTest::shapeFailed, + &AbstractShaperTest::shapeZeroGlyphs, &AbstractShaperTest::shapeBeginEndOutOfRange, - &AbstractShaperTest::shapeEmptyText, &AbstractShaperTest::glyphsIntoEmpty, &AbstractShaperTest::glyphsIntoInvalidViewSizes}); @@ -352,13 +350,15 @@ void AbstractShaperTest::shape() { bool shapeCalled = false; } shaper{FakeFont}; - /* Initially it shouldn't call into any of the implementations */ + /* There's no special behavior, it calls into the implementations even if + nothing has been shaped yet */ CORRADE_COMPARE(shaper.glyphCount(), 0); - CORRADE_COMPARE(shaper.script(), Script::Unspecified); - CORRADE_COMPARE(shaper.language(), ""); - CORRADE_COMPARE(shaper.direction(), Direction::Unspecified); + CORRADE_COMPARE(shaper.script(), Script::LinearA); + CORRADE_COMPARE(shaper.language(), "eh-UH"); + CORRADE_COMPARE(shaper.direction(), Direction::BottomToTop); - /* Shaping fills glyph count and allows calling into the implementations */ + /* Shaping fills glyph count. A real implementation would then return + (different) detected script/language/direction values, for example. */ CORRADE_COMPARE(shaper.shape("some text", 3, 8, { Feature::ContextualLigatures, {Feature::Kerning, 2, 5, false} @@ -496,7 +496,7 @@ void AbstractShaperTest::shapeScriptLanguageDirectionNotImplemented() { CORRADE_COMPARE(shaper.direction(), Direction::Unspecified); } -void AbstractShaperTest::shapeFailed() { +void AbstractShaperTest::shapeZeroGlyphs() { struct: AbstractShaper { using AbstractShaper::AbstractShaper; @@ -519,16 +519,16 @@ void AbstractShaperTest::shapeFailed() { void doGlyphsInto(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&) const override {} } shaper{FakeFont}; - /* The implementation is responsible for printing a message, the base class - doesn't */ CORRADE_COMPARE(shaper.shape("some text", 3, 8), 0); - /* After a failure it shouldn't call into any of the implementations - either */ + /* It calls into the implementations even in case no glyphs were actually + shaped. It could be for example a zero-length slice of a larger string + for which script/language/direction detection was performed, so it's + still useful to get the values after */ CORRADE_COMPARE(shaper.glyphCount(), 0); - CORRADE_COMPARE(shaper.script(), Script::Unspecified); - CORRADE_COMPARE(shaper.language(), ""); - CORRADE_COMPARE(shaper.direction(), Direction::Unspecified); + CORRADE_COMPARE(shaper.script(), Script::LinearA); + CORRADE_COMPARE(shaper.language(), "eh-UH"); + CORRADE_COMPARE(shaper.direction(), Direction::BottomToTop); } void AbstractShaperTest::shapeBeginEndOutOfRange() { @@ -586,35 +586,6 @@ void AbstractShaperTest::shapeBeginEndOutOfRange() { TestSuite::Compare::String); } -void AbstractShaperTest::shapeEmptyText() { - CORRADE_SKIP_IF_NO_ASSERT(); - - struct: AbstractShaper { - using AbstractShaper::AbstractShaper; - - UnsignedInt doShape(Containers::StringView, UnsignedInt, UnsignedInt, Containers::ArrayView) override { - CORRADE_FAIL("This shouldn't be called"); - return 5; - } - - void doGlyphsInto(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&) const override {} - } shaper{FakeFont}; - - /* Capture correct function name */ - CORRADE_VERIFY(true); - - std::ostringstream out; - Error redirectError{&out}; - shaper.shape(""); - shaper.shape("hello", 3, 3); - shaper.shape("hello", 5, ~UnsignedInt{}); - CORRADE_COMPARE_AS(out.str(), - "Text::AbstractShaper::shape(): shaped text at begin 0 is empty\n" - "Text::AbstractShaper::shape(): shaped text at begin 3 is empty\n" - "Text::AbstractShaper::shape(): shaped text at begin 5 is empty\n", - TestSuite::Compare::String); -} - void AbstractShaperTest::glyphsIntoEmpty() { struct: AbstractShaper { using AbstractShaper::AbstractShaper; diff --git a/src/MagnumPlugins/MagnumFont/Test/MagnumFontTest.cpp b/src/MagnumPlugins/MagnumFont/Test/MagnumFontTest.cpp index a32864f19..b1440fce0 100644 --- a/src/MagnumPlugins/MagnumFont/Test/MagnumFontTest.cpp +++ b/src/MagnumPlugins/MagnumFont/Test/MagnumFontTest.cpp @@ -54,6 +54,7 @@ struct MagnumFontTest: TestSuite::Tester { void properties(); void shape(); + void shapeEmpty(); void shaperReuse(); void fileCallbackImage(); @@ -83,7 +84,8 @@ MagnumFontTest::MagnumFontTest() { addInstancedTests({&MagnumFontTest::shape}, Containers::arraySize(ShapeData)); - addTests({&MagnumFontTest::shaperReuse, + addTests({&MagnumFontTest::shapeEmpty, + &MagnumFontTest::shaperReuse, &MagnumFontTest::fileCallbackImage, &MagnumFontTest::fileCallbackImageNotFound}); @@ -169,14 +171,28 @@ void MagnumFontTest::shape() { }), TestSuite::Compare::Container); } +void MagnumFontTest::shapeEmpty() { + Containers::Pointer font = _fontManager.instantiate("MagnumFont"); + CORRADE_VERIFY(font->openFile(Utility::Path::join(MAGNUMFONT_TEST_DIR, "font.conf"), 0.0f)); + + Containers::Pointer shaper = font->createShaper(); + + /* Shouldn't crash or do anything rogue */ + CORRADE_COMPARE(shaper->shape("Wave", 2, 2), 0); +} + void MagnumFontTest::shaperReuse() { Containers::Pointer font = _fontManager.instantiate("MagnumFont"); CORRADE_VERIFY(font->openFile(Utility::Path::join(MAGNUMFONT_TEST_DIR, "font.conf"), 0.0f)); Containers::Pointer shaper = font->createShaper(); - /* Short text */ + /* Empty text */ { + CORRADE_COMPARE(shaper->shape("Wave", 2, 2), 0); + + /* Short text. Empty shape shouldn't have caused any broken state. */ + } { CORRADE_COMPARE(shaper->shape("We"), 2); UnsignedInt ids[2]; Vector2 offsets[2];