noSmooth() makes textures with NEAREST interpolation blurry

RandomGamingDev commented 1 year ago

Most appropriate sub-area of p5.js?

[ ] Accessibility
[ ] Color
[ ] Core/Environment/Rendering
[ ] Data
[ ] DOM
[ ] Events
[ ] Image
[ ] IO
[ ] Math
[ ] Typography
[ ] Utilities
[X] WebGL
[ ] Build Process
[ ] Unit Testing
[ ] Internalization
[ ] Friendly Errors
[ ] Other (specify if possible)

p5.js version

1.7.0

Web browser and version

115.0.5790.111

Operating System

Windows 10

Steps to reproduce this

Steps:

Create a small texture that you can tell is or isn't pixelated
Create the canvas and set it to noSmooth()
Set the interpolation to NEAREST

Snippet:


let img;
let canvas;

function preload() {
  img = loadImage('<pixelated image goes here>');
}

function setup() {
  canvas = createCanvas(400, 400, WEBGL);
  noSmooth(); // ironically the image only smooths/blurs if this is on

  canvas.getTexture(img).setInterpolation(NEAREST, NEAREST);
}

function draw() {
  background(0);
  image(img, -width / 2, -height / 2, width, height);
}

In case you don't want to copy and paste it here's the sketch: https://editor.p5js.org/PotatoBoy/sketches/vgaD3vTok

davepagurek commented 1 year ago

Some added context: It looks blurry for me in Chrome, but crisp in Firefox:

RandomGamingDev commented 1 year ago

Same for me after checking.

davepagurek commented 1 year ago

Oh I think I know what's going on here. noSmooth() for WebGL calls setAttributes('antialias', false) (which may or may not be the right thing to do, but that's an aside.) Setting attributes recreates the canvas and invalidates previous references to it. The recreation is hard to avoid, as far as I'm aware there isn't an API to change those other than in getContext on a new canvas. We can maybe still make the return value of createCanvas be aware of this and update itself internally though (see https://github.com/processing/p5.js/issues/5902).

For now, calling _renderer.getTexture(img).setInterpolation(NEAREST, NEAREST); works in Chrome, since _renderer is always a reference to the current canvas.

RandomGamingDev commented 1 year ago

Ah, alr

deveshidwivedi commented 9 months ago

Hi! I went through the existing discussions on this issue and similar ones. I was thinking if making these changes would solve the issue?

p5.prototype.noSmooth = function() {
  if (!this._renderer.isP3D) {
    if ('imageSmoothingEnabled' in this.drawingContext) {
      this.drawingContext.imageSmoothingEnabled = false;
    }
  } else {
    const currentRenderer = this._renderer;
    if (this._renderer !== currentRenderer) {
      this._renderer.parent = this;
      this._renderer.drawingContext = this.drawingContext;
    }
  }
  return this;
};

This would avoid canvas recreation, reassigning current renderer's parent and drawing context.

davepagurek commented 9 months ago

I think unfortunately if we need to change the attributes of the canvas, we need to recreate the canvas, since MDN says:

[getContext] returns a drawing context on the canvas, or null if [...] the canvas has already been set to a different context mode. [...] It is not possible to get a different drawing context object on a given canvas element.

...but that said, your idea of updating the properties of old renderer objects could be a good approach to fixing https://github.com/processing/p5.js/issues/5902! I would probably do it in setAttributes rather than noSmooth, since that method is called by noSmooth, and both have the same problem of invalidating references to renderers. What I'd maybe do is:

For each parent class (p5 or p5.Graphics), keep an array of past renderers
Whenever a new renderer is made from the parent (through setAttributes or even the initial createCanvas maybe):
- Update the elt property of all the old renderers to the new canvas element
- Update the drawingContext property
- For WebGL, update the GL property too (same as drawingContext)
- Then, add the new renderer to the list

That said, although this is necessary for setAttributes regardless (https://github.com/processing/p5.js/issues/5902), there's also the separate issue of whether or not noSmooth should turn off antialiasing or just change the texture filtering of whichever texture you use next. Anyone have any opinions on that?

processing / p5.js