[core] Make Image safer

Provide Image::copy, which handles copying rectangles from a source to a destination, with thorough bounds checking.

Also fixes an indexing error in SpriteAtlas, where the top row of pixels in a wrapped image was copied from the wrong source row.
This commit is contained in:
John Firebaugh
2017-02-10 17:18:18 -08:00
parent c6d5eaf479
commit 94f011895c
15 changed files with 163 additions and 116 deletions

View File

@@ -8,7 +8,7 @@ namespace mbgl {
FrameHistory::FrameHistory() {
changeOpacities.fill(0);
std::fill(opacities.data.get(), opacities.data.get() + opacities.bytes(), 0);
opacities.fill(0);
}
void FrameHistory::record(const TimePoint& now, float zoom, const Duration& duration) {

View File

@@ -26,7 +26,7 @@ public:
private:
std::array<TimePoint, 256> changeTimes;
std::array<uint8_t, 256> changeOpacities;
const AlphaImage opacities{ { 256, 1 } };
AlphaImage opacities{ { 256, 1 } };
int16_t previousZoomIndex = 0;
TimePoint previousTime;

View File

@@ -231,52 +231,32 @@ optional<SpriteAtlasPosition> SpriteAtlas::getPosition(const std::string& name,
};
}
void copyBitmap(const uint32_t *src, const uint32_t srcStride, const uint32_t srcX, const uint32_t srcY,
uint32_t *const dst, const uint32_t dstStride, const uint32_t dstX, const uint32_t dstY, int dstSize,
const int width, const int height, const SpritePatternMode mode) {
int srcI = srcY * srcStride + srcX;
int dstI = dstY * dstStride + dstX;
int x, y;
if (mode == SpritePatternMode::Repeating) {
// add 1 pixel wrapped padding on each side of the image
dstI -= dstStride;
for (y = -1; y <= height; y++, srcI = ((y + height) % height + srcY) * srcStride + srcX, dstI += dstStride) {
for (x = -1; x <= width; x++) {
const int dstIndex = (dstI + x + dstSize) % dstSize;
dst[dstIndex] = src[srcI + ((x + width) % width)];
}
}
} else {
for (y = 0; y < height; y++, srcI += srcStride, dstI += dstStride) {
for (x = 0; x < width; x++) {
const int dstIndex = (dstI + x + dstSize) % dstSize;
dst[dstIndex] = src[srcI + x];
}
}
}
}
void SpriteAtlas::copy(const Holder& holder, const SpritePatternMode mode) {
if (!image.valid()) {
image = PremultipliedImage({ static_cast<uint32_t>(std::ceil(size.width * pixelRatio)),
static_cast<uint32_t>(std::ceil(size.height * pixelRatio)) });
std::fill(image.data.get(), image.data.get() + image.bytes(), 0);
image.fill(0);
}
const uint32_t* srcData =
reinterpret_cast<const uint32_t*>(holder.spriteImage->image.data.get());
if (!srcData) return;
uint32_t* const dstData = reinterpret_cast<uint32_t*>(image.data.get());
if (!holder.spriteImage->image.valid()) {
return;
}
const int padding = 1;
const uint32_t padding = 1;
const uint32_t x = (holder.pos.x + padding) * pixelRatio;
const uint32_t y = (holder.pos.y + padding) * pixelRatio;
const uint32_t w = holder.spriteImage->image.size.width;
const uint32_t h = holder.spriteImage->image.size.height;
copyBitmap(srcData, holder.spriteImage->image.size.width, 0, 0, dstData, image.size.width,
(holder.pos.x + padding) * pixelRatio, (holder.pos.y + padding) * pixelRatio,
image.size.width * image.size.height, holder.spriteImage->image.size.width,
holder.spriteImage->image.size.height, mode);
PremultipliedImage::copy(holder.spriteImage->image, image, { 0, 0 }, { x, y }, { w, h });
if (mode == SpritePatternMode::Repeating) {
// Add 1 pixel wrapped padding on each side of the image.
PremultipliedImage::copy(holder.spriteImage->image, image, { 0, h - 1 }, { x, y - 1 }, { w, 1 }); // T
PremultipliedImage::copy(holder.spriteImage->image, image, { 0, 0 }, { x, y + h }, { w, 1 }); // B
PremultipliedImage::copy(holder.spriteImage->image, image, { w - 1, 0 }, { x - 1, y }, { 1, h }); // L
PremultipliedImage::copy(holder.spriteImage->image, image, { 0, 0 }, { x + w, y }, { 1, h }); // R
}
dirty = true;
}

View File

@@ -34,17 +34,8 @@ SpriteImagePtr createSpriteImage(const PremultipliedImage& image,
PremultipliedImage dstImage({ width, height });
auto srcData = reinterpret_cast<const uint32_t*>(image.data.get());
auto dstData = reinterpret_cast<uint32_t*>(dstImage.data.get());
// Copy from the source image into our individual sprite image
for (uint32_t y = 0; y < height; ++y) {
const auto dstRow = y * width;
const auto srcRow = (y + srcY) * image.size.width + srcX;
for (uint32_t x = 0; x < width; ++x) {
dstData[dstRow + x] = srcData[srcRow + x];
}
}
PremultipliedImage::copy(image, dstImage, { srcX, srcY }, { 0, 0 }, { width, height });
return std::make_unique<const SpriteImage>(std::move(dstImage), ratio, sdf);
}

View File

@@ -3,6 +3,7 @@
#include <mbgl/text/glyph_range.hpp>
#include <mbgl/util/rect.hpp>
#include <mbgl/util/traits.hpp>
#include <mbgl/util/image.hpp>
#include <cstdint>
#include <vector>
@@ -89,7 +90,7 @@ public:
uint32_t id = 0;
// A signed distance field of the glyph with a border (see above).
std::string bitmap;
AlphaImage bitmap;
// Glyph metrics
GlyphMetrics metrics;

View File

@@ -124,48 +124,26 @@ Rect<uint16_t> GlyphAtlas::addGlyph(uintptr_t tileUID,
return Rect<uint16_t>{ 0, 0, 0, 0 };
}
uint16_t buffered_width = glyph.metrics.width + SDFGlyph::borderSize * 2;
uint16_t buffered_height = glyph.metrics.height + SDFGlyph::borderSize * 2;
// Guard against mismatches between the glyph bitmap size and the size mandated by
// its metrics.
if (size_t(buffered_width * buffered_height) != glyph.bitmap.size()) {
return Rect<uint16_t>{ 0, 0, 0, 0 };
}
// Add a 1px border around every image.
const uint16_t padding = 1;
uint16_t pack_width = buffered_width + 2 * padding;
uint16_t pack_height = buffered_height + 2 * padding;
const uint32_t padding = 1;
uint16_t width = glyph.bitmap.size.width + 2 * padding;
uint16_t height = glyph.bitmap.size.height + 2 * padding;
// Increase to next number divisible by 4, but at least 1.
// This is so we can scale down the texture coordinates and pack them
// into 2 bytes rather than 4 bytes.
pack_width += (4 - pack_width % 4);
pack_height += (4 - pack_height % 4);
width += (4 - width % 4);
height += (4 - height % 4);
Rect<uint16_t> rect = bin.allocate(pack_width, pack_height);
Rect<uint16_t> rect = bin.allocate(width, height);
if (rect.w == 0) {
Log::Error(Event::OpenGL, "glyph bitmap overflow");
return rect;
}
// Verify that binpack didn't produce a rect that goes beyond the size of the image.
// This should never happen.
assert(rect.x + rect.w <= image.size.width);
assert(rect.y + rect.h <= image.size.height);
face.emplace(glyph.id, GlyphValue { rect, tileUID });
// Copy the bitmap
const uint8_t* source = reinterpret_cast<const uint8_t*>(glyph.bitmap.data());
for (uint32_t y = 0; y < buffered_height; y++) {
uint32_t y1 = image.size.width * (rect.y + y + padding) + rect.x + padding;
uint32_t y2 = buffered_width * y;
for (uint32_t x = 0; x < buffered_width; x++) {
image.data[y1 + x] = source[y2 + x];
}
}
AlphaImage::copy(glyph.bitmap, image, { 0, 0 }, { rect.x + padding, rect.y + padding }, glyph.bitmap.size);
dirty = true;

View File

@@ -101,7 +101,7 @@ private:
std::mutex mtx;
BinPack<uint16_t> bin;
std::unordered_map<FontStack, std::map<uint32_t, GlyphValue>, FontStackHash> index;
const AlphaImage image;
AlphaImage image;
std::atomic<bool> dirty;
mbgl::optional<gl::Texture> texture;
};

View File

@@ -25,6 +25,7 @@ void parseGlyphPBF(GlyphSet& glyphSet, const GlyphRange& glyphRange, const std::
auto glyph_pbf = fontstack_pbf.get_message();
SDFGlyph glyph;
protozero::data_view glyphData;
bool hasID = false, hasWidth = false, hasHeight = false, hasLeft = false,
hasTop = false, hasAdvance = false;
@@ -36,7 +37,7 @@ void parseGlyphPBF(GlyphSet& glyphSet, const GlyphRange& glyphRange, const std::
hasID = true;
break;
case 2: // bitmap
glyph.bitmap = glyph_pbf.get_string();
glyphData = glyph_pbf.get_view();
break;
case 3: // width
glyph.metrics.width = glyph_pbf.get_uint32();
@@ -64,26 +65,35 @@ void parseGlyphPBF(GlyphSet& glyphSet, const GlyphRange& glyphRange, const std::
}
}
// Only treat this glyph as a correct glyph if it has all required fields. It also
// needs to satisfy a few metrics conditions that ensure that the glyph isn't bogus.
// All other glyphs are malformed. We're also discarding all glyphs that are outside
// the expected glyph range.
if (!hasID || !hasWidth || !hasHeight || !hasLeft || !hasTop || !hasAdvance ||
glyph.metrics.width >= 256 || glyph.metrics.height >= 256 ||
glyph.metrics.left < -128 || glyph.metrics.left >= 128 ||
glyph.metrics.top < -128 || glyph.metrics.top >= 128 ||
glyph.metrics.advance >= 256 ||
glyph.id < glyphRange.first || glyph.id > glyphRange.second) {
continue;
}
// If the area of width/height is non-zero, we need to adjust the expected size
// with the implicit border size, otherwise we expect there to be no bitmap at all.
const uint32_t expectedBitmapSize =
glyph.metrics.width && glyph.metrics.height
? (glyph.metrics.width + 2 * SDFGlyph::borderSize) *
(glyph.metrics.height + 2 * SDFGlyph::borderSize)
: 0;
if (glyph.metrics.width && glyph.metrics.height) {
const Size size {
glyph.metrics.width + 2 * SDFGlyph::borderSize,
glyph.metrics.height + 2 * SDFGlyph::borderSize
};
// Only treat this glyph as a correct glyph if it has all required fields, and if
// the bitmap has the correct length. It also needs to satisfy a few metrics conditions
// that ensure that the glyph isn't bogus. All other glyphs are malformed.
// We're also discarding all glyphs that are outside the expected glyph range.
if (hasID && hasWidth && hasHeight && hasLeft && hasTop && hasAdvance &&
glyph.metrics.width < 256 && glyph.metrics.height < 256 &&
glyph.metrics.left >= -128 && glyph.metrics.left < 128 &&
glyph.metrics.top >= -128 && glyph.metrics.top < 128 &&
glyph.metrics.advance < 256 && glyph.bitmap.size() == expectedBitmapSize &&
glyph.id >= glyphRange.first && glyph.id <= glyphRange.second) {
glyphSet.insert(glyph.id, std::move(glyph));
if (size.area() != glyphData.size()) {
continue;
}
glyph.bitmap = AlphaImage(size, reinterpret_cast<const uint8_t*>(glyphData.data()), glyphData.size());
}
glyphSet.insert(glyph.id, std::move(glyph));
}
}
}