Canvas feature#2290
Conversation
📝 WalkthroughWalkthrough
ChangesCanvas primitive drawing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/widgets/src/display/Canvas.ts (1)
123-135: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider batching
markDirty()infillCirclefor consistency withfillRect.
fillCirclecallssetPixelper pixel, each invoking bounds-checking and potentiallymarkDirty()individually. The existingfillRect(lines 62–81) instead manipulates_pixelsdirectly and callsmarkDirty()once. For large circles this means up to(2r+1)²setPixelcalls with per-call overhead. AligningfillCirclewith thefillRectpattern would reduce redundant work.♻️ Proposed optimization: direct pixel manipulation with batched dirty flag
public fillCircle(cx: number, cy: number, r: number): void { cx = Math.floor(cx); cy = Math.floor(cy); r = Math.floor(r); - for (let y = -r; y <= r; y++) { - for (let x = -r; x <= r; x++) { - if (x * x + y * y <= r * r) { - this.setPixel(cx + x, cy + y); - } + let dirty = false; + const r2 = r * r; + for (let y = -r; y <= r; y++) { + const py = cy + y; + if (py < 0 || py >= this._canvasHeight) continue; + for (let x = -r; x <= r; x++) { + if (x * x + y * y > r2) continue; + const px = cx + x; + if (px < 0 || px >= this._canvasWidth) continue; + const idx = py * this._canvasWidth + px; + if (this._pixels[idx] !== 1) { + this._pixels[idx] = 1; + dirty = true; + } } } + if (dirty) { + this.markDirty(); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widgets/src/display/Canvas.ts` around lines 123 - 135, Optimize fillCircle by matching fillRect’s direct _pixels manipulation pattern: perform bounds checks while writing circle pixels directly, avoid calling setPixel for each pixel, and invoke markDirty() once only if at least one pixel was changed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/widgets/src/display/Canvas.ts`:
- Around line 123-135: Optimize fillCircle by matching fillRect’s direct _pixels
manipulation pattern: perform bounds checks while writing circle pixels
directly, avoid calling setPixel for each pixel, and invoke markDirty() once
only if at least one pixel was changed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3966d62b-083e-4663-9bab-472a8e766a98
📒 Files selected for processing (1)
packages/widgets/src/display/Canvas.ts
Description
This PR adds a
CanvasandBrailleCanvaswidget to@termuijs/widgets. It provides an HTML5 Canvas-like API for drawing lines, rectangles, and primitive graphics in the terminal, utilizing braille and block characters for sub-cell rendering resolution.Related Issue
Closes #2250
Which package(s)?
@termuijs/widgetsType of Change
type:bug)type:feature)type:docs)type:testing)type:refactor)type:design)type:accessibility)type:performance)type:devops)type:security)Checklist
needs-starcheck blocks your merge otherwise.bun vitest runbun run buildbun run typecheckCONTRIBUTING.md.type: short description.markDirty()(if your change affects rendering).anytypes without an inline comment explaining why.GSSoC 2026 Participation
https://gssoc.girlscript.org/profile/YOUR_PROFILE_ID_HEREScreenshots / Recordings (UI changes)
Notes for the Reviewer
The implementation uses Bresenham's line algorithm combined with UTF-8 braille character mapping (
\u2800to\u28FF) to achieve 2x4 sub-pixel drawing accuracy in standard terminal cells.Summary by CodeRabbit