fix(shaders): don't re-convert raw canvas shaders#40
Merged
Conversation
Raw Canvas shaders (CanvasShaderNode) expose `render`, not `program`, so the `!props.shader.program` guard let them fall through and re-run through `convertToShader`, corrupting an already-built shader. WebGL worked only because WebGlShaderNode happens to have `program`. Both backends' shader nodes extend CoreShaderNode, which always carries a `shaderType`, as does the DOM-renderer test fake. A raw StyleEffects props object never does. Switch the guard to a single `'shaderType' in shader` check that covers WebGL, Canvas, and the DOM test path uniformly — and won't need another `|| '...' in shader` clause when a new backend lands. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Problem
Raw Canvas shaders still go through
convertToShaderwhen set on each node, even though a raw shader already carries everything it needs to render. WebGL handles this correctly; Canvas does not.The guard in
ElementNode._renderwas:A
CanvasShaderNodeexposesrender, notprogram, so it falls through the!props.shader.programcheck and gets wrongly re-run throughconvertToShader. WebGL only works becauseWebGlShaderNodehappens to have aprogramfield.Solution
This is an alternative to #34. Instead of widening the check to
'program' in shader || 'render' in shader(which enumerates backend-specific fields and needs a new clause per backend), use the single field that every built shader shares and a rawStyleEffectsprops object never has:shaderType.WebGlShaderNodeandCanvasShaderNodeboth extendCoreShaderNode, which declaresreadonly shaderType.{ shaderType, props, program: {} }.StyleEffects(border*/shadow/rounded/gradients/holePunch) has noshaderType.So
!('shaderType' in props.shader)cleanly means "this is a raw style-props object, build a shader from it" and covers WebGL, Canvas, and the DOM test path uniformly.Behavior parity
props.shadershaderType?StyleEffectspropsVerification
npm run tsc— cleannpm test— 134 passednpm run lint— 0 errors🤖 Generated with Claude Code