Use uploadU32DataTextureRows for LoD index uploads - #406
Open
sawa-zen wants to merge 1 commit into
Open
Conversation
updateLodIndices duplicated the texSubImage2D path inline instead of calling the existing helper, and the inline copy did not save and restore the pixelStorei parameters. Leaving UNPACK_FLIP_Y_WEBGL at false desynchronises three.js's WebGLState cache: the cache still believes the value is true, so three skips the pixelStorei call before its own uploads and every subsequent texture is transferred flipped. uploadU32DataTextureRows already handles this. Fixes sparkjsdev#405
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.
Fixes #405.
updateLodIndicesduplicates thetexSubImage2Dpath inline instead of callinguploadU32DataTextureRows, which is already imported in this file and used a few hundredlines above for the ordering texture. The inline copy is missing the save/restore of the
pixelStoreiparameters that the helper performs.Leaving
UNPACK_FLIP_Y_WEBGLatfalsedesynchronises three.js'sWebGLStatecache.The cache still believes the value is
true, so three skips thepixelStoreicall beforeits own uploads, and every subsequent texture is transferred with the wrong unpack state —
unrelated textures (text atlases, UI images) render vertically flipped.
Replacing the block with the helper also picks up the
UNPACK_PREMULTIPLY_ALPHA_WEBGLsave/restore and
renderer.state.unbindTexture(), and drops ~20 lines.Behaviour is otherwise identical: same target texture, same 4096 x rows region, same
RGBA_INTEGER/UNSIGNED_INTupload. Therenderer.properties.has(...)guard is kept, sothe "texture not found" throw inside the helper stays unreachable here as before.
Repro for the underlying issue (plain three.js, no build step):
https://sawa-zen.github.io/three-texture-flip-repro/?pollute=1
npx biome checkandnpm testpass.npx tsc --noEmitreports only the pre-existingspark-rsmodule errors from not having runbuild:wasm; nothing inSparkRenderer.ts.Note: the comment in
uploadU32DataTextureRowsmentionsrenderer.state.pixelStoreias analternative for three.js >= r184. Happy to switch to that instead if you prefer, but I kept this
patch to "call the existing helper" so it stays minimal.