Skip to content

Regression in 4.4.3: get() and set() are reading and writing from/to the wrong coordinates. #1131

Open
@PhilipSharman

Description

@PhilipSharman

Most appropriate sub-area of Processing 4?

Core/Environment/Rendering

Processing version

4.4.3

Operating system

MacOS 11.7.10

Steps to reproduce this

  1. Run the attached script to demonstrate the behavior.

In v4.4.2, the color that get() reads is white, as expected. And set() draws around (350, 350) as expected. But in v4.4.3 (and v4.4.4) get() reads blue, and set() draws around (175, 175).

It appears that both get() and set() are operating on (x/2. y/2) instead of (x,y).

snippet

////////////////////////////////////////////////////////////////////////////////////////////////////////////// //<>// //<>// //<>// //<>//
// Define colors
color white = color(255);
color black = color(0);
color red = color(255, 0, 0);
color blue = color(0, 0, 255);

//////////////////////////////////////////////////////////////////////////////////////////////////////////////
// Setup
void setup() {
    size(400, 400);
    background(white);
    noLoop();
}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////
// Draw
void draw() {
    stroke(blue);
    fill(blue);
    rect(165, 165, 20, 20);

    // BUG 1: The color c is blue but it should be white.
    // It seems to be reading from (x/2, y/2) = (175, 175) instead of (350, 350).
    color c = get(350, 350);
    showColor(c);            // R/G/B = 0.0 0.0 255.0 = blue

    // BUG 2: This draws around (175, 175). It should draw around (350, 350).
    for (int x = 340; x < 360; x++) {
        for (int y = 340; y < 360; y++) {
            set(x, y, red);
        }
    }

    //save("screenshot.xxx.png");
}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////
// Show information about a color
void showColor(color c) {
    print("R/G/B =", red(c), green(c), blue(c) );
    if (c == white) {
        println(" = white");
    } else if (c == blue) {
        println(" = blue");
    } else {
        println("");
    }
}
//////////////////////////////////////////////////////////////////////////////////////////////////////////////

Additional context

No response

Would you like to work on the issue?

No

Activity

changed the title [-]Regression in 4.4.3: get() and set() are reading and writing to/from the wrong coordinates.[/-] [+]Regression in 4.4.3: get() and set() are reading and writing from/to the wrong coordinates.[/+] on Jun 1, 2025
SableRaf

SableRaf commented on Jun 2, 2025

@SableRaf
Collaborator

Hi @PhilipSharman,

This is related to a recent change in Processing 4.4.3 where we updated the default pixelDensity() setting to match your display’s density. It makes sketches look sharper on high-DPI screens, but it has caused a few side effects that we are addressing as they are reported.

Thanks for catching that one!

tychedelia

tychedelia commented on Jun 2, 2025

@tychedelia

I'm not sure that this is a bug -- or at least, the switch to setting pixelDensity by default was a breaking change. The difficulty here is that, as far as I can tell, get/set are currently expressed in terms of physical pixels loaded from the underlying graphics buffer. For sketches prior to forcing a high dpi pixel density in 4.4.3, logical = physical and so that was no problem.

The problem is, if you assume get/set are logical coordinates, this introduces a number of other problems, particularly if fractional scaling support were to be implemented (which it should, I'm guessing things might be wonky on fractional displays atm). Basically, when converting a logical to physical coordinate, what should happen? Should we use nearest neighbor or area sampling? What if the user wants to be able to fetch each physical pixel (i.e. logical sub-pixel)? This could represent a breaking change in the other direction.

I think some solutions here could be to:

Either way @PhilipSharman you should be able to work around this at the moment by setting pixelDensity(1); in your project setup.

SableRaf

SableRaf commented on Jun 3, 2025

@SableRaf
Collaborator

Hi @tychedelia and thanks for your input and thoughtful suggestions.

I think matching displayDensity was the right call. It makes all sketches look better on high DPI screens by default, and I’d rather not revert it if we can avoid it. That said, we’ll probably need to look more closely at what else this change may have affected.

Besides, I believe most users expect “pixels” to refer to logical coordinates (size() works that way, for example) and probably don't know about pixel density. Introducing the notion of physical pixels too early could be confusing for beginners.

I’d suggest keeping get() and set() in logical space, averaging over physical pixels when needed. That keeps most existing code working as before and matches user expectation.

If necessary for advanced use, a pixelMode(PHYSICAL) flag feels most consistent with Processing’s API design. It would be a breaking change, but only for users already working with pixelDensity(x) and get()/set(), who likely understand the distinction between physical and logical pixels.

tychedelia

tychedelia commented on Jun 3, 2025

@tychedelia

I think matching displayDensity was the right call. It makes all sketches look better on high DPI screens by default, and I’d rather not revert it unless absolutely necessary.

For sure, it's def the right call from a default perspective. I agree that the logical/physical distinction can be pretty confusing to new users. It hurts my head constantly too! Especially when fractional scaling is involved.

My concern is with the pixels array that the get/set methods point users towards using. Even if you do sampling/blending on the CPU in get/set, the backing buffer will still be misized.

If you're okay with declaring all pixel operations to always be logical, I think the right call then would be to add a downres framebuffer and perform sampling on the GPU, always reading back into a pixel buffer that is the same as size (and vice versa for upres). That way, pixelDensity would only affect the size of allocated GPU textures and not anything on the CPU.

SableRaf

SableRaf commented on Jun 3, 2025

@SableRaf
Collaborator

That seems reasonable. I'm mostly focused on how it works in user space and the low-level details are a bit over my head ✈️ As long as we agree on the user-facing behavior, I’ll defer to those with a deeper understanding of the optimal implementation. Thanks for taking the time to dig into this!

Edit: This post on Reddit has a good demonstration of the issue https://www.reddit.com/r/processing/comments/1l5siua/odd_get_behaviour/

tychedelia

tychedelia commented on Jun 12, 2025

@tychedelia

Have a working solution that I opened in #1145, but as I noted at the bottom, I'm still not totally sure forcing all operations to be logical is the right approach? Or at least it makes me nervous. Pedagogically, I think it is and continue to agree with the decision in general to force high dpi scaling when available, but there's definite backwards compatibility and documentation things that should be thought about more careful, I think.

SableRaf

SableRaf commented on Jun 14, 2025

@SableRaf
Collaborator

Hi @tychedelia, thanks so much for your thoughtful work on this and for opening the draft PR! Sorry we’ve been a bit slow to respond; we’re in the middle of reviewing candidates for the 2025 pr05 grant and juggling other org work. I hope to be a little more on top of things starting next month.

Happy to think through how we can communicate this change better, both in the reference and maybe with clearer console warnings. Let me know if you have any suggestions for how best to do this.

Thanks again for diving into this!

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggood first issueGood for newcomershelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @PhilipSharman@SableRaf@tychedelia

      Issue actions

        Regression in 4.4.3: get() and set() are reading and writing from/to the wrong coordinates. · Issue #1131 · processing/processing4