Skip to content

Fix textureWrap() dropping the y mode when passed a modes object - #9290

Open
harshiltewari2004 wants to merge 1 commit into
processing:mainfrom
harshiltewari2004:fix-textureWrap-object-setter
Open

harshiltewari2004 wants to merge 1 commit into
processing:mainfrom
harshiltewari2004:fix-textureWrap-object-setter

Conversation

@harshiltewari2004

Copy link
Copy Markdown
Contributor

Resolves #9164

Changes

textureWrap() accepts the object its own getter returns, but the object
branch reassigned wrapX before reading wrapX.y, so the y mode became
undefined and fell through to CLAMP with no message. Reading both values
before either assignment fixes the round trip.

Per the discussion on the issue:

  • Docs key mismatch. The getter was added in be4de981 (1 Feb) and has
    always returned {x, y}; the {wrapX, wrapY} prose arrived later in
    85d6ccc (24 Mar), a docs-only commit that also added the @return
    annotation saying {x, y}. It was never the real shape, so the prose is
    corrected to match the code.
  • Unrecognised wrap values now emit a non-blocking FES warning, gated on
    p5.disableFriendlyErrors, raised at the user-facing call rather than in
    renderer internals, and flagged so it logs once per sketch. Renderer
    behaviour is unchanged — an unrecognised value still resolves to clamping on
    both backends (explicitly in setWebGLTextureParams, and via the WebGPU
    sampler default when the constantMapping lookup returns undefined).
  • textureWrap(null) no longer throws a TypeError.

Param validator

Writing the tests surfaced that the validator rejects the object form itself:
textureWrap(textureWrap()) logged "Expected constant ... but received
[object Object]"
, because the only declared overloads were the two constants
and the no-arg getter. So the documented round trip produced a false FES
warning on every correct call.

Added an Object overload to the JSDoc and to docs/parameterData.json,
following c119cb99 (endShape). Tests in test/unit/core/param_errors.js.

Tests

Seven tests in test/unit/webgl/p5.Texture.js — the getter and the object
branch had no coverage before. One asserts MIRRORED_REPEAT reaches
texParameteri after a round trip, so the fix is verified at the GL call and
not just in states.

Four in test/unit/core/param_errors.js for the overloads.

Two questions, @davepagurek

  1. textureWrap() already gets an auto-generated validator warning for an
    invalid constant, so the warning we discussed fires as a second message.
    Should I narrow mine to only the values unpacked from an object — which the
    auto-validator can't inspect — or drop it?
  2. The validator throws a TypeError on a null-prototype object:
    buildTypeMismatchMessage() calls toString() on the argument, and
    Object.create(null) has none. Not specific to textureWrap, so the test
    for that path disables the FES with a comment. Happy to file it separately.

Note for reviewers

textureMode() in the same file warns via console.warn and skips the
assignment on an invalid mode. This uses the FES and keeps the assignment, per
the issue discussion. Happy to align them if you'd prefer consistency.

The two backends handle wrap modes in mirrored places — WebGL in
setTextureParams (stubbing getSampler), WebGPU in getSampler (stubbing
setTextureParams). Validating at the user-facing call covers both without
touching either renderer.

Test run

npx vitest run — 2753 passed, 1 failed:
test/unit/webgpu/p5.Shader.js → "handle passing a value from a vertex hook
to a fragment hook with swizzle assignment" (expected 255 to be close to 0).
It fails identically on a clean checkout with these changes stashed, so it's
unrelated to this PR.

The object branch reassigned wrapX before reading wrapX.y, so the y
mode became undefined and silently fell back to CLAMP. Read both
values before either assignment.

Also guard against null, warn via the FES for unrecognised wrap modes,
correct the getter docs to {x, y}, and add an Object overload so the
param validator stops rejecting the documented round-trip form.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant