Browse Source

Text: empty Renderer runs should still contribute to the bounding rect.

One obvious use case is rendering a text that's just a bunch of \n
characters. With the current code, it would result in a completely empty
rectangle no matter how many newlines there would be, which isn't right
because -- even if nothing is actually visible -- it would break
anything that relies on the bounding rectangle for alignment and
positioning of other content.

Another case is for example the Ui library, which -- once ported to
use the new Renderer instead of the lower-level APIs -- wants to use the
bounding rectangle for cursor and selection positioning. And if it would
be empty for empty text, it'd mean empty input boxes would have
invisible cursor. Not good.

The old Renderer implementation seems to have handled this correctly --
but in f3d6ab4916, when porting to the new
APIs, I discarded that from the test, thinking it was some unnecessary
behavior. It wasn't, it was just never tested directly so I forgot it
was needed.
pull/674/head
Vladimír Vondruš 1 year ago
parent
commit
be2f4971d8
  1. 151
      src/Magnum/Text/Renderer.cpp
  2. 2
      src/Magnum/Text/Test/RendererGLTest.cpp
  3. 152
      src/Magnum/Text/Test/RendererTest.cpp

151
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<Vector2> 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<Vector2> 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<UnsignedInt> glyphIds = state.glyphIds.sliceSize(state.renderingGlyphCount, glyphCount);
/* Those views will be empty if no glyphs were shaped */
const Containers::StridedArrayView1D<Vector2> 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<Vector2> 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<UnsignedInt> 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<Range2D, Range1Dui> 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<Range2D, Range1Dui> 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 = {};

2
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
}

152
src/Magnum/Text/Test/RendererTest.cpp

@ -141,6 +141,7 @@ struct RendererTest: TestSuite::Tester {
void addFontNotFoundInCache();
void multipleBlocks();
void emptyLines();
template<class T> 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>({
&RendererTest::indicesVertices<UnsignedByte>,
&RendererTest::indicesVertices<UnsignedShort>,
@ -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 char32_t>&, const Containers::StridedArrayView1D<UnsignedInt>&) override {
CORRADE_FAIL("This shouldn't be called");
}
Vector2 doGlyphSize(UnsignedInt) override { return {}; }
Vector2 doGlyphAdvance(UnsignedInt) override { return {}; }
Containers::Pointer<AbstractShaper> 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 char32_t>&, const Containers::StridedArrayView1D<UnsignedInt>&) override {
CORRADE_FAIL("This shouldn't be called");
}
Vector2 doGlyphSize(UnsignedInt) override { return {}; }
Vector2 doGlyphAdvance(UnsignedInt) override { return {}; }
Containers::Pointer<AbstractShaper> 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<const FeatureRange>) override {
return end - begin;
}
void doGlyphIdsInto(const Containers::StridedArrayView1D<UnsignedInt>&) const override {
CORRADE_FAIL("This shouldn't be called");
}
void doGlyphOffsetsAdvancesInto(const Containers::StridedArrayView1D<Vector2>&, const Containers::StridedArrayView1D<Vector2>&) const override {
CORRADE_FAIL("This shouldn't be called");
}
void doGlyphClustersInto(const Containers::StridedArrayView1D<UnsignedInt>&) 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<class T> void RendererTest::indicesVertices() {
auto&& data = IndicesVerticesData[testCaseInstanceId()];
setTestCaseDescription(data.name);

Loading…
Cancel
Save