Line2NodeMaterial: Align wide lines with WebGLRenderer version.#32234
Line2NodeMaterial: Align wide lines with WebGLRenderer version.#32234Mugen87 merged 1 commit intomrdoob:devfrom
WebGLRenderer version.#32234Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
|
|
||
| // adjust for clip-space to screen-space conversion // maybe resolution should be based on viewport ... | ||
| offset.assign( offset.div( viewport.w ) ); | ||
| offset.assign( offset.div( viewport.w.div( screenDPR ) ) ); |
There was a problem hiding this comment.
This bug occurred because the definitions are not consistent.
renderer.setViewport() is in logical pixel units. Can you improve the nomenclature?
There was a problem hiding this comment.
I think there is a difference with what is defined via the renderer API and what TSL objects represent in the shader. That has been mentioned here #29347 (comment). I don't think you can come up with a solution that satisfies every use case. Keeping the TSL objects in physical pixel units seems correct to me. And I would leave the renderer API as it is, especially to avoid any kind of breakage.
There was a problem hiding this comment.
Keeping the TSL objects in physical pixel units seems correct to me.
Yes.
And I would leave the renderer API as it is,
Yes.
So the only possible changes are in the margins: revist the docs and inline docs and ensure the units are clearly stated.
Also the internal nomenclature could be reviewed.
_viewport // logical (css) pixel units
_currentViewport // physical pixel units
state.viewport // physical pixel units
Fixed #32229.
Description
As suggested in #32229 (comment), the PR makes sure the wide line implementation for
WebGPURendereruses CSS pixel units to ensure consistent line widths across devices. For this, the DPR must be factored out of the viewport.With this PR, the wide line demos now render with equal width for
WebGPURendererandWebGLRenderer.