Javascript create images on canvas in loop

Question

I am Looping through my Images and try to put all in my canvas but I get this error:

My Html:

<div class="container-fluid">

    <canvas id="signature-canvas" class="signature-canvas"></canvas>

</div>

signatures.js:121 Uncaught TypeError: Failed to execute 'drawImage' on 'CanvasRenderingContext2D': The provided value is not of type '(CSSImageValue or HTMLImageElement or SVGImageElement or HTMLVideoElement or HTMLCanvasElement or ImageBitmap or OffscreenCanvas)' at Image.images.(anonymous function).onload (http://localhost/app/public/js/signatures.js:121:25)

my javascript

var canvasModule = {

    elements: {
        canvas: document.getElementById('signature-canvas')
    },

    render: function () {
        this.createImageOnCanvas();
    },

    getCanvasContext: function () {
        return this.elements.canvas.getContext('2d');
    },

    getCanvasWidth: function () {
        return this.elements.canvas.width;
    },

    getCanvasHeight: function () {
        return this.elements.canvas.height;
    },

    createImageOnCanvas: function () {

        var itemWidth = this.getCanvasWidth() / 7;
        var itemHeight = this.getCanvasHeight() / 3;

        var u = 0;
        var context = this.getCanvasContext();
        var images = [];
        var imagesWidth = [];
        var imagesHeight = [];

        for (var i = 1; i < 8; i++) {
            images[i] = new Image();

            images[i].src = 'images/sample/1'+i+'.png';

            imagesWidth[i] = images[i].width;
            imagesHeight[i] = images[i].height;

            images[i].onload = function () {
                context.drawImage(images[i], u, 0, itemWidth, itemHeight);
            }

            u = u + itemWidth;
        }

    }

}


$(document).ready(canvasModule.render());

I got your script working when image is not defined using an array, maybe the onload is not working propperly with an array? https://codepen.io/anon/pen/vpZpXY?editors=0010 — , Jan 02 '18 at 11:42
Line 29: function this.getCanvasContext() doesn't exist, I replaced this with this.getCanvas(). — , Jan 02 '18 at 11:43
@JoostMeijer the function does exist, so your script works now? — Jaan, Jan 02 '18 at 11:51
Oh you're right, I accidently removed a part copying to my editor. But yes the script works, look at the codepen.io. — , Jan 02 '18 at 11:59
Rule of thumb: never create a function inside a loop. This is a classic case of variable hoisting. In your case, `var i` is hoisted for callbacks. [please read this](http://javascriptissexy.com/javascript-variable-scope-and-hoisting-explained/) — VikDev, Jan 02 '18 at 12:10

score 1 · Accepted Answer · answered Jan 02 '18 at 12:04

1

You have a problem here:

 images[i].onload = function () {           
     context.drawImage(images[i], u, 0, itemWidth, itemHeight);
 }

because i is just a var which is hoisted to the top of the function and whose value will be incorrect by the time the callback is invoked, instead of a let variable.

Your u variable for setting the horizontal offset suffers from the same problem.

If your JS environment doesn't support let you can use this instead:

images[i].onload = (function(i, u) {
     context.drawImage(images[i], u, 0, itemWidth, itemHeight);
}).bind(this, i, u);

answered Jan 02 '18 at 12:04

Alnitak

334,560
70
407
495

better use `this`. – Kaiido Jan 02 '18 at 12:08
@Kaiido what do you mean? The `.bind` call already preserves `this`. – Alnitak Jan 02 '18 at 12:11
Ok this worked, but all Images are stacked now instead of being next to each other any idea how to solve this? – Jaan Jan 02 '18 at 12:12
@Jaan did you use my updated answer where I pass `u` into the callback as well? – Alnitak Jan 02 '18 at 12:13
@Alnitak yes I used that one somehow it does not increment? – Jaan Jan 02 '18 at 12:14
I mean using `drawImage(this, ...` is always clearer than `drawImage(images[i], ...).bind(...)` – Kaiido Jan 02 '18 at 12:15
@Jaan Have you used `let` or `.bind`? If the former, you have to declare `let u = 0` as well. – Alnitak Jan 02 '18 at 12:15
@Kaiido true, except in this case the OP needs to pass another variable as well. An IIFE would also work, although `.bind` is IMHO cleaner. – Alnitak Jan 02 '18 at 12:16
Nevermind it was my fault it works nice – Jaan Jan 02 '18 at 12:16
Well using `this` still avoids the bind of `i`, but anyway, if it were my code, I would split all this in two steps: load all, draw all. Only two declared functions and no hoisting nightmare, `u` would simply become `i * itemWidth` where `i` is the iterator of the draw all func. – Kaiido Jan 02 '18 at 12:18
@Alnitak btw do you have an idea why is Image Quality is so bad in my script? – Jaan Jan 02 '18 at 15:17
@Jaan I expect you've got a mismatch between the CSS width and height of the image elements compared to the actual onscreen size (which AFAICR defaults to 300x300) – Alnitak Jan 02 '18 at 16:25

Javascript create images on canvas in loop

1 Answers1