Merge pull request #13 from assetgraph/retina

Handle background-size on sprite images

lib/spriteBackgroundImages.js

@@ -45,7 +45,8 @@ function getImageAssetFromCanvas(canvas, assetType, assetGraph, cb) {
     }
 }
 
-function calculateSpritePadding(paddingStr) {
+function calculateSpritePadding(paddingStr, asset) {
+    var padding;
     if (paddingStr) {
         // Strip units ('px' assumed)
         var tokens = [];
@@ -56,24 +57,29 @@ function calculateSpritePadding(paddingStr) {
             }
         });
         if (tokens.length === 4) {
-            return tokens;
+            padding = tokens;
         } else if (tokens.length === 3) {
-            return [tokens[0], tokens[1], tokens[2], tokens[1]]; // T, L+R, B
+            padding = [tokens[0], tokens[1], tokens[2], tokens[1]]; // T, L+R, B
         } else if (tokens.length === 2) {
-            return [tokens[0], tokens[1], tokens[0], tokens[1]]; // T+B, L+R
+            padding = [tokens[0], tokens[1], tokens[0], tokens[1]]; // T+B, L+R
         } else if (tokens.length === 1) {
-            return [tokens[0], tokens[0], tokens[0], tokens[0]];
+            padding = [tokens[0], tokens[0], tokens[0], tokens[0]];
         }
+    } else {
+        padding = [0, 0, 0, 0];
     }
-    return [0, 0, 0, 0];
+
+    return padding.map(function (size) {
+        return Math.max(size, Math.max(asset.devicePixelRatio) - 1);
+    });
 }
 
 function getRelationSpriteInfoFromIncomingRelation(incomingRelation) {
     var parsedQueryString = queryString.parse(incomingRelation.href.match(/\?([^#]*)/)[1]);
     return {
         groupName: parsedQueryString.sprite || 'default',
         noGroup: 'spriteNoGroup' in parsedQueryString,
-        padding: calculateSpritePadding(parsedQueryString.padding),
+        padding: calculateSpritePadding(parsedQueryString.padding, incomingRelation.to),
         asset: incomingRelation.to
     };
 }
@@ -110,6 +116,8 @@ module.exports = function () {
         }
 
         var spriteGroups = {};
+
+        // Find sprite annotated images and create a data structure with their information
         assetGraph.findRelations({type: 'CssImage', to: {isImage: true}, href: /\?(?:|[^#]*&)sprite(?:[=&#]|$)/}).forEach(function (relation) {
             var relationSpriteInfo = getRelationSpriteInfoFromIncomingRelation(relation),
                 spriteGroup = (spriteGroups[relationSpriteInfo.groupName] = spriteGroups[relationSpriteInfo.groupName] || {
@@ -127,6 +135,7 @@ module.exports = function () {
             }
         });
 
+        // Extract sprite grouping information va -sprite- prefixed properties in stylesheets
         assetGraph.findAssets({type: 'Css'}).forEach(function (cssAsset) {
             cssAsset.eachRuleInParseTree(function (cssRule) {
                 if (cssRule.type !== 1) { // cssom.CSSRule.STYLE_RULE
@@ -190,7 +199,8 @@ module.exports = function () {
                 var callback = this,
                     spriteGroup = spriteGroups[spriteGroupName],
                     imageInfos = _.values(spriteGroup.imageInfosById),
-                    spriteInfo = spriteGroup.placeHolders && spriteGroup.placeHolders[0] || {};
+                    spriteInfo = spriteGroup.placeHolders && spriteGroup.placeHolders[0] || {},
+                    packingData;
 
                 seq(imageInfos)
                     .parMap(function (imageInfo) {
@@ -210,9 +220,12 @@ module.exports = function () {
                             horizontal: 'horizontal',
                             vertical: 'vertical'
                         }[spriteInfo.packer] || 'tryAll';
-                        var packingData = packers[packerName].pack(imageInfos),
-                            canvas = new Canvas(packingData.width, packingData.height),
+
+                        packingData =  packers[packerName].pack(imageInfos);
+
+                        var canvas = new Canvas(packingData.width, packingData.height),
                             ctx = canvas.getContext('2d');
+
                         imageInfos = packingData.imageInfos;
                         if ('backgroundColor' in spriteInfo) {
                             ctx.fillStyle = spriteInfo.backgroundColor;
@@ -281,6 +294,11 @@ module.exports = function () {
                                 ['selector-for-group', 'packer', 'image-format', 'background-color', 'important'].forEach(function (propertyName) {
                                     spriteInfo.cssRule.style.removeProperty('-sprite-' + propertyName);
                                 });
+
+                                // If background-size is set, we should update it, The correct size is now the sprites size
+                                if (spriteInfo.cssRule.style.getPropertyValue('background-size')) {
+                                    spriteInfo.cssRule.style.setProperty('background-size', packingData.width + 'px ' + packingData.height + 'px');
+                                }
                             });
                         }
                         imageInfos.forEach(function (imageInfo) {
@@ -292,8 +310,8 @@ module.exports = function () {
                                     existingBackgroundValue = style.background,
                                     backgroundOffsetsWereUpdated = false,
                                     offsets = [
-                                        imageInfo.x,
-                                        imageInfo.y
+                                        Math.round(imageInfo.x / imageInfo.asset.devicePixelRatio), // FIXME: Rounding issues?
+                                        Math.round(imageInfo.y / imageInfo.asset.devicePixelRatio)
                                     ],
                                     existingOffsets;
 
@@ -351,6 +369,21 @@ module.exports = function () {
                                 ['group', 'padding', 'no-group-selector', 'important'].forEach(function (propertyName) {
                                     style.removeProperty('-sprite-' + propertyName);
                                 });
+
+                                // Background-sizes change when spriting, upadte appropriately
+                                if (imageInfo.asset.devicePixelRatio === 1) {
+                                    // Device pixel ratio is default. Remove property and let the defaults rule
+                                    incomingRelation.cssRule.style.removeProperty('background-size');
+                                } else {
+                                    // Device pixel ratio is non-default, Set it explicitly with the ratio applied
+                                    var dpr = incomingRelation.to.devicePixelRatio;
+
+                                    // TODO: Figure out if rounding might become a problem
+                                    var width = packingData.width / dpr;
+                                    var height = packingData.height / dpr;
+                                    incomingRelation.cssRule.style.setProperty('background-size', width + 'px ' + height + 'px');
+                                }
+
                                 if (relationSpriteInfo.noGroup || !spriteGroup.placeHolders) {
                                     // The user specified that this selector needs its own background-image/background
                                     // property pointing at the sprite rather than relying on the Html elements also being

package.json

@@ -25,10 +25,10 @@
     "underscore": "=1.4.2"
   },
   "optionalDependencies": {
-    "canvas": "=1.1.2"
+    "canvas": "=1.1.6"
   },
   "devDependencies": {
-    "assetgraph": "1.14.4",
+    "assetgraph": "1.15.1",
     "coveralls": "^2.11.1",
     "istanbul": "^0.3.0",
     "jshint": "=2.5.1",

test/spriteBackgroundImages.js

@@ -1,4 +1,5 @@
-/*global describe, it*/var _ = require('underscore'),
+/*global describe, it*/
+var _ = require('underscore'),
     expect = require('./unexpected-with-plugins'),
     AssetGraph = require('assetgraph'),
     spriteBackgroundImages = require('../lib/spriteBackgroundImages');
@@ -257,4 +258,61 @@ describe('spriteBackgroundImages', function () {
             })
             .run(done);
     });
+
+    it('should get the background-position right when spriting a @2x image', function (done) {
+        new AssetGraph({root: __dirname + '/../testdata/spriteBackgroundImages/retina/'})
+            .loadAssets('index.html')
+            .populate()
+            .queue(function (assetGraph) {
+                expect(assetGraph, 'to contain assets', 'Css', 1);
+                expect(assetGraph, 'to contain assets', 'Png', 2);
+                expect(assetGraph, 'to contain assets', { type: 'Png', devicePixelRatio: 1 }, 1);
+                expect(assetGraph, 'to contain assets', { type: 'Png', devicePixelRatio: 2 }, 1);
+
+                assetGraph.findRelations({ type: 'CssImage', cssRule: { selectorText: '.regular' } }).forEach(function (relation) {
+                    expect(relation.to.devicePixelRatio, 'to be', 1);
+                    expect(relation.cssRule.style, 'not to have property', 'background-size');
+                });
+
+                assetGraph.findRelations({ type: 'CssImage', cssRule: { selectorText: '.retina' } }).forEach(function (relation) {
+                    expect(relation.to.devicePixelRatio, 'to be', 2);
+                    expect(relation.cssRule.style, 'to have property', 'background-size');
+                });
+            })
+            .queue(spriteBackgroundImages())
+            .queue(function (assetGraph) {
+                expect(assetGraph, 'to contain asset', 'Png', 1);
+                expect(assetGraph, 'to contain relations', 'CssImage', 2);
+                expect(assetGraph, 'to contain relations', { type: 'CssImage', cssRule: { selectorText: '.regular' } }, 1);
+                expect(assetGraph, 'to contain relations', { type: 'CssImage', cssRule: { selectorText: '.retina' } }, 1);
+
+                assetGraph.findRelations({ type: 'CssImage', cssRule: { selectorText: '.regular' } }).forEach(function (relation) {
+                    expect(relation.cssRule.style, 'not to have property', 'background-size');
+                });
+
+                assetGraph.findRelations({ type: 'CssImage', cssRule: { selectorText: '.retina' } }).forEach(function (relation) {
+                    expect(relation.cssRule.style, 'to have property', 'background-size');
+                    expect(relation.cssRule.style.getPropertyValue('background-size'), 'to be', '89px 59px');
+                    expect(relation.cssRule.style.getPropertyValue('background-position'), 'to be', '-30px 0');
+                });
+            })
+            .run(done);
+    });
+
+    it('should sprite retina @2x inline styled backgrounds correctly', function (done) {
+        new AssetGraph({root: __dirname + '/../testdata/spriteBackgroundImages/retina/'})
+            .loadAssets('inline-style.html')
+            .populate()
+            .queue(function (assetGraph) {
+                expect(assetGraph, 'to contain assets', 'Css', 3);
+                expect(assetGraph, 'to contain assets', 'Png', 2);
+                expect(assetGraph, 'to contain relations', 'CssImage', 4);
+            })
+            .queue(spriteBackgroundImages())
+            .queue(function (assetGraph) {
+                expect(assetGraph, 'to contain asset', 'Png', 1);
+                expect(assetGraph, 'to contain relations', 'CssImage', 1);
+            })
+            .run(done);
+    });
 });

testdata/spriteBackgroundImages/retina/index.html

@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <style type="text/css">
+            div {
+                width: 59px;
+                height: 59px;
+            }
+            .regular {
+                background-image: url(settings.png?sprite=default);
+            }
+            .retina {
+                background-image: url(settings@2x.png?sprite=default);
+                background-size: 59px 59px;
+            }
+        </style>
+    </head>
+
+    <body>
+        <div class="regular"></div>
+        <div class="retina"></div>
+    </body>
+</html>

testdata/spriteBackgroundImages/retina/inline-style.html

@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <style type="text/css">
+            .person-avatar {
+                -sprite-selector-for-group: people;
+                width: 59px;
+                height: 59px;
+                background-size: 59px;
+            }
+
+            .regular {
+                background-image: url(settings.png?sprite=people);
+            }
+            .retina {
+                background-image: url(settings@2x.png?sprite=people);
+            }
+        </style>
+    </head>
+
+    <body>
+        <div class="person-avatar" style="background-image: url(settings.png?sprite=people)"></div>
+        <div class="person-avatar" style="background-image: url(settings@2x.png?sprite=people)"></div>
+        <div class="person-avatar regular"></div>
+        <div class="person-avatar retina"></div>
+    </body>
+</html>