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

get(x, y, w, h) output image width/height is doubled #6540

Closed
2 of 17 tasks
ffd8 opened this issue Nov 8, 2023 · 4 comments · Fixed by #6542
Closed
2 of 17 tasks

get(x, y, w, h) output image width/height is doubled #6540

ffd8 opened this issue Nov 8, 2023 · 4 comments · Fixed by #6542

Comments

@ffd8
Copy link
Contributor

ffd8 commented Nov 8, 2023

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

1.8.0

Web browser and version

119.0.6045.123

Operating System

MacOSX 12.5.1

Steps to reproduce this

Steps:

  1. set canvas to specific size, ie. createCanvas(400, 400
  2. fb = get(0, 0, width, height) to grab the whole canvas
  3. check size of of print(fb.width), returns 800...

I loooove using visual feedback in p5.js, using get() to grab the image at the end of the draw, and place it back at the top of the draw scaled or rotated... just tried loading some works i had made the past weeks after updating to 1.8.0 and nothing worked.. computer ran slow.. no feedback, weird.. tested code in v1.7.0 and voila, worked great again. Turns out there's a bug? in v1.8.0 for the get() function that's doubling the image size... probably has something to do with the pixelDensity or some new WEBGL(2?) aspect of the canvas?? I know by default when one says createCanvas(400, 400) it's actually 800,800 on a retina display unless using pixelDensity(1).. however it seems like a bug that the get() image size is that doubled size compared to that of the canvas. I think the 3rd example for get() in the docs even looks different than expected.. it's asking for half the image, but showing 1/4 of it due to the scaling issue. Change the demo code with the following two lines and the problem is clear:

let c = get(0, 0, width / 2, height / 2);
image(c, 0, 0);

Aaand digging into the render js, found the change that is causing it! Curious if/why or for what other function this needed to change...

Here's a basic example Example:
https://editor.p5js.org/ffd8/sketches/SUf60UZhg

Snippet:

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

function draw() {
  background(220);
  fb = get(0, 0, width, height)
  print(fb.width)
}```
@ffd8
Copy link
Contributor Author

ffd8 commented Nov 8, 2023

@davepagurek @Gaurav-1306 – curious if you also experience the issue mentioned above. I tracked down the commit that changed the get() along with similar changes for pixelDensity(), which came out of this issue.

@davepagurek
Copy link
Contributor

oh I bet it's because the next line is region._pixelDensity = pd; instead of region.pixelDensity(pd); -- the latter should divide the width and height back again:

p5.js/src/image/p5.Image.js

Lines 253 to 255 in 17304ce

// Adjust canvas dimensions based on pixel density
this.width /= density;
this.height /= density;

@Gaurav-1306
Copy link
Contributor

I understand the mistake there. Let me make a new commit. Will change according to Dave's suggestion.

@davepagurek
Copy link
Contributor

Hey @Gaurav-1306, I've got a PR opened here already: #6542 Could you give this one a review and see if it makes sense to you too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants