-
-
Notifications
You must be signed in to change notification settings - Fork 102
Ensure logical coordinates are used for all operations #1145
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
Conversation
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 > ConwayTopics > Shaders > DomeProjectionTopics > 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.PSurfaceAWT9ドル.callDraw(PSurfaceAWT.java:1388)
at processing.core.PSurfaceNone$AnimationThread.run(PSurfaceNone.java:356)
Uh oh!
There was an error while loading. Please reload this page.
Problem
Fixes #1131
When
pixelDensity,get/setoperations no longer work on logical but instead physical pixels. This is confusing for users who think that their image issizepixels. 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
PImageapi was always intended to be physical pixels and indeed we now introduce a weird distinction where basePImagewill 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:
PGraphicsJava2D