views:

249

answers:

5

Alright, I thought I had this whole setTimeout thing perfect but I seem to be horribly mistaken.

I'm using excanvas and javascript to draw a map of my home state, however the drawing procedure chokes the browser. Right now I'm forced to pander to IE6 because I'm in a big organisation, which is probably a large part of the slowness.

So what I thought I'd do is build a procedure called distributedDrawPolys (I'm probably using the wrong word there, so don't focus on the word distributed) which basically pops the polygons off of a global array in order to draw 50 of them at a time.

This is the method that pushes the polygons on to the global array and runs the setTimeout:

 for (var x = 0; x < polygon.length; x++) {
      coordsObject.push(polygon[x]);
      fifty++;
      if (fifty > 49) {
           timeOutID = setTimeout(distributedDrawPolys, 5000);
           fifty = 0;
      }
 }

I put an alert at the end of that method, it runs in practically a second.

The distributed method looks like:

 function distributedDrawPolys()
 {
      if (coordsObject.length > 0) {
           for (x = 0; x < 50; x++) { //Only do 50 polygons
                var polygon = coordsObject.pop();
                var coordinate = polygon.selectNodes("Coordinates/point");
                var zip = polygon.selectNodes("ZipCode");
                var rating = polygon.selectNodes("Score");
                if (zip[0].text.indexOf("HH") == -1) {
                     var lastOriginCoord = [];
                     for (var y = 0; y < coordinate.length; y++) {
                          var point = coordinate[y];
                          latitude = shiftLat(point.getAttribute("lat"));
                          longitude = shiftLong(point.getAttribute("long"));
                          if (y == 0) {
                               lastOriginCoord[0] = point.getAttribute("long");
                               lastOriginCoord[1] = point.getAttribute("lat");
                          }
                          if (y == 1) {
                               beginPoly(longitude, latitude);
                          }
                          if (y > 0) {
                               if (translateLongToX(longitude) > 0 && translateLongToX(longitude) < 800 && translateLatToY(latitude) > 0 && translateLatToY(latitude) < 600) {
                                    drawPolyPoint(longitude, latitude);
                               }
                          }
                     }
                     y = 0;
                     if (zip[0].text != targetZipCode) {
                          if (rating[0] != null) {
                               if (rating[0].text == "Excellent") {
                                    endPoly("rgb(0,153,0)");
                               }
                               else if (rating[0].text == "Good") {
                                    endPoly("rgb(153,204,102)");
                               }
                               else if (rating[0].text == "Average") {
                                    endPoly("rgb(255,255,153)");
                               }
                          }
                          else { endPoly("rgb(255,255,255)"); }
                     }
                     else {
                     endPoly("rgb(255,0,0)");
                     }
                }
           }
      }
 }

Edit: fixed the format

So I thought the setTimeout method would allow the site to draw the polygons in groups so the users would be able to interact with the page while it was still drawing. What am I doing wrong here?

+6  A: 

If your loop is running in under a second, all of your setTimeout calls will stack up trying to fire off about five seconds later.

If you want to give the browser breathing room for intermediate rendering, push all of your objects on the stack, then call the function with a limit, and have the function schedule itself when it's done that many objects. Semi-pseudocode:

var theArray = [];
var limit = 50;

function showStuff() {
    for (...) {
        // push objects on theArray
    }

    renderArrayInBatches();
}

function renderArrayInBatches() {
    var counter;

    for (counter = limit; counter > 0; --counter) {
        // pop an object and render it
    }
    if (theArray.length > 0) {
        setTimeout(renderArrayInBatches, 10);
    }
}

That builds the array all in one go, then triggers the first batch (up to limit) of rendering. At the end of the first batch, if there's more rendering to do, it schedules it to happen about 10ms later. In fact, it will happen no sooner than 10ms later and quite possibly later than that, if the browser is still busy with other things. (Re 10ms: most browsers won't schedule something for sooner than 10ms from now.) (Edit Andy E points out, quite correctly, that you may as well fold the logic related to what needs to be rendered into the rendering function directly rather than first building the array, then processing it. Doesn't change the above much except for the array part, how you do the chaining and the "breathing room" remains the same.)

Not knowing the excanvas stuff you're using, you may find you need to adjust the timeout time, but I tend to doubt it -- it's basically a "yield" operation which lets the browser do some things and come back to you.

Note that the psuedo-code sample above is using what appear to be globals. I wouldn't recommend actually using globals. You might even want to do this instead:

function showStuff() {
    var theArray = [];
    var limit = 50;

    for (...) {
        // push objects on theArray
    }

    renderArrayInBatches();

    function renderArrayInBatches() {
        var counter;

        for (counter = limit; counter > 0; --counter) {
            // pop an object and render it
        }
        if (theArray.length > 0) {
            setTimeout(renderArrayInBatches, 10);
        }
    }
}

...but I didn't like to complicate the main answer by introducing the closure (although technically both codeblocks involve closures).

T.J. Crowder
Nice answer. @C Bauer, gotta remember that setTimeout doesn't block. It just schedules a function to run at some time in the future, so the timeouts can stack up.
Jonathon
I was typing a similar answer, so +1. Another option would be to do away with the for loop and have each `distributedDrawPolys()` call set the timer for the next, 10ms apart or so.
Andy E
Hi TJ, I just wanted to say that I appreciate the amount of effort you put into this answer and, once I understood what I had confused myself about setTimeout in the first place (In part due to Andy E's comment), it made more sense. However, I wouldn't have really gotten there without jitter's example.
C Bauer
@C. Bauer: Happy to help!
T.J. Crowder
+1  A: 

Change the code to

for (var x = 0; x < polygon.length; x++) {
    coordsObject.push(polygon[x]);
}
distributedDrawPolys();

function distributedDrawPolys()
{
    if (coordsObject.length > 0) {
        for (x = 0; x < 50; x++) {
            ...
        }
        setTimeout("distributedDrawPolys()", 5000); //render next 50 polys in 5 sec
    }
}
jitter
Browser is still locking up when I run the code like this :/ (I also put the settimeout inside of the if statement)
C Bauer
Oops sorry. Forgot the quotations marks. Try again please
jitter
EDIT - NVM I'm an idiot! Thanks!!!
C Bauer
@jitter: You don't need, or want, to put the function in quotes. (Avoid all forms of `eval` when you can, including implicit ones.) The appropriate `setTimeout` would be: `setTimeout(distributedDrawPolys, 5000);` (note that it's not in quotes, and does *not* have the `()` after it -- it's the function reference we want, not the function's return value).
T.J. Crowder
**@C Bauer:** The setTimeout doesn't block, it only schedules a delayed execution. The difference in this example compared to yours, is that you schedule all the calls to start after 5 seconds, while **jitter** example function runs itself after 5 seconds. So it does one bunch, then schedules to run itself after 5 seconds for another bunch, etc.-.
awe
@jitter: Yes I know and understand, I was just answering to his first comment here about saying the browser locking up..
awe
A: 

This doesn't work as expected. By the time your first function starts executing, your global array already contains 50 elements. You just end up operating on the same data 50 times.

What you can do is to chain your setTimeout so that the next function executes after the previous method.

Here's a simple implementation:

var coordObj = [...]; //50 or whatever elements
(function() {
    if (coordObj.length === 0) return; //Guardian
    var obj = coordObj.pop(); //or .shift(), based on the order desired.
    doStuffWithCoordObj(obj);
    setTimeout(arguments.callee,0); //call this anonymous function after a timeout
})();
Chetan Sastry
The pop method actually removes the returned array object from the global whenever it's run, so the data does not get reused.
C Bauer
@C Bauer: Good catch, I didn't look inside that function. In any case, this isn't meant to be THE solution. It is just a pattern for doing long non-blocking UI updates.
Chetan Sastry
Updated with the right approach.
Chetan Sastry
+1  A: 

No, you want something different.

var batchsize = 50; 
function drawPolys(start) {
    for (var x = start; x < polygon.length; x++) {
        drawOnePolygon(polygon[x]);
        if (start + batchsize <= x) {
            // quit this invocation, and schedule the next
            setTimeout(function(){drawPolys(x+1);}, 400);
            return;  // important
        }
    }
}

then drawOnePolygon must be something like this:

function drawOnePolygon(p) {
    var coordinate = polygon.selectNodes("Coordinates/point");
    //...and so on, continuing from your code above.
}

kick it off with:

drawPolys(0); 
Cheeso
A: 

If you call it once every five seconds, and it does 1 second of work each time, the browser will be choked for interaction 20% of the time.

You could try and chop up your big function and run it in chunks to make the experience smoother.

npup