Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support shader-based filters after resizing/changing pixel density #6399

Closed
1 of 17 tasks
davepagurek opened this issue Sep 6, 2023 · 6 comments · Fixed by #6480
Closed
1 of 17 tasks

Support shader-based filters after resizing/changing pixel density #6399

davepagurek opened this issue Sep 6, 2023 · 6 comments · Fixed by #6480

Comments

@davepagurek
Copy link
Contributor

Most appropriate sub-area of p5.js?

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

p5.js version

main branch

Web browser and version

Firefox 117

Operating System

MacOS 12.5.1

Steps to reproduce this

Note: this is a follow up to the conversation on #6324

The feature works great so far! This is just another edge case that we can try to clean up for more robustness.

Steps:

  1. Create a canvas and apply a filter without using CPU mode
  2. Resize the canvas
  3. Apply another filter without using CPU mode

It looks like only the size of the previous canvas is getting rendered, and rendering appears to break. In this example, the top left square is the size of the canvas pre-resize:
image

Snippet:

createCanvas(20, 20)

clear()
circle(50,50,50)
filter(BLUR, 3)

resizeCanvas(100, 100)

clear()
circle(50,50,50)
filter(BLUR, 3) // works if you add `false` as a 3rd argument

Live: https://editor.p5js.org/davepagurek/sketches/EwgqqaiDI

@davepagurek davepagurek added the Bug label Sep 6, 2023
@davepagurek davepagurek changed the title Support shader-based filters after resizing Support shader-based filters after resizing/changing pixel density Sep 6, 2023
@davepagurek
Copy link
Contributor Author

I believe changing the pixel density will also have similar effects, so I'm updating the title of the issue to include that too.

@aferriss
Copy link
Contributor

aferriss commented Sep 6, 2023

Good catch! @wong-justin do you want to take a look at this?

@wong-justin
Copy link
Contributor

Yes I'll have a look. Thanks @davepagurek for catching it

wong-justin added a commit to wong-justin/p5.js that referenced this issue Sep 8, 2023
@davepagurek davepagurek added this to In Progress in p5.js WebGL Projects Sep 19, 2023
@davepagurek davepagurek mentioned this issue Oct 12, 2023
7 tasks
@davepagurek
Copy link
Contributor Author

Currently, we create a new filter shader graphic if one does not yet exist here, matching the main canvas's size and density:

if (!this.filterGraphicsLayer) {
// the real _pInst is buried when this is a secondary p5.Graphics
const pInst =
this._pInst instanceof p5.Graphics ? this._pInst._pInst : this._pInst;
// create secondary layer
this.filterGraphicsLayer =
new p5.Graphics(
this.width,
this.height,
constants.WEBGL,
pInst
);
// geometries/borders on this layer should always be invisible
this.filterGraphicsLayer.noStroke();
}

However, once it has been made, we never check if it needs to be resized. I think to fix this, we can add an else branch to that if, and inside it, check if the width/height and pixel density still match, and if not, call this.filterGraphicsLayer.resizeCanvas(this.width, this.height) and this.filterGraphicsLayer.pixelDensity(this.pixelDensity()).

@Gaurav-1306
Copy link
Contributor

@davepagurek

if (!this.filterGraphicsLayer) { 
   // the real _pInst is buried when this is a secondary p5.Graphics 
   const pInst = this._pInst instanceof p5.Graphics ? this._pInst._pInst : this._pInst; 
  
   // create secondary layer 
   this.filterGraphicsLayer = new p5.Graphics( 
       this.width, 
       this.height, 
       constants.WEBGL, 
       pInst 
   ); 
   // geometries/borders on this layer should always be invisible 
   this.filterGraphicsLayer.noStroke(); 
} else {
   // Check if dimensions or pixel density need to be resized
   if (
       this.filterGraphicsLayer.width !== this.width ||
       this.filterGraphicsLayer.height !== this.height ||
       this.filterGraphicsLayer.pixelDensity() !== this.pixelDensity()
   ) {
       // Resize the graphics layer
       this.filterGraphicsLayer.resizeCanvas(this.width, this.height);
       this.filterGraphicsLayer.pixelDensity(this.pixelDensity());
   }
}

do you think this looks good. I think we can have the else and if merged into one and using else if. what do you think?

@davepagurek
Copy link
Contributor Author

Thanks, @Gaurav-1306, that looks good!

I think we can have the else and if merged into one and using else if.

Agreed, that works!

Gaurav-1306 added a commit to Gaurav-1306/p5.js that referenced this issue Oct 17, 2023
davepagurek added a commit that referenced this issue Oct 17, 2023
@davepagurek davepagurek moved this from In Progress to DONE! 🎉 in p5.js WebGL Projects Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: DONE! 🎉
p5.js WebGL Projects
  
DONE! 🎉
Development

Successfully merging a pull request may close this issue.

4 participants