Browse Source

Text: only allow creating processed GlyphCacheGL by subclasses.

So the users don't accidentally create an instance with the processed
format / size being different only to get greeted by a nasty bounds or
format assertion inside flushImage() / doSetImage().

Now it's only allowed to be done by a subclass (such as
DistanceFieldGlyphCacheGL, or, ahem, MagnumFont), and doSetImage()
additionally has an assert to notify the implementer that a subclass
needs to provide its own override.

This also feels kind of messy, honestly. It's as if the
DistanceFieldGlyphCache should never have been a subclass, maybe?
pull/650/head
Vladimír Vondruš 2 years ago
parent
commit
6b31924b0b
  1. 5
      src/Magnum/Text/CMakeLists.txt
  2. 3
      src/Magnum/Text/GlyphCacheGL.cpp
  3. 46
      src/Magnum/Text/GlyphCacheGL.h
  4. 2
      src/Magnum/Text/Test/CMakeLists.txt
  5. 54
      src/Magnum/Text/Test/GlyphCacheGLTest.cpp
  6. 2
      src/MagnumPlugins/MagnumFont/MagnumFont.cpp

5
src/Magnum/Text/CMakeLists.txt

@ -62,10 +62,9 @@ set(MagnumText_PRIVATE_HEADERS
Implementation/printFourCC.h)
if(MAGNUM_TARGET_GL)
list(APPEND MagnumText_SRCS
GlyphCacheGL.cpp)
list(APPEND MagnumText_GracefulAssert_SRCS
DistanceFieldGlyphCacheGL.cpp)
DistanceFieldGlyphCacheGL.cpp
GlyphCacheGL.cpp)
list(APPEND MagnumText_HEADERS
DistanceFieldGlyphCacheGL.h
GlyphCacheGL.h)

3
src/Magnum/Text/GlyphCacheGL.cpp

@ -108,6 +108,9 @@ GlyphCacheGL::GlyphCacheGL(NoCreateT) noexcept: AbstractGlyphCache{NoCreate}, _t
GlyphCacheFeatures GlyphCacheGL::doFeatures() const { return {}; }
void GlyphCacheGL::doSetImage(const Vector2i& offset, const ImageView2D& image) {
CORRADE_ASSERT(format() == processedFormat() && size() == processedSize(),
"Text::GlyphCacheGL::flushImage(): subclass expected to provide a doSetImage() implementation to handle different processed format or size", );
/* On ES2 without EXT_unpack_subimage and on WebGL 1 there's no possibility
to upload just a slice of the input, upload the whole image instead by
ignoring the PixelStorage properties of the input */

46
src/Magnum/Text/GlyphCacheGL.h

@ -102,26 +102,6 @@ class MAGNUM_TEXT_EXPORT GlyphCacheGL: public AbstractGlyphCache {
*/
explicit GlyphCacheGL(PixelFormat format, const Vector2i& size, const Vector2i& padding = Vector2i{1});
/**
* @brief Construct with a specific processed format and size
* @param format Source image format
* @param size Source image size size in pixels
* @param processedFormat Processed image format
* @param processedSize Processed glyph cache texture size in
* pixels
* @param padding Padding around every glyph in pixels. See
* @ref Text-AbstractGlyphCache-padding for more information about
* the default.
* @m_since_latest
*
* The @p size and @p processedSize is expected to be non-zero. All
* glyphs are saved in @p format relative to @p size and with
* @p padding, although the actual glyph cache texture is in
* @p processedFormat and has @p processedSize.
* @see @ref AbstractGlyphCache(PixelFormat, const Vector2i&, const Vector2i&)
*/
explicit GlyphCacheGL(PixelFormat format, const Vector2i& size, PixelFormat processedFormat, const Vector2i& processedSize, const Vector2i& padding = Vector2i{1});
#ifdef MAGNUM_BUILD_DEPRECATED
/**
* @brief Constructor
@ -178,6 +158,32 @@ class MAGNUM_TEXT_EXPORT GlyphCacheGL: public AbstractGlyphCache {
/** @brief Cache texture */
GL::Texture2D& texture() { return _texture; }
protected:
/**
* @brief Construct with a specific processed format and size
* @param format Source image format
* @param size Source image size size in pixels
* @param processedFormat Processed image format
* @param processedSize Processed glyph cache texture size in
* pixels
* @param padding Padding around every glyph in pixels. See
* @ref Text-AbstractGlyphCache-padding for more information about
* the default.
* @m_since_latest
*
* The @p size and @p processedSize is expected to be non-zero. All
* glyphs are saved in @p format relative to @p size and with
* @p padding, although the actual glyph cache texture is in
* @p processedFormat and has @p processedSize.
*
* Meant to be only used by subclasses that advertise
* @ref GlyphCacheFeature::ImageProcessing and reimplement
* @ref doSetImage() to take the differences between @p format, @p size
* and @p processedFormat, @p processedSize into account.
* @see @ref AbstractGlyphCache(PixelFormat, const Vector2i&, const Vector2i&)
*/
explicit GlyphCacheGL(PixelFormat format, const Vector2i& size, PixelFormat processedFormat, const Vector2i& processedSize, const Vector2i& padding = Vector2i{1});
#ifdef DOXYGEN_GENERATING_OUTPUT
private:
#else

2
src/Magnum/Text/Test/CMakeLists.txt

@ -116,7 +116,7 @@ if(MAGNUM_TARGET_GL)
corrade_add_test(TextGlyphCacheGLTest GlyphCacheGLTest.cpp
LIBRARIES
MagnumText
MagnumTextTestLib
MagnumOpenGLTester
MagnumDebugTools)
corrade_add_test(TextRendererGLTest RendererGLTest.cpp

54
src/Magnum/Text/Test/GlyphCacheGLTest.cpp

@ -23,8 +23,11 @@
DEALINGS IN THE SOFTWARE.
*/
#include <sstream>
#include <Corrade/Containers/StringView.h>
#include <Corrade/Containers/StridedArrayView.h>
#include <Corrade/Containers/StringStl.h> /**< @todo remove once Debug is stream-free */
#include <Corrade/TestSuite/Compare/String.h>
#include <Corrade/Utility/Algorithms.h>
#include "Magnum/Image.h"
@ -66,6 +69,8 @@ struct GlyphCacheGLTest: GL::OpenGLTester {
void setImage();
void setImageFourChannel();
void flushImageSubclassProcessedFormatSize();
};
GlyphCacheGLTest::GlyphCacheGLTest() {
@ -84,7 +89,9 @@ GlyphCacheGLTest::GlyphCacheGLTest() {
&GlyphCacheGLTest::constructMove,
&GlyphCacheGLTest::setImage,
&GlyphCacheGLTest::setImageFourChannel});
&GlyphCacheGLTest::setImageFourChannel,
&GlyphCacheGLTest::flushImageSubclassProcessedFormatSize});
}
void GlyphCacheGLTest::construct() {
@ -112,19 +119,12 @@ void GlyphCacheGLTest::constructNoPadding() {
}
void GlyphCacheGLTest::constructProcessed() {
struct: GlyphCacheGL {
using GlyphCacheGL::GlyphCacheGL;
struct Cache: GlyphCacheGL {
explicit Cache(PixelFormat format, const Vector2i& size, PixelFormat processedFormat, const Vector2i& processedSize, const Vector2i& padding): GlyphCacheGL{format, size, processedFormat, processedSize, padding} {}
GlyphCacheFeatures doFeatures() const override {
return GlyphCacheFeature::ImageProcessing;
}
/* The symbols are private, we don't actually need them here, so just
override with an empty implementation */
void doSetImage(const Vector2i&, const ImageView2D&) override {}
void doSetProcessedImage(const Vector2i&, const ImageView2D&) override {}
Image3D doProcessedImage() override {
CORRADE_INTERNAL_ASSERT_UNREACHABLE();
}
} cache{PixelFormat::R8Unorm, {1024, 2048}, PixelFormat::RGBA8Unorm, {128, 256}, {3, 2}};
MAGNUM_VERIFY_NO_GL_ERROR();
@ -139,19 +139,12 @@ void GlyphCacheGLTest::constructProcessed() {
}
void GlyphCacheGLTest::constructProcessedNoPadding() {
struct: GlyphCacheGL {
using GlyphCacheGL::GlyphCacheGL;
struct Cache: GlyphCacheGL {
explicit Cache(PixelFormat format, const Vector2i& size, PixelFormat processedFormat, const Vector2i& processedSize): GlyphCacheGL{format, size, processedFormat, processedSize} {}
GlyphCacheFeatures doFeatures() const override {
return GlyphCacheFeature::ImageProcessing;
}
/* The symbols are private, we don't actually need them here, so just
override with an empty implementation */
void doSetImage(const Vector2i&, const ImageView2D&) override {}
void doSetProcessedImage(const Vector2i&, const ImageView2D&) override {}
Image3D doProcessedImage() override {
CORRADE_INTERNAL_ASSERT_UNREACHABLE();
}
} cache{PixelFormat::R8Unorm, {1024, 2048}, PixelFormat::RGBA8Unorm, {128, 256}};
MAGNUM_VERIFY_NO_GL_ERROR();
@ -376,6 +369,29 @@ void GlyphCacheGLTest::setImageFourChannel() {
DebugTools::CompareImage);
}
void GlyphCacheGLTest::flushImageSubclassProcessedFormatSize() {
CORRADE_SKIP_IF_NO_ASSERT();
struct Cache: GlyphCacheGL {
explicit Cache(PixelFormat format, const Vector2i& size, PixelFormat processedFormat, const Vector2i& processedSize): GlyphCacheGL{format, size, processedFormat, processedSize} {}
GlyphCacheFeatures doFeatures() const override {
return GlyphCacheFeature::ImageProcessing;
}
};
Cache differentFormat{PixelFormat::R8Unorm, {32, 32}, PixelFormat::RGBA8Unorm, {32, 32}};
Cache differentSize{PixelFormat::R8Unorm, {32, 32}, PixelFormat::R8Unorm, {16, 32}};
std::ostringstream out;
Error redirectError{&out};
differentFormat.flushImage({{}, {32, 32}});
differentSize.flushImage({{}, {32, 32}});
CORRADE_COMPARE_AS(out.str(),
"Text::GlyphCacheGL::flushImage(): subclass expected to provide a doSetImage() implementation to handle different processed format or size\n"
"Text::GlyphCacheGL::flushImage(): subclass expected to provide a doSetImage() implementation to handle different processed format or size\n",
TestSuite::Compare::String);
}
}}}}
CORRADE_TEST_MAIN(Magnum::Text::Test::GlyphCacheGLTest)

2
src/MagnumPlugins/MagnumFont/MagnumFont.cpp

@ -166,7 +166,7 @@ Containers::Pointer<AbstractGlyphCache> MagnumFont::doCreateGlyphCache() {
/** @todo figure out a nicer way, and ideally how to do this with
fillGlyphCache() instead */
struct Cache: GlyphCacheGL {
using GlyphCacheGL::GlyphCacheGL;
explicit Cache(PixelFormat format, const Vector2i& size, PixelFormat processedFormat, const Vector2i& processedSize, const Vector2i& padding): GlyphCacheGL{format, size, processedFormat, processedSize, padding} {}
GlyphCacheFeatures doFeatures() const override {
return GlyphCacheFeature::ImageProcessing;

Loading…
Cancel
Save