Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

I was just wondering if there's a better way to do this? If I ever have to worry about earlier images not being loaded before the later images, i.e. image[3] being loaded after image[21]. Any information on this is greatly appreciated since I have to load about 100 images at the start:

for (var i = 0; i < 22; i++) {
   images[i] = new Image(); 
   images[i].src = whatever[i] + ".png"; 
    if (i == 21) {
        images[21].onload = function() {
            for (var j = 0; j < 22; j++) {
            context.drawImage( images[j], (test * 60), (240 + ((Math.floor(j * 0.25)) * 70)) );
            }
            test ++;
            if(test > 4) {
                test = 1;
            }
        }
    }
}

Any input is greatly appreciated, thanks in advance

share|improve this question
1  
Are you sure this works? Can we see a demo? – Joseph the Dreamer May 31 at 8:06

1 Answer

up vote 0 down vote accepted

Sprite!

I suggest you place the images in a single image, like a "sprite sheet". This means that you need to pre-assemble the images into one image, load that single image and draw it on the canvas.

That way:

  • You only load one image, thus reducing your http requests to the server to one only.
  • You draw to the canvas only once. No more loops, no more multiple drawImage.

Order in the court!

Loading images are async, thus the need for the event handler. This also means that the images will arrive in any order, and not necessarily in the order you called them down. In your case, the image at index 21 might come earlier than you'd expect, even before all others have loaded.

I suggest you add an onload to each image and have it check if all images have been loaded before doing anything with them. A simple way to do this is to increment a counter and check if the counter value matches the expected number.

When the count is equal to the expected length, run the code you want to run.

No aftershocks?

If the entire operation was just to load into the canvas the images, it's fine.

However, you aren't loading them for no reason now are you? You are loading them for something. I suggest you wrap this operation in a function, have it accept your url array, and a callback that runs when it finishes loading and rendering all your images on the canvas.

Something like:

loadImagesToCanvas([
  //urls
],function(renderedCanvas){
  //run when done
});

Math.floor() is slow and long

In some browsers, Math.floor() is a slow way to trim off the decimal places of a number. Try bitwise OR (|) instead:

var wholeNumber = decimalNumber | 0;

Unnecessary math

If some consecutive operations use some value resulting from some math operation, cache that value in a variable rather than doing the math again.

share|improve this answer
Wow thanks a lot for your thorough input! I can't believe I forgot about sprites. Thanks once again for all the different tips, really helpful! – Hate Names May 31 at 15:39

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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