fix(image): fix pixelDensity idempotence and high-DPI canvas resize - #9167
Pcmhacker-piro wants to merge 3 commits into
Conversation
…rocessing#9152) - Derive logical dimensions from physical canvas dimensions and pixel density in pixelDensity() so repeated calls are idempotent and resetting to 1 restores original size - Scale backing canvas by pixel density in resize() to preserve high-DPI resolution - Fix Color reference in Image.prototype.set() - Add unit tests for pixelDensity and high-DPI resize
|
🎉 Thanks for opening this pull request! For guidance on contributing, check out our contributor guidelines and other resources for contributors! Thank You! |
| a = imgOrCol[3]; | ||
| //this.updatePixels.call(this); | ||
| } | ||
| } else if (imgOrCol instanceof p5.Color) { |
There was a problem hiding this comment.
@limzykenneth I wanted to get your thoughts on this: if we want to have the ability to load all modules as separate files in the future then we may want to use p5. prefixes more to avoid imports, but if we're thinking of implementing different combinations of modules as separate single-file builds then switching to imports like this makes sense. It's a little confusing if we have both, e.g. for WebGPU where that feels like it should be a separate file, because it means using a different convention in one spot than another, but maybe that's ok?
There was a problem hiding this comment.
Thanks @davepagurek! To avoid introducing a cross-module import of Color while also fixing the issue where referencing p5.Color threw ReferenceError: p5 is not defined (since p5 is not in module scope within p5.Image.js), I've updated this to use duck typing (imgOrCol?.isColor). This matches how Color is checked in p5.Shader and RendererWebGPU (leveraging isColor = true from p5.Color.js), removing the import and keeping separate module/bundle builds clean. Pushed the update in ebdb78c!
Continuous ReleaseCDN linkPublished PackagesCommit hash: 0fb2274 Previous deploymentsThis is an automated message. |
…avoid cross-module import
|
Hi @davepagurek, I have updated the PR to address your feedback:
Ready for final review whenever you have a chance! Thank you! |
3 participants
Resolves #9152
Changes:
src/image/p5.Image.js, fixedpixelDensity(density)to derive logical dimensions from physical canvas dimensions (this.width = this.canvas.width / density,this.height = this.canvas.height / density). This makes repeated calls idempotent (no compounding dimension shrinkage) and allows resettingpixelDensity(1)to accurately restore original dimensions.src/image/p5.Image.js, fixedresize(width, height)to scale the backing canvas and temporary canvas bythis._pixelDensity(width * pd,height * pd). This prevents high-DPI images from having their canvas backing store downscaled to logical size, which caused subsequentget()calls to return[0, 0, 0, 0]as out-of-bounds andset()to fail writes.imgOrCol?.isColor) inImage.prototype.set()to avoid cross-moduleColorimports and preventReferenceError: p5 is not defined.test/unit/image/p5.Image.jscovering:pixelDensity()getter and setter behaviorpixelDensity()idempotence and restoring dimensionspixelDensity > 1inresize()get()andset()after resizing a high-DPI imagePR Checklist
npm run lintpassesAI Usage Disclosure
AI tools were used assistively to help write unit test cases and verify edge cases. The changes have been tested and reviewed locally.