1

I want to compensate for the possibility of a user changing the window size that contains a canvas element. I had something along the lines of:

class RenderClass{
   constructor(){
     this.Canvas = document.getElementById("GameView").getContext("2d");
     this.CanvasWidth = this.Canvas.canvas.width;
     this.CanvasWidth = this.Canvas.canvas.height;
   }
}

I have a callback setup where whenever the window is resized, I do the following:

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

My problem is that that RenderClass.CanvasWidth and RenderClass.CanvasHeight does not equal to RenderClass.Canvas.canvas.width and RenderClass.Canvas.canvas.height when the window size has changed.

Is there a way to accomplish this without having code that explicitly makes these two values equal? I was under the impression almost everything in JavaScript is returned by reference so when I modified the actual Canvas object that CanvasWidth and CanvasHeight would automatically return those values?

ggorlen
  • 26,337
  • 5
  • 34
  • 50
  • Those assignments just copy primitives, they don't provide references into the object. `document.getElementById("GameView").getContext("2d");` returns a context, not a canvas. If you find yourself writing things like `RenderClass.Canvas.canvas.width`, you may want to rethink your design. – ggorlen May 26 '21 at 21:07

2 Answers2

0

While you have direct access to the canvas element you can get width and height from it directly. Just as you set them.

If you still need to keep CanvasWidth/CanvasHeight class properties and be sure they're in sync with actual canvas' dimenstions I'd propose to use getters/setters for that:

class RenderClass{
   constructor(id){
     this.Canvas = document.getElementById(id).getContext("2d");
   }
   
   get canvasWidth() {
     return this.Canvas.canvas.width
   }
   set canvasWidth(width) {
     this.Canvas.canvas.width = width
   }
   
   get canvasHeight() {
     return this.Canvas.canvas.height
   }
   set canvasHeight(height) {
     this.Canvas.canvas.height = height
   }
}

const render = new RenderClass("GameView")

window.addEventListener('resize', () => {
  render.canvasWidth = window.innerWidth
  render.canvasHeight = window.innerHeight

  console.log(render.canvasWidth, render.canvasHeight)
})
html, body, #GameView {
  width: 100%;
  height: 100%;
}

body {
  margin: 0;
}
<canvas id="GameView" />
aleksxor
  • 709
  • 4
  • 10
0

Those assignments just copy primitives, they don't provide references into the object.

You can (and should) minimize the problem down to:

const o = {foo: 42};
let {foo} = o;
foo = 600;
console.log(o); // what happened? I expected {foo: 600}...

Additionally, document.getElementById("GameView").getContext("2d"); returns a canvas context, not a canvas. It should be named as such, context. It so happens you can grab the canvas from the context but it's sort of awkward and invasive: RenderClass.Canvas.canvas.width (yikes...).

What's the point of this class exactly? If it's responsible for rendering, it shouldn't allow getters on canvas height and width -- those are properties of the canvas member, not the renderer. If you want to wrap the canvas class, write a class Canvas that has these getters and setters, then offer a getter into the underlying canvas on the Renderer class.

The syntax for setting width would be renderer.canvas.width = 42 rather than renderer.setWdith = 42.

The other option would be to completely hide the canvas as an implementation detail of a renderer. In that design, the canvas would not be exposed at all, so the height/width getters/setters would make sense. But then there'd not be a getter for the canvas exposed by the Renderer interface -- all interactions between the client and the underlying Renderer#canvas would be mediated by the Renderer interface.

Passing in an id to the constructor for RenderClass is odd. There's no reason to tie down the class to only work with a subset of HTML elements that happen to have ids. A renderer doesn't need to know about the document. Better is to pass in an arbitrary canvas element rather than baking getElementById into the constructor. Think of it like the ReactDOM.render call (if you're familiar with React), which lets the client specify the element. It'd be frustrating if React forced you to use an id and did the document traversal call under the hood -- we should be able to inject mocks into this.

As a final note, variables, including instance variables, should be named lowercase -- uppercase is for classes. CSS ids should be kebab case, #game-view, not PascalCase. It's poor style to name a class FooClass -- just Foo. The capital is enough to distinguish it as a class.

Playing along with the class design (right now, I don't think you need classes at all, but I assume you have bigger abstractions to build), here's a rewrite suggestion:

class Canvas {
  constructor(canvas) {
    this._canvas = canvas;
    this._context = canvas.getContext("2d");
  }
  
  get width() {
    return this._canvas.width;
  }
  set width(width) {
    this._canvas.width = width;
  }
  
  get height() {
    return this._canvas.height;
  }
  set height(height) {
    this._canvas.height = height;
  }
  
  // ...
}

class Renderer {
  constructor(cavas) {
    this._canvas = canvas;
  }
  
  get canvas() {
    return this._canvas;
  }
  set canvas(canvas) {
    this._canvas = canvas;
  }
  
  // ...
}

const canvas = new Canvas(document.querySelector("#game-view"));
const renderer = new Renderer(canvas);

const handleResize = () => {
  renderer.canvas.width = window.innerWidth;
  renderer.canvas.height = window.innerHeight;
  console.log(
    renderer.canvas.width,
    renderer.canvas.height
  );
};
window.addEventListener("resize", handleResize);
handleResize();
html, body, #game-view {
  width: 100%;
  height: 100%;
}

body {
  margin: 0;
}
<canvas id="game-view"></canvas>

Don't forget to debounce your resize calls.

ggorlen
  • 26,337
  • 5
  • 34
  • 50