Repository navigation
Fix textureWrap() dropping the y mode when passed a modes object - #9290
Open
harshiltewari2004 wants to merge 1 commit into
Open
harshiltewari2004 wants to merge 1 commit into
harshiltewari2004 wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Resolves #9164
Changes
textureWrap()accepts the object its own getter returns, but the objectbranch reassigned
wrapXbefore readingwrapX.y, so the y mode becameundefinedand fell through toCLAMPwith no message. Reading both valuesbefore either assignment fixes the round trip.
Per the discussion on the issue:
be4de981(1 Feb) and hasalways returned
{x, y}; the{wrapX, wrapY}prose arrived later in85d6ccc(24 Mar), a docs-only commit that also added the@returnannotation saying
{x, y}. It was never the real shape, so the prose iscorrected to match the code.
p5.disableFriendlyErrors, raised at the user-facing call rather than inrenderer 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 WebGPUsampler default when the
constantMappinglookup returnsundefined).textureWrap(null)no longer throws aTypeError.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
Objectoverload to the JSDoc and todocs/parameterData.json,following
c119cb99(endShape). Tests intest/unit/core/param_errors.js.Tests
Seven tests in
test/unit/webgl/p5.Texture.js— the getter and the objectbranch had no coverage before. One asserts
MIRRORED_REPEATreachestexParameteriafter a round trip, so the fix is verified at the GL call andnot just in
states.Four in
test/unit/core/param_errors.jsfor the overloads.Two questions, @davepagurek
textureWrap()already gets an auto-generated validator warning for aninvalid 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?
TypeErroron a null-prototype object:buildTypeMismatchMessage()callstoString()on the argument, andObject.create(null)has none. Not specific totextureWrap, so the testfor that path disables the FES with a comment. Happy to file it separately.
Note for reviewers
textureMode()in the same file warns viaconsole.warnand skips theassignment 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(stubbinggetSampler), WebGPU ingetSampler(stubbingsetTextureParams). Validating at the user-facing call covers both withouttouching either renderer.
Test run
npx vitest run— 2753 passed, 1 failed:test/unit/webgpu/p5.Shader.js→ "handle passing a value from a vertex hookto 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.