Skip to content

[p5.js 2.0+ Bug Report]: textureWrap() silently drops the y wrap mode when passed the object returned by its own getter #9164

Description

@harshiltewari2004

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • WebGPU
  • p5.strands
  • Build process
  • Unit testing
  • Internationalization
  • Friendly errors
  • Other (specify if possible)

p5.js version

2.3.2

Web browser and version

Chrome 152.0.7977.83 (Official Build) (arm64)

Operating system

macOS 26.6.2 (Build 25G83), MacBook Air M1

Steps to reproduce this

What happens

fn.textureWrap has a branch that accepts the object its own getter returns, so textureWrap(textureWrap()) is meant to round-trip. It doesn't — the y mode comes back undefined, and the texture silently falls back to CLAMP.

function setup() {
  createCanvas(100, 100, WEBGL);

  textureWrap(REPEAT, MIRROR);
  console.log(textureWrap());   // { x: 'repeat', y: 'mirror' }  — correct

  textureWrap(textureWrap());   // round-trip through the object branch
  console.log(textureWrap());   // { x: 'repeat', y: undefined }  — y is gone
}

Same result for any hand-written object: textureWrap({ x: REPEAT, y: MIRROR }).

Why

src/webgl/material.js:3457-3461:

// accept what is returned from the getter
if (wrapX.hasOwnProperty('x') && wrapX.hasOwnProperty('y')) {
  wrapX = wrapX.x;
  wrapY = wrapX.y;
}

The first assignment replaces wrapX with a string, so the second line reads .y off that string rather than off the original object, and gets undefined.

Why it's silent

undefined is stored via setValue('textureWrapY', undefined) and travels to setWebGLTextureParams in src/webgl/utils.js, where the wrap branching is REPEAT → … else MIRROR → … else CLAMP_TO_EDGE. An unrecognised value lands in the final else and is treated as CLAMP.

Worth noting the asymmetry: REPEAT and MIRROR each console.warn when they have to downgrade to CLAMP on a non-power-of-two texture, so the existing code does consider a silent downgrade worth reporting. undefined produces no message at all.

A second, smaller defect in the same function

The prose documentation at src/webgl/material.js:3287-3288 says the getter returns:

{ wrapX: CLAMP, wrapY: REPEAT }

The returned keys are actually x and y. The @return annotation directly below (line 3447) already documents {x, y} correctly, and test/unit/core/properties.js:31 uses the {x, y} shape — so the prose is the part that's wrong, not the code. Anyone writing textureWrap().wrapX from the docs gets undefined.

Suggested fix

Read both keys off the original object before either is reassigned. Destructuring evaluates the right-hand side first, so nothing gets clobbered:

if (wrapX && typeof wrapX === 'object' && 'x' in wrapX && 'y' in wrapX) {
  ({ x: wrapX, y: wrapY } = wrapX);
}

This also removes the current TypeError on textureWrap(null), since null.hasOwnProperty throws before any check runs.

Plus a one-line docs correction for the key names.

Test coverage

test/unit/webgl/p5.Texture.js has three textureWrap() tests, all on the plain two-argument setter path. Neither the getter nor the object branch is covered. I'd add:

  • the getter returns {x, y} with the current modes
  • textureWrap(textureWrap()) preserves both modes
  • textureWrap({x: REPEAT, y: MIRROR}) sets them independently

Open questions before I pick an approach

  1. Docs or code for the key mismatch? Correcting the prose to {x, y} is the non-breaking option and matches the annotation and existing tests, so that's my assumption — but if wrapX/wrapY is the preferred public shape, that's a different (breaking) change and I'd rather not guess.
  2. Should an unrecognised wrap value warn? Right now anything that isn't REPEAT or MIRROR quietly becomes CLAMP in setWebGLTextureParams. Surfacing that through the FES feels consistent with the existing power-of-two warnings, but it's wider than this bug and would affect every caller, so I'd keep it out unless you'd like it included.
  3. Is the null guard in scope here, or better as its own thing?

Happy to take this if the direction looks right.

Activity

  1. Shruti2110-coder commented on Sep 12, 2026

    @Shruti2110-coder
    Contributor

    @harshiltewari2004 Can I work on this?

  2. mayanksingh-27 commented on Sep 14, 2026

    @mayanksingh-27

    I’ve prepared a focused fix for the textureWrap() object setter.

    Before: After textureWrap(REPEAT, MIRROR), passing the getter result back with textureWrap(textureWrap()) sets the Y mode to undefined. The function replaces wrapX with its x value before reading wrapX.y.

    After: Read both values from the object first, then update the modes. The same round trip preserves { x: REPEAT, y: MIRROR }.

    if (Object.hasOwn(wrapX, 'x') && Object.hasOwn(wrapX, 'y')) {
      const { x, y } = wrapX;
      wrapX = x;
      wrapY = y;
    }

    Object.hasOwn retains the existing own-property check and supports objects without a prototype. This fix leaves invalid-mode warning behavior unchanged.

    Files to change:

    • src/webgl/material.js: fix the object setter and correct the getter’s documentation example to { x, y }.
    • test/unit/webgl/p5.Texture.js: add regression tests.

    Tests: Check the getter’s x and y values; verify that passing its result back preserves both modes; verify distinct modes in a supplied object; check an object without a prototype; and confirm the Y mode reaches WebGL after the round trip.

    The isolated reproduction passes with this fix. I still need to run the full browser test suite when its dependencies are available. Does this approach and scope look right?

  3. harshiltewari2004 commented on Sep 15, 2026

    @harshiltewari2004
    ContributorAuthor

    Thanks both for the interest, and @mayanksingh-27 for writing that up — your snippet matches what I had in mind, reading both values off the object before either variable is reassigned.

    I filed this intending to implement it myself, which is why the issue ends with an offer to take it. I haven't opened a PR yet because the first open question changes what the diff touches: if the prose docs get corrected to {x, y} it's a small fix plus a doc line, but if wrapX/wrapY is the preferred public shape then it's a breaking change to the getter and a different piece of work. I didn't want to code into an unapproved design.

    One note on the snippet: Object.hasOwn(null, 'x') throws a TypeError just like the current null.hasOwnProperty, so textureWrap(null) is still unhandled either way — that's open question 3 above.

    @ksen0 could you please weigh in on the key-naming question, and assign this to whoever you think is best placed to take it? Happy to do the work if it comes my way, and equally happy to review if it goes to someone else.

  4. harshiltewari2004 commented on Sep 27, 2026

    @harshiltewari2004
    ContributorAuthor

    @davepagurek could I get your call on this one? Rather than leave it as an open question, here's the narrow scope I'd propose:

    • Fix the object branch in fn.bezierOrder's neighbour fn.textureWrap (src/webgl/material.js) so both values are read off the object before either variable is reassigned. Behaviour for textureWrap(REPEAT, MIRROR) is unchanged.
    • Correct the prose docs above the getter to say {x, y} rather than {wrapX, wrapY}. The @return annotation and test/unit/core/properties.js already use {x, y}, so this is the non-breaking direction.
    • Leave two things out of scope: textureWrap(null) still throws a TypeError (true of the current code and of every fix suggested so far), and unrecognised wrap values still fall through to CLAMP without a warning. Both seem like separate calls.
    • Add regression tests in test/unit/webgl/p5.Texture.js covering the getter shape and the round trip, neither of which is currently tested.

    If that scope looks right, could I be assigned?

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions