When creating an image with createGraphics(), the image is corrupted if the size is specified as a decimal

[reona396]

trafficstars

Most appropriate sub-area of p5.js?

• [ ] Accessibility (Web Accessibility)
• [ ] Build tools and processes
• [ ] Color
• [ ] Core/Environment/Rendering
• [ ] Data
• [ ] DOM
• [ ] Events
• [ ] Friendly error system
• [X] Image
• [ ] IO (Input/Output)
• [ ] Localization
• [ ] Math
• [ ] Unit Testing
• [ ] Typography
• [ ] Utilities
• [ ] WebGL
• [ ] Other (specify if possible)

p5.js version

1.4.0

Web browser and version

102.0.5005.115 (Official Build) （x86_64）

Operating System

MacOSX 10.15.7

Steps to reproduce this

When creating an image with createGraphics(), the image is corrupted if the size is specified as a decimal.

Snippet:

Using the code in loadPixels() as a reference, I wrote code to repeat the image created in createGraphics(), up and down.

Normal case

let img;

function setup() {
  createCanvas(200, 200);

  img = createGraphics(width, height);
  img.background(252, 186, 3);
  img.textAlign(CENTER, CENTER);
  img.textSize(180);
  img.text("🦄", width / 2, height / 2);

  let d = img.pixelDensity();
  let halfImage = 4 * (img.width * d) * ((img.height * d) / 2);
  img.loadPixels();
  for (let i = 0; i < halfImage; i++) {
    img.pixels[i + halfImage] = img.pixels[i];
  }
  img.updatePixels();

  image(img, 0, 0);
}

Buggy case

If the size of createGraphics() is a decimal, the color and position of the image is bugged.

let img;

function setup() {
  createCanvas(200.5, 200.5);

  img = createGraphics(width, height);
  img.background(252, 186, 3);
  img.textAlign(CENTER, CENTER);
  img.textSize(180);
  img.text("🦄", width / 2, height / 2);

  let d = img.pixelDensity();
  let halfImage = 4 * (img.width * d) * ((img.height * d) / 2);
  img.loadPixels();
  for (let i = 0; i < halfImage; i++) {
    img.pixels[i + halfImage] = img.pixels[i];
  }
  img.updatePixels();

  image(img, 0, 0);
}

let img;

function setup() {
  createCanvas(200.9, 200.0);

  img = createGraphics(width, height);
  img.background(252, 186, 3);
  img.textAlign(CENTER, CENTER);
  img.textSize(180);
  img.text("🦄", width / 2, height / 2);

  let d = img.pixelDensity();
  let halfImage = 4 * (img.width * d) * ((img.height * d) / 2);
  img.loadPixels();
  for (let i = 0; i < halfImage; i++) {
    img.pixels[i + halfImage] = img.pixels[i];
  }
  img.updatePixels();

  image(img, 0, 0);
}

[limzykenneth]

I'm not sure if there will be an easy way to make canvas with fractional dimensions work without some kind of undefined behaviour as it is not expected that fractional dimensions would be used to define those dimensions (that I know of). Is there a reason for wanting to use fractional dimensions for defining canvas sizes?

P.S. I quite like the glitchy effect regardless 😝

[reona396]

I encountered this bug when I was multiplying a decimal to adjust the canvas size to a larger or smaller size. So, because I was matching that canvas size with the size of the image created in createGraphics(), a fraction was entered in the createGraphics() size specification.

[limzykenneth]

Could you floor() the values passed to createGraphics() so that they will always be integers?

[reona396]

If you use floor(), it works fine. As you said, it is an irregular case to specify a decimal number. Therefore, I think p5.js can use floor() internally to convert it to an integer in advance.

[davepagurek]

it is an irregular case to specify a decimal number

Just a thought, it might sometimes make sense if you set pixel density. e.g. if you set pixelDensity(2) and make a canvas that's 100.5x100.5, then the actual underlying canvas size is 201x201, which is valid. If we round anything, it should maybe be the dimensions of the internal canvas rather than the user-facing width and height

[limzykenneth]

I'm happy to go with @davepagurek suggestion and floor the internal canvas dimension. We'll need to take care to not cause inconsistency where elsewhere in the codebase there is still expectation of non-floored dimensions though (not sure how prevalent this is).

[xrcyz]

I'd like to have a crack at this, if it's okay with all?

[davepagurek]

Thanks @xrcyz! I've assigned this to you.

[xrcyz]

Looking for some feedback!

In the original ticket, the pixel translation and color offset is (I believe) expected behaviour. The canvas has an odd height after applying pixelDensity, so the index halfImage starts writing halfway through a row (and halfway through a pixel, if the width is also odd).

However, there is still weird behaviour if you do something like createCanvas(200,200.25); pixelDensity(1); because we end up with img.elt.height !== img.height * pixelDensity().

Since elt.height seems to be exclusively set in p5.Renderer.prototype.resize, this is my working solution:

p5.Renderer.prototype.resize = function(w, h) {
  this.elt.width = Math.floor(w * this._pInst._pixelDensity); //floor to a whole number
  this.elt.height = Math.floor(h * this._pInst._pixelDensity); //floor to a whole number
  this.width = this.elt.width / this._pInst._pixelDensity; //back-calculate width
  this.height = this.elt.height / this._pInst._pixelDensity; //back-calculate height
  this.elt.style.width = `${w}px`;
  this.elt.style.height = `${h}px`;
  if (this._isMainCanvas) {
    this._pInst._setProperty('width', this.width);
    this._pInst._setProperty('height', this.height);
  }
};

It requires p5.prototype.resizeCanvas and p5.Graphics to get their dimensions from this._renderer.

  this._renderer.resize(w, h);
  this.width = this._renderer.width;
  this.height = this._renderer.height;

Example with createGraphics:

let img;

function setup() {
  
  createCanvas(400, 400.5);

  noSmooth();
  background(0);

  img = createGraphics(30, 30.5); //img.height => 30.5; img.elt.height => 61
  img.pixelDensity(4);
  img.resizeCanvas(30, 30.25); //img.height => 30.25; img.elt.height => 121
  img.resizeCanvas(30, 30.77); //img.height => 30.75; img.elt.height => 123

  let d = img.pixelDensity();
  let halfImage = 4 * (img.width * d) * ((img.height * d) / 2);
  img.loadPixels();
  for (let i = 0; i < halfImage; i += 4) {
    
    let x = (i/4/d) % img.width; 

    img.pixels[i + 0] = 255 * i / halfImage;
    img.pixels[i + 1] = 255 * x / img.width;
    img.pixels[i + 2] = 0;
    img.pixels[i + 3] = 255
    
    img.pixels[i + 0 + halfImage] = img.pixels[i + 0];
    img.pixels[i + 1 + halfImage] = img.pixels[i + 1];
    img.pixels[i + 2 + halfImage] = img.pixels[i + 2];
    img.pixels[i + 3 + halfImage] = img.pixels[i + 3];
  }
  img.updatePixels();

  image(img, 0, 0, width, height);

  console.log("width: " + img.width);
  console.log("height: " + img.height);
  console.log("pixelDensity: " + d);
  console.log("img.elt.width: " + img.elt.width);
  console.log("img.elt.height: " + img.elt.height);
  console.log("pixels.length: " + img.pixels.length); 
  console.log("halfImage: " + halfImage);
  console.log("pixel index of halfImage: " + (halfImage / 4));

 console.assert(abs(img.width * img.height * d**2 * 4 - img.pixels.length) < 1E-5, "pixels.length != width * height * pixelDensity()**2 * 4"); 
}

Example with createCanvas:

let cnv; 

function setup() {
  
  cnv = createCanvas(400, 400.5); //height => 400.5; cnv.elt.height => 801
  pixelDensity(4);
  resizeCanvas(400,400.25); //img.height => 400.25; cnv.elt.height => 1601
  resizeCanvas(400,400.77); //img.height => 400.75; cnv.elt.height => 1603
  
  noSmooth();
  background(0);

  let d = pixelDensity();
  let halfImage = 4 * (width * d) * ((height * d) / 2);
  loadPixels();
  for (let i = 0; i < halfImage; i += 4) {
    
    let x = (i/4/d) % width; 

    pixels[i + 0] = 255 * i / halfImage;
    pixels[i + 1] = 255 * x / width;
    pixels[i + 2] = 0;
    pixels[i + 3] = 255
    
    pixels[i + 0 + halfImage] = pixels[i + 0];
    pixels[i + 1 + halfImage] = pixels[i + 1];
    pixels[i + 2 + halfImage] = pixels[i + 2];
    pixels[i + 3 + halfImage] = pixels[i + 3];
  }
  updatePixels();

  console.log("width: " + width);
  console.log("height: " + height);
  console.log("pixelDensity: " + d);
  console.log("img.elt.width: " + cnv.elt.width);
  console.log("img.elt.height: " + cnv.elt.height);
  console.log("pixels.length: " + pixels.length); 
  console.log("halfImage: " + halfImage);
  console.log("pixel index of halfImage: " + (halfImage / 4));

 console.assert(abs(width * height * d**2 * 4 - pixels.length) < 1E-5, "pixels.length != width * height * pixelDensity()**2 * 4"); 

}

A criticism of this approach is that you can end up with weird canvas heights of (for example) 100.3333333 if you set pixelDensity(3);resizeCanvas(100,100.5);

Is this an acceptable solution?

[davepagurek]

A criticism of this approach is that you can end up with weird canvas heights of (for example) 100.3333333 if you set pixelDensity(3);resizeCanvas(100,100.5);

To weigh the pros and cons:

Pros:

• If we don't do this, we might cause some math to be off when accessing pixels if they assume that width * pixelDensity() is the full width
• The values we report are more accurate to the underlying data, so it's clearer what's going on when trying to align to integer pixels

Cons:

• When you read back width after setting it, you could get a different number
• Any sketches have a loop like for (let i = 0; i < width; i++) might run into issues

My personal opinion is that your approach still makes the most sense, as debugging is probably easier with the more accurately reported values (and this all only applies when you set the size to a non-integer value, which I feel is probably not a common use case anyway.) Let me know if I've missed anything though!

[xrcyz]

Okay, had another play with this today.

A better approach might be to floor inputs to createCanvas, createGraphics and allow an optional pixelDensity parameter in resizeCanvas, to support edge cases like resizeCanvas(100.25, 100.25, pixelDensity = 4).

p5.prototype.resizeCanvas = function(w, h, d, noRedraw) {
    //..intro..

    //allow decimal dimensions when pixelDensity multiplies to an int
    const decimalWidth = d && (w % 1) !== 0 && (w * d) % 1 === 0;
    const decimalHeight = d && (h % 1) !== 0 && (h * d) % 1 === 0;
    this.width = decimalWidth ? w : Math.floor(w);
    this.height = decimalHeight ? h : Math.floor(h);
    this._renderer.resize(this.width, this.height);

    //..outro..
};

It is worth noting there are tripwires for the player whichever way you set it up: the player either has to floor the inputs to resizeCanvas, or get the width and height "outputs" from resizeCanvas. The former is more transparent in my opinion.

Since the crux of the issue is looping through pixels, this has got me wondering: can I use the Friendly Error System to throw loadPixels if the width or height are not integers?

My working test case:

let cnv;
let img;

const FLOOR_DIMS = true; //choose whether to floor the canvas dimensions

function setup() {
  cnv = createCanvas(400, 400);
  
  img = createGraphics(300.5, 300.5); 
  img.pixelDensity(1);
  writePixels(img.width, img.height); 
}


function draw()
{
    w = map(sin(frameCount / 100), -1, 1, 200, 400);
    if(FLOOR_DIMS) 
    { 
      w = floor(w); 
    }
    testResize(w, w);
    background(0);
    image(img, 0, 0);
}

function testResize(w, h) {

    img.resizeCanvas(w, h);

    //img width and height may not be the same as what you set!
    if(!FLOOR_DIMS)
    {
        w = img.width;
        h = img.height;
    }

    writePixels(w, h);
}

function writePixels(w, h)
{
    let d = img.pixelDensity();
    let halfHeight = floor(h * d / 2);
    let pixelsPerRow = w * d;
    let elementsPerPixel = 4;
    let halfImage = elementsPerPixel * pixelsPerRow * halfHeight;
    img.loadPixels();
    for (let i = 0; i < halfImage; i += 4) {
        
        let x = (i/d/elementsPerPixel) % w; 

        img.pixels[i + 0] = 255 * i / halfImage;
        img.pixels[i + 1] = 255 * x / w;
        img.pixels[i + 2] = 0;
        img.pixels[i + 3] = 255

        img.pixels[i + 0 + halfImage] = img.pixels[i + 0];
        img.pixels[i + 1 + halfImage] = img.pixels[i + 1];
        img.pixels[i + 2 + halfImage] = img.pixels[i + 2];
        img.pixels[i + 3 + halfImage] = img.pixels[i + 3];
    }
    img.updatePixels();

    console.assert(w * h * d**2 * 4 === img.pixels.length, "pixels.length != width * height * pixelDensity()**2 * 4"); 
}

[davepagurek]

A better approach might be to floor inputs to createCanvas, createGraphics and allow an optional pixelDensity parameter in resizeCanvas, to support edge cases like resizeCanvas(100.25, 100.25, pixelDensity = 4).

I like this approach, both for resizeCanvas and createCanvas, as it also lets you create the right size of underlying canvas in one go instead of doing effectively two resizes (first to get the target width/height, then a second time to get the right pixel density.)

How do you feel about an API like this, to preserve backwards compatibility and give us the ability to add more options in the future? You can either pass a boolean for noRedraw, or an options object, which would let you specify pixel density without noRedraw or vice versa:

resizeCanvas(width: number, height: number, noRedraw: boolean = false)
resizeCanvas(width: number, height: number, options: { noRedraw?: boolean; pixelDensity?: number })

Since the crux of the issue is looping through pixels, this has got me wondering: can I use the Friendly Error System to throw loadPixels if the width or height are not integers?

This is a great idea! Definitely a useful warning for newcomers.

[limzykenneth]

I might not be understanding this fully but I don't quite understand the pixelDensity parameter passed to resizeCanvas here, specifically why do we need it instead of using the pixelDensity() function (and the internal value) and does it overwite the pixel density value globally or just for the resized canvas?

In terms of API, using options object argument is not very common in p5.js, so while not to say we definitely should not use it here but just to consider familiarity of users that they may not be used to options object argument at all.

[davepagurek]

I might not be understanding this fully but I don't quite understand the pixelDensity parameter passed to resizeCanvas here, specifically why do we need it instead of using the pixelDensity() function (and the internal value) and does it overwite the pixel density value globally or just for the resized canvas?

I think the idea is that, if you're setting the size and pixel density at once, you would know what to round to immediately, instead of having to store the target width/height to use when calling pixelDensity. E.g. If you set width/height to 100.5, if you know immediately that the pixel density should be 2, you can use the provided size as-is. If you round the width/height first and only call pixelDensity(2) afterwards, then you might not end up with an underlying canvas size of 201. Storing the target size in a separate variable would allow us to get around that problem too.

In terms of API, using options object argument is not very common in p5.js, so while not to say we definitely should not use it here but just to consider familiarity of users that they may not be used to options object argument at all.

That's a good point! My worry is mostly that method signatures end up long and with many overloads that are hard to tell apart, but we can also avoid that if we opt for a solution that doesn't need more parameters passed in.