Account for pixel density when masking images

[Papershine]

Resolves #6770

Changes:

Previously, image pixel densities were hard coded to be 1, but when image pixel densities were changed to be variable, the mask function was not updated, resulting in incorrectly sized images being masked. By taking into account the pixel density when calculating the actual pixel width and height for masking, the issue should be fixed.

Screenshots of the change:

On a high pixel density display, the previous behaviour of the code snippet in #6770 has been restored:

PR Checklist

• npm run lint passes
• Unit tests are included / updated

[welcome]

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page!

[Papershine]

Potential changes needed:

• If we try to mask two images with different pixel densities, things don't seem to work well currently (which makes sense). Potentially, we might want to upscale an image if we see that the pixel densities of the two images are different?

• May also want to add a unit test to test non-1 pixel densities - please let me know if that is necessary, thanks!

[davepagurek]

Thanks for taking this on! I think those two ideas you mentioned would be great, and a test will help make sure we don't accidentally break this again in the future. We have a new visual test system (https://github.com/processing/p5.js/blob/main/contributor_docs/unit_testing.md#visual-tests), maybe that could be a good fit for a test for this?

[davepagurek]

p5.Image also has a pixel density too now, maybe we can add a branch to handle when the input is an image too?

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

Line 150 in 37d3324

this._pixelDensity = 1;

[Papershine]

I'm trying to set up and test masking when manually changing the pixel density, but things keep bugging out. Looking into the code for pixelDensity, I think there are some problems here

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

Lines 256 to 260 in 304ee90

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

If the original pixel density is 2, and we want to make it 4, it seems setting pixelDensity(4) would break the correct width and height of the image. Is it something that should be fixed?

[davepagurek]

I think the idea is that when you import e.g. an 800x800 image from a file, we have no idea what the density is, so we assume 1x density. But if we tell p5 that actually it should be treated like a 400x400 image at 2x density, then it will change its width and height to accommodate. So in this code path, we don't want to change any of the underlying pixels, just reinterpret it.

I'll be different if you create it from a p5.Graphics though: for that, since it's a canvas that you draw to, we give you a new blank canvas at the size you ask for. So if you start with an 800x800 canvas and then tell it to use a pixel density of 2, under the hood, you get 1600x1600 pixels under the hood. Then if you call .get() on that canvas, you should also end up with an image that also has width and height of 800 at density 2, for a total of 1600x1600 pixels.

So for tests, I think you'd either (1) make a graphic at the size and density you want and then turn it into an image with .get(), or (2) start with an image with the full number of pixels you want, and then tell p5 about its density to scale down the width/height.

[taliacotton]

Hi team! Wondering if there's a plan to integrate this update into the newest release of p5. Would be grateful! Thank you!!

[davepagurek]

@Papershine are you interested in continuing to finish up the tests? No worries if not, we can always continue development from what you've started!

[Papershine]

Hey @davepagurek, sorry about the delay! I've been extremely busy with school and still doing exams right now, it would be great if anybody interested could take over and finish this PR, thank you so much! If nobody is interested, I could work on the tests in a week when my exams are done.

[Papershine]

A test for pixel density 2, as well as a test for the case where pixel densities aren't the same, have been added.

Do we also want to have a test with createGraphics instead of createImage?

[davepagurek]

Thanks @Papershine, the tests so far look great! A createGraphics test would be good too just to make sure we've handled everything, then I think we're good to go!

[Papershine]

Just added that!

[davepagurek]

Thanks for the code and the tests!

[davepagurek]

@all-contributors please add @Papershine for code, test

[allcontributors]

@davepagurek

I've put up a pull request to add @Papershine! 🎉