Nodes: Add LineDashedNodeMaterial #26528
Conversation
| const node = this.getFloat( this.scope ); | ||
| this.node = node; | ||
|
|
||
| return node; |
There was a problem hiding this comment.
The code below should work, or maybe we have some issue in the build process.
class LineMaterialNode extends MaterialNode {
constructor( scope ) {
super( scope );
}
construct( /* builder */ ) {
return this.getFloat( this.scope );
}
}There was a problem hiding this comment.
I tried that initially, but it failed as described. I traced it through the path for varying() where the reference is included in the vertex shader flow and the uniform name is omitted in the code produced.
The output from the construct() phase is entered into the nodes properties[ 'fragment' ] so isn't available at that point. I don't understand the code well enough to known if there is a better solution.
There was a problem hiding this comment.
Hmm... Make sense, construct() must build the output node once. I will fix this.
| import NodeMaterial, { addNodeMaterial } from './NodeMaterial.js'; | ||
| import { attribute } from '../core/AttributeNode.js'; | ||
| import { varying } from '../core/VaryingNode.js'; | ||
| import { lineScale, lineDashSize, lineGapSize } from '../accessors/LineMaterialNode..js'; |
There was a problem hiding this comment.
Seems to have two points in ..js
|
|
||
| export default LineMaterialNode; | ||
|
|
||
| export const lineScale = nodeImmutable( LineMaterialNode, LineMaterialNode.SCALE ); |
There was a problem hiding this comment.
I think the ideal nomenclature would be:
materialLineScale
materialLineDashSize
materialLineGapSizeFor that we can use properties node-based, e.g:
import { dashSize } from '../core/PropertyNode.js';
class LineDashedNodeMaterial extends NodeMaterial {
constructor( parameters ) {
...
this.dashSizeNode = null;
...
}
constructVariants( { stack } ) {
const dashSizeNode = this.dashSizeNode ? float( this.dashSizeNode ) : materialDashSize;
stack.assign( dashSize, dashSizeNode );
...
}
|
|
||
| class LineMaterialNode extends MaterialNode { | ||
|
|
||
| constructor( scope ) { |
There was a problem hiding this comment.
Do we need such a pass-through constructor? I think these lines (7-11) can be just removed.
| @@ -57,5 +57,7 @@ export const iridescenceThickness = nodeImmutable( PropertyNode, 'float', 'Iride | |||
| export const specularColor = nodeImmutable( PropertyNode, 'color', 'SpecularColor' ); | |||
| export const shininess = nodeImmutable( PropertyNode, 'float', 'Shininess' ); | |||
| export const output = nodeImmutable( PropertyNode, 'vec4', 'Output' ); | |||
| export const dashSize = nodeImmutable( PropertyNode, 'float', 'dashScale' ); | |||
| export const gapSize= nodeImmutable( PropertyNode, 'float', 'gapSize' ); | |||
There was a problem hiding this comment.
Missing space before the = sign.
Add support for dashed lines.
@sunag
materialReferences() fail when accessed across code flows. The generate() methods added resolve the problem but there is probably a better fix. The required outputNodes for the lineScale reference are cached against the fragment shader but required in the vertex shader. Without the changes the required uniform name is missing from the generated code.
ie (pseudo code).
uniform lineScale;
varying = ( * lineLength );
instead of:
varying = ( lineScale * lineLength );