From e577354567c467ce5188020ba3e3a4b132a341d6 Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Tue, 12 Jan 2016 13:30:39 -0800 Subject: [PATCH] [core] remove bilinear, nearest scaling, fix #3164 --- package.json | 2 +- src/mbgl/sprite/sprite_atlas.cpp | 78 +++++++++------------ src/mbgl/util/rect.hpp | 1 - src/mbgl/util/scaling.cpp | 112 ------------------------------- src/mbgl/util/scaling.hpp | 23 ------- test/miscellaneous/bilinear.cpp | 54 --------------- test/sprite/sprite_atlas.cpp | 24 +++---- test/test.gypi | 1 - 8 files changed, 41 insertions(+), 254 deletions(-) delete mode 100644 src/mbgl/util/scaling.cpp delete mode 100644 src/mbgl/util/scaling.hpp delete mode 100644 test/miscellaneous/bilinear.cpp diff --git a/package.json b/package.json index 273e1a4dd08..0e1ca758b68 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ ], "devDependencies": { "aws-sdk": "^2.2.21", - "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#c401ec0acb064f819cddd182c63ee9e019bfb18f", + "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#c8982f16ef5d39164b2e4f33bc3f993f9ac17502", "node-gyp": "^3.2.1", "request": "^2.67.0", "tape": "^4.2.2" diff --git a/src/mbgl/sprite/sprite_atlas.cpp b/src/mbgl/sprite/sprite_atlas.cpp index a20f6f0fe36..497721a61c8 100644 --- a/src/mbgl/sprite/sprite_atlas.cpp +++ b/src/mbgl/sprite/sprite_atlas.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #include @@ -45,9 +44,6 @@ Rect SpriteAtlas::allocateImage(float src_width, float s return rect; } - rect.originalW = pixel_width; - rect.originalH = pixel_height; - return rect; } @@ -101,6 +97,34 @@ mapbox::util::optional SpriteAtlas::getPosition(const std:: }; } +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 bool wrap) { + + int srcI = srcY * srcStride + srcX; + int dstI = dstY * dstStride + dstX; + int x, y; + + if (wrap) { + // 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 bool wrap) { if (!data) { data = std::make_unique(pixelWidth * pixelHeight); @@ -109,51 +133,13 @@ void SpriteAtlas::copy(const Holder& holder, const bool wrap) { const uint32_t *srcData = reinterpret_cast(holder.texture->data.data()); if (!srcData) return; - const vec2 srcSize { holder.texture->pixelWidth, holder.texture->pixelHeight }; - const Rect srcPos { 0, 0, srcSize.x, srcSize.y }; - const auto& dst = holder.pos; - - const int offset = 1; - uint32_t *const dstData = data.get(); - const vec2 dstSize{ pixelWidth, pixelHeight }; - const Rect dstPos{ static_cast((offset + dst.x) * pixelRatio), - static_cast((offset + dst.y) * pixelRatio), - static_cast(dst.originalW * pixelRatio), - static_cast(dst.originalH * pixelRatio) }; - util::bilinearScale(srcData, srcSize, srcPos, dstData, dstSize, dstPos, wrap); + const int padding = 1; - // Add borders around the copied image if required. - if (wrap) { - // We're copying from the same image so we don't have to scale again. - const uint32_t border = 1; - const uint32_t borderX = dstPos.x != 0 ? border : 0; - const uint32_t borderY = dstPos.y != 0 ? border : 0; - - // Left border - util::nearestNeighborScale( - dstData, dstSize, { dstPos.x + dstPos.w - borderX, dstPos.y, borderX, dstPos.h }, - dstData, dstSize, { dstPos.x - borderX, dstPos.y, borderX, dstPos.h }); - - // Right border - util::nearestNeighborScale(dstData, dstSize, { dstPos.x, dstPos.y, border, dstPos.h }, - dstData, dstSize, - { dstPos.x + dstPos.w, dstPos.y, border, dstPos.h }); - - // Top border - util::nearestNeighborScale( - dstData, dstSize, { dstPos.x - borderX, dstPos.y + dstPos.h - borderY, - dstPos.w + border + borderX, borderY }, - dstData, dstSize, - { dstPos.x - borderX, dstPos.y - borderY, dstPos.w + 2 * borderX, borderY }); - - // Bottom border - util::nearestNeighborScale( - dstData, dstSize, { dstPos.x - borderX, dstPos.y, dstPos.w + 2 * borderX, border }, - dstData, dstSize, - { dstPos.x - borderX, dstPos.y + dstPos.h, dstPos.w + border + borderX, border }); - } + copyBitmap(srcData, holder.texture->pixelWidth, 0, 0, + dstData, pixelWidth, (holder.pos.x + padding) * pixelRatio, (holder.pos.y + padding) * pixelRatio, pixelWidth * pixelHeight, + holder.texture->pixelWidth, holder.texture->pixelHeight, wrap); dirty = true; } diff --git a/src/mbgl/util/rect.hpp b/src/mbgl/util/rect.hpp index 7bb3214b9bc..1dc8cb8af96 100644 --- a/src/mbgl/util/rect.hpp +++ b/src/mbgl/util/rect.hpp @@ -9,7 +9,6 @@ struct Rect { inline Rect(T x_, T y_, T w_, T h_) : x(x_), y(y_), w(w_), h(h_) {} T x = 0, y = 0; T w = 0, h = 0; - T originalW = 0, originalH = 0; template inline Rect operator *(Number value) const { diff --git a/src/mbgl/util/scaling.cpp b/src/mbgl/util/scaling.cpp deleted file mode 100644 index 9b32650511a..00000000000 --- a/src/mbgl/util/scaling.cpp +++ /dev/null @@ -1,112 +0,0 @@ -#include "scaling.hpp" - -namespace { - -using namespace mbgl; - -inline uint8_t bilinearInterpolate(uint8_t tl, uint8_t tr, uint8_t bl, uint8_t br, double dx, double dy) { - const double t = dx * (tr - tl) + tl; - const double b = dx * (br - bl) + bl; - return t + dy * (b - t); -} - -template -inline const uint8_t& b(const uint32_t& w) { - return reinterpret_cast(&w)[i]; -} - -template -inline uint8_t& b(uint32_t& w) { - return reinterpret_cast(&w)[i]; -} - -vec2 getFactor(const Rect& srcPos, const Rect& dstPos) { - return { - double(srcPos.w) / dstPos.w, - double(srcPos.h) / dstPos.h - }; -} - -vec2 getBounds(const vec2& srcSize, const Rect& srcPos, - const vec2& dstSize, const Rect& dstPos, - const vec2& factor) { - if (srcPos.x > srcSize.x || srcPos.y > srcSize.y || - dstPos.x > dstSize.x || dstPos.y > dstSize.y) { - // Source or destination position is out of range. - return { 0, 0 }; - } - - // Make sure we don't read/write values out of range. - return { std::min(uint32_t(double(srcSize.x - srcPos.x) / factor.x), - std::min(dstSize.x - dstPos.x, dstPos.w)), - std::min(uint32_t(double(srcSize.y - srcPos.y) / factor.y), - std::min(dstSize.y - dstPos.y, dstPos.h)) }; -} -} // namespace - -namespace mbgl { -namespace util { - -void bilinearScale(const uint32_t* srcData, const vec2& srcSize, - const Rect& srcPos, uint32_t* dstData, const vec2& dstSize, - const Rect& dstPos, bool wrap) { - const auto factor = getFactor(srcPos, dstPos); - const auto bounds = getBounds(srcSize, srcPos, dstSize, dstPos, factor); - - uint32_t x, y; - size_t i = dstSize.x * dstPos.y + dstPos.x; - for (y = 0; y < bounds.y; y++) { - const double fractY = y * factor.y; - const uint32_t Y0 = fractY; - const uint32_t Y1 = wrap ? (Y0 + 1) % srcPos.h : (Y0 + 1); - const uint32_t srcY0 = srcPos.y + Y0; - const uint32_t srcY1 = std::min(srcPos.y + Y1, srcSize.y - 1); - for (x = 0; x < bounds.x; x++) { - const double fractX = x * factor.x; - const uint32_t X0 = fractX; - const uint32_t X1 = wrap ? (X0 + 1) % srcPos.w : (X0 + 1); - const uint32_t srcX0 = srcPos.x + X0; - const uint32_t srcX1 = std::min(srcPos.x + X1, srcSize.x - 1); - - const uint32_t tl = srcData[srcSize.x * srcY0 + srcX0]; - const uint32_t tr = srcData[srcSize.x * srcY0 + srcX1]; - const uint32_t bl = srcData[srcSize.x * srcY1 + srcX0]; - const uint32_t br = srcData[srcSize.x * srcY1 + srcX1]; - - const double dx = fractX - X0; - const double dy = fractY - Y0; - uint32_t& dst = dstData[i + x]; - b<0>(dst) = bilinearInterpolate(b<0>(tl), b<0>(tr), b<0>(bl), b<0>(br), dx, dy); - b<1>(dst) = bilinearInterpolate(b<1>(tl), b<1>(tr), b<1>(bl), b<1>(br), dx, dy); - b<2>(dst) = bilinearInterpolate(b<2>(tl), b<2>(tr), b<2>(bl), b<2>(br), dx, dy); - b<3>(dst) = bilinearInterpolate(b<3>(tl), b<3>(tr), b<3>(bl), b<3>(br), dx, dy); - } - i += dstSize.x; - } -} - -void nearestNeighborScale(const uint32_t* srcData, const vec2& srcSize, - const Rect& srcPos, uint32_t* dstData, - const vec2& dstSize, const Rect& dstPos) { - const auto factor = getFactor(srcPos, dstPos); - const auto bounds = getBounds(srcSize, srcPos, dstSize, dstPos, factor); - - double fractSrcY = srcPos.y; - double fractSrcX; - size_t i = dstSize.x * dstPos.y + dstPos.x; - uint32_t srcY; - uint32_t x, y; - for (y = 0; y < bounds.y; y++) { - fractSrcX = srcPos.x; - srcY = srcSize.x * uint32_t(fractSrcY); - for (x = 0; x < bounds.x; x++) { - dstData[i + x] = srcData[srcY + uint32_t(fractSrcX)]; - fractSrcX += factor.x; - } - i += dstSize.x; - fractSrcY += factor.y; - } -} - -} // namespace util -} // namespace mbgl diff --git a/src/mbgl/util/scaling.hpp b/src/mbgl/util/scaling.hpp deleted file mode 100644 index a2d582de1c0..00000000000 --- a/src/mbgl/util/scaling.hpp +++ /dev/null @@ -1,23 +0,0 @@ -#ifndef MBGL_UTIL_SCALING -#define MBGL_UTIL_SCALING - - -#include -#include - -#include - -namespace mbgl { -namespace util { - -void bilinearScale(const uint32_t* srcData, const vec2& srcSize, - const Rect& srcPos, uint32_t* dstData, const vec2& dstSize, - const Rect& dstPos, bool wrap); - -void nearestNeighborScale(const uint32_t* srcData, const vec2& srcSize, - const Rect& srcPos, uint32_t* dstData, - const vec2& dstSize, const Rect& dstPos); -} // namespace util -} // namespace mbgl - -#endif diff --git a/test/miscellaneous/bilinear.cpp b/test/miscellaneous/bilinear.cpp deleted file mode 100644 index 3184e5739d4..00000000000 --- a/test/miscellaneous/bilinear.cpp +++ /dev/null @@ -1,54 +0,0 @@ -#include "../fixtures/util.hpp" -#include -#include -#include -#include -#include - -#include -#include - -using namespace mbgl; - -TEST(Bilinear, Scaling) { - // We're reading from a custom "image format" that has a uint32_t width/height prefix and then - // raw RGBA data. We can't use the built-in PNG reading routines because they are reading - // premultiplied images by default. - const std::string sprite = util::decompress(util::read_file("test/fixtures/sprites/bright.bin")); - const uint8_t *src = reinterpret_cast(sprite.data()); - ASSERT_GT(sprite.length(), 8u); - const uint32_t width = src[0] << 24 | src[1] << 16 | src[2] << 8 | src[3]; - const uint32_t height = src[4] << 24 | src[5] << 16 | src[6] << 8 | src[7]; - ASSERT_EQ(sprite.length(), 2 * sizeof(uint32_t) + width * height * sizeof(uint32_t)); - - const uint32_t *srcData = reinterpret_cast(src + 8); - const vec2 srcSize { width, height }; - const vec2 dstSize { 128, 128 }; - - UnassociatedImage dst { dstSize.x, dstSize.y }; - uint32_t *dstData = reinterpret_cast(dst.data.get()); - std::fill(dstData, dstData + dstSize.x * dstSize.y, 0xFFFF00FF); - - util::bilinearScale(srcData, srcSize, { 0, 0, 24, 24 }, dstData, dstSize, { 8, 8, 24, 24 }, false); - util::bilinearScale(srcData, srcSize, { 26, 0, 24, 24 }, dstData, dstSize, { 0, 40, 48, 48 }, false); - util::bilinearScale(srcData, srcSize, { 26, 26, 24, 24 }, dstData, dstSize, { 52, 40, 36, 36 }, false); - util::bilinearScale(srcData, srcSize, { 26, 26, 24, 24 }, dstData, dstSize, { 52, 40, 36, 36 }, false); - util::bilinearScale(srcData, srcSize, { 104, 0, 24, 24 }, dstData, dstSize, { 96, 0, 48, 48 }, false); - util::bilinearScale(srcData, srcSize, { 52, 260, 24, 24 }, dstData, dstSize, { 108, 108, 38, 38 }, false); - util::bilinearScale(srcData, srcSize, { 380, 0, 24, 24 }, dstData, dstSize, { 36, 0, 24, 24 }, false); - util::bilinearScale(srcData, srcSize, { 396, 396, 24, 24 }, dstData, dstSize, { 0, 0, 50, 50 }, false); - util::bilinearScale(srcData, srcSize, { 380, 182, 12, 12 }, dstData, dstSize, { 52, 80, 24, 24 }, false); - - // From the bottom - util::bilinearScale(srcData, srcSize, { 252, 380, 12, 12 }, dstData, dstSize, { 0, 90, 12, 12 }, false); - util::bilinearScale(srcData, srcSize, { 252, 380, 12, 12 }, dstData, dstSize, { 18, 90, 24, 24 }, false); - - const std::string data { reinterpret_cast(dstData), dstSize.x * dstSize.y * sizeof(uint32_t) }; - util::write_file("test/fixtures/sprites/atlas_actual.png", encodePNG(util::premultiply(std::move(dst)))); - util::write_file("test/fixtures/sprites/atlas_actual.bin", util::compress(data)); - - const std::string reference = util::decompress(util::read_file("test/fixtures/sprites/atlas_reference.bin")); - - EXPECT_EQ(reference.size(), data.size()); - EXPECT_TRUE(0 == std::memcmp(data.data(), reference.data(), data.size())); -} diff --git a/test/sprite/sprite_atlas.cpp b/test/sprite/sprite_atlas.cpp index fd78e971fcf..8e3c4317c08 100644 --- a/test/sprite/sprite_atlas.cpp +++ b/test/sprite/sprite_atlas.cpp @@ -34,8 +34,6 @@ TEST(Sprite, SpriteAtlas) { EXPECT_EQ(0, metro.pos.y); EXPECT_EQ(20, metro.pos.w); EXPECT_EQ(20, metro.pos.h); - EXPECT_EQ(18, metro.pos.originalW); - EXPECT_EQ(18, metro.pos.originalH); EXPECT_EQ(18, metro.texture->width); EXPECT_EQ(18, metro.texture->height); EXPECT_EQ(18, metro.texture->pixelWidth); @@ -45,12 +43,12 @@ TEST(Sprite, SpriteAtlas) { EXPECT_TRUE(atlas.getData()); auto pos = *atlas.getPosition("metro", false); - EXPECT_DOUBLE_EQ(20, pos.size[0]); - EXPECT_DOUBLE_EQ(20, pos.size[1]); + EXPECT_DOUBLE_EQ(18, pos.size[0]); + EXPECT_DOUBLE_EQ(18, pos.size[1]); EXPECT_DOUBLE_EQ(1.0f / 63, pos.tl[0]); EXPECT_DOUBLE_EQ(1.0f / 112, pos.tl[1]); - EXPECT_DOUBLE_EQ(21.0f / 63, pos.br[0]); - EXPECT_DOUBLE_EQ(21.0f / 112, pos.br[1]); + EXPECT_DOUBLE_EQ(19.0f / 63, pos.br[0]); + EXPECT_DOUBLE_EQ(19.0f / 112, pos.br[1]); auto missing = atlas.getImage("doesnotexist", false); EXPECT_FALSE(missing); @@ -68,12 +66,10 @@ TEST(Sprite, SpriteAtlas) { EXPECT_EQ(0, metro2.pos.y); EXPECT_EQ(20, metro2.pos.w); EXPECT_EQ(20, metro2.pos.h); - EXPECT_EQ(18, metro2.pos.originalW); - EXPECT_EQ(18, metro2.pos.originalH); const size_t bytes = atlas.getTextureWidth() * atlas.getTextureHeight() * 4; const auto hash = test::crc64(reinterpret_cast(atlas.getData()), bytes); - EXPECT_EQ(0x9875FC0595489A9Fu, hash) << std::hex << hash; + EXPECT_EQ(11868256915183397177u, hash) << std::hex << hash; // util::write_file( // "test/fixtures/annotations/atlas1.png", @@ -98,10 +94,8 @@ TEST(Sprite, SpriteAtlasSize) { auto metro = *atlas.getImage("metro", false); EXPECT_EQ(0, metro.pos.x); EXPECT_EQ(0, metro.pos.y); - EXPECT_EQ(20, metro.pos.w); - EXPECT_EQ(20, metro.pos.h); - EXPECT_EQ(18, metro.pos.originalW); - EXPECT_EQ(18, metro.pos.originalH); + EXPECT_EQ(16, metro.pos.w); + EXPECT_EQ(16, metro.pos.h); EXPECT_EQ(18, metro.texture->width); EXPECT_EQ(18, metro.texture->height); EXPECT_EQ(18, metro.texture->pixelWidth); @@ -110,7 +104,7 @@ TEST(Sprite, SpriteAtlasSize) { const size_t bytes = atlas.getTextureWidth() * atlas.getTextureHeight() * 4; const auto hash = test::crc64(reinterpret_cast(atlas.getData()), bytes); - EXPECT_EQ(0x2CDDA7DBB04D116Du, hash) << std::hex << hash; + EXPECT_EQ(18324190582232646342u, hash) << std::hex << hash; // util::write_file( // "test/fixtures/annotations/atlas2.png", @@ -134,8 +128,6 @@ TEST(Sprite, SpriteAtlasUpdates) { EXPECT_EQ(0, one.pos.y); EXPECT_EQ(20, one.pos.w); EXPECT_EQ(16, one.pos.h); - EXPECT_EQ(16, one.pos.originalW); - EXPECT_EQ(12, one.pos.originalH); EXPECT_EQ(16, one.texture->width); EXPECT_EQ(12, one.texture->height); EXPECT_EQ(16, one.texture->pixelWidth); diff --git a/test/test.gypi b/test/test.gypi index f8818e20840..21b20388767 100644 --- a/test/test.gypi +++ b/test/test.gypi @@ -47,7 +47,6 @@ 'miscellaneous/async_task.cpp', 'miscellaneous/clip_ids.cpp', 'miscellaneous/binpack.cpp', - 'miscellaneous/bilinear.cpp', 'miscellaneous/comparisons.cpp', 'miscellaneous/functions.cpp', 'miscellaneous/geo.cpp',