Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm just learning the basics of animating with Javascript and HTML5. I'm trying to create this scrolling logos element for my site, and I'm pretty sure that my code is very inefficient. Can anyone give me any tips on how to do a better job of what I'm trying to do here?

http://jsbin.com/anagug/1/edit

Thanks!

EDIT: Here is the code:

        var ctx;
        var count = 0;
        var xpos;
        var ypos;
        var revealTimer;

        var img = new Image();
        img.src = "http://www.firewalkcreative.com/2012/images/logos_1.png";

        function switchImage(){

            xpos = count * 150;
            ypos = 0;

            count++;
            ctx.globalAlpha = 0.1;
            revealTimer = setInterval(revealImage,100);

            if(count == 5)
                count = 0;

        }


        function revealImage() {
            ctx.drawImage(img, 0, 0, 840, 90, xpos, count, 840, 90);
            ctx.drawImage(img, 0, 0, 840, 90, xpos-900, 0, 840, 90);
            ctx.save();
            ctx.globalAlpha += 0.1;
            if (ctx.globalAlpha >= 1.0)
                clearInterval(revealTimer);
            ctx.restore();
        }

        function init() {

            ctx = document.getElementById("canvas").getContext("2d");
            setInterval(switchImage,3000);
        }
share|improve this question
3  
drawing objects is always going to be expensive. It would be much smoother to draw those elements before the timer function starts, so that you're not expending resources to draw every time the function fires every 5 seconds –  j-man86 Aug 16 '12 at 6:25
 
I don't see any scrolling, only fading. Anyway, it may be because I'm not experienced with it, but I wouldn't use canvas for this. –  RoToRa Aug 17 '12 at 13:09
add comment

migrated from stackoverflow.com Aug 16 '12 at 9:02

This question came from our site for professional and enthusiast programmers.

1 Answer

Always do a clearInterval() before doing a new setInterval() on the same variable. Doesn't matter if it has been cleared or not.

e.g.

clearInterval(revealImage);
revealTimer = setInterval(revealImage,100);

You're code has a bug where the revealTimer is being reset before being cleared.

Why do you need to reset it?

function init() {   
    ctx = document.getElementById("canvas").getContext("2d");
    setInterval(switchImage,3000);
    setInterval(revealImage,100);
}

Should work just as well.

share|improve this answer
add comment

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.