current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 2 down vote favorite

The following is a code written in Javascript. It let's me change the appearance of a canvas depending on user input. The question is, how can I make this code more CPU efficient? I ask that because this code is just barely snappy enough on the iPad 2 and any improvement in speed would be really great to have.

With speed I am mainly focusing on that loop that iterates thousands of times (right now 3.600).

function repeat(){
//pc, input_mouse_x/y becomes input x/y.
if(device_type==0){
    ix=input_mouse_x;
    iy=input_mouse_y;
    //if pc's left mouse button isn't clicked then this isn't a valid input.
    if(input_mouse_button_left==0)  {ix='-';iy='-';}
    }
//ipad, input_touch becomes x/y.
if(device_type==1){
    ix=input_touch[0][0];
    iy=input_touch[0][1];
    //iy=trigonometry.flip({'number':iy,'around':screen_center});
    iy+=(screen_center-iy)*2;
    }

var pencil_x=ix;
var pencil_y=iy; pencil_y+=(screen_center-pencil_y)*2;
var pencil_width=30;
var pencil_height=30;
var data_quantity=(pencil_width*pencil_height*4);
var pencil_width_half=(pencil_width/2);
var pencil_height_half=(pencil_height/2);
var pencil_begin_x=(pencil_x-pencil_width_half);
var pencil_begin_y=(pencil_y-pencil_height_half);
var image_data=drawing_board_canvas_twod.getImageData(pencil_begin_x,pencil_begin_y,pencil_width,pencil_height);

//everything in this loop must be as cpu efficient as possible.
for(var cdata=3;cdata<3600;cdata+=4){
    //data to pixel, and then to x/y.
    var cpixel=Math.floor(cdata/4);
    var cx=(cpixel%pencil_width);
    var cy=parseInt(cpixel/pencil_width);

    //find distance to center of pencil.
    var to_center_x=Math.abs(pencil_width_half-cx);
    var to_center_y=Math.abs(pencil_height_half-cy);
    var to_center=Math.pow((Math.pow(to_center_x,2)+Math.pow(to_center_y,2)), 0.5);

    if(to_center<17){
        image_data.data[cdata]=0;//the higher the denser.
        }
    }
drawing_board_canvas_twod.putImageData(image_data,pencil_begin_x,pencil_begin_y);
}
share|improve this question

1 Answer 1

up vote 3 down vote accepted

The short answer is that you can't, not really anyways. You have no real control over how this code will be compiled. Since you mention iPad, the code is likely to be JIT-compiled to bytecode, prior to execution. How this is done depends (in part) on how the code is written, and how JS is implemented. The second factor, the implementation, is beyond your control.

You're also working with a canvas element. That's not JS that causes the bottleneck there, that's the DOM, which is gouverned by W3C, and is not part of ECMAScript in any way. JS merely has an API, via which it can request the DOM to change state. The DOM API isn't all that good: it's badly designed, slow, counter intuitive and one of the main reasons why many people claim JS is a terrible language. JS isn't, it's the DOM. But I'm going off course.

Inside that loop, which repeats 3600 times, you're calling a function which isn't part of your post (perhaps provide the source for that). But my guess is, that this function interacts with the dom (perhaps moves something about). If so: request a repaint, using the requestAnimationFrame method, it makes a huge difference!

After that, to further optimize your code, use jslint. As the page says, it will hurt your feelings, but it's the best way to teach yourself some good practices, like not declaring 10001 vars seperatly, but using:

var i, j, another = 123, someArray = [], andAnObject = {};

The main bottleneck, however is putImageData. this JSPerf demonstrates an alternative, up to x5000 more performant alternative!
It uses a neat little trick to buffer the states: it draws from one canvas onto the other, which is a lot faster. As is loading the image, then using:

drawing_board_canvas_twod.getContext('2d').drawImage(image_data, pencil_begin_x, pencil_begin_y);

But the way the image_data is gotten in your code still relies on the horrid getImageData, which is outpaced even by getting the img data pixel-by-pixed. Perhaps refer to this question on StackOverflow and all linked pages for details...

For now, the next to last thing I'd suggest is: look into how you call this repeat function of yours. If it's called in response to an event, check to see if you can't, perhaps, delegate that event, eliminating any excess event listeners is always good for performance. Are you using libs like jQuery? Get rid of them if you can, because they are slower than VanillaJS, if you know how to write good vanillaJS, of course.

The last, but certainly not least important thing I'd suggest: never ever use globals. Globals are risky, slow, and make for hellish debugging: your code relies on global variables, because the repeat function starts of with:

if(device_type==0)

device_type is not declared in the scope of repeat, so that implies it being a global variable. Learn about closures, scopes and how to use them. It's what makes JS worth your while.

share|improve this answer
    
putImageData is a built in canvas function. w3schools.com/tags/canvas_putimagedata.asp –  Hermann Ingjaldsson Aug 7 '13 at 14:46
    
@HermannIngjaldsson: Good point, mind you w3schools is a bad place for info, use MDN instead, as w3fools.com pointed out, w3schools is often lacking, or even providing dead wrong information. Avoid until fixed! –  Elias Van Ootegem Aug 7 '13 at 14:55
    
In the 3600 function, the only functions I'm calling are parseInt(developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/…) and Math functions, i don't think they mess with the DOM. –  Hermann Ingjaldsson Aug 7 '13 at 15:03
1  
That doesn't matter, you're using 'image_data' which is a reference. Besides, a loop of 3600 times calling core functions isn't likely to be the bottleneck, a single DOM call like yours can easily take twice as long... –  Elias Van Ootegem Aug 7 '13 at 17:51

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.