Simplify and fix sprite atlas coordinate calculations

src/data/bucket/symbol_bucket.js

@@ -373,16 +373,15 @@ class SymbolBucket {
  let shapedIcon;
  if (feature.icon) {
  const image = icons[feature.icon];
- const iconOffset = this.layers[0].getLayoutValue('icon-offset', {zoom: this.zoom}, feature.properties);
- shapedIcon = shapeIcon(image, iconOffset);
-
  if (image) {
+ shapedIcon = shapeIcon(image,
+ this.layers[0].getLayoutValue('icon-offset', {zoom: this.zoom}, feature.properties));
  if (this.sdfIcons === undefined) {
  this.sdfIcons = image.sdf;
  } else if (this.sdfIcons !== image.sdf) {
  util.warnOnce('Style sheet warning: Cannot mix SDF and non-SDF icons in one buffer');
  }
- if (image.pixelRatio !== 1) {
+ if (!image.isNativePixelRatio) {
  this.iconsNeedLinear = true;
  } else if (layout['icon-rotate'] !== 0 || !this.layers[0].isLayoutValueFeatureConstant('icon-rotate')) {
  this.iconsNeedLinear = true;

src/render/draw_line.js

@@ -73,13 +73,13 @@ function drawLineTile(program, painter, tile, buffers, layer, coord, layerData,
  gl.uniform1f(program.u_sdfgamma, painter.lineAtlas.width / (Math.min(widthA, widthB) * 256 * browser.devicePixelRatio) / 2);
 
  } else if (image) {
- imagePosA = painter.spriteAtlas.getPosition(image.from, true);
- imagePosB = painter.spriteAtlas.getPosition(image.to, true);
+ imagePosA = painter.spriteAtlas.getPattern(image.from);
+ imagePosB = painter.spriteAtlas.getPattern(image.to);
  if (!imagePosA || !imagePosB) return;
 
- gl.uniform2f(program.u_pattern_size_a, imagePosA.size[0] * image.fromScale / tileRatio, imagePosB.size[1]);
- gl.uniform2f(program.u_pattern_size_b, imagePosB.size[0] * image.toScale / tileRatio, imagePosB.size[1]);
- gl.uniform2f(program.u_texsize, painter.spriteAtlas.width, painter.spriteAtlas.height);
+ gl.uniform2f(program.u_pattern_size_a, imagePosA.displaySize[0] * image.fromScale / tileRatio, imagePosB.displaySize[1]);
+ gl.uniform2f(program.u_pattern_size_b, imagePosB.displaySize[0] * image.toScale / tileRatio, imagePosB.displaySize[1]);
+ gl.uniform2fv(program.u_texsize, painter.spriteAtlas.getPixelSize());
  }
 
  gl.uniform2f(program.u_gl_units_to_pixels, 1 / painter.transform.pixelsToGLUnits[0], 1 / painter.transform.pixelsToGLUnits[1]);

src/render/draw_symbol.js

@@ -2,7 +2,6 @@
 
 const assert = require('assert');
 const util = require('../util/util');
-const browser = require('../util/browser');
 const drawCollisionDebug = require('./draw_collision_debug');
 const pixelsToTileUnits = require('../source/pixels_to_tile_units');
 const interpolationFactor = require('../style-spec/function').interpolationFactor;
@@ -136,10 +135,10 @@ function setSymbolDrawState(program, painter, layer, tileZoom, isText, isSDF, ro
  const iconSizeScaled = !layer.isLayoutValueFeatureConstant('icon-size') ||
  !layer.isLayoutValueZoomConstant('icon-size') ||
  layer.getLayoutValue('icon-size', { zoom: tr.zoom }) !== 1;
- const iconScaled = iconSizeScaled || browser.devicePixelRatio !== painter.spriteAtlas.pixelRatio || iconsNeedLinear;
+ const iconScaled = iconSizeScaled || iconsNeedLinear;
  const iconTransformed = pitchWithMap || tr.pitch;
  painter.spriteAtlas.bind(gl, isSDF || mapMoving || iconScaled || iconTransformed);
- gl.uniform2f(program.u_texsize, painter.spriteAtlas.width, painter.spriteAtlas.height);
+ gl.uniform2fv(program.u_texsize, painter.spriteAtlas.getPixelSize());
  }
 
  gl.activeTexture(gl.TEXTURE1);

src/render/pattern.js

@@ -11,27 +11,27 @@ const pixelsToTileUnits = require('../source/pixels_to_tile_units');
  */
 exports.isPatternMissing = function(image, painter) {
  if (!image) return false;
- const imagePosA = painter.spriteAtlas.getPosition(image.from, true);
- const imagePosB = painter.spriteAtlas.getPosition(image.to, true);
+ const imagePosA = painter.spriteAtlas.getPattern(image.from);
+ const imagePosB = painter.spriteAtlas.getPattern(image.to);
  return !imagePosA || !imagePosB;
 };
 
 exports.prepare = function (image, painter, program) {
  const gl = painter.gl;
 
- const imagePosA = painter.spriteAtlas.getPosition(image.from, true);
- const imagePosB = painter.spriteAtlas.getPosition(image.to, true);
+ const imagePosA = painter.spriteAtlas.getPattern(image.from);
+ const imagePosB = painter.spriteAtlas.getPattern(image.to);
  assert(imagePosA && imagePosB);
 
  gl.uniform1i(program.u_image, 0);
  gl.uniform2fv(program.u_pattern_tl_a, imagePosA.tl);
  gl.uniform2fv(program.u_pattern_br_a, imagePosA.br);
  gl.uniform2fv(program.u_pattern_tl_b, imagePosB.tl);
  gl.uniform2fv(program.u_pattern_br_b, imagePosB.br);
- gl.uniform2f(program.u_texsize, painter.spriteAtlas.width, painter.spriteAtlas.height);
+ gl.uniform2fv(program.u_texsize, painter.spriteAtlas.getPixelSize());
  gl.uniform1f(program.u_mix, image.t);
- gl.uniform2fv(program.u_pattern_size_a, imagePosA.size);
- gl.uniform2fv(program.u_pattern_size_b, imagePosB.size);
+ gl.uniform2fv(program.u_pattern_size_a, imagePosA.displaySize);
+ gl.uniform2fv(program.u_pattern_size_b, imagePosB.displaySize);
  gl.uniform1f(program.u_scale_a, image.fromScale);
  gl.uniform1f(program.u_scale_b, image.toScale);
 

src/symbol/quads.js

@@ -59,14 +59,18 @@ function SymbolQuad(anchorPoint, tl, tr, bl, br, tex, anchorAngle, glyphAngle, m
  * @private
  */
 function getIconQuads(anchor, shapedIcon, boxScale, line, layer, alongLine, shapedText, globalProperties, featureProperties) {
- const rect = shapedIcon.image.rect;
+ const image = shapedIcon.image;
  const layout = layer.layout;
 
+ // If you have a 10px icon that isn't perfectly aligned to the pixel grid it will cover 11 actual
+ // pixels. The quad needs to be padded to account for this, otherwise they'll look slightly clipped
+ // on one edge in some cases.
  const border = 1;
- const left = shapedIcon.left - border;
- const right = left + rect.w / shapedIcon.image.pixelRatio;
- const top = shapedIcon.top - border;
- const bottom = top + rect.h / shapedIcon.image.pixelRatio;
+
+ const top = shapedIcon.top - border / image.pixelRatio;
+ const left = shapedIcon.left - border / image.pixelRatio;
+ const bottom = shapedIcon.bottom + border / image.pixelRatio;
+ const right = shapedIcon.right + border / image.pixelRatio;
  let tl, tr, br, bl;
 
  // text-fit mode
@@ -122,7 +126,15 @@ function getIconQuads(anchor, shapedIcon, boxScale, line, layer, alongLine, shap
  br = br.matMult(matrix);
  }
 
- return [new SymbolQuad(new Point(anchor.x, anchor.y), tl, tr, bl, br, shapedIcon.image.rect, 0, 0, minScale, Infinity)];
+ // Icon quad is padded, so texture coordinates also need to be padded.
+ const textureRect = {
+ x: image.textureRect.x - border,
+ y: image.textureRect.y - border,
+ w: image.textureRect.w + border * 2,
+ h: image.textureRect.h + border * 2
+ };
+
+ return [new SymbolQuad(new Point(anchor.x, anchor.y), tl, tr, bl, br, textureRect, 0, 0, minScale, Infinity)];
 }
 
 /**

src/symbol/shaping.js

@@ -310,14 +310,12 @@ function align(positionedGlyphs, justify, horizontalAlign, verticalAlign, maxLin
 }
 
 function shapeIcon(image, iconOffset) {
- if (!image || !image.rect) return null;
-
  const dx = iconOffset[0];
  const dy = iconOffset[1];
- const x1 = dx - image.width / 2;
- const x2 = x1 + image.width;
- const y1 = dy - image.height / 2;
- const y2 = y1 + image.height;
+ const x1 = dx - image.displaySize[0] / 2;
+ const x2 = x1 + image.displaySize[0];
+ const y1 = dy - image.displaySize[1] / 2;
+ const y2 = y1 + image.displaySize[1];
 
  return new PositionedIcon(image, y1, y2, x1, x2);
 }

src/symbol/sprite_atlas.js

@@ -7,28 +7,62 @@ const window = require('../util/window');
 const Evented = require('../util/evented');
 const padding = 1;
 
+// This wants to be a class, but is sent to workers, so must be a plain JSON blob.
+function spriteAtlasElement(image) {
+ const textureRect = {
+ x: image.rect.x + padding,
+ y: image.rect.y + padding,
+ w: image.rect.w - padding * 2,
+ h: image.rect.h - padding * 2
+ };
+ return {
+ sdf: image.sdf,
+ pixelRatio: image.pixelRatio,
+ isNativePixelRatio: image.pixelRatio === browser.devicePixelRatio,
+ textureRect: textureRect,
+
+ // Redundant calculated members.
+ tl: [
+ textureRect.x,
+ textureRect.y
+ ],
+ br: [
+ textureRect.x + textureRect.w,
+ textureRect.y + textureRect.h
+ ],
+ displaySize: [
+ textureRect.w / image.pixelRatio,
+ textureRect.h / image.pixelRatio
+ ]
+ };
+}
+
 // The SpriteAtlas class is responsible for turning a sprite and assorted
 // other images added at runtime into a texture that can be consumed by WebGL.
 class SpriteAtlas extends Evented {
 
  constructor(width, height) {
  super();
-
- this.width = width;
- this.height = height;
-
- this.shelfPack = new ShelfPack(width, height);
  this.images = {};
  this.data = false;
  this.texture = 0; // WebGL ID
  this.filter = 0; // WebGL ID
- this.pixelRatio = browser.devicePixelRatio > 1 ? 2 : 1;
+ this.width = width * browser.devicePixelRatio;
+ this.height = height * browser.devicePixelRatio;
+ this.shelfPack = new ShelfPack(this.width, this.height);
  this.dirty = true;
  }
 
+ getPixelSize() {
+ return [
+ this.width,
+ this.height
+ ];
+ }
+
  allocateImage(pixelWidth, pixelHeight) {
- const width = pixelWidth / this.pixelRatio + 2 * padding;
- const height = pixelHeight / this.pixelRatio + 2 * padding;
+ const width = pixelWidth + 2 * padding;
+ const height = pixelHeight + 2 * padding;
 
  const rect = this.shelfPack.packOne(width, height);
  if (!rect) {
@@ -69,16 +103,15 @@ class SpriteAtlas extends Evented {
  return this.fire('error', {error: new Error('There is not enough space to add this image.')});
  }
 
- const image = {
+ this.images[name] = {
  rect,
- width: width / pixelRatio,
- height: height / pixelRatio,
- sdf: false,
- pixelRatio: pixelRatio / this.pixelRatio
+ width,
+ height,
+ pixelRatio,
+ sdf: false
  };
- this.images[name] = image;
 
- this.copy(pixels, width, rect, {pixelRatio, x: 0, y: 0, width, height}, false);
+ this.copy(pixels, width, rect, {x: 0, y: 0, width, height}, false);
 
  this.fire('data', {dataType: 'style'});
  }
@@ -95,9 +128,19 @@ class SpriteAtlas extends Evented {
  this.fire('data', {dataType: 'style'});
  }
 
- getImage(name, wrap) {
+ // Return metrics for an icon image.
+ getIcon(name) {
+ return this._getImage(name, false);
+ }
+
+ // Return metrics for repeating pattern image.
+ getPattern(name) {
+ return this._getImage(name, true);
+ }
+
+ _getImage(name, wrap) {
  if (this.images[name]) {
- return this.images[name];
+ return spriteAtlasElement(this.images[name]);
  }
 
  if (!this.sprite) {
@@ -116,44 +159,23 @@ class SpriteAtlas extends Evented {
 
  const image = {
  rect,
- width: pos.width / pos.pixelRatio,
- height: pos.height / pos.pixelRatio,
+ width: pos.width,
+ height: pos.height,
  sdf: pos.sdf,
- pixelRatio: pos.pixelRatio / this.pixelRatio
+ pixelRatio: pos.pixelRatio
  };
  this.images[name] = image;
 
  if (!this.sprite.imgData) return null;
  const srcImg = new Uint32Array(this.sprite.imgData.buffer);
  this.copy(srcImg, this.sprite.width, rect, pos, wrap);
 
- return image;
- }
-
- // Return position of a repeating fill pattern.
- getPosition(name, repeating) {
- const image = this.getImage(name, repeating);
- const rect = image && image.rect;
-
- if (!rect) {
- return null;
- }
-
- const width = image.width * image.pixelRatio;
- const height = image.height * image.pixelRatio;
-
- return {
- size: [image.width, image.height],
- tl: [(rect.x + padding), (rect.y + padding)],
- br: [(rect.x + padding + width), (rect.y + padding + height)]
- };
+ return spriteAtlasElement(image);
  }
 
  allocate() {
  if (!this.data) {
- const w = Math.floor(this.width * this.pixelRatio);
- const h = Math.floor(this.height * this.pixelRatio);
- this.data = new Uint32Array(w * h);
+ this.data = new Uint32Array(this.width * this.height);
  for (let i = 0; i < this.data.length; i++) {
  this.data[i] = 0;
  }
@@ -171,9 +193,9 @@ class SpriteAtlas extends Evented {
  /* source x */ srcPos.x,
  /* source y */ srcPos.y,
  /* dest buffer */ dstImg,
- /* dest stride */ this.width * this.pixelRatio,
- /* dest x */ (dstPos.x + padding) * this.pixelRatio,
- /* dest y */ (dstPos.y + padding) * this.pixelRatio,
+ /* dest stride */ this.getPixelSize()[0],
+ /* dest x */ dstPos.x + padding,
+ /* dest y */ dstPos.y + padding,
  /* icon dimension */ srcPos.width,
  /* icon dimension */ srcPos.height,
  /* wrap */ wrap
@@ -186,18 +208,18 @@ class SpriteAtlas extends Evented {
 
  setSprite(sprite) {
  if (sprite && this.canvas) {
- this.canvas.width = this.width * this.pixelRatio;
- this.canvas.height = this.height * this.pixelRatio;
+ this.canvas.width = this.width;
+ this.canvas.height = this.height;
  }
  this.sprite = sprite;
  }
 
  addIcons(icons, callback) {
- for (let i = 0; i < icons.length; i++) {
- this.getImage(icons[i]);
+ const result = {};
+ for (const icon of icons) {
+ result[icon] = this.getIcon(icon);
  }
-
- callback(null, this.images);
+ callback(null, result);
  }
 
  bind(gl, linear) {
@@ -228,8 +250,8 @@ class SpriteAtlas extends Evented {
  gl.TEXTURE_2D, // enum target
  0, // ind level
  gl.RGBA, // ind internalformat
- this.width * this.pixelRatio, // GLsizei width
- this.height * this.pixelRatio, // GLsizei height
+ this.width, // GLsizei width
+ this.height, // GLsizei height
  0, // ind border
  gl.RGBA, // enum format
  gl.UNSIGNED_BYTE, // enum type
@@ -241,8 +263,8 @@ class SpriteAtlas extends Evented {
  0, // int level
  0, // int xoffset
  0, // int yoffset
- this.width * this.pixelRatio, // long width
- this.height * this.pixelRatio, // long height
+ this.width, // long width
+ this.height, // long height
  gl.RGBA, // enum format
  gl.UNSIGNED_BYTE, // enum type
  new Uint8Array(this.data.buffer) // Object pixels

test/integration/render-tests/icon-pixelratio-mismatch/default/style.json

@@ -2,9 +2,6 @@
  "version": 8,
  "metadata": {
  "test": {
- "ignored": {
- "native": "https://github.com/mapbox/mapbox-gl-native/issues/3164"
- },
  "pixelRatio": 0.5,
  "height": 256
  }
@@ -53,4 +50,4 @@
  }
  }
  ]
-}
+}