GeezerVison is showing github.com — Ensure logical coordinates are used for all operations by tychedelia · Pull Request #1145 · processing/processing4
Conversation
tychedelia
commented
Jun 12, 2025
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Problem
Fixes #1131
When pixelDensity, get/set operations no longer work on logical but instead physical pixels. This is confusing for users who think that their image is size pixels. This behavior will become particular problematic when fractional scaling is introduced as it's more difficult to reason about that size * 2.
Solution
Ensure that operations are always done in logical pixels, upscaling/downscaling into physical pixels when doing write operations to the backing texture.
Add a new pixel access mode constant that determines whether linear or nearest neighbor sampling should be done when scaling. This isn't super helpful right now but will become more important when fractional scaling is introduced.
To be honest, I'm still not 100% sure this is the right solution. It's clear that the PImage api was always intended to be physical pixels and indeed we now introduce a weird distinction where base PImage will continue to use physical pixels since it doesn't make sense to refer to images as having a scaling factor. It's probably good for users to not have to know about these concepts but also introduces inherent confusion when using high dpi scaling. More robust documentation could probably help here but I'm still a little apprehensive as this is a substantial change (i.e. a totally new contract that we always use logical pixels).
TODO:
- Check whether off screen rendering still works
- Confirm whether we need to do gamma correction when doing software rendering in
PGraphicsJava2D
- Test more examples to see if anything broke
- Double check if there's a better existing pattern for the gpu copies when binding the output texture is necessary
- Double check depth writes still work + msaa can be configured
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
SableRaf and catilac reacted with eyes emoji
|
Hi @tychedelia and thanks for your work on this so far!
It seems to properly address the issue for get() and set() as described in #1131, however I also tried it on some of the examples that broke with the pixelDensity change. I hope this is helpful.
In particular, the following shader examples are still broken with this PR:
Topics > Shaders > Conway
Topics > Shaders > DomeProjection
Topics > Shaders > Landscape
There might be other broken examples but these are the ones I remember trying before.
I also tried the example ControlP5colorWheel from the ControlP5 contributed library, which now crashes with the following error (that's new with this PR):
updatePixels(x, y, w, h) is not available with this renderer.
java.lang.ArrayIndexOutOfBoundsException: arraycopy: last source index 40400 out of bounds for int[40000]
at java.base/java.lang.System.arraycopy(Native Method)
at java.desktop/sun.awt.image.IntegerInterleavedRaster.setDataElements(IntegerInterleavedRaster.java:426)
at processing.awt.PGraphicsJava2D$ImageCache.update(PGraphicsJava2D.java:1791)
at processing.awt.PGraphicsJava2D.imageImpl(PGraphicsJava2D.java:1600)
at processing.core.PGraphics.image(PGraphics.java:3911)
at controlP5.ColorWheel$ColorWheelView.display(Unknown Source)
at controlP5.ColorWheel$ColorWheelView.display(Unknown Source)
at controlP5.Controller.draw(Unknown Source)
at controlP5.ControllerGroup.drawControllers(Unknown Source)
at controlP5.ControllerGroup.draw(Unknown Source)
at controlP5.ControlWindow.draw(Unknown Source)
at controlP5.ControlWindow.draw(Unknown Source)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at processing.core.PApplet$RegisteredMethods.handle(PApplet.java:1342)
at processing.core.PApplet.handleMethods(PApplet.java:1489)
at processing.core.PApplet.handleDraw(PApplet.java:2139)
at processing.awt.PSurfaceAWT$9.callDraw(PSurfaceAWT.java:1388)
at processing.core.PSurfaceNone$AnimationThread.run(PSurfaceNone.java:356)
|
tychedelia reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This branch has not been deployed
No deployments
Reviewers
catilac
Awaiting requested review from catilac
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.