From 24ed82fd23bb99e87070d4a8fa90f5bbbb59ea89 Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Wed, 10 Aug 2016 00:57:27 -0400
Subject: [PATCH] Unref glyphs when tiles unload

---
 debug/atlases.html                 |  2 +-
 js/source/tile.js                  |  4 ++
 js/symbol/glyph_atlas.js           | 91 ++++++++++++------------------
 js/symbol/glyph_source.js          | 44 ++++++++++++---
 test/js/data/symbol_bucket.test.js |  4 +-
 5 files changed, 78 insertions(+), 67 deletions(-)

diff --git a/debug/atlases.html b/debug/atlases.html
index e78dff105af..ec33656eb15 100644
--- a/debug/atlases.html
+++ b/debug/atlases.html
@@ -55,7 +55,7 @@
 <div id='debugsize'>
 </div>
 
-<script src='mapbox-gl.js'></script>
+<script src='dist/mapbox-gl-dev.js'></script>
 <script src='access-token.js'></script>
 
 <script>
diff --git a/js/source/tile.js b/js/source/tile.js
index 1c223546a01..b801c49d8f0 100644
--- a/js/source/tile.js
+++ b/js/source/tile.js
@@ -102,6 +102,10 @@ Tile.prototype = {
      * @private
      */
     unloadVectorData: function(painter) {
+        if (painter && painter.glyphSource) {
+            painter.glyphSource.unloadTile(this.uid);
+        }
+
         for (var id in this.buckets) {
             var bucket = this.buckets[id];
             bucket.destroy(painter.gl);
diff --git a/js/symbol/glyph_atlas.js b/js/symbol/glyph_atlas.js
index 01aabc6e211..1177420f989 100644
--- a/js/symbol/glyph_atlas.js
+++ b/js/symbol/glyph_atlas.js
@@ -4,64 +4,31 @@ var ShelfPack = require('shelf-pack');
 var util = require('../util/util');
 
 module.exports = GlyphAtlas;
+
+
 function GlyphAtlas(width, height) {
     this.width = width;
     this.height = height;
 
-    this.bin = new ShelfPack(width, height);
-    this.index = {};
-    this.ids = {};
+    this.sprite = new ShelfPack(width, height);
     this.data = new Uint8Array(width * height);
+    this.tilebins = {};
 }
 
-GlyphAtlas.prototype.getGlyphs = function() {
-    var glyphs = {},
-        split,
-        name,
-        id;
-
-    for (var key in this.ids) {
-        split = key.split('#');
-        name = split[0];
-        id = split[1];
-
-        if (!glyphs[name]) glyphs[name] = [];
-        glyphs[name].push(id);
-    }
-
-    return glyphs;
-};
-
-GlyphAtlas.prototype.getRects = function() {
-    var rects = {},
-        split,
-        name,
-        id;
-
-    for (var key in this.ids) {
-        split = key.split('#');
-        name = split[0];
-        id = split[1];
-
-        if (!rects[name]) rects[name] = {};
-        rects[name][id] = this.index[key];
-    }
-
-    return rects;
-};
 
-
-GlyphAtlas.prototype.addGlyph = function(id, name, glyph, buffer) {
+GlyphAtlas.prototype.addGlyph = function(glyph, uid, buffer) {
     if (!glyph) return null;
 
-    var key = name + "#" + glyph.id;
+    if (!this.tilebins[uid]) {
+        this.tilebins[uid] = [];
+    }
 
     // The glyph is already in this texture.
-    if (this.index[key]) {
-        if (this.ids[key].indexOf(id) < 0) {
-            this.ids[key].push(id);
-        }
-        return this.index[key];
+    var bin = this.sprite.getBin(glyph.id);
+    if (bin) {
+        this.tilebins[uid].push(bin);
+        this.sprite.ref(bin);
+        return bin;
     }
 
     // The glyph bitmap has zero width.
@@ -83,23 +50,23 @@ GlyphAtlas.prototype.addGlyph = function(id, name, glyph, buffer) {
     packWidth += (4 - packWidth % 4);
     packHeight += (4 - packHeight % 4);
 
-    var rect = this.bin.packOne(packWidth, packHeight);
-    if (!rect) {
+    bin = this.sprite.packOne(packWidth, packHeight, glyph.id);
+    if (!bin) {
         this.resize();
-        rect = this.bin.packOne(packWidth, packHeight);
+        bin = this.sprite.packOne(packWidth, packHeight, glyph.id);
     }
-    if (!rect) {
+    if (!bin) {
         util.warnOnce('glyph bitmap overflow');
         return null;
     }
 
-    this.index[key] = rect;
-    this.ids[key] = [id];
+    this.tilebins[uid].push(bin);
+
 
     var target = this.data;
     var source = glyph.bitmap;
     for (var y = 0; y < bufferedHeight; y++) {
-        var y1 = this.width * (rect.y + y + padding) + rect.x + padding;
+        var y1 = this.width * (bin.y + y + padding) + bin.x + padding;
         var y2 = bufferedWidth * y;
         for (var x = 0; x < bufferedWidth; x++) {
             target[y1 + x] = source[y2 + x];
@@ -108,9 +75,21 @@ GlyphAtlas.prototype.addGlyph = function(id, name, glyph, buffer) {
 
     this.dirty = true;
 
-    return rect;
+    return bin;
+};
+
+
+GlyphAtlas.prototype.removeTileGlyphs = function(uid) {
+    var bins = this.tilebins[uid];
+    if (!bins) return;
+
+    for (var i = 0; i < bins.length; i++) {
+        this.sprite.unref(bins[i]);
+    }
+    delete this.tilebins[uid];
 };
 
+
 GlyphAtlas.prototype.resize = function() {
     var origw = this.width,
         origh = this.height;
@@ -128,7 +107,7 @@ GlyphAtlas.prototype.resize = function() {
 
     this.width *= 2;
     this.height *= 2;
-    this.bin.resize(this.width, this.height);
+    this.sprite.resize(this.width, this.height);
 
     var buf = new ArrayBuffer(this.width * this.height),
         src, dst;
@@ -140,6 +119,7 @@ GlyphAtlas.prototype.resize = function() {
     this.data = new Uint8Array(buf);
 };
 
+
 GlyphAtlas.prototype.bind = function(gl) {
     this.gl = gl;
     if (!this.texture) {
@@ -156,6 +136,7 @@ GlyphAtlas.prototype.bind = function(gl) {
     }
 };
 
+
 GlyphAtlas.prototype.updateTexture = function(gl) {
     this.bind(gl);
     if (this.dirty) {
diff --git a/js/symbol/glyph_source.js b/js/symbol/glyph_source.js
index 6224c1cc532..ca7617d5e89 100644
--- a/js/symbol/glyph_source.js
+++ b/js/symbol/glyph_source.js
@@ -8,6 +8,7 @@ var Protobuf = require('pbf');
 
 module.exports = GlyphSource;
 
+
 /**
  * A glyph source has a URL from which to load new glyphs and manages
  * GlyphAtlases in which to store glyphs used by the requested fontstacks
@@ -23,6 +24,7 @@ function GlyphSource(url) {
     this.loading = {};
 }
 
+
 GlyphSource.prototype.getSimpleGlyphs = function(fontstack, glyphIDs, uid, callback) {
     if (this.stacks[fontstack] === undefined) {
         this.stacks[fontstack] = {};
@@ -48,8 +50,10 @@ GlyphSource.prototype.getSimpleGlyphs = function(fontstack, glyphIDs, uid, callb
 
         if (stack[range]) {
             var glyph = stack[range].glyphs[glyphID];
-            var rect  = atlas.addGlyph(uid, fontstack, glyph, buffer);
-            if (glyph) glyphs[glyphID] = new SimpleGlyph(glyph, rect, buffer);
+            var bin  = atlas.addGlyph(glyph, uid, buffer);
+            if (glyph) {
+                glyphs[glyphID] = new SimpleGlyph(glyph, bin, buffer);
+            }
         } else {
             if (missing[range] === undefined) {
                 missing[range] = [];
@@ -59,7 +63,9 @@ GlyphSource.prototype.getSimpleGlyphs = function(fontstack, glyphIDs, uid, callb
         }
     }
 
-    if (!remaining) callback(undefined, glyphs, fontstack);
+    if (!remaining) {
+        callback(undefined, glyphs, fontstack);
+    }
 
     var onRangeLoaded = function(err, range, data) {
         if (!err) {
@@ -67,12 +73,16 @@ GlyphSource.prototype.getSimpleGlyphs = function(fontstack, glyphIDs, uid, callb
             for (var i = 0; i < missing[range].length; i++) {
                 var glyphID = missing[range][i];
                 var glyph = stack.glyphs[glyphID];
-                var rect  = atlas.addGlyph(uid, fontstack, glyph, buffer);
-                if (glyph) glyphs[glyphID] = new SimpleGlyph(glyph, rect, buffer);
+                var bin  = atlas.addGlyph(glyph, uid, buffer);
+                if (glyph) {
+                    glyphs[glyphID] = new SimpleGlyph(glyph, bin, buffer);
+                }
             }
         }
         remaining--;
-        if (!remaining) callback(undefined, glyphs, fontstack);
+        if (!remaining) {
+            callback(undefined, glyphs, fontstack);
+        }
     }.bind(this);
 
     for (var r in missing) {
@@ -80,21 +90,35 @@ GlyphSource.prototype.getSimpleGlyphs = function(fontstack, glyphIDs, uid, callb
     }
 };
 
+
+// Mark all glyphs used by this tile as unused.
+GlyphSource.prototype.unloadTile = function(uid) {
+    var fontstacks = Object.keys(this.atlases);
+    for (var i = 0; i < fontstacks.length; i++) {
+        this.atlases[fontstacks[i]].removeTileGlyphs(uid);
+    }
+};
+
+
 // A simplified representation of the glyph containing only the properties needed for shaping.
-function SimpleGlyph(glyph, rect, buffer) {
+function SimpleGlyph(glyph, bin, buffer) {
     var padding = 1;
     this.advance = glyph.advance;
     this.left = glyph.left - buffer - padding;
     this.top = glyph.top + buffer + padding;
-    this.rect = rect;
+    this.rect = bin;
 }
 
+
 GlyphSource.prototype.loadRange = function(fontstack, range, callback) {
-    if (range * 256 > 65535) return callback('glyphs > 65535 not supported');
+    if (range * 256 > 65535) {
+        return callback('glyphs > 65535 not supported');
+    }
 
     if (this.loading[fontstack] === undefined) {
         this.loading[fontstack] = {};
     }
+
     var loading = this.loading[fontstack];
 
     if (loading[range]) {
@@ -115,10 +139,12 @@ GlyphSource.prototype.loadRange = function(fontstack, range, callback) {
     }
 };
 
+
 GlyphSource.prototype.getGlyphAtlas = function(fontstack) {
     return this.atlases[fontstack];
 };
 
+
 /**
  * Use CNAME sharding to load a specific glyph range over a randomized
  * but consistent subdomain.
diff --git a/test/js/data/symbol_bucket.test.js b/test/js/data/symbol_bucket.test.js
index 9838f29e379..b96bad2ca1e 100644
--- a/test/js/data/symbol_bucket.test.js
+++ b/test/js/data/symbol_bucket.test.js
@@ -25,10 +25,10 @@ test('SymbolBucket', function(t) {
     var symbolQuadsArray = new SymbolQuadsArray();
     var symbolInstancesArray = new SymbolInstancesArray();
     var collision = new Collision(0, 0, collisionBoxArray);
-    var atlas = new GlyphAtlas(1024, 1024);
+    var atlas = new GlyphAtlas(128, 128);
     for (var id in glyphs) {
         glyphs[id].bitmap = true;
-        glyphs[id].rect = atlas.addGlyph(id, 'Test', glyphs[id], 3);
+        glyphs[id].rect = atlas.addGlyph(glyphs[id], 1, 3);
     }
 
     var stacks = { 'Test': glyphs };