2

I have a canvas element. It draws some lines, based on art vanishing points.

I'm trying to draw a house (for now its just a box) from a single vanishing point. The size of the box is dictated by a delta variable. If I change the value manually, it does this:

enter image description here

I wanted to have a slider that changes the delta variable. But I get some really weird effects. Namely lines are drawn out of frame to the right. I dumped console.log statements everywhere but I still cannot find the problem (how does one even debug canvas issues?)

enter image description here

var canvas = document.querySelector("canvas");

canvas.width = window.innerWidth;
canvas.height = window.innerHeight;

canvas.width = 1600;
canvas.height = 800;

var ct = canvas.getContext("2d");

// TODO
// 1. Make a center point
// 2. Draw lines jutting from center
// 3. Draw a line parallel to canvas bottom
// 4. Draw an adjoining item upward

// x, y
// right, down

// Nomenclature
// x0a
// coordinate type, vanishingPt#, endPtName

// Vanishing point 0
var x0 = 400;
var y0 = 400;

// Vanishing point end 0a
var x0a = 0;
var y0a = 2 * y0;

// Vanishing point end 0b
var x0b = 2 * x0;
var y0b = 2 * y0;

// Define delta
var delta = 700;

function init() {
  console.log(delta, "delta");
  console.log(x0b, "x0b");
  console.log(y0b, "y0b");
  console.log(x0, "x0");
  console.log(y0, "y0");
  // First Line
  ct.beginPath();
  ct.moveTo(x0, y0);
  ct.lineTo(x0a, y0a);
  ct.strokeStyle = 'red';
  ct.stroke();

  // Second Line
  ct.beginPath();
  ct.moveTo(x0, y0);
  ct.lineTo(x0b, x0b);
  ct.strokeStyle = 'green';
  ct.stroke();

  // House based on second Line
  ct.beginPath();
  ct.moveTo(x0b, y0b); // starting point
  ct.lineTo(x0b + delta, y0b); // right x+100
  ct.lineTo(x0b + delta, y0b - delta); // up y-100
  ct.lineTo(x0b, y0b - delta); // left x-100
  ct.lineTo(x0b, y0b); // down y+100
  ct.lineTo(x0b, y0b - delta); // back up y-100
  //calculate
  ct.lineTo(x0, y0);
  ct.lineTo(x0b + delta, y0b - delta);
  ct.strokeStyle = 'blue';
  ct.stroke();
}

init();

var slider = document.getElementById("myRange");

slider.oninput = function () {
  delta = this.value;
  requestAnimationFrame(init()); // redraw everything
}
body {
  background-color: lightgray;
  display: flex;
  justify-content: center;
  align-items: center;
}
.slideContainer {
  position: fixed;
  right: 30px;
  top: 30px;
  background-color: lightblue;
  z-index: 20;
}
canvas {
  border: 1px dotted red;
  padding: 80px;
  background-color: lightgray;
  transform: scale(0.5);
}
<!DOCTYPE html>
<html>

<head>
  <link rel="stylesheet" href="style.css" />
</head>

<body>
  <div class="wrapper">
    <div class="slideContainer">
      <input type="range" min="1" max="800" value="50" class="slider" id="myRange">
    </div>
    <canvas id="canvas"></canvas>
  </div>
  <script src="script.js"></script>
</body>

</html>
Vincent Tang
  • 2,648
  • 3
  • 28
  • 49

2 Answers2

2

Decouple render from input events

Your question as been answered but your question has some bad practice parts that need to be pointed out or they get copied.

oninput is a mouse move driven event.

Never call a render function or requestAnimationFrame from any events that are the result of mouse move events. Mouse move events on many devices can fire at rates much higher than the display rate (as much as 1000 times a second). The display can only display 1 frame every 60th second, drawing more will not be seen and can chew through the client's batteries.

If you call requestAnimationFrame from within a mouse driven input event you end up queuing many renders for the next display refresh, and as requestAnimationFrame tries to balance the load, it may queue renders for the next frame, thus the latest update can be up to 2 display frames late. Most frames will never be seen and you still chew up power.

Use a semaphore and a standard render loop that monitors the semaphore and redraws only when needed, and only once per frame. (see example)

Don`t scale down the canvas.

Unless you are transforming the canvas as part of an animation dont scale it down via the CSS rule transform: scale(0.5); (or any other scaling method) Rendering performance is all about pixels per second, if you half the size of the displayed canvas that means you need to render 4 times as many pixels, and use 4 times as much memory.

You can do the scaling via the canvas 2D API and will save the clients battery life, and increase performance, doing so.

Example

I have totally re-written the code, hopefully it will help. The two main points, Updates, and Scale are commented. Added code to use points rather than x,y coords as I am lazy.

requestAnimationFrame(update); // start anim loop

const ctx = canvas.getContext("2d");
const width = 1600;  // The ideal resolution
const height = 800;  // used to scale content
canvas.width = innerWidth;
canvas.height = innerHeight;


//Scales 2D context to always show the ideal resolution area
const scaleToFit = () => {  // sets canvas scale to fit content
    var scale = Math.min(canvas.width / width, canvas.height / height);
    ctx.setTransform(scale, 0, 0, scale, 0, 0);
}

var redraw = true;   // when true scene is redrawn ready for the next display refresh

// Working with points is easier
const point = (x = 0, y = 0) => ({x, y});
const pointCpy = (p, x = 0, y = 0) => ({x: p.x + x, y: p.y + y});
const scalePoint = (origin, point, scale) => {
    point.x = (point.x - origin.x) * scale + origin.x;
    point.y = (point.y - origin.y) * scale + origin.y;
};

const p1 = point(400,400);
const pA = point(p1.x, p1.y * 2);
const pB = point(p1.x * 2, p1.y * 2);

var delta = 50;

// the slider input event should not directly trigger a render
slider.addEventListener("input",(e) => {   
    delta = Number(e.target.value); 
    redraw = true;               // use a semaphore to indicate content needs to redraw.
});

function update() {  // this is the render loop it only draws when redraw is true
    if (redraw) {        // monitor semaphore
        redraw = false;  // clear semaphore
        ctx.setTransform(1,0,0,1,0,0);  // resets transform
        ctx.clearRect(0,0,ctx.canvas.width, ctx.canvas.height);
        scaleToFit();
        draw();
    }
    requestAnimationFrame(update);
}

// this was your function init()
function draw() {
    drawLine(p1, pA, "red");
    drawLine(p1, pB, "green");
    drawVBox(pB, delta, p1, "blue");
}


function drawVBox(p, size, vp, col, width) { // p is bottom left vp is vanish point
    ctx.strokeStyle = col;
    ctx.lineWidth = width;
    const p0 = pointCpy(p);           // get corners
    const p1 = pointCpy(p, size);      
    const p2 = pointCpy(p, size, -size);
    const p3 = pointCpy(p, 0, -size);
    drawPoly(col, width, p0, p1, p2, p3)

    ctx.beginPath();    // draw vanish lines 
    pathLine(p0, vp);
    pathLine(p1, vp);
    pathLine(p2, vp);
    pathLine(p3, vp);
    ctx.stroke();
    
    const scale = 1 - size / (800 * 2);
    scalePoint(vp, p0, scale);
    scalePoint(vp, p1, scale);
    scalePoint(vp, p2, scale);
    scalePoint(vp, p3, scale);
    drawPoly(col, width, p0, p1, p2, p3);
}   

// Use function to do common tasks and save your self a lot of typing
function drawLine(p1, p2, col, width = 1) { 
    ctx.strokeStyle = col;
    ctx.lineWidth = width;
    ctx.beginPath();
    ctx.lineTo(p1.x, p1.y);  // First point after beginPath can be lineTo
    ctx.lineTo(p2.x, p2.y);
    ctx.stroke();
}
function drawPoly(col,width, ...points) { 
    ctx.strokeStyle = col;
    ctx.lineWidth = width;
    ctx.beginPath();
    for(const p of points){
        ctx.lineTo(p.x, p.y);  // First point after beginPath can be lineTo
    }
    ctx.closePath(); // draw closing line
    ctx.stroke();
}
function pathLine(p1, p2) { 
    ctx.moveTo(p1.x, p1.y);  
    ctx.lineTo(p2.x, p2.y);
}
canvas {

  position : absolute;
  top: 0px;
  left: 0px;
  background-color: lightgray;
  z-index: -20;
}
<canvas id="canvas"></canvas>
<input type="range" min="1" max="800" value="50" id="slider">
<code id="info"></code>
Community
  • 1
  • 1
Blindman67
  • 41,565
  • 7
  • 47
  • 102
  • wow thanks so much for the codereview :). I'm still a beginner at using `` so this was very helpful – Vincent Tang Nov 26 '18 at 00:05
  • That will obviously depend on the type of application, but for the ones that only do draw in response to user events, keeping an rAF loop active is probably not the best direction either ;-) You might want to *throttle* your event handlers instead, so that your tab can actually be free of computation when not interacting with it. Here is an old and basic implementation of a [rAF based throttler](https://stackoverflow.com/a/38943316/3702797) – Kaiido Nov 26 '18 at 15:05
  • @Kaiido frames are call as soon as possible after the display refresh, How do you stop renders in the time after the frame and before the next display refresh? Even the most basic render `ctx.clearRect(0,0,1,1)` takes 2000 time longer than `if(redraw){ ...}` if `redraw` is `false` Your throttle on average lets through 1 extra render per frame (rounding down and on slow machine) when moving the mouse, so in one second of move events your throttle generates unneeded renders consuming the equivalent of half an hour idle frame calls or 1 render = 33 seconds of rAF non renders – Blindman67 Nov 26 '18 at 17:19
  • @Blindman67 I think you forgot rAF itself in your calculations ;-) even if internal code, it is far from being a noop. And I don't see where you see the extra render, throttling causes one lost render on average (we are always on the late frame) – Kaiido Nov 26 '18 at 22:30
  • @Your throttle lets extra rAF calls in the time between rAF exit, and the next display refresh, which could be as long as 15+ms. Seq.. Display refresh, then mouse event requests frame, Frame gets called immediately after event exits and clears the active flag and exits, Then another mouse event fires, and requests another frame, that gets called after event exits. thats two renders in less than 16ms. How do you stop renders in the time after the frame and before the next display refresh? – Blindman67 Nov 26 '18 at 23:01
  • Ah I think I see where your misconception comes from. No, rAF doesn't execute the callback ASAP, rAF has its own task in [the event loop](https://www.w3.org/TR/html51/webappapis.html#event-loops-processing-model). That task is itself called only when required. [Live demo](https://jsfiddle.net/05ch3weu/2). Now, having a non empty *animation callback list* makes the browser think your tab is fully active unless fully hidden, and thus it will run the event loop at normal rate plus enter the rendering tasks @60Hz, which is not just an `if(false)` task ;-) – Kaiido Nov 27 '18 at 00:40
  • Thinking about it once more, if it was working the way you thought, how would the simple fn=>rAF(fn) fire at 60 fps? If it was called immediately at the end of the event loop then a new rAF would get added before the next screen refresh, itself called immediately and adding a new one etc. just like the crazyLoop in my fiddle, however it's not how it works. Besides, cancelAnimationFrame would be pretty useless if we had to call it from the same event loop. – Kaiido Nov 27 '18 at 14:52
  • @Kaiido Found out why you are not seeing what used to happen, Seams that Chrome and FF both no longer fire mousemoves at rates higher than 1/60th (in line with pointer and touch events). I only know this now via experiment with Chrome never letting more than one mouse move per frame (between rAF calls), and FF very occasionally letting past a second mouse move (after display refresh and before frame request, thus counts 2, with prev frame counting 0) When this happened I do not know. Which explains why some of my apps have lost mouse move direction precision. – Blindman67 Nov 29 '18 at 14:39
  • @Blindman67 no. Read my last comment. What would make the normal rAF loop wait but not others? And in my fiddle the crazyLoop enters every event loop (didn't measure, but hundreds+ a frame), and if you happen to click on "batch" it will fire recursively 1000 times more per event loop. So my throttler works :-) This has nothing to do with mouse event rate being threshold by the UA... And anyway, the main point is that having a non empty animation callbacks list marks your document as animated and thus active and thus unpausable by the browser, whoch is not just an if(false) for device batteries – Kaiido Nov 29 '18 at 23:51
  • @Kaiido Calling a rAf from within a rAF callback's execution context forces the next call back to be on the next frame. Calling rAF outside a rAF callback's execution context, or calling rAF more than once in a rAF callback will result in 2 or more callbacks being called per frame. It is designed to allow multiple independent rAF loops to run together at 60fps. And no there is no compositing if there is no change to the backbuffers (dirty flag) (use dev tools to see). Adding a single UI event to the page will chew more power than an empty rAF loop. – Blindman67 Nov 30 '18 at 02:16
  • What you say makes no sense... What kind of magic would there be in rAF callbacks executor that would say here I should wait here I should not. And I didn't say they'd go to actually paint the frame (what the dev-tools you're talking about shows) but they still keep the whole event loop alive just because the document is marked as animated. Please read the specs, and check the Performance tab of your dev-tools. Having a continuous loop makes continuous work for the UA. Having nothing makes nothing. A browser is not just a js executor... – Kaiido Nov 30 '18 at 02:43
  • And even if we did assume there is such a magic, how would you make the cancelAnimationFrame work with your logic? Do you think they also save the state before executing the code in the callback just in case so they can magically restore it? No, the truth is there is a special task in the event loop that UAs are allowed to visit only at a certain frame rate, and only if they think they have to (.7 [here](https://www.w3.org/TR/html51/webappapis.html#event-loops-processing-model)) and that this task will itself call the sub-task of executing the rAF callbacks. No magics. – Kaiido Nov 30 '18 at 02:46
  • And actually even though they don't have to, my Chrome 70 does update + composite layers every frame when rAF is set. https://jsfiddle.net/L890zep6/ – Kaiido Nov 30 '18 at 03:06
  • @Kaiido Not magic, its called execution context, it is how rAF knows when to call the callback.. `cancelAnimationFrame` works the same as cancel any timer, supply the handle (you do know rAF has a return, why would that be?). And mouse event keeps UI events alive, which is a great deal more complicated than rAF. As for saved state I have no clue what you are talking Try this `(function A(){requestAnimationFrame(A);requestAnimationFrame(A)})()` – Blindman67 Nov 30 '18 at 03:09
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/184503/discussion-between-kaiido-and-blindman67). – Kaiido Nov 30 '18 at 03:11
  • @Kaiido In a fiddle!! please!?!, the fiddle code you run is heavily modified, the page is full of events, and there are 10s of thousands of lines of JS running,. Write a simple HTML doc and test that. – Blindman67 Nov 30 '18 at 03:15
  • Still you can see that when the rAF loop is off there is nothing reported in the perf records, and that there are js exec + composite + update when on. – Kaiido Nov 30 '18 at 03:45
1

After fixing the Syntax Error of calling requestAnimationFrame(init()) instead of requestAnimationFrame(init), note the (), all that remains is to coerce your HTMLInput's value to a Number so you don't end up doing "800" + 150 which results in "800150".

myRange.oninput = function() {
  console.log(this.value + 150);
}
<input type="range" min="1" max="800" value="50" class="slider" id="myRange">

var canvas = document.querySelector("canvas");

canvas.width = window.innerWidth;
canvas.height = window.innerHeight;

canvas.width = 1600;
canvas.height = 800;

var ct = canvas.getContext("2d");

// TODO
// 1. Make a center point
// 2. Draw lines jutting from center
// 3. Draw a line parallel to canvas bottom
// 4. Draw an adjoining item upward

// x, y
// right, down

// Nomenclature
// x0a
// coordinate type, vanishingPt#, endPtName

// Vanishing point 0
var x0 = 400;
var y0 = 400;

// Vanishing point end 0a
var x0a = 0;
var y0a = 2 * y0;

// Vanishing point end 0b
var x0b = 2 * x0;
var y0b = 2 * y0;

// Define delta
var delta = 700;

function init() {
  console.log(delta, "delta");
  console.log(x0b, "x0b");
  console.log(y0b, "y0b");
  console.log(x0, "x0");
  console.log(y0, "y0");
  // First Line
  ct.beginPath();
  ct.moveTo(x0, y0);
  ct.lineTo(x0a, y0a);
  ct.strokeStyle = 'red';
  ct.stroke();

  // Second Line
  ct.beginPath();
  ct.moveTo(x0, y0);
  ct.lineTo(x0b, x0b);
  ct.strokeStyle = 'green';
  ct.stroke();

  // House based on second Line
  ct.beginPath();
  ct.moveTo(x0b, y0b); // starting point
  ct.lineTo(x0b + delta, y0b); // right x+100
  ct.lineTo(x0b + delta, y0b - delta); // up y-100
  ct.lineTo(x0b, y0b - delta); // left x-100
  ct.lineTo(x0b, y0b); // down y+100
  ct.lineTo(x0b, y0b - delta); // back up y-100
  //calculate
  ct.lineTo(x0, y0);
  ct.lineTo(x0b + delta, y0b - delta);
  ct.strokeStyle = 'blue';
  ct.stroke();
}

init();

var slider = document.getElementById("myRange");

slider.oninput = function () {
  // coerce to Number
  delta = +this.value;
  requestAnimationFrame(init); // redraw everything
}
body {
  background-color: lightgray;
  display: flex;
  justify-content: center;
  align-items: center;
}
.slideContainer {
  position: fixed;
  right: 30px;
  top: 30px;
  background-color: lightblue;
  z-index: 20;
}
canvas {
  border: 1px dotted red;
  padding: 80px;
  background-color: lightgray;
  transform: scale(0.5);
}
<div class="wrapper">
    <div class="slideContainer">
      <input type="range" min="1" max="800" value="50" class="slider" id="myRange">
    </div>
    <canvas id="canvas"></canvas>
  </div>
Kaiido
  • 87,051
  • 7
  • 143
  • 194