JavaScript - Settting property on the object in the image loading function, the property is not set after the external function

Question

Sometimes JavaScript does not make sense to me, consider the following code that generates a photomosaic based on x / y-plates. I am trying to set the .Done property to true as soon as each Mosaic image is loaded, but for some reason it is always false, 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 in the tile object is always false, even if it is explicitly set to true inside tileImage.onload = function() . And I can guarantee that this function is called because I placed an alert() call inside it. Right now, it is just stuck inside an infinite loop constantly calling tryDisplayMosaic() .

Also, if I placed a loop immediately before calling tryDisplayMosaic() , and in this loop I set .Done = true , then the .Done property will be true, and alert('All images downloaded'); called out.

Answer 1

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

 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 a tile inside a loop like this does not make it covered by the statement block of the loop body. The only thing that gives you scope in Javascript is the function body.

Answer 2

I see that you are creating closures inside loops, which is a common mistake . To avoid this, you can create a more general closure outside the loop that processes all loading events and performs the function after loading all images:

 // 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 completely different from your code, but in my opinion, it is much prettier. To do this, you need to set the number of downloaded images and the function that must be performed after all images have been downloaded.

Note. I have not tested it, I'm not sure about this part of .call(this) , maybe the forum can help if it is incorrect.

I tested my code and it works great. In addition, I improved it, so you keep a pointer to the loaded image inside the tileData array, see JSBin for an example.