Text: make glyph caches pad by one pixel by default to avoid artifacts.

Took me quite a while to realize what was going on, but in retrospect
it's obvious -- the rasterizer just *rounds* the sub-pixel-positioned
glyph quads to nearest pixels. Which then can cause the neighboring
glyph data to leak to it (because the texture is then sampled not
directly on the edge pixel, but slightly outside of it), or it can also
cut away the edge, when it gets rounded in the other direction.

This was a problem with the original -- laughably inefficient -- atlas
packer as well, but because that packer had excessive padding around
everything, only the second edge-cutting artifact manifested, and that
one is rather subtle unless you know what to look for.

This means the packing is now slightly worse than before and sizes that
previously worked may no longer fit anymore. But since the new atlas
packer is relatively new (well, from September, time sure flies
different here), and the improvement compared to the original packer is
still quite massive, I don't think this is a problem.

src/Magnum/Text/AbstractGlyphCache.cpp

@@ -132,16 +132,16 @@ AbstractGlyphCache::AbstractGlyphCache(const PixelFormat format, const Vector3i&
     arrayAppend(_state->fonts, InPlaceInit, 0u, nullptr);
 }
 
-AbstractGlyphCache::AbstractGlyphCache(const PixelFormat format, const Vector3i& size): AbstractGlyphCache{format, size, {}} {}
+AbstractGlyphCache::AbstractGlyphCache(const PixelFormat format, const Vector3i& size): AbstractGlyphCache{format, size, Vector2i{1}} {}
 
 AbstractGlyphCache::AbstractGlyphCache(const PixelFormat format, const Vector2i& size, const Vector2i& padding): AbstractGlyphCache{format, Vector3i{size, 1}, padding} {}
 
-AbstractGlyphCache::AbstractGlyphCache(const PixelFormat format, const Vector2i& size): AbstractGlyphCache{format, size, {}} {}
+AbstractGlyphCache::AbstractGlyphCache(const PixelFormat format, const Vector2i& size): AbstractGlyphCache{format, size, Vector2i{1}} {}
 
 #ifdef MAGNUM_BUILD_DEPRECATED
 AbstractGlyphCache::AbstractGlyphCache(const Vector2i& size, const Vector2i& padding): AbstractGlyphCache{PixelFormat::R8Unorm, size, padding} {}
 
-AbstractGlyphCache::AbstractGlyphCache(const Vector2i& size): AbstractGlyphCache{PixelFormat::R8Unorm, size, {}} {}
+AbstractGlyphCache::AbstractGlyphCache(const Vector2i& size): AbstractGlyphCache{PixelFormat::R8Unorm, size, Vector2i{1}} {}
 #endif
 
 AbstractGlyphCache::AbstractGlyphCache(NoCreateT) noexcept {}

src/Magnum/Text/AbstractGlyphCache.h

@@ -245,6 +245,27 @@ font-specific glyph IDs that resolved to @cpp 0 @ce with @ref glyphId(),
 rasterize them to the cache in the next round, and then update the rendered
 text again.
 
+@section Text-AbstractGlyphCache-padding Glyph padding
+
+Fonts commonly shape the glyphs to sub-pixel positions, the GPU rasterizer then
+rounds the actual quads to nearest pixels, which then can lead to neighboring
+pixels to leak into the rendered glyph or, conversely, the rendered glyph
+having an edge cut. Because of that, by default the glyphs are padded with one
+pixel on each side, which should prevent such artifacts even if rendering the
+glyphs 2x supersampled.
+
+If you have a pixel-perfect font that gets always shaped to whole pixel
+positions, you can set it back to zero in the constructor. Or for example if
+you can ensure that at least lines get placed on whole pixel positions and the
+line advance is whole pixels as well, you can se it to zero in one dimension at
+least, assuming neither @ref TextureTools::AtlasLandfillFlag::RotatePortrait
+nor @relativeref{TextureTools::AtlasLandfillFlag,RotateLandscape} is enabled in
+@ref atlas().
+
+On the other hand, if you're rendering more than 2x supersampled or for example
+using the cache for generating a distance field, you may want to increase the
+padding even further.
+
 @section Text-AbstractGlyphCache-subclassing Subclassing
 
 A subclass needs to implement the @ref doFeatures() and @ref doSetImage()
@@ -264,14 +285,16 @@ class MAGNUM_TEXT_EXPORT AbstractGlyphCache {
          * @brief Construct a 2D array glyph cache
          * @param format     Source image format
          * @param size       Source image size in pixels
-         * @param padding    Padding around every glyph in pixels
+         * @param padding    Padding around every glyph in pixelss. See
+         *      @ref Text-AbstractGlyphCache-padding for more information about
+         *      the default.
          * @m_since_latest
          *
          * The @p size is expected to be non-zero.
          * @see @ref AbstractGlyphCache(PixelFormat, const Vector2i&, const Vector2i&)
          */
         #ifdef DOXYGEN_GENERATING_OUTPUT
-        explicit AbstractGlyphCache(PixelFormat format, const Vector3i& size, const Vector2i& padding = {});
+        explicit AbstractGlyphCache(PixelFormat format, const Vector3i& size, const Vector2i& padding = Vector2i{1});
         #else
         /* To not need to include Vector */
         explicit AbstractGlyphCache(PixelFormat format, const Vector3i& size, const Vector2i& padding);
@@ -282,7 +305,9 @@ class MAGNUM_TEXT_EXPORT AbstractGlyphCache {
          * @brief Construct a 2D glyph cache
          * @param format     Source image format
          * @param size       Source image size in pixels
-         * @param padding    Padding around every glyph in pixels
+         * @param padding    Padding around every glyph in pixels. See
+         *      @ref Text-AbstractGlyphCache-padding for more information about
+         *      the default.
          * @m_since_latest
          *
          * Equivalent to calling
@@ -290,7 +315,7 @@ class MAGNUM_TEXT_EXPORT AbstractGlyphCache {
          * with depth set to @cpp 1 @ce.
          */
         #ifdef DOXYGEN_GENERATING_OUTPUT
-        explicit AbstractGlyphCache(PixelFormat format, const Vector2i& size, const Vector2i& padding = {});
+        explicit AbstractGlyphCache(PixelFormat format, const Vector2i& size, const Vector2i& padding = Vector2i{1});
         #else
         /* To not need to include Vector */
         explicit AbstractGlyphCache(PixelFormat format, const Vector2i& size, const Vector2i& padding);
@@ -301,7 +326,9 @@ class MAGNUM_TEXT_EXPORT AbstractGlyphCache {
         /**
          * @brief Construct a 2D glyph cache
          * @param size       Source image size in pixels
-         * @param padding    Padding around every glyph in pixels
+         * @param padding    Padding around every glyph in pixelss. See
+         *      @ref Text-AbstractGlyphCache-padding for more information about
+         *      the default.
          *
          * Calls @ref AbstractGlyphCache(PixelFormat, const Vector2i&, const Vector2i&)
          * with @p format set to @ref PixelFormat::R8Unorm.
@@ -309,7 +336,7 @@ class MAGNUM_TEXT_EXPORT AbstractGlyphCache {
          *      instead.
          */
         #ifdef DOXYGEN_GENERATING_OUTPUT
-        explicit AbstractGlyphCache(const Vector2i& size, const Vector2i& padding = {});
+        explicit AbstractGlyphCache(const Vector2i& size, const Vector2i& padding = Vector2i{1});
         #else
         /* To not need to include Vector */
         CORRADE_DEPRECATED("use AbstractGlyphCache(PixelFormat, const Vector2i&, const Vector2i&) instead") explicit AbstractGlyphCache(const Vector2i& size, const Vector2i& padding);
@@ -385,7 +412,11 @@ class MAGNUM_TEXT_EXPORT AbstractGlyphCache {
         CORRADE_DEPRECATED("use size() instead") Vector2i textureSize() const;
         #endif
 
-        /** @brief Glyph padding */
+        /**
+         * @brief Glyph padding
+         *
+         * @see @ref Text-AbstractGlyphCache-padding
+         */
         Vector2i padding() const;
 
         /**

src/Magnum/Text/GlyphCache.h

@@ -80,9 +80,10 @@ class MAGNUM_TEXT_EXPORT GlyphCache: public AbstractGlyphCache {
          * @brief Constructor
          *
          * Same as calling the above with @p originalSize and @p size being set
-         * to the same value.
+         * to the same value. See @ref Text-AbstractGlyphCache-padding for more
+         * information about the default @p padding.
          */
-        explicit GlyphCache(GL::TextureFormat internalFormat, const Vector2i& size, const Vector2i& padding = {});
+        explicit GlyphCache(GL::TextureFormat internalFormat, const Vector2i& size, const Vector2i& padding = Vector2i{1});
 
         /**
          * @brief Constructor
@@ -103,9 +104,10 @@ class MAGNUM_TEXT_EXPORT GlyphCache: public AbstractGlyphCache {
          * @brief Constructor
          *
          * Same as calling the above with @p originalSize and @p size being set
-         * to the same value.
+         * to the same value. See @ref Text-AbstractGlyphCache-padding for more
+         * information about the default @p padding.
          */
-        explicit GlyphCache(const Vector2i& size, const Vector2i& padding = {});
+        explicit GlyphCache(const Vector2i& size, const Vector2i& padding = Vector2i{1});
 
         /**
          * @brief Construct without creating the internal state and the OpenGL texture object

src/Magnum/Text/Test/AbstractFontTest.cpp

@@ -1228,7 +1228,8 @@ void AbstractFontTest::layout() {
     font.openFile({}, {});
     CORRADE_COMPARE(font.size(), 0.5f);
 
-    DummyGlyphCache cache{PixelFormat::R8Unorm, {10, 20}};
+    /* Default padding is 1 to avoid artifacts, set that to 0 to simplify */
+    DummyGlyphCache cache{PixelFormat::R8Unorm, {10, 20}, {}};
 
     UnsignedInt fontId = cache.addFont(15, &font);
 

src/Magnum/Text/Test/AbstractGlyphCacheTest.cpp

@@ -309,7 +309,8 @@ void AbstractGlyphCacheTest::constructNoPadding() {
     DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}};
     CORRADE_COMPARE(cache.format(), PixelFormat::R32F);
     CORRADE_COMPARE(cache.size(), (Vector3i{1024, 512, 3}));
-    CORRADE_COMPARE(cache.padding(), Vector2i{});
+    /* 1 by default to avoid artifacts */
+    CORRADE_COMPARE(cache.padding(), Vector2i{1});
     CORRADE_COMPARE(cache.fontCount(), 0);
     /* Invalid glyph is always present */
     CORRADE_COMPARE(cache.glyphCount(), 1);
@@ -317,7 +318,7 @@ void AbstractGlyphCacheTest::constructNoPadding() {
     CORRADE_COMPARE(cache.atlas().size(), (Vector3i{1024, 512, 3}));
     CORRADE_COMPARE(cache.atlas().filledSize(), (Vector3i{1024, 512, 0}));
     CORRADE_COMPARE(cache.atlas().flags(), TextureTools::AtlasLandfillFlag::WidestFirst);
-    CORRADE_COMPARE(cache.atlas().padding(), Vector2i{});
+    CORRADE_COMPARE(cache.atlas().padding(), Vector2i{1});
     CORRADE_COMPARE(cache.image().format(), PixelFormat::R32F);
     CORRADE_COMPARE(cache.image().size(), (Vector3i{1024, 512, 3}));
 
@@ -356,14 +357,15 @@ void AbstractGlyphCacheTest::construct2DNoPadding() {
     DummyGlyphCache cache{PixelFormat::R32F, {1024, 512}};
     CORRADE_COMPARE(cache.format(), PixelFormat::R32F);
     CORRADE_COMPARE(cache.size(), (Vector3i{1024, 512, 1}));
-    CORRADE_COMPARE(cache.padding(), Vector2i{});
+    /* 1 by default to avoid artifacts */
+    CORRADE_COMPARE(cache.padding(), Vector2i{1});
     CORRADE_COMPARE(cache.fontCount(), 0);
     /* Invalid glyph is always present */
     CORRADE_COMPARE(cache.glyphCount(), 1);
     CORRADE_COMPARE(cache.atlas().size(), (Vector3i{1024, 512, 1}));
     CORRADE_COMPARE(cache.atlas().filledSize(), (Vector3i{1024, 0, 1}));
     CORRADE_COMPARE(cache.atlas().flags(), TextureTools::AtlasLandfillFlag::WidestFirst);
-    CORRADE_COMPARE(cache.atlas().padding(), Vector2i{});
+    CORRADE_COMPARE(cache.atlas().padding(), Vector2i{1});
 
     /* The rest shouldn't be any different */
 }
@@ -416,13 +418,14 @@ void AbstractGlyphCacheTest::constructDeprecatedNoPadding() {
     CORRADE_IGNORE_DEPRECATED_PUSH
     CORRADE_COMPARE(cache.textureSize(), (Vector2i{1024, 512}));
     CORRADE_IGNORE_DEPRECATED_POP
-    CORRADE_COMPARE(cache.padding(), Vector2i{});
+    /* 1 by default to avoid artifacts */
+    CORRADE_COMPARE(cache.padding(), Vector2i{1});
     CORRADE_COMPARE(cache.fontCount(), 0);
     CORRADE_COMPARE(cache.glyphCount(), 1);
     CORRADE_COMPARE(cache.atlas().size(), (Vector3i{1024, 512, 1}));
     CORRADE_COMPARE(cache.atlas().filledSize(), (Vector3i{1024, 0, 1}));
     CORRADE_COMPARE(cache.atlas().flags(), TextureTools::AtlasLandfillFlag::WidestFirst);
-    CORRADE_COMPARE(cache.atlas().padding(), Vector2i{});
+    CORRADE_COMPARE(cache.atlas().padding(), Vector2i{1});
 }
 #endif
 
@@ -538,7 +541,8 @@ void AbstractGlyphCacheTest::setInvalidGlyph2D() {
 void AbstractGlyphCacheTest::setInvalidGlyphOutOfRange() {
     CORRADE_SKIP_IF_NO_ASSERT();
 
-    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}};
+    /* Default padding is 1, test that it works for zero as well */
+    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}, {}};
 
     std::ostringstream out;
     Error redirectError{&out};
@@ -804,7 +808,10 @@ void AbstractGlyphCacheTest::addGlyphIndexOutOfRange() {
 void AbstractGlyphCacheTest::addGlyphAlreadyAdded() {
     CORRADE_SKIP_IF_NO_ASSERT();
 
-    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}};
+    /* Default padding of 1 makes it impossible to add a glyph at zero offset
+       as it's out of range. Don't want to bother with that here so resetting
+       it to 0. */
+    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}, {}};
 
     cache.addFont(9);
     UnsignedInt fontId = cache.addFont(3);
@@ -822,7 +829,8 @@ void AbstractGlyphCacheTest::addGlyphAlreadyAdded() {
 void AbstractGlyphCacheTest::addGlyphOutOfRange() {
     CORRADE_SKIP_IF_NO_ASSERT();
 
-    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}};
+    /* Default padding is 1, test that it works for zero as well */
+    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}, {}};
 
     UnsignedInt fontId = cache.addFont(9);
 
@@ -889,7 +897,10 @@ void AbstractGlyphCacheTest::addGlyphOutOfRangePadded() {
 void AbstractGlyphCacheTest::addGlyphTooMany() {
     CORRADE_SKIP_IF_NO_ASSERT();
 
-    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512}};
+    /* Default padding of 1 makes it impossible to add a glyph at zero offset
+       as it's out of range. Don't want to bother with that here so resetting
+       it to 0. */
+    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512}, {}};
 
     /* Adding a font with over 65k potential glyphs is okay */
     UnsignedInt fontId = cache.addFont(100000);
@@ -1472,8 +1483,8 @@ void AbstractGlyphCacheTest::processedImageNotImplemented() {
 }
 
 void AbstractGlyphCacheTest::access() {
-    /* Padding tested well enough in addGlyph(), not using it here */
-    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}};
+    /* Padding tested well enough in addGlyph(), resetting it back to 0 here */
+    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}, {}};
 
     cache.setInvalidGlyph({5, 7}, 2, {{5, 10}, {15, 30}});
 
@@ -1540,8 +1551,8 @@ void AbstractGlyphCacheTest::access() {
 }
 
 void AbstractGlyphCacheTest::accessBatch() {
-    /* Padding tested well enough in addGlyph(), not using it here */
-    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}};
+    /* Padding tested well enough in addGlyph(), resetting it back to 0 here */
+    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}, {}};
 
     cache.setInvalidGlyph({5, 7}, 2, {{5, 10}, {15, 30}});
 
@@ -1594,7 +1605,10 @@ void AbstractGlyphCacheTest::accessInvalid() {
     /* Silly test name, but these all test debug asserts while
        accessBatchInvalid() tests non-debug asserts */
 
-    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}};
+    /* Default padding of 1 makes it impossible to add a glyph at zero offset
+       as it's out of range. Don't want to bother with that here so resetting
+       it to 0. */
+    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}, {}};
 
     cache.addFont(9);
     UnsignedInt fontId = cache.addFont(3);
@@ -1628,7 +1642,10 @@ void AbstractGlyphCacheTest::accessInvalid() {
 void AbstractGlyphCacheTest::accessBatchInvalid() {
     CORRADE_SKIP_IF_NO_ASSERT();
 
-    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}};
+    /* Default padding of 1 makes it impossible to add a glyph at zero offset
+       as it's out of range. Don't want to bother with that here so resetting
+       it to 0. */
+    DummyGlyphCache cache{PixelFormat::R32F, {1024, 512, 3}, {}};
 
     cache.addFont(9);
     UnsignedInt fontId = cache.addFont(3);

src/Magnum/Text/Test/RendererGLTest.cpp

@@ -106,7 +106,8 @@ struct TestFont: AbstractFont {
 };
 
 GlyphCache testGlyphCache(AbstractFont& font) {
-    GlyphCache cache{{20, 20}};
+    /* Default padding is 1 to avoid artifacts, set that to 0 to simplify */
+    GlyphCache cache{{20, 20}, {}};
 
     /* Add one more font to verify the right one gets picked */
     cache.addFont(96);

src/Magnum/Text/Test/RendererTest.cpp

@@ -361,7 +361,8 @@ struct DummyGlyphCache: AbstractGlyphCache {
 };
 
 DummyGlyphCache testGlyphCache(AbstractFont& font) {
-    DummyGlyphCache cache{PixelFormat::R8Unorm, {20, 20}};
+    /* Default padding is 1 to avoid artifacts, set that to 0 to simplify */
+    DummyGlyphCache cache{PixelFormat::R8Unorm, {20, 20}, {}};
 
     /* Add one more font to verify the right one gets picked */
     cache.addFont(96);
@@ -378,7 +379,8 @@ DummyGlyphCache testGlyphCache(AbstractFont& font) {
 }
 
 DummyGlyphCache testGlyphCacheArray(AbstractFont& font) {
-    DummyGlyphCache cache{PixelFormat::R8Unorm, {20, 20, 3}};
+    /* Default padding is 1 to avoid artifacts, set that to 0 to simplify */
+    DummyGlyphCache cache{PixelFormat::R8Unorm, {20, 20, 3}, {}};
 
     /* Add one more font to verify the right one gets picked */
     cache.addFont(96);
@@ -1049,8 +1051,9 @@ void RendererTest::multiline() {
     } font;
     font.openFile({}, 0.5f);
 
-    /* Just a single glyph that scales to {1, 1} in the end */
-    DummyGlyphCache cache{PixelFormat::R8Unorm, {20, 20}};
+    /* Just a single glyph that scales to {1, 1} in the end. Default padding is
+       1 which would prevent this, set it back to 0. */
+    DummyGlyphCache cache{PixelFormat::R8Unorm, {20, 20}, {}};
     UnsignedInt fontId = cache.addFont(1, &font);
     cache.addGlyph(fontId, 0, {}, {{}, {2, 2}});
 

src/MagnumPlugins/MagnumFontConverter/Test/MagnumFontConverterTest.cpp

@@ -323,7 +323,8 @@ void MagnumFontConverterTest::exportFontEmptyCache() {
 
         GlyphCacheFeatures doFeatures() const override { return {}; }
         void doSetImage(const Vector2i&, const ImageView2D&) override {}
-    } cache{PixelFormat::R8Unorm, {8, 4}};
+    /* Default padding is 1 to avoid artifacts, set that to 0 to simplify */
+    } cache{PixelFormat::R8Unorm, {8, 4}, {}};
 
     /* Associate the font with the cache. The case where it's not even that is
        tested in exportFontNotFoundInCache() below. */