JavaScript - Settting property on Object in Image load function, property not set once outside function

Question

Sometimes JavaScript doesn't make sense to me, consider the following code that generates a photo mosaic based on x/y tiles. I'm trying to set a .Done property to true once each Mosaic image has been downloaded, but it's always false for some reason, what am I doing wrong?

var tileData = [];

function generate()
{
    var image = new Image();
    image.onload = function()
    {
        // Build up the 'tileData' array with tile objects from this Image

        for (var i = 0; i < tileData.length; i++)
        {
            var tile = tileData[i];

            var tileImage = new Image();
            tileImage.onload = function()
            {
                // Do something with this tile Image
                tile.Done = true;
            };
            tileImage.src = tile.ImageUrl;
        }
    };
    image.src = 'Penguins.jpg';

    tryDisplayMosaic();
}

function tryDisplayMosaic()
{
    setTimeout(function()
    {
        for (var i = 0; i < tileData.length; i++)
        {
            var tile = tileData[i];

            if (!tile.Done)
            {
                tryDisplayMosaic();
                return;
            }
        }

        // If we get here then all the tiles have been downloaded
        alert('All images downloaded');
    }, 2000);
}

Now for some reason the .Done property on the tile object is always false, even though it is explicitly being set to true inside tileImage.onload = function(). And I can ensure you that this function DOES get called because I've placed an alert() call inside it. Right now it's just stuck inside an infinite loop calling tryDisplayMosaic() constantly.

Also if I place a loop just before tryDisplayMosaic() is called, and in that loop I set .Done = true, then .Done property is true and alert('All images downloaded'); will get called.

Answer 1

The variable "tile" in the top loop is shared by every one of the "onload" functions you create; the very same single variable, in other words. Try this, to give each one its own variable:

tileImage.onload = (function(myTile) {
  return function()
        {
            // Do something with this tile Image
            myTile.Done = true;
        };
})(tile);

Why is this so? Because unlike C or Java, declaring "tile" inside the loop like that does not make it scoped to the statement block of the loop body. The only thing that gives you scope in Javascript is a function body.

Answer 2

What I see is that you create closures inside loops, which is a common mistake. To avoid this, you could create a more general closure, outside the loop, which handles all the load events and executes a function after all images are loaded:

// This array must be filled somewhere...
var tileData = [];

var ImageLoader = function(){
    var numOfImages = 0;
    var numLoaded   = 0;
    var callBack    = function(){};

    function imageLoaded(){
        numLoaded++;
        if(numLoaded===numOfImages){
            // All images are loaded, now call the callback function
            callBack.call(this);
        }
    }

    function init(numberOfImages, fn){
        numOfImages = numberOfImages;
        callBack = fn;
    }

    return {
        imageLoaded: imageLoaded,
        init: init
    };
}();

function tryDisplayMosaic(){
    alert('All images downloaded');
}

function generate(){
    // (1) Set the number of images that are to be loaded and (all tiles + penguin)
    // (2) Set the function that must be called after all images are loaded
    ImageLoader.init(tileData.length+1, tryDisplayMosaic);

    // Load this Penguins image
    var image = new Image();
    image.src = 'Penguins.jpg';
    image.onload = ImageLoader.imageLoaded;

    // Go through each image and load it
    for (var i=0; i<tileData.length; i++) {
        image = new Image();
        image.src = tileData[i].ImageUrl;
        image.onload = ImageLoader.imageLoaded;
    }
}

This code is totally different from your code, but it's a lot prettier in my opinion. It requires that you set the number of images that you need to load and a function that must be executed after all images are loaded.

Note: I haven't tested it, I'm not sure about this .call(this) part, maybe the forum could help if it is incorrect.

I have tested my code and it works fine. Additionally I've improved it, so you save a pointer to the loaded image inside the tileData array, see JSBin for a working example.