feat: implement strokeUniform property for zoom-invariant stroke widths #789
Conversation
…ring zoom Adds strokeUniform boolean property to Path and Group classes that maintains constant stroke width in screen space regardless of scale transformations. Key features: - strokeUniform property on Path and Group (defaults to false) - getEffectiveStrokeWidth() utility function compensates for world scale - Cross-renderer support (Canvas, SVG, WebGL) - Group propagation applies to all child shapes - Backward compatible with existing code Resolves issue #546 for zoom-invariant stroke widths when using ZUI. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Jono <[email protected]>
|
Claude finished @jonobr1's task —— View job PR Review: strokeUniform Property ImplementationI've completed a comprehensive analysis of the Todo List:
Overall Assessment: ✅ High Quality ImplementationThe implementation is well-architected and follows Two.js patterns consistently. Here's my detailed feedback: Strengths✅ Excellent Architecture
✅ Robust Mathematical Implementation
✅ Cross-Renderer ConsistencyAll three renderers correctly call
✅ Backward Compatibility
✅ TypeScript Support
Areas for Improvement
|
|
@claude instead of |
|
Claude finished @jonobr1's task —— View job Renaming strokeUniform to strokeAttenuation ✅I have successfully renamed Todo List:
Summary of Changes:Property Behavior:
Files Modified:
Backward Compatibility: Note: According to CLAUDE.md, you should manually run |
- Replace strokeUniform property with strokeAttenuation across codebase - Invert boolean logic: strokeAttenuation=true (default) enables scaling, false maintains constant screen space width - Update property names, flags, documentation, and TypeScript definitions - Maintain backward compatibility by defaulting to true (normal scaling behavior) - Update getEffectiveStrokeWidth utility to use inverted logic 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Jono <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR implements a new strokeAttenuation property for Path and Group classes to maintain constant stroke width in screen space during zoom operations. When set to false, stroke widths automatically compensate for world transform scale.
Key changes:
- Adds
getEffectiveStrokeWidthutility function to calculate scale-compensated stroke widths - Implements
strokeAttenuationproperty on Path and Group classes (defaults totruefor backward compatibility) - Updates all renderers (Canvas, SVG, WebGL) to use the new effective stroke width calculation
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/math.js | Adds getEffectiveStrokeWidth utility function for scale compensation |
| src/path.js | Implements strokeAttenuation property with getter/setter and flag management |
| src/group.js | Adds strokeAttenuation property that propagates to child shapes |
| src/renderers/canvas.js | Updates Canvas renderer to use effective stroke width calculation |
| src/renderers/svg.js | Updates SVG renderer to use effective stroke width calculation |
| src/renderers/webgl.js | Updates WebGL renderer to use effective stroke width calculation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| for (let i = 0; i < this.children.length; i++) { | ||
| const child = this.children[i]; | ||
| if (child.strokeAttenuation !== undefined) { | ||
| child.strokeAttenuation = v; | ||
| } |
There was a problem hiding this comment.
The setter iterates through all children every time the property is set, which could be inefficient for groups with many children. Consider implementing lazy propagation or only updating children that have strokeAttenuation property.
| for (let i = 0; i < this.children.length; i++) { | |
| const child = this.children[i]; | |
| if (child.strokeAttenuation !== undefined) { | |
| child.strokeAttenuation = v; | |
| } | |
| // Only update children that have a defined strokeAttenuation property | |
| const attenuableChildren = this.children.filter(child => child && child.strokeAttenuation !== undefined); | |
| for (let i = 0; i < attenuableChildren.length; i++) { | |
| attenuableChildren[i].strokeAttenuation = v; |
Co-authored-by: Copilot <[email protected]>
Introduces the strokeAttenuation property to Two.Points and Two.Text, allowing stroke width to remain constant in screen space or scale with transformations. Updates internal flags, property lists, and TypeScript definitions to support this feature.
This commit adds comprehensive JSDoc comments to core classes and methods in the Two.js build, improving code documentation and developer experience. It also refactors static property assignments to use modern class field syntax, removes legacy helper functions, and introduces the getEffectiveStrokeWidth utility. Minor updates to constants and test files are included.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if (typeof this.fill === 'object' && this.fill && 'dispose' in this.fill) { | ||
| this.fill.dispose(); | ||
| } else if (typeof this.fill === 'object' && this.fill && 'unbind' in this.fill) { | ||
| ) { |
Check notice
Code scanning / CodeQL
Syntax error Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix the syntax error, we must remove the stray unmatched parenthesis on line 502 within the else if block. Upon inspection, it appears that line 502 () {) should simply be removed to restore proper function block syntax. The corresponding else if block should take its statement on the next line. No other modifications appear to be needed – it looks like the else if block is already structured properly, only the accidental orphaned parenthesis is present. No imports or additional code is needed; the only required change is to delete line 502, which contains just ) {.
| @@ -499,7 +499,6 @@ | ||
| if (typeof this.fill === 'object' && this.fill && 'dispose' in this.fill) { | ||
| this.fill.dispose(); | ||
| } else if (typeof this.fill === 'object' && this.fill && 'unbind' in this.fill) { | ||
| ) { | ||
| this.fill.unbind(); | ||
| } | ||
|
|
Summary
strokeUniformboolean property for Path and Group classesResolves Issue #546
This addresses the original request for maintaining stroke width upon zoom using ZUI or other scaling operations. When
strokeUniformis set totrue, the stroke width automatically compensates for world transform scale, maintaining constant visual thickness regardless of zoom level.Backward Compatible
falsefor existing behavior🤖 Generated with Claude Code