4
\$\begingroup\$

I got this as part of some interview assignment. After some head scratching I think I have done quite well as the result can be seen here:

enter image description here

After transformation(10x10 tile):

enter image description here

More crisp picture with smaller tile size:

enter image description here

So, the requirement were:

  1. Not to use any external library.
  2. Only front end code.
  3. Should be a production ready.
  4. Use HTML5 features where appropriate.
  5. Make the code modular.
  6. Make the code testable.

Brief problem description:

  1. A user selects a local image file.
  2. The app loads that image, divides the image into tiles, computes the average color of each tile, fetches a tile from the server for that color, and composites the results into a photomosaic of the original image.
  3. The composited photomosaic should be displayed according to the following constraints:
    • tiles should be rendered a complete row at a time (a user should never see a row with some completed tiles and some incomplete)
    • the mosaic should be rendered from the top row to the bottom row.
  4. The client app should make effective use of parallelism and asynchrony.

    • tiles should be rendered a complete row at a time (a user should never see a row with some completed tiles and some incomplete)
    • the mosaic should be rendered from the top row to the bottom row.

Code:

client.js

'use strict';

/** @fileoverview */

(function(window, document, app) {

  var DOMURL = window.URL || window.webkitURL;
  var SVG_URL = '/color/';

  function Tile(pixelData, x, y) {
    this.hex = Utils.rgbToHex(pixelData);
    this.x = x * TILE_WIDTH;
    this.y = y * TILE_HEIGHT; 
  };

  /**
   * Draws a offscreen canvas to get averaged rgb per tile.
   * see http://stackoverflow.com/a/17862644/4260745
   */
  function getAverageRgb(image) {
    var canvas = document.createElement('canvas');
    var ctx = canvas.getContext('2d');
    canvas.width = image.width / TILE_WIDTH;
    canvas.height = image.height / TILE_HEIGHT;
    ctx.drawImage(image, 0, 0, canvas.width, canvas.height);
    return ctx;
  };

  function getTilesMetaData(sourceImage) {
    var res = [];
    var context = getAverageRgb(sourceImage);
    var NUM_TILES_X = sourceImage.width / TILE_WIDTH;
    var NUM_TILES_Y = sourceImage.height / TILE_HEIGHT;
    var data = context.getImageData(0, 0, NUM_TILES_X, NUM_TILES_Y).data;
    var i = 0;
    for (var row = 0; row < NUM_TILES_Y; row++) {
      for (var col = 0; col < NUM_TILES_X; col++) {
        res.push(new Tile(data.subarray(i * 4, i * 4 + 3), col, row));
        i++;
      }
    }
    return res;
  };

  /**
   * Draws PhotoMosaic on screen.
   */
  function drawMosiac(image) {
    var canvas = document.createElement('canvas');
    var ctx = canvas.getContext('2d');
    canvas.width = image.width;
    canvas.height = image.height;

    var chunkSize = image.width / TILE_WIDTH;
    var tilesMetadata = getTilesMetaData(image);
    var chunks = tilesMetadata.splice(0, chunkSize);
    var promises = [];
    while (chunks.length > 0) {
      promises.push(
          Promise.all(chunks.map(function(data) { return getSvg(data); })));
      chunks = tilesMetadata.splice(0, chunkSize);
    }
    function renderRow(promises) {
      // Base case.
      if (promises.length === 0) return;
      promises.shift().then(function(results) {
        results.forEach(function(result) {
          renderSVGTile(ctx, result.svg, {x: result.x, y: result.y});
        });
        // Resolve the remaining promises.
        renderRow(promises);
      });
    };
    renderRow(promises);
    return canvas;
  };

  function createSVGUrl(svg) {
    var svgBlob = new Blob([svg], {type: 'image/svg+xml;charset=utf-8'});
    return DOMURL.createObjectURL(svgBlob);
  };

  function renderSVGTile(ctx, svg, pos) {
    var img = new Image();
    var url = createSVGUrl(svg);
    img.onload = function() {
      try {
        ctx.drawImage(img, pos.x, pos.y);
        ctx.imageSmoothingEnabled = false;
        ctx.mozImageSmoothingEnabled = false;
        DOMURL.revokeObjectURL(url);
      } catch(e) {
        throw new Error('Could not render image' + e);
      }
    };
    img.src = url;
  };

  function getSvg(data) {
    return Utils.httpGet(SVG_URL + data.hex)
        .then(function(svg) { return {svg: svg, x: data.x, y: data.y}; })
        .catch(function(error) {
          // One or more promises were rejected
          console.log(error);
        });
  };

  function handleFileUpload(callback) {
    var img = new Image();
    img.src = window.URL.createObjectURL(this);
    // img.crossOrigin = 'anonymous';
    img.onload = function() { callback(this); }
  };

  var Utils = {
    componentToHex: function componentToHex(c) {
      var hex = c.toString(16);
      return hex.length == 1 ? '0' + hex : hex;
    },

    rgbToHex: function rgbToHex(rgb) {
      return this.componentToHex(rgb[0]) + this.componentToHex(rgb[1]) +
          this.componentToHex(rgb[2]);
    },

    httpGet: function httpGet(url) {
      // Return a new promise.
      return new Promise(function(resolve, reject) {
        // Do the usual XHR stuff
        var req = new XMLHttpRequest();
        req.open('GET', url);

        req.onload = function() {
          // This is called even on 404 etc
          // so check the status
          if (req.status == 200) {
            // Resolve the promise with the response text
            resolve(req.response);
          } else {
            // Otherwise reject with the status text
            // which will hopefully be a meaningful error
            reject(Error(req.statusText));
          }
        };

        // Handle network errors
        req.onerror = function() { reject(Error('Network Error')); };

        // Make the request
        req.send();
      });
    }
  };

  app.run = function run() {
    var inputElement = document.getElementById('input');
    var ul = document.getElementById('image-list');
    var li = document.createElement('li');
    ul.appendChild(li);
    inputElement.addEventListener('change', function() {
      handleFileUpload.call(
          this.files[0], function(image) {
            var canvas = drawMosiac(image);
            li.appendChild(canvas);
          });
    }, false);
  };
})(window, document, window.app || (window.app = {}));

mosaic.js

// Constants shared between client and server.

var TILE_WIDTH = 8;
var TILE_HEIGHT = 8;

var exports = exports || null;
if (exports) {
  exports.TILE_WIDTH = TILE_WIDTH;
  exports.TILE_HEIGHT = TILE_HEIGHT;
}

index.html

<!DOCTYPE html>
<html>
  <head>
    <script type="text/javascript" src="js/mosaic.js"></script>
    <script type="text/javascript" src="js/client.js"></script>
    <title>Mosaic</title>
    <style>
      .container {
        margin: 0 auto;
        width: 50%;
      }
      .container ul {
        list-style: none;
        margin-left: 0;
      }
    </style>
  </head>
  <body>
    <div class="container">
      <ul id="image-list">
      </ul>
      <input id="input" type="file" accept="image/*">
    </div>
    <script>
      (function(app) {
        document.addEventListener('DOMContentLoaded', function() {
          app.run();
        });
     })(window.app || (window.app = {}));
    </script>

  </body>
</html>

Note: Only client.js and index.html are written by me.

\$\endgroup\$
  • \$\begingroup\$ Isn't Promise an external library, prohibited by the rules? \$\endgroup\$ – 200_success Jul 5 '16 at 18:54
  • 1
    \$\begingroup\$ @200_success no, developer.mozilla.org/en/docs/Web/JavaScript/Reference/… \$\endgroup\$ – CodeYogi Jul 6 '16 at 3:24
  • \$\begingroup\$ I have rolled back the last edit. Please see What to do when someone answers. Feel free to post a follow-up question instead. \$\endgroup\$ – Mast Jul 12 '16 at 12:03
  • 1
    \$\begingroup\$ @Mast in that case I would like to take my bounty back, how can I do it? \$\endgroup\$ – CodeYogi Jul 12 '16 at 12:05
  • 1
    \$\begingroup\$ Bounty reverted. I suggest that you post a follow-up question instead, then you may put a bounty on that after a few days if you want. \$\endgroup\$ – Simon Forsberg Jul 12 '16 at 12:22
1
\$\begingroup\$

My notes;

  • getAverageRgb does not do what it says, it gets a context, this is bad
  • drawMosiac <- spelling matters
  • You should really comment that

    ctx.drawImage(image, 0, 0, canvas.width, canvas.height);
    

    actually scales the image, which gives you hence an average value, I spend way too long scratching my head over this

  • I would re-order the statements in getAverageRgb:

    function getAverageRgb(image) {
      var canvas = document.createElement('canvas');
      canvas.width = image.width / TILE_WIDTH;
      canvas.height = image.height / TILE_HEIGHT;
      var ctx = canvas.getContext('2d');
      ctx.drawImage(image, 0, 0, canvas.width, canvas.height);
      return ctx;
    };
    

    if the middle var bothers you, you could declare the ctx upfront:

    function getAverageRgb(image) {
      var canvas = document.createElement('canvas'),
          ctx;
    

    The reason is that you modify canvas which impacts the context which just looks wrong (I understand it works).

  • On the whole the flow of your program is hard to follow, case in point is function renderRow, why define it there ? The code jumps from run at the bottom to renderMosaic in the middle which then calls functions under and above it, I think you can do better.
  • I probably would have create an SVG object/helper to group the generic SVG related functions
  • This stuff kills me:

        // Return a new promise.
        return new Promise(function(resolve, reject) {
    

    and

        // Make the request
        req.send();
    

    and

        // Otherwise reject with the status text
        // which will hopefully be a meaningful error
        reject(Error(req.statusText));
    

    Anybody who knows a little JavaScript knows what does lines do, but you dedicate comments to them. Other parts which are far less obvious get no comments. Most of your comments are a waste of space, you should review them.

  • From a test-ability perspective, renderRow cannot be tested by itself

\$\endgroup\$
  • \$\begingroup\$ For the scaling part I have the SO link, I thought that would be sufficient since that part is too tricky to explain in few lines of comments. Also, can you give some example for I probably would have create an SVG object/helper to group the generic SVG related functions. \$\endgroup\$ – CodeYogi Jul 7 '16 at 18:56
  • \$\begingroup\$ I agree that the trickery of the assignment and time limit kept me away from cleaning up the code. \$\endgroup\$ – CodeYogi Jul 7 '16 at 18:56
  • \$\begingroup\$ @CodeYogi For the SVG object I would use the exact thing you used for the Utils object . \$\endgroup\$ – konijn Jul 7 '16 at 20:36
  • \$\begingroup\$ getAverageRgb does what it says ctx is returned as a side effect. \$\endgroup\$ – CodeYogi Jul 11 '16 at 11:14
  • 1
    \$\begingroup\$ @CodeYogi I am guessing you gave minus 1 ;) If you ask a developer what averagergb should return, that developer would expect an rgb value. A return value cannot be reasonably considered a 'side effect'. Lastly, on codereview, a reviewer can review any or all parts of the code. \$\endgroup\$ – konijn Jul 12 '16 at 14:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.