From 0083542dc85c49f58a8f1ec8765e74a2526c6da7 Mon Sep 17 00:00:00 2001 From: Aaron Boodman Date: Sat, 12 Apr 2014 13:42:23 -0700 Subject: [PATCH] Fix: sometimes the foursquare photos don't completely fill the available space Change-Id: I6cafd5ebdd73da81e4fbdfd73ecfc596213c7433 --- .../ui/blob_item_foursquare_content.js | 2 +- server/camlistored/ui/thumber.js | 49 +++++++++++++------ server/camlistored/ui/thumber_test.js | 20 ++++++-- 3 files changed, 50 insertions(+), 21 deletions(-) diff --git a/server/camlistored/ui/blob_item_foursquare_content.js b/server/camlistored/ui/blob_item_foursquare_content.js index fd2229a83..3d3ae35c7 100644 --- a/server/camlistored/ui/blob_item_foursquare_content.js +++ b/server/camlistored/ui/blob_item_foursquare_content.js @@ -160,7 +160,7 @@ cam.BlobItemFoursquareContent.Handler.prototype.createContent = function(size) { size: size, venueId: this.venueId_, venueName: this.venueName_, - photo: this.thumber_ ? this.thumber_.getSrc(size.height) : '', + photo: this.thumber_ ? this.thumber_.getSrc(size) : '', date: this.startDate_, }); }; diff --git a/server/camlistored/ui/thumber.js b/server/camlistored/ui/thumber.js index 18e6bc53c..dc683961a 100644 --- a/server/camlistored/ui/thumber.js +++ b/server/camlistored/ui/thumber.js @@ -20,29 +20,46 @@ goog.require('goog.string'); // Utility to efficiently choose thumbnail URLs for use by the UI. // -// Sizes are rounded to the next highest power of two for cache friendliness. Also, the last requested size is remembered, and if the requested size is smaller than the last size, then we continue using the old URL. -cam.Thumber = function(pathname) { +// Sizes are bucketized for cache friendliness. Also, the last requested size is remembered, and if the requested size is smaller than the last size, then we continue using the old URL. +cam.Thumber = function(pathname, opt_aspect) { this.pathname_ = pathname; - this.lastSize_ = 0; + this.lastHeight_ = 0; + this.aspect_ = opt_aspect || 1; }; -cam.Thumber.MAX_SIZE = 2000; +// We originally just used powers of 2, but we need sizes between 200 and 400 all the time in the UI and it seemed wasteful to jump to 512. Having an explicit list will make it easier to tune the buckets more in the future if necessary. +cam.Thumber.SIZES = [64, 128, 256, 375, 500, 750, 1000, 1500, 2000]; cam.Thumber.fromImageMeta = function(imageMeta) { - return new cam.Thumber(goog.string.subs('thumbnail/%s/%s', imageMeta.blobRef, (imageMeta.file && imageMeta.file.fileName) || imageMeta.blobRef + '.jpg')); + return new cam.Thumber(goog.string.subs('thumbnail/%s/%s', imageMeta.blobRef, (imageMeta.file && imageMeta.file.fileName) || imageMeta.blobRef + '.jpg'), + imageMeta.image.width / imageMeta.image.height); }; -cam.Thumber.prototype.getSrc = function(displayHeight) { - this.lastSize_ = this.getSizeToRequest_(displayHeight); - return this.pathname_ + '?mh=' + this.lastSize_ + '&tv=' + (goog.global.CAMLISTORE_CONFIG ? goog.global.CAMLISTORE_CONFIG.thumbVersion : 1); -}; - -cam.Thumber.prototype.getSizeToRequest_ = function(displayHeight) { - if (this.lastSize_ >= displayHeight) { - return this.lastSize_; +// @param {number|goog.math.Size} minSize The minimum size of the required thumbnail. If this is a number, it is the minimum height. If it is goog.math.Size, then it is the min size of both dimensions. +cam.Thumber.prototype.getSrc = function(minSize) { + var minWidth, minHeight; + if (typeof minSize == 'number') { + minHeight = minSize; + minWidth = 0; + } else { + minWidth = minSize.width; + minHeight = minSize.height; } - for (var size = 64; (size <= displayHeight && size < cam.Thumber.MAX_SIZE); size <<= 1) { - } - return Math.min(size, cam.Thumber.MAX_SIZE); + this.lastHeight_ = this.getSizeToRequest_(minWidth, minHeight); + return goog.string.subs('%s?mh=%s&tv=%s', this.pathname_, this.lastHeight_, goog.global.CAMLISTORE_CONFIG ? goog.global.CAMLISTORE_CONFIG.thumbVersion : 1); +}; + +cam.Thumber.prototype.getSizeToRequest_ = function(minWidth, minHeight) { + if (this.lastHeight_ >= minHeight && ((this.lastHeight_ * this.aspect_) >= minWidth)) { + return this.lastHeight_; + } + var newHeight; + for (var i = 0; i < cam.Thumber.SIZES.length; i++) { + newHeight = cam.Thumber.SIZES[i]; + if (newHeight >= minHeight && ((newHeight * this.aspect_) >= minWidth)) { + break; + } + } + return newHeight; }; diff --git a/server/camlistored/ui/thumber_test.js b/server/camlistored/ui/thumber_test.js index 860c508b0..099df549f 100644 --- a/server/camlistored/ui/thumber_test.js +++ b/server/camlistored/ui/thumber_test.js @@ -16,6 +16,7 @@ limitations under the License. var assert = require('assert'); +goog.require('goog.math.Size'); goog.require('goog.Uri'); goog.require('cam.Thumber'); @@ -23,7 +24,7 @@ goog.require('cam.Thumber'); describe('cam.Thumber', function() { describe('#getSrc', function() { - it('should go up in powers of two', function() { + it('it should bucketize properly', function() { var thumber = new cam.Thumber('foo.png'); assert.equal(128, goog.Uri.parse(thumber.getSrc(100)).getParameterValue('mh')); assert.equal(128, goog.Uri.parse(thumber.getSrc(128)).getParameterValue('mh')); @@ -33,9 +34,10 @@ describe('cam.Thumber', function() { it('should max out at a sane size', function() { var thumber = new cam.Thumber('foo.png'); - assert.equal(cam.Thumber.MAX_SIZE, goog.Uri.parse(thumber.getSrc(1999)).getParameterValue('mh')); - assert.equal(cam.Thumber.MAX_SIZE, goog.Uri.parse(thumber.getSrc(2000)).getParameterValue('mh')); - assert.equal(cam.Thumber.MAX_SIZE, goog.Uri.parse(thumber.getSrc(2001)).getParameterValue('mh')); + var maxSize = cam.Thumber.SIZES[cam.Thumber.SIZES.length - 1]; + assert.equal(maxSize, goog.Uri.parse(thumber.getSrc(1999)).getParameterValue('mh')); + assert.equal(maxSize, goog.Uri.parse(thumber.getSrc(2000)).getParameterValue('mh')); + assert.equal(maxSize, goog.Uri.parse(thumber.getSrc(2001)).getParameterValue('mh')); }); it('should only increase in size, never decrease', function() { @@ -46,5 +48,15 @@ describe('cam.Thumber', function() { assert.equal(128, goog.Uri.parse(thumber.getSrc(50)).getParameterValue('mh')); assert.equal(256, goog.Uri.parse(thumber.getSrc(129)).getParameterValue('mh')); }); + + it('should handle Size objects properly', function() { + var thumber = new cam.Thumber('foo.png', 2); + assert.equal(128, goog.Uri.parse(thumber.getSrc(new goog.math.Size(100, 100))).getParameterValue('mh')); + thumber = new cam.Thumber('foo.png', 0.5); + assert.equal(256, goog.Uri.parse(thumber.getSrc(new goog.math.Size(100, 100))).getParameterValue('mh')); + + assert.equal(256, goog.Uri.parse(thumber.getSrc(new goog.math.Size(128, 100))).getParameterValue('mh')); + assert.equal(375, goog.Uri.parse(thumber.getSrc(new goog.math.Size(129, 100))).getParameterValue('mh')); + }); }); });