From 63558e06ad4fbc33ad3bdec57dc0ead2b369a7b0 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 17 May 2017 08:58:36 -0700 Subject: [PATCH] [core] Remove obsolete modular arithmetic on atlas coordinates We switched to 16-bit coordinates in attributes in b0cb8715ed74678b4d0f05829fa71a590e41b2f6, so there's no longer any reason to require that they are divisible by four. --- mapbox-gl-js | 2 +- src/mbgl/programs/symbol_program.cpp | 2 +- src/mbgl/programs/symbol_program.hpp | 4 ++-- src/mbgl/sprite/sprite_atlas.cpp | 17 +++++------------ src/mbgl/text/glyph_atlas.cpp | 10 ++-------- test/sprite/sprite_atlas.test.cpp | 8 ++++---- 6 files changed, 15 insertions(+), 28 deletions(-) diff --git a/mapbox-gl-js b/mapbox-gl-js index 0e0f2e543..9dc82f694 160000 --- a/mapbox-gl-js +++ b/mapbox-gl-js @@ -1 +1 @@ -Subproject commit 0e0f2e543276182b06796d68157d4fcf062a4906 +Subproject commit 9dc82f694dc985c62c509f09f712b1a0ffdd0e05 diff --git a/src/mbgl/programs/symbol_program.cpp b/src/mbgl/programs/symbol_program.cpp index 86f61c4ad..1fb4ae5c5 100644 --- a/src/mbgl/programs/symbol_program.cpp +++ b/src/mbgl/programs/symbol_program.cpp @@ -51,7 +51,7 @@ Values makeValues(const bool isText, values.translateAnchor, state) }, uniforms::u_extrude_scale::Value{ extrudeScale }, - uniforms::u_texsize::Value{ std::array {{ float(texsize.width) / 4, float(texsize.height) / 4 }} }, + uniforms::u_texsize::Value{ std::array {{ float(texsize.width), float(texsize.height) }} }, uniforms::u_zoom::Value{ float(state.getZoom()) }, uniforms::u_rotate_with_map::Value{ values.rotationAlignment == AlignmentType::Map }, uniforms::u_texture::Value{ 0 }, diff --git a/src/mbgl/programs/symbol_program.hpp b/src/mbgl/programs/symbol_program.hpp index 9eaa14095..809eacdd6 100644 --- a/src/mbgl/programs/symbol_program.hpp +++ b/src/mbgl/programs/symbol_program.hpp @@ -67,8 +67,8 @@ struct SymbolLayoutAttributes : gl::Attributes< static_cast(::round(o.y * 64)) }}, {{ - static_cast(tx / 4), - static_cast(ty / 4), + tx, + ty, mbgl::attributes::packUint8Pair( static_cast(labelminzoom * 10), // 1/10 zoom levels: z16 == 160 static_cast(labelangle) diff --git a/src/mbgl/sprite/sprite_atlas.cpp b/src/mbgl/sprite/sprite_atlas.cpp index 0f4cde56a..2175077c1 100644 --- a/src/mbgl/sprite/sprite_atlas.cpp +++ b/src/mbgl/sprite/sprite_atlas.cpp @@ -14,6 +14,8 @@ namespace mbgl { +static constexpr uint32_t padding = 1; + SpriteAtlasElement::SpriteAtlasElement(Rect rect_, const style::Image::Impl& image, Size size_, float pixelRatio) @@ -23,7 +25,6 @@ SpriteAtlasElement::SpriteAtlasElement(Rect rect_, width(image.getWidth()), height(image.getHeight()) { - const float padding = 1; const float w = image.getWidth() * relativePixelRatio; const float h = image.getHeight() * relativePixelRatio; @@ -148,17 +149,10 @@ optional SpriteAtlas::getImage(const std::string& id, }; } - const uint16_t pixelWidth = std::ceil(entry.image->image.size.width / pixelRatio); - const uint16_t pixelHeight = std::ceil(entry.image->image.size.height / pixelRatio); + const uint16_t width = std::ceil(entry.image->image.size.width / pixelRatio) + 2 * padding; + const uint16_t height = std::ceil(entry.image->image.size.height / pixelRatio) + 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. - const uint16_t packWidth = (pixelWidth + 1) + (4 - (pixelWidth + 1) % 4); - const uint16_t packHeight = (pixelHeight + 1) + (4 - (pixelHeight + 1) % 4); - - // We have to allocate a new area in the bin, and store an empty image in it. - Rect rect = bin.allocate(packWidth, packHeight); + Rect rect = bin.allocate(width, height); if (rect.w == 0) { if (debug::spriteWarnings) { Log::Warning(Event::Sprite, "sprite atlas bitmap overflow"); @@ -187,7 +181,6 @@ void SpriteAtlas::copy(const Entry& entry, optional> Entry::*entr const PremultipliedImage& src = entry.image->image; const Rect& rect = *(entry.*entryRect); - const uint32_t padding = 1; const uint32_t x = (rect.x + padding) * pixelRatio; const uint32_t y = (rect.y + padding) * pixelRatio; const uint32_t w = src.size.width; diff --git a/src/mbgl/text/glyph_atlas.cpp b/src/mbgl/text/glyph_atlas.cpp index b43971f70..c6083fe0c 100644 --- a/src/mbgl/text/glyph_atlas.cpp +++ b/src/mbgl/text/glyph_atlas.cpp @@ -184,14 +184,8 @@ Rect GlyphAtlas::addGlyph(GlyphValue& value) { // Add a 1px border around every image. const uint32_t padding = 1; - uint16_t width = value.bitmap.size.width + 2 * padding; - uint16_t height = value.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. - width += (4 - width % 4); - height += (4 - height % 4); + const uint16_t width = value.bitmap.size.width + 2 * padding; + const uint16_t height = value.bitmap.size.height + 2 * padding; Rect rect = bin.allocate(width, height); if (rect.w == 0) { diff --git a/test/sprite/sprite_atlas.test.cpp b/test/sprite/sprite_atlas.test.cpp index 37a50dddd..269e5b4b6 100644 --- a/test/sprite/sprite_atlas.test.cpp +++ b/test/sprite/sprite_atlas.test.cpp @@ -91,8 +91,8 @@ TEST(SpriteAtlas, Size) { float imagePixelRatio = metro.relativePixelRatio * atlas.getPixelRatio(); EXPECT_EQ(0, metro.pos.x); EXPECT_EQ(0, metro.pos.y); - EXPECT_EQ(16, metro.pos.w); - EXPECT_EQ(16, metro.pos.h); + EXPECT_EQ(15, metro.pos.w); + EXPECT_EQ(15, metro.pos.h); EXPECT_EQ(18, metro.width); EXPECT_EQ(18, metro.height); EXPECT_EQ(18u, metro.width * imagePixelRatio); @@ -118,8 +118,8 @@ TEST(SpriteAtlas, Updates) { float imagePixelRatio = one.relativePixelRatio * atlas.getPixelRatio(); EXPECT_EQ(0, one.pos.x); EXPECT_EQ(0, one.pos.y); - EXPECT_EQ(20, one.pos.w); - EXPECT_EQ(16, one.pos.h); + EXPECT_EQ(18, one.pos.w); + EXPECT_EQ(14, one.pos.h); EXPECT_EQ(16, one.width); EXPECT_EQ(12, one.height); EXPECT_EQ(16u, one.width * imagePixelRatio);