diff --git a/src/Magnum/Text/Renderer.cpp b/src/Magnum/Text/Renderer.cpp index 36685b698..0df069b28 100644 --- a/src/Magnum/Text/Renderer.cpp +++ b/src/Magnum/Text/Renderer.cpp @@ -501,8 +501,11 @@ RendererCore& RendererCore::reset() { void RendererCore::alignAndFinishLine() { State& state = *_state; - CORRADE_INTERNAL_DEBUG_ASSERT(state.lineGlyphBegin != state.renderingGlyphCount && state.resolvedAlignment); + CORRADE_INTERNAL_DEBUG_ASSERT(state.resolvedAlignment); + /* Note that the slice can be empty in case we rendered an empty line -- + we still want to run this code in that case to correctly position the + rectangle */ const Range2D alignedLineRectangle = alignRenderedLine( state.lineRectangle, state.layoutDirection, @@ -542,77 +545,91 @@ RendererCore& RendererCore::add(AbstractShaper& shaper, const Float size, const state.renderingLineAdvance = Vector2::yAxis(-font.lineHeight()*scale); } + /* Run through the code at least once even if the line is empty, to ensure + font ascent and descent is reflected in the output rectangle in that + case */ Containers::StringView line = text.slice(begin, end); - while(line) { + for(;;) { /* Find the next newline. The text to shape is until lineEnd.begin(), the next line (if any) starts at lineEnd.end(). If lineEnd is empty, we reached the end of the input text -- it's *not* an end of the line, because the next add() call may continue with it. */ const Containers::StringView lineEnd = line.findOr('\n', line.end()); - /* If the line is not empty and produced some glyphs, render them */ - if(const UnsignedInt glyphCount = lineEnd.begin() != line.begin() ? shaper.shape(text, line.begin() - text.begin(), lineEnd.begin() - text.begin(), features) : 0) { - /* If we need to add more glyphs than what's in the capacity, - allocate more */ - if(state.glyphPositions.size() < state.renderingGlyphCount + glyphCount) { - allocateGlyphs( - #ifndef CORRADE_NO_ASSERT - "Text::RendererCore::add():", - #endif - state.renderingGlyphCount + glyphCount); - #ifdef CORRADE_GRACEFUL_ASSERT - /* For testing only -- if allocation failed, bail */ - if(state.glyphPositions.size() < state.renderingGlyphCount + glyphCount) - return *this; + /* If the line is not empty, shape it */ + const UnsignedInt glyphCount = lineEnd.begin() != line.begin() ? shaper.shape(text, line.begin() - text.begin(), lineEnd.begin() - text.begin(), features) : 0; + + /* If we need to add more glyphs than what's in the capacity, allocate + more */ + if(state.glyphPositions.size() < state.renderingGlyphCount + glyphCount) { + allocateGlyphs( + #ifndef CORRADE_NO_ASSERT + "Text::RendererCore::add():", #endif - } + state.renderingGlyphCount + glyphCount); + #ifdef CORRADE_GRACEFUL_ASSERT + /* For testing only -- if allocation failed, bail */ + if(state.glyphPositions.size() < state.renderingGlyphCount + glyphCount) + return *this; + #endif + } - const Containers::StridedArrayView1D glyphOffsetsPositions = state.glyphPositions.sliceSize(state.renderingGlyphCount, glyphCount); - /* The glyph advance array may be aliasing IDs and clusters. Pick - only a suffix of the same size as the remaining capacity -- that - memory is guaranteed to be unused yet. */ - const std::size_t remainingCapacity = state.glyphPositions.size() - state.renderingGlyphCount; - const Containers::StridedArrayView1D glyphAdvances = state.glyphAdvances.sliceSize(state.glyphAdvances.size() - remainingCapacity, glyphCount); - shaper.glyphOffsetsAdvancesInto( - glyphOffsetsPositions, - glyphAdvances); - - /* Render line glyph positions, aliasing the offsets */ - const Range2D rectangle = renderLineGlyphPositionsInto( - shaper.font(), - size, - state.layoutDirection, - glyphOffsetsPositions, - glyphAdvances, - state.renderingLineCursor, - glyphOffsetsPositions); - - /* Retrieve the glyph IDs and clusters, convert the glyph IDs to - cache-global. Do it only after finalizing the positions so the - glyphAdvances array can alias the IDs. */ - const Containers::StridedArrayView1D glyphIds = state.glyphIds.sliceSize(state.renderingGlyphCount, glyphCount); + /* Those views will be empty if no glyphs were shaped */ + const Containers::StridedArrayView1D glyphOffsetsPositions = state.glyphPositions.sliceSize(state.renderingGlyphCount, glyphCount); + /* The glyph advance array may be aliasing IDs and clusters. Pick only + a suffix of the same size as the remaining capacity -- that memory + is guaranteed to be unused yet. */ + const std::size_t remainingCapacity = state.glyphPositions.size() - state.renderingGlyphCount; + const Containers::StridedArrayView1D glyphAdvances = state.glyphAdvances.sliceSize(state.glyphAdvances.size() - remainingCapacity, glyphCount); + + /* Query glyph advances. If there are none, avoid a virtual call. */ + if(glyphCount) shaper.glyphOffsetsAdvancesInto( + glyphOffsetsPositions, + glyphAdvances); + + /* Render line glyph positions, aliasing the offsets. Do this even if + there are no glyphs, as we want the rectangle to contain at least + font ascent and descent in that case. */ + const Range2D rectangle = renderLineGlyphPositionsInto( + shaper.font(), + size, + state.layoutDirection, + glyphOffsetsPositions, + glyphAdvances, + state.renderingLineCursor, + glyphOffsetsPositions); + + /* Retrieve the glyph IDs and clusters, convert the glyph IDs to + cache-global. Do it only after finalizing the positions so the + glyphAdvances array can alias the IDs. This doesn't need to be done + if there are no glyphs, saving two virtual calls. */ + const Containers::StridedArrayView1D glyphIds = state.glyphIds.sliceSize(state.renderingGlyphCount, glyphCount); + if(glyphCount) { shaper.glyphIdsInto(glyphIds); state.glyphCache.glyphIdsInto(*glyphCacheFontId, glyphIds, glyphIds); if(state.flags & RendererCoreFlag::GlyphClusters) shaper.glyphClustersInto(state.glyphClusters.sliceSize(state.renderingGlyphCount, glyphCount)); + } - /* If we're aligning based on glyph bounds, calculate a rectangle - from scratch instead of using a rectangle based on advances and - font metrics. Join the resulting rectangle with one that's - maintained for the line so far. */ - state.lineRectangle = Math::join(state.lineRectangle, - (UnsignedByte(state.alignment) & Implementation::AlignmentGlyphBounds) ? - glyphQuadBounds(state.glyphCache, scale, glyphOffsetsPositions, glyphIds) : - rectangle); + /* If we're aligning based on glyph bounds, calculate a rectangle from + scratch instead of using a rectangle based on advances and font + metrics. Join the resulting rectangle with one that's maintained for + the line so far. Again do this even if there are no glyphs, as we + want the rectangle to contain at least font ascent and descent in + that case. */ + state.lineRectangle = Math::join(state.lineRectangle, + (UnsignedByte(state.alignment) & Implementation::AlignmentGlyphBounds) ? + glyphQuadBounds(state.glyphCache, scale, glyphOffsetsPositions, glyphIds) : + rectangle); - state.renderingGlyphCount += glyphCount; - } + state.renderingGlyphCount += glyphCount; /* If the alignment isn't resolved yet and the shaper detected any - usable direction (or we're at the end of the line where we need - it), resolve it. If there's no usable direction detected yet, maybe - it will be next time. */ - if(!state.resolvedAlignment) { + usable direction from passed glyphs (or we're at the end of the line + where we need it), resolve it. If there's no usable direction + detected yet, maybe it will be next time. If there are no glyphs and + we don't need it yet, avoid the virtual calls. */ + if(!state.resolvedAlignment && (glyphCount || lineEnd)) { /* In this case it may happen that we query direction on a shaper for which shape() wasn't called yet, for example if shaping a text starting with \n and the previous text shaping gave back @@ -635,7 +652,7 @@ RendererCore& RendererCore::add(AbstractShaper& shaper, const Float size, const above or being there from the previous add() call, align them. If alignment based on bounds is requested, calculate a special rectangle for it. */ - if(state.lineGlyphBegin != state.renderingGlyphCount) + if(state.lineRectangle != Range2D{}) alignAndFinishLine(); /* Move the cursor for the next line */ @@ -643,6 +660,18 @@ RendererCore& RendererCore::add(AbstractShaper& shaper, const Float size, const state.renderingLineCursor = state.renderingLineStart; } + /* If there's no newline found after, we have nothing to do in the next + iteration and can break out of the loop. This also handles the case + of completely empty input -- it enters the loop exactly once. If + there is a newline, at this point the current line should be already + finished by alignAndFinishLine() above. Same assert is in render() + below. */ + if(!lineEnd) + break; + else CORRADE_INTERNAL_DEBUG_ASSERT( + state.lineGlyphBegin == state.renderingGlyphCount && + state.lineRectangle == Range2D{}); + /* For the next iteration cut away everything that got processed, including the \n */ line = line.suffix(lineEnd.end()); @@ -709,9 +738,9 @@ Containers::Pair RendererCore::render() { state.layoutDirection, ShapeDirection::Unspecified); - /* Align the last unfinished line. In most cases there will be, unless the - last text passed to add() was ending with a \n. */ - if(state.lineGlyphBegin != state.renderingGlyphCount) + /* Align the last unfinished line. In most cases there will be, unless + render() was called from an empty state. */ + if(state.lineRectangle != Range2D{}) alignAndFinishLine(); /* Align the block. Now it's respecting the alignment relative to the @@ -730,8 +759,8 @@ Containers::Pair RendererCore::render() { i += state.cursor; /* Reset all block-related state, marking the renderer as not in progress - anymore. Line-related state should be reset after the alignLine() above - already. */ + anymore. Line-related state should be reset after the + alignAndFinishLine() above already. */ const UnsignedInt blockRunBegin = state.blockRunBegin; state.rendering = false; state.resolvedAlignment = {}; diff --git a/src/Magnum/Text/Test/RendererGLTest.cpp b/src/Magnum/Text/Test/RendererGLTest.cpp index ff023042c..44fc3808a 100644 --- a/src/Magnum/Text/Test/RendererGLTest.cpp +++ b/src/Magnum/Text/Test/RendererGLTest.cpp @@ -1187,7 +1187,7 @@ void RendererGLTest::deprecatedMutableText() { string, it makes the output empty. */ CORRADE_IGNORE_DEPRECATED_PUSH renderer.render(""); - CORRADE_COMPARE(renderer.rectangle(), Range2D{}); + CORRADE_COMPARE(renderer.rectangle(), (Range2D{{0.0f, -1.75f}, {0.0f, 1.75f}})); CORRADE_COMPARE(renderer.mesh().count(), 0); CORRADE_IGNORE_DEPRECATED_POP } diff --git a/src/Magnum/Text/Test/RendererTest.cpp b/src/Magnum/Text/Test/RendererTest.cpp index 40df2497f..1b8b5c2a6 100644 --- a/src/Magnum/Text/Test/RendererTest.cpp +++ b/src/Magnum/Text/Test/RendererTest.cpp @@ -141,6 +141,7 @@ struct RendererTest: TestSuite::Tester { void addFontNotFoundInCache(); void multipleBlocks(); + void emptyLines(); template void indicesVertices(); @@ -1124,12 +1125,13 @@ const struct { first ---- second ------- first */ 4, 6, 2, 1, 13, 11, 13, 11, 7, 8 }}, - /* Empty parts have their font metrics ignored */ + /* Empty part font metrics are considered as well. See the emptyLines() + test for extended verification. */ {"an empty part with taller font", {InPlaceInit, { {3, 5, 1.0f}, {5, 5, 10.0f}, {5, 8, 1.0f}, - }}, {}, Alignment::LineRight, ShapeDirection{}, 0, false, 24.0f, {InPlaceInit, { + }}, {}, Alignment::LineRight, ShapeDirection{}, 0, false, 120.0f, {InPlaceInit, { {1.0f, 4}, {1.0f, 10}, }}, { @@ -1788,6 +1790,8 @@ RendererTest::RendererTest() { addInstancedTests({&RendererTest::multipleBlocks}, Containers::arraySize(MultipleBlocksData)); + addTests({&RendererTest::emptyLines}); + addInstancedTests({ &RendererTest::indicesVertices, &RendererTest::indicesVertices, @@ -6470,6 +6474,150 @@ void RendererTest::multipleBlocks() { }), TestSuite::Compare::Container); } +void RendererTest::emptyLines() { + struct: AbstractFont { + FontFeatures doFeatures() const override { return {}; } + bool doIsOpened() const override { return _opened; } + void doClose() override { _opened = false; } + + Properties doOpenFile(Containers::StringView, Float) override { + _opened = true; + return {4.0f, 5.0f, -3.0f, 8.0f, 0}; + } + + void doGlyphIdsInto(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&) override { + CORRADE_FAIL("This shouldn't be called"); + } + Vector2 doGlyphSize(UnsignedInt) override { return {}; } + Vector2 doGlyphAdvance(UnsignedInt) override { return {}; } + + Containers::Pointer doCreateShaper() override { return {}; } + + bool _opened = false; + } font1; + font1.openFile({}, 0.0f); + + struct: AbstractFont { + FontFeatures doFeatures() const override { return {}; } + bool doIsOpened() const override { return _opened; } + void doClose() override { _opened = false; } + + Properties doOpenFile(Containers::StringView, Float) override { + _opened = true; + return {6.0f, 1.0f, -2.0f, 12.0f, 0}; + } + + void doGlyphIdsInto(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&) override { + CORRADE_FAIL("This shouldn't be called"); + } + Vector2 doGlyphSize(UnsignedInt) override { return {}; } + Vector2 doGlyphAdvance(UnsignedInt) override { return {}; } + + Containers::Pointer doCreateShaper() override { return {}; } + + bool _opened = false; + } font2; + font2.openFile({}, 0.0f); + + struct: AbstractShaper { + using AbstractShaper::AbstractShaper; + + UnsignedInt doShape(Containers::StringView, UnsignedInt begin, UnsignedInt end, Containers::ArrayView) override { + return end - begin; + } + + void doGlyphIdsInto(const Containers::StridedArrayView1D&) const override { + CORRADE_FAIL("This shouldn't be called"); + } + void doGlyphOffsetsAdvancesInto(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&) const override { + CORRADE_FAIL("This shouldn't be called"); + } + void doGlyphClustersInto(const Containers::StridedArrayView1D&) const override {} + } shaper1{font1}, shaper2{font2}; + + struct: AbstractGlyphCache { + using AbstractGlyphCache::AbstractGlyphCache; + + GlyphCacheFeatures doFeatures() const override { return {}; } + } glyphCache{PixelFormat::R8Unorm, {16, 16}}; + glyphCache.addFont(1, &font1); + glyphCache.addFont(1, &font2); + + RendererCore renderer{glyphCache}; + + renderer.setCursor({-100.0f, 300.0f}); + + /* Just a render() call alone will produce an empty rect at the cursor */ + CORRADE_COMPARE( + renderer.render(), + Containers::pair(Range2D{{-100.0f, 300.0f}, {-100.0f, 300.0f}}, Range1Dui{})); + + /* Empty text produces a rectangle spanning just the ascent (2.0*5) and + descent (-2.0*3) */ + CORRADE_COMPARE( + renderer + .setAlignment(Alignment::LineLeft) + .render(shaper1, 8.0f, ""), + Containers::pair(Range2D{{-100.0f, 294.0f}, {-100.0f, 310.0f}}, Range1Dui{})); + + /* Empty text from two fonts takes the max. The first font has bigger + ascent (0.5*5), the second font has bigger descent (-2.0*2). */ + CORRADE_COMPARE( + renderer + .add(shaper1, 2.0f, "") + .add(shaper2, 12.0f, "") + .render(), + Containers::pair(Range2D{{-100.0f, 296.0f}, {-100.0f, 302.5f}}, Range1Dui{})); + + /* Same, just different order and alignment to top instead of line */ + CORRADE_COMPARE( + renderer + .setAlignment(Alignment::TopLeft) + .add(shaper2, 12.0f, "") + .add(shaper1, 2.0f, "") + .render(), + Containers::pair(Range2D{{-100.0f, 293.5f}, {-100.0f, 300.0f}}, Range1Dui{})); + + /* Another empty lines with a single font takes into account the line + advance (1.5*8). Resetting the alignment back to line for easier testing + but using direction-aware one to verify it's correctly resolved in this + case as well. */ + CORRADE_COMPARE( + renderer + .setAlignment(Alignment::LineBegin) + .render(shaper1, 6.0f, "\n"), + Containers::pair(Range2D{{-100.0f, 295.5f - 12.0f}, {-100.0f, 307.5f}}, Range1Dui{})); + + /* Multiple lines should cause multiple advances. Horizontal alignment has + no effect on this. */ + CORRADE_COMPARE( + renderer + .setAlignment(Alignment::LineRight) + .render(shaper1, 6.0f, "\n\n\n\n"), + Containers::pair(Range2D{{-100.0f, 295.5f - 4*12.0f}, {-100.0f, 307.5f}}, Range1Dui{})); + + /* Two lines with different fonts should take ascent of the first font (5), + line advance of the first font (8) and descent of the second font (-2) + into account. Ascent of the second font is 1, which should thus not + affect the first line. */ + CORRADE_COMPARE( + renderer + .add(shaper1, 4.0f, "") + .add(shaper2, 6.0f, "\n") + .render(), + Containers::pair(Range2D{{-100.0f, 298.0f - 8.0f}, {-100.0f, 305.0f}}, Range1Dui{})); + + /* With the order swapped, the second run should contribute to the first + line ascent as well, changing it from 1 to 5. Second line descent is + then -3, line advance is taken from the other font as well. */ + CORRADE_COMPARE( + renderer + .add(shaper2, 6.0f, "") + .add(shaper1, 4.0f, "\n") + .render(), + Containers::pair(Range2D{{-100.0f, 297.0f - 12.0f}, {-100.0f, 305.0f}}, Range1Dui{})); +} + template void RendererTest::indicesVertices() { auto&& data = IndicesVerticesData[testCaseInstanceId()]; setTestCaseDescription(data.name);