From 04c73e5ae13a1cc223e3be817d1c4220457aff25 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Fri, 8 May 2015 14:52:33 -0700 Subject: [PATCH 1/2] fix integer overflow in line_atlas --- src/mbgl/geometry/line_atlas.cpp | 2 +- src/mbgl/geometry/line_atlas.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mbgl/geometry/line_atlas.cpp b/src/mbgl/geometry/line_atlas.cpp index 42d238077..a0bdbc0e5 100644 --- a/src/mbgl/geometry/line_atlas.cpp +++ b/src/mbgl/geometry/line_atlas.cpp @@ -14,7 +14,7 @@ using namespace mbgl; LineAtlas::LineAtlas(uint16_t w, uint16_t h) : width(w), height(h), - data(new char[w * h]), + data(new uint8_t[w * h]), dirty(true) { } diff --git a/src/mbgl/geometry/line_atlas.hpp b/src/mbgl/geometry/line_atlas.hpp index 3683272f9..1ce6f901f 100644 --- a/src/mbgl/geometry/line_atlas.hpp +++ b/src/mbgl/geometry/line_atlas.hpp @@ -34,7 +34,7 @@ public: private: std::recursive_mutex mtx; - char *const data = nullptr; + uint8_t *const data = nullptr; std::atomic dirty; uint32_t texture = 0; int nextRow = 0; From b7b532ce28e2f7571697fb53aa33f33b7a16bd5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Mon, 11 May 2015 11:28:11 +0200 Subject: [PATCH 2/2] manage atlas textures with unique_ptrs --- src/mbgl/geometry/glyph_atlas.cpp | 11 ++++++----- src/mbgl/geometry/glyph_atlas.hpp | 2 +- src/mbgl/geometry/line_atlas.cpp | 9 ++++----- src/mbgl/geometry/line_atlas.hpp | 2 +- src/mbgl/geometry/sprite_atlas.cpp | 21 +++++++-------------- src/mbgl/geometry/sprite_atlas.hpp | 2 +- 6 files changed, 20 insertions(+), 27 deletions(-) diff --git a/src/mbgl/geometry/glyph_atlas.cpp b/src/mbgl/geometry/glyph_atlas.cpp index f2978de98..bb230e4ef 100644 --- a/src/mbgl/geometry/glyph_atlas.cpp +++ b/src/mbgl/geometry/glyph_atlas.cpp @@ -4,6 +4,8 @@ #include #include +#include + #include #include @@ -14,7 +16,7 @@ GlyphAtlas::GlyphAtlas(uint16_t width_, uint16_t height_) : width(width_), height(height_), bin(width_, height_), - data(new char[width_ *height_]), + data(util::make_unique(width_ * height_)), dirty(true) { } @@ -88,13 +90,12 @@ Rect GlyphAtlas::addGlyph(uintptr_t tileUID, face.emplace(glyph.id, GlyphValue { rect, tileUID }); // Copy the bitmap - char *target = data.get(); - const char *source = glyph.bitmap.data(); + const uint8_t* source = reinterpret_cast(glyph.bitmap.data()); for (uint32_t y = 0; y < buffered_height; y++) { uint32_t y1 = width * (rect.y + y) + rect.x; uint32_t y2 = buffered_width * y; for (uint32_t x = 0; x < buffered_width; x++) { - target[y1 + x] = source[y2 + x]; + data[y1 + x] = source[y2 + x]; } } @@ -116,7 +117,7 @@ void GlyphAtlas::removeGlyphs(uintptr_t tileUID) { const Rect& rect = value.rect; // Clear out the bitmap. - char *target = data.get(); + uint8_t *target = data.get(); for (uint32_t y = 0; y < rect.h; y++) { uint32_t y1 = width * (rect.y + y) + rect.x; for (uint32_t x = 0; x < rect.w; x++) { diff --git a/src/mbgl/geometry/glyph_atlas.hpp b/src/mbgl/geometry/glyph_atlas.hpp index dfa568f0f..8bc21da09 100644 --- a/src/mbgl/geometry/glyph_atlas.hpp +++ b/src/mbgl/geometry/glyph_atlas.hpp @@ -49,7 +49,7 @@ private: std::mutex mtx; BinPack bin; std::map> index; - std::unique_ptr data; + const std::unique_ptr data; std::atomic dirty; uint32_t texture = 0; }; diff --git a/src/mbgl/geometry/line_atlas.cpp b/src/mbgl/geometry/line_atlas.cpp index a0bdbc0e5..91ac15639 100644 --- a/src/mbgl/geometry/line_atlas.cpp +++ b/src/mbgl/geometry/line_atlas.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include @@ -14,7 +15,7 @@ using namespace mbgl; LineAtlas::LineAtlas(uint16_t w, uint16_t h) : width(w), height(h), - data(new uint8_t[w * h]), + data(util::make_unique(w * h)), dirty(true) { } @@ -23,8 +24,6 @@ LineAtlas::~LineAtlas() { Environment::Get().abandonTexture(texture); texture = 0; - - delete[] data; } LinePatternPos LineAtlas::getDashPosition(const std::vector &dasharray, bool round) { @@ -162,7 +161,7 @@ void LineAtlas::bind() { 0, // GLint border GL_ALPHA, // GLenum format GL_UNSIGNED_BYTE, // GLenum type - data // const GLvoid * data + data.get() // const GLvoid * data )); } else { MBGL_CHECK_ERROR(glTexSubImage2D( @@ -174,7 +173,7 @@ void LineAtlas::bind() { height, // GLsizei height GL_ALPHA, // GLenum format GL_UNSIGNED_BYTE, // GLenum type - data // const GLvoid *pixels + data.get() // const GLvoid *pixels )); } diff --git a/src/mbgl/geometry/line_atlas.hpp b/src/mbgl/geometry/line_atlas.hpp index 1ce6f901f..296c997f7 100644 --- a/src/mbgl/geometry/line_atlas.hpp +++ b/src/mbgl/geometry/line_atlas.hpp @@ -34,7 +34,7 @@ public: private: std::recursive_mutex mtx; - uint8_t *const data = nullptr; + const std::unique_ptr data; std::atomic dirty; uint32_t texture = 0; int nextRow = 0; diff --git a/src/mbgl/geometry/sprite_atlas.cpp b/src/mbgl/geometry/sprite_atlas.cpp index 9d0aeac8b..5d47793cf 100644 --- a/src/mbgl/geometry/sprite_atlas.cpp +++ b/src/mbgl/geometry/sprite_atlas.cpp @@ -33,9 +33,7 @@ bool SpriteAtlas::resize(const float newRatio) { pixelRatio = newRatio; if (data) { - uint32_t *old_data = data; - - data = nullptr; + const auto oldData = std::move(data); allocate(); const int old_w = width * oldRatio; @@ -44,19 +42,15 @@ bool SpriteAtlas::resize(const float newRatio) { const int new_h = height * newRatio; // Basic image scaling. TODO: Replace this with better image scaling. - uint32_t *img_new = reinterpret_cast(data); - const uint32_t *img_old = reinterpret_cast(old_data); - for (int y = 0; y < new_h; y++) { const int old_yoffset = ((y * old_h) / new_h) * old_w; const int new_yoffset = y * new_w; for (int x = 0; x < new_w; x++) { const int old_x = (x * old_w) / new_w; - img_new[new_yoffset + x] = img_old[old_yoffset + old_x]; + data[new_yoffset + x] = oldData[old_yoffset + old_x]; } } - ::operator delete(old_data); dirty = true; fullUploadRequired = true; @@ -144,8 +138,8 @@ void SpriteAtlas::allocate() { if (!data) { dimension w = static_cast(width * pixelRatio); dimension h = static_cast(height * pixelRatio); - data = static_cast(::operator new(w * h * sizeof(uint32_t))); - std::fill(data, data + w * h, 0); + data = util::make_unique(w * h); + std::fill(data.get(), data.get() + w * h, 0); } } @@ -158,7 +152,7 @@ void SpriteAtlas::copy(const Rect& dst, const SpritePosition& src, co const Rect srcPos { src.x, src.y, src.width, src.height }; allocate(); - uint32_t *dstData = reinterpret_cast(data); + uint32_t *const dstData = data.get(); const vec2 dstSize { static_cast(width * pixelRatio), static_cast(height * pixelRatio) }; const Rect dstPos { static_cast(dst.x * pixelRatio), @@ -274,7 +268,7 @@ void SpriteAtlas::bind(bool linear) { 0, // GLint border GL_RGBA, // GLenum format GL_UNSIGNED_BYTE, // GLenum type - data // const GLvoid * data + data.get() // const GLvoid * data )); fullUploadRequired = false; } else { @@ -287,7 +281,7 @@ void SpriteAtlas::bind(bool linear) { height * pixelRatio, // GLsizei height GL_RGBA, // GLenum format GL_UNSIGNED_BYTE, // GLenum type - data // const GLvoid *pixels + data.get() // const GLvoid *pixels )); } @@ -303,5 +297,4 @@ SpriteAtlas::~SpriteAtlas() { std::lock_guard lock(mtx); Environment::Get().abandonTexture(texture); texture = 0; - ::operator delete(data), data = nullptr; } diff --git a/src/mbgl/geometry/sprite_atlas.hpp b/src/mbgl/geometry/sprite_atlas.hpp index 6c4a381aa..d34013cd6 100644 --- a/src/mbgl/geometry/sprite_atlas.hpp +++ b/src/mbgl/geometry/sprite_atlas.hpp @@ -77,7 +77,7 @@ private: util::ptr sprite; std::map> images; std::set uninitialized; - uint32_t *data = nullptr; + std::unique_ptr data; std::atomic dirty; bool fullUploadRequired = true; uint32_t texture = 0;