ic
/
magnum
mirror of https://github.com/mosra/magnum.git
6
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Text: returning std::unique_ptr instead of raw pointer. 

Avoids memory leaks (I made one in the test, fail).
pull/34/head
Vladimír Vondruš 13 years ago
parent
20a9d9a374
commit
8003cd435a
14 changed files with 59 additions and 49 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 10
      src/Plugins/MagnumFont/MagnumFont.cpp
  2. 4
      src/Plugins/MagnumFont/MagnumFont.h
  3. 2
      src/Plugins/MagnumFont/Test/MagnumFontTest.cpp
  4. 2
      src/Plugins/MagnumFont/pluginRegistrationMagnumFont.cpp
  5. 2
      src/Plugins/MagnumFontConverter/Test/MagnumFontConverterTest.cpp
  6. 2
      src/Plugins/MagnumFontConverter/pluginRegistrationMagnumFontConverter.cpp
  7. 8
      src/Text/AbstractFont.cpp
  8. 13
      src/Text/AbstractFont.h
  9. 14
      src/Text/AbstractFontConverter.cpp
  10. 15
      src/Text/AbstractFontConverter.h
  11. 20
      src/Text/Test/AbstractFontConverterTest.cpp
  12. 2
      src/Text/Test/AbstractFontTest.cpp
  13. 4
      src/Text/Test/TextRendererGLTest.cpp
  14. 10
      src/Text/TextRenderer.cpp

10
src/Plugins/MagnumFont/MagnumFont.cpp
Unescape View File

@ -176,12 +176,12 @@ Vector2 MagnumFont::doGlyphAdvance(const UnsignedInt glyph) {
return glyph < _opened->glyphAdvance.size() ? _opened->glyphAdvance[glyph] : Vector2();
}
GlyphCache* MagnumFont::doCreateGlyphCache() {
std::unique_ptr<GlyphCache> MagnumFont::doCreateGlyphCache() {
/* Set cache image */
auto cache = new Text::GlyphCache(
std::unique_ptr<GlyphCache> cache(new Text::GlyphCache(
_opened->conf.value<Vector2i>("originalImageSize"),
_opened->image.size(),
_opened->conf.value<Vector2i>("padding"));
_opened->conf.value<Vector2i>("padding")));
cache->setImage({}, _opened->image);
/* Fill glyph map */
@ -192,8 +192,8 @@ GlyphCache* MagnumFont::doCreateGlyphCache() {
return cache;
}
AbstractLayouter* MagnumFont::doLayout(const GlyphCache& cache, Float size, const std::string& text) {
return new MagnumFontLayouter(_opened->glyphId, _opened->glyphAdvance, cache, this->size(), size, text);
std::unique_ptr<AbstractLayouter> MagnumFont::doLayout(const GlyphCache& cache, Float size, const std::string& text) {
return std::unique_ptr<MagnumFontLayouter>(new MagnumFontLayouter(_opened->glyphId, _opened->glyphAdvance, cache, this->size(), size, text));
}
namespace {

4
src/Plugins/MagnumFont/MagnumFont.h
Unescape View File

@ -125,9 +125,9 @@ class MagnumFont: public AbstractFont {
Vector2 doGlyphAdvance(UnsignedInt glyph) override;
GlyphCache* doCreateGlyphCache() override;
std::unique_ptr<GlyphCache> doCreateGlyphCache() override;
AbstractLayouter* doLayout(const GlyphCache& cache, Float size, const std::string& text) override;
std::unique_ptr<AbstractLayouter> doLayout(const GlyphCache& cache, Float size, const std::string& text) override;
Data* _opened;

2
src/Plugins/MagnumFont/Test/MagnumFontTest.cpp
Unescape View File

@ -63,7 +63,7 @@ void MagnumFontTest::layout() {
cache.insert(font.glyphId(U'W'), {25, 34}, {{0, 8}, {16, 128}});
cache.insert(font.glyphId(U'e'), {25, 12}, {{16, 4}, {64, 32}});
AbstractLayouter* layouter = font.layout(cache, 0.5f, "Wave");
auto layouter = font.layout(cache, 0.5f, "Wave");
CORRADE_VERIFY(layouter);
CORRADE_COMPARE(layouter->glyphCount(), 4);

2
src/Plugins/MagnumFont/pluginRegistrationMagnumFont.cpp
Unescape View File

@ -25,4 +25,4 @@
#include "MagnumFont/MagnumFont.h"
CORRADE_PLUGIN_REGISTER(MagnumFont, Magnum::Text::MagnumFont,
"cz.mosra.magnum.Text.AbstractFont/0.2")
"cz.mosra.magnum.Text.AbstractFont/0.2.1")

2
src/Plugins/MagnumFontConverter/Test/MagnumFontConverterTest.cpp
Unescape View File

@ -66,7 +66,7 @@ void MagnumFontConverterTest::exportFont() {
void doClose() {}
bool doIsOpened() const { return true; }
Features doFeatures() const { return {}; }
AbstractLayouter* doLayout(const GlyphCache&, Float, const std::string&) { return nullptr; }
std::unique_ptr<AbstractLayouter> doLayout(const GlyphCache&, Float, const std::string&) { return nullptr; }
UnsignedInt doGlyphId(const char32_t character) {
switch(character) {

2
src/Plugins/MagnumFontConverter/pluginRegistrationMagnumFontConverter.cpp
Unescape View File

@ -25,4 +25,4 @@
#include "MagnumFontConverter/MagnumFontConverter.h"
CORRADE_PLUGIN_REGISTER(MagnumFontConverter, Magnum::Text::MagnumFontConverter,
"cz.mosra.magnum.Text.AbstractFontConverter/0.1")
"cz.mosra.magnum.Text.AbstractFontConverter/0.1.1")

8
src/Text/AbstractFont.cpp
Unescape View File

@ -28,6 +28,8 @@
#include <Containers/Array.h>
#include <Utility/Unicode.h>
#include "Text/GlyphCache.h"
namespace Magnum { namespace Text {
AbstractFont::AbstractFont(): _size(0.0f) {}
@ -136,7 +138,7 @@ void AbstractFont::doFillGlyphCache(GlyphCache&, const std::vector<char32_t>&)
CORRADE_ASSERT(false, "Text::AbstractFont::fillGlyphCache(): feature advertised but not implemented", );
}
GlyphCache* AbstractFont::createGlyphCache() {
std::unique_ptr<GlyphCache> AbstractFont::createGlyphCache() {
CORRADE_ASSERT(isOpened(),
"Text::AbstractFont::createGlyphCache(): no font opened", nullptr);
CORRADE_ASSERT(features() & Feature::PreparedGlyphCache,
@ -145,11 +147,11 @@ GlyphCache* AbstractFont::createGlyphCache() {
return doCreateGlyphCache();
}
GlyphCache* AbstractFont::doCreateGlyphCache() {
std::unique_ptr<GlyphCache> AbstractFont::doCreateGlyphCache() {
CORRADE_ASSERT(false, "Text::AbstractFont::createGlyphCache(): feature advertised but not implemented", nullptr);
}
AbstractLayouter* AbstractFont::layout(const GlyphCache& cache, const Float size, const std::string& text) {
std::unique_ptr<AbstractLayouter> AbstractFont::layout(const GlyphCache& cache, const Float size, const std::string& text) {
CORRADE_ASSERT(isOpened(), "Text::AbstractFont::layout(): no font opened", nullptr);
return doLayout(cache, size, text);

13
src/Text/AbstractFont.h
Unescape View File

@ -28,8 +28,9 @@
* @brief Class Magnum::Text::AbstractFont, Magnum::Text::AbstractLayouter
*/
#include <tuple>
#include <memory>
#include <string>
#include <tuple>
#include <PluginManager/AbstractPlugin.h>
#include "Magnum.h"
@ -65,7 +66,7 @@ checked by the implementation:
there is any file opened.
*/
class MAGNUM_TEXT_EXPORT AbstractFont: public PluginManager::AbstractPlugin {
CORRADE_PLUGIN_INTERFACE("cz.mosra.magnum.Text.AbstractFont/0.2")
CORRADE_PLUGIN_INTERFACE("cz.mosra.magnum.Text.AbstractFont/0.2.1")
public:
/**
@ -185,7 +186,7 @@ class MAGNUM_TEXT_EXPORT AbstractFont: public PluginManager::AbstractPlugin {
* Other fonts support only partial glyph cache filling, see
* @ref fillGlyphCache().
*/
GlyphCache* createGlyphCache();
std::unique_ptr<GlyphCache> createGlyphCache();
/**
* @brief Layout the text using font's own layouter
@ -195,7 +196,7 @@ class MAGNUM_TEXT_EXPORT AbstractFont: public PluginManager::AbstractPlugin {
*
* @see fillGlyphCache(), createGlyphCache()
*/
AbstractLayouter* layout(const GlyphCache& cache, Float size, const std::string& text);
std::unique_ptr<AbstractLayouter> layout(const GlyphCache& cache, Float size, const std::string& text);
#ifdef DOXYGEN_GENERATING_OUTPUT
private:
@ -262,10 +263,10 @@ class MAGNUM_TEXT_EXPORT AbstractFont: public PluginManager::AbstractPlugin {
/**
* @brief Implementation for createGlyphCache()
*/
virtual GlyphCache* doCreateGlyphCache();
virtual std::unique_ptr<GlyphCache> doCreateGlyphCache();
/** @brief Implementation for layout() */
virtual AbstractLayouter* doLayout(const GlyphCache& cache, Float size, const std::string& text) = 0;
virtual std::unique_ptr<AbstractLayouter> doLayout(const GlyphCache& cache, Float size, const std::string& text) = 0;
};
CORRADE_ENUMSET_OPERATORS(AbstractFont::Features)

14
src/Text/AbstractFontConverter.cpp
Unescape View File

@ -30,6 +30,8 @@
#include <Utility/Assert.h>
#include <Utility/Unicode.h>
#include "Text/GlyphCache.h"
namespace Magnum { namespace Text {
AbstractFontConverter::AbstractFontConverter() = default;
@ -167,7 +169,7 @@ bool AbstractFontConverter::doExportGlyphCacheToFile(GlyphCache& cache, const st
return true;
}
GlyphCache* AbstractFontConverter::importGlyphCacheFromData(const std::vector<std::pair<std::string, Containers::ArrayReference<const unsigned char>>>& data) const {
std::unique_ptr<GlyphCache> AbstractFontConverter::importGlyphCacheFromData(const std::vector<std::pair<std::string, Containers::ArrayReference<const unsigned char>>>& data) const {
CORRADE_ASSERT(features() >= (Feature::ImportGlyphCache|Feature::ConvertData),
"Text::AbstractFontConverter::importGlyphCacheFromData(): feature not supported", nullptr);
CORRADE_ASSERT(!data.empty(),
@ -176,7 +178,7 @@ GlyphCache* AbstractFontConverter::importGlyphCacheFromData(const std::vector<st
return doImportGlyphCacheFromData(data);
}
GlyphCache* AbstractFontConverter::doImportGlyphCacheFromData(const std::vector<std::pair<std::string, Containers::ArrayReference<const unsigned char>>>& data) const {
std::unique_ptr<GlyphCache> AbstractFontConverter::doImportGlyphCacheFromData(const std::vector<std::pair<std::string, Containers::ArrayReference<const unsigned char>>>& data) const {
CORRADE_ASSERT(!(features() & Feature::MultiFile),
"Text::AbstractFontConverter::importGlyphCacheFromData(): feature advertised but not implemented", nullptr);
CORRADE_ASSERT(data.size() == 1,
@ -185,7 +187,7 @@ GlyphCache* AbstractFontConverter::doImportGlyphCacheFromData(const std::vector<
return doImportGlyphCacheFromSingleData(data[0].second);
}
GlyphCache* AbstractFontConverter::importGlyphCacheFromSingleData(Containers::ArrayReference<const unsigned char> data) const {
std::unique_ptr<GlyphCache> AbstractFontConverter::importGlyphCacheFromSingleData(Containers::ArrayReference<const unsigned char> data) const {
CORRADE_ASSERT(features() >= (Feature::ImportGlyphCache|Feature::ConvertData),
"Text::AbstractFontConverter::importGlyphCacheFromSingleData(): feature not supported", nullptr);
CORRADE_ASSERT(!(features() & Feature::MultiFile),
@ -194,19 +196,19 @@ GlyphCache* AbstractFontConverter::importGlyphCacheFromSingleData(Containers::Ar
return doImportGlyphCacheFromSingleData(data);
}
GlyphCache* AbstractFontConverter::doImportGlyphCacheFromSingleData(Containers::ArrayReference<const unsigned char>) const {
std::unique_ptr<GlyphCache> AbstractFontConverter::doImportGlyphCacheFromSingleData(Containers::ArrayReference<const unsigned char>) const {
CORRADE_ASSERT(false,
"Text::AbstractFontConverter::importGlyphCacheFromSingleData(): feature advertised but not implemented", nullptr);
}
GlyphCache* AbstractFontConverter::importGlyphCacheFromFile(const std::string& filename) const {
std::unique_ptr<GlyphCache> AbstractFontConverter::importGlyphCacheFromFile(const std::string& filename) const {
CORRADE_ASSERT(features() & Feature::ImportGlyphCache,
"Text::AbstractFontConverter::importGlyphCacheFromFile(): feature not supported", nullptr);
return doImportGlyphCacheFromFile(filename);
}
GlyphCache* AbstractFontConverter::doImportGlyphCacheFromFile(const std::string& filename) const {
std::unique_ptr<GlyphCache> AbstractFontConverter::doImportGlyphCacheFromFile(const std::string& filename) const {
CORRADE_ASSERT(features() & Feature::ConvertData && !(features() & Feature::MultiFile),
"Text::AbstractFontConverter::importGlyphCacheFromFile(): not implemented", nullptr);

15
src/Text/AbstractFontConverter.h
Unescape View File

@ -28,6 +28,7 @@
* @brief Class Magnum::Text::AbstractFontConverter
*/
#include <memory>
#include <PluginManager/AbstractPlugin.h>
#include "Magnum.h"
@ -61,7 +62,7 @@ checked by the implementation:
array passed.
*/
class MAGNUM_TEXT_EXPORT AbstractFontConverter: public PluginManager::AbstractPlugin {
CORRADE_PLUGIN_INTERFACE("cz.mosra.magnum.Text.AbstractFontConverter/0.1")
CORRADE_PLUGIN_INTERFACE("cz.mosra.magnum.Text.AbstractFontConverter/0.1.1")
public:
/**
@ -222,7 +223,7 @@ class MAGNUM_TEXT_EXPORT AbstractFontConverter: public PluginManager::AbstractPl
* @see @ref features(), @ref importGlyphCacheFromFile(),
* @ref exportGlyphCacheToData()
*/
GlyphCache* importGlyphCacheFromData(const std::vector<std::pair<std::string, Containers::ArrayReference<const unsigned char>>>& data) const;
std::unique_ptr<GlyphCache> importGlyphCacheFromData(const std::vector<std::pair<std::string, Containers::ArrayReference<const unsigned char>>>& data) const;
/**
* @brief Import glyph cache from single raw data
@ -234,7 +235,7 @@ class MAGNUM_TEXT_EXPORT AbstractFontConverter: public PluginManager::AbstractPl
* @see @ref features(), @ref importGlyphCacheFromFile(),
* @ref exportFontToSingleData()
*/
GlyphCache* importGlyphCacheFromSingleData(Containers::ArrayReference<const unsigned char> data) const;
std::unique_ptr<GlyphCache> importGlyphCacheFromSingleData(Containers::ArrayReference<const unsigned char> data) const;
/**
* @brief Import glyph cache from file
@ -247,7 +248,7 @@ class MAGNUM_TEXT_EXPORT AbstractFontConverter: public PluginManager::AbstractPl
* @see @ref features(), @ref importGlyphCacheFromData(),
* @ref exportGlyphCacheToFile()
*/
GlyphCache* importGlyphCacheFromFile(const std::string& filename) const;
std::unique_ptr<GlyphCache> importGlyphCacheFromFile(const std::string& filename) const;
#ifndef DOXYGEN_GENERATING_OUTPUT
private:
@ -326,10 +327,10 @@ class MAGNUM_TEXT_EXPORT AbstractFontConverter: public PluginManager::AbstractPl
* If the plugin doesn't have @ref Feature::MultiFile, default
* implementation calls @ref doImportGlyphCacheFromSingleData().
*/
virtual GlyphCache* doImportGlyphCacheFromData(const std::vector<std::pair<std::string, Containers::ArrayReference<const unsigned char>>>& data) const;
virtual std::unique_ptr<GlyphCache> doImportGlyphCacheFromData(const std::vector<std::pair<std::string, Containers::ArrayReference<const unsigned char>>>& data) const;
/** @brief Implementation for importGlyphCacheFromSingleData() */
virtual GlyphCache* doImportGlyphCacheFromSingleData(Containers::ArrayReference<const unsigned char> data) const;
virtual std::unique_ptr<GlyphCache> doImportGlyphCacheFromSingleData(Containers::ArrayReference<const unsigned char> data) const;
/**
* @brief Implementation for @ref importGlyphCacheFromFile()
@ -338,7 +339,7 @@ class MAGNUM_TEXT_EXPORT AbstractFontConverter: public PluginManager::AbstractPl
* have @ref Feature::MultiFile, default implementation opens the file
* and calls @ref doImportGlyphCacheFromSingleData() with its contents.
*/
virtual GlyphCache* doImportGlyphCacheFromFile(const std::string& filename) const;
virtual std::unique_ptr<GlyphCache> doImportGlyphCacheFromFile(const std::string& filename) const;
private:
#ifndef _WIN32

20
src/Text/Test/AbstractFontConverterTest.cpp
Unescape View File

@ -28,6 +28,7 @@
#include <Utility/Directory.h>
#include "Text/AbstractFontConverter.h"
#include "Text/GlyphCache.h"
#include "testConfigure.h"
@ -232,8 +233,9 @@ class SingleGlyphCacheDataImporter: public Text::AbstractFontConverter {
private:
Features doFeatures() const override { return Feature::ConvertData|Feature::ImportGlyphCache; }
GlyphCache* doImportGlyphCacheFromSingleData(const Containers::ArrayReference<const unsigned char> data) const override {
if(data.size() == 1 && data[0] == 0xa5) return reinterpret_cast<GlyphCache*>(0xdeadbeef);
std::unique_ptr<GlyphCache> doImportGlyphCacheFromSingleData(const Containers::ArrayReference<const unsigned char> data) const override {
if(data.size() == 1 && data[0] == 0xa5)
return std::unique_ptr<GlyphCache>(reinterpret_cast<GlyphCache*>(0xdeadbeef));
return nullptr;
}
};
@ -244,15 +246,21 @@ void AbstractFontConverterTest::importGlyphCacheFromSingleData() {
/* doImportFromData() should call doImportFromSingleData() */
SingleGlyphCacheDataImporter importer;
const unsigned char data[] = {0xa5};
GlyphCache* cache = importer.importGlyphCacheFromData({{{}, data}});
CORRADE_COMPARE(cache, reinterpret_cast<GlyphCache*>(0xdeadbeef));
std::unique_ptr<GlyphCache> cache = importer.importGlyphCacheFromData({{{}, data}});
CORRADE_COMPARE(cache.get(), reinterpret_cast<GlyphCache*>(0xdeadbeef));
/* The pointer is invalid, avoid deletion */
cache.release();
}
void AbstractFontConverterTest::importGlyphCacheFromFile() {
/* doImportFromFile() should call doImportFromSingleData() */
SingleGlyphCacheDataImporter importer;
GlyphCache* cache = importer.importGlyphCacheFromFile(Utility::Directory::join(TEXT_TEST_DIR, "data.bin"));
CORRADE_COMPARE(cache, reinterpret_cast<GlyphCache*>(0xdeadbeef));
std::unique_ptr<GlyphCache> cache = importer.importGlyphCacheFromFile(Utility::Directory::join(TEXT_TEST_DIR, "data.bin"));
CORRADE_COMPARE(cache.get(), reinterpret_cast<GlyphCache*>(0xdeadbeef));
/* The pointer is invalid, avoid deletion */
cache.release();
}
}}}

2
src/Text/Test/AbstractFontTest.cpp
Unescape View File

@ -63,7 +63,7 @@ class SingleDataFont: public Text::AbstractFont {
Vector2 doGlyphAdvance(UnsignedInt) override { return {}; }
AbstractLayouter* doLayout(const GlyphCache&, Float, const std::string&) override {
std::unique_ptr<AbstractLayouter> doLayout(const GlyphCache&, Float, const std::string&) override {
return nullptr;
}

4
src/Text/Test/TextRendererGLTest.cpp
Unescape View File

@ -73,8 +73,8 @@ class TestFont: public Text::AbstractFont {
UnsignedInt doGlyphId(char32_t) override { return 0; }
Vector2 doGlyphAdvance(UnsignedInt) override { return {}; }
AbstractLayouter* doLayout(const GlyphCache&, Float size, const std::string& text) override {
return new TestLayouter(size, text.size());
std::unique_ptr<AbstractLayouter> doLayout(const GlyphCache&, Float size, const std::string& text) override {
return std::unique_ptr<AbstractLayouter>(new TestLayouter(size, text.size()));
}
};

10
src/Text/TextRenderer.cpp
Unescape View File

@ -61,7 +61,7 @@ struct Vertex {
}
std::tuple<std::vector<Vector2>, std::vector<Vector2>, std::vector<UnsignedInt>, Rectangle> AbstractTextRenderer::render(AbstractFont& font, const GlyphCache& cache, Float size, const std::string& text) {
AbstractLayouter* const layouter = font.layout(cache, size, text);
const auto layouter = font.layout(cache, size, text);
const UnsignedInt vertexCount = layouter->glyphCount()*4;
/* Output data */
@ -115,12 +115,11 @@ std::tuple<std::vector<Vector2>, std::vector<Vector2>, std::vector<UnsignedInt>,
std::vector<UnsignedInt> indices(layouter->glyphCount()*6);
createIndices<UnsignedInt>(indices.data(), layouter->glyphCount());
delete layouter;
return std::make_tuple(std::move(positions), std::move(texcoords), std::move(indices), rectangle);
}
std::tuple<Mesh, Rectangle> AbstractTextRenderer::render(AbstractFont& font, const GlyphCache& cache, Float size, const std::string& text, Buffer& vertexBuffer, Buffer& indexBuffer, Buffer::Usage usage) {
AbstractLayouter* const layouter = font.layout(cache, size, text);
const auto layouter = font.layout(cache, size, text);
const UnsignedInt vertexCount = layouter->glyphCount()*4;
const UnsignedInt indexCount = layouter->glyphCount()*6;
@ -190,7 +189,6 @@ std::tuple<Mesh, Rectangle> AbstractTextRenderer::render(AbstractFont& font, con
.setIndexCount(indexCount)
.setIndexBuffer(indexBuffer, 0, indexType, 0, vertexCount);
delete layouter;
return std::make_tuple(std::move(mesh), rectangle);
}
@ -328,7 +326,7 @@ void AbstractTextRenderer::reserve(const uint32_t glyphCount, const Buffer::Usag
}
void AbstractTextRenderer::render(const std::string& text) {
AbstractLayouter* layouter = font.layout(cache, size, text);
const auto layouter = font.layout(cache, size, text);
CORRADE_ASSERT(layouter->glyphCount() <= _capacity,
"Text::TextRenderer::render(): capacity" << _capacity << "too small to render" << layouter->glyphCount() << "glyphs", );
@ -370,8 +368,6 @@ void AbstractTextRenderer::render(const std::string& text) {
/* Update index count */
_mesh.setIndexCount(layouter->glyphCount()*6);
delete layouter;
}
#ifndef DOXYGEN_GENERATING_OUTPUT

Write Preview
Loading…
Cancel
Save