Skip to content

fix(highway_3d): size Butterchurn output canvas buffer to fill the highway - #820

Merged
byrongamatos merged 2 commits into
mainfrom
fix/highway-3d-butterchurn-scaling
Jul 10, 2026
Merged

byrongamatos merged 2 commits into
mainfrom
fix/highway-3d-butterchurn-scaling

Conversation

@ChrisBeWithYou

@ChrisBeWithYou ChrisBeWithYou commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

The 3D-highway Butterchurn background sized only the output canvas CSS size and called setRendererSize(), but never sized the canvas drawing buffer (canvas.width/height). Butterchurn does not size the output canvas itself — its on-screen pass viewports to the reported size into the default framebuffer — so the buffer stayed at the browser default 300×150 while the viewport was the full highway. Only the bottom-left ~300×150 of the pattern was drawn, then CSS-stretched across the whole highway: zoomed, soft, aspect-wrong, and worse the larger the panel.

Fix

plugins/highway_3d/screen.js:

  • New _bcApplySize(cssW, cssH): sets the drawing buffer to the device-pixel render size (round(css × min(DPR, 1.5))), confines every layer (canvas / backdrop / scrim / tint) to the highway rect, and reports the same device px to setRendererSize so buffer == on-screen viewport == full fill.
  • Seed the buffer at create and switch createVisualizer to pixelRatio:1, textureRatio:1 — DPR is now folded into the reported size, so buffer == viewport == internal texsize (no double-counting).
  • render() and resize() both route through _bcApplySize.

plugins/highway_3d/plugin.json: version 3.31.33.31.4.

Testing

No plugin unit tests exist; validated visually on the :8000 testbed (3D Highway → background style = Butterchurn) — pattern now fills the highway at correct aspect/sharpness across panel sizes. Tint/scrim/opacity background layers unchanged (still a deliberately subdued background).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved highway 3D visualizer sizing across different screen resolutions and pixel densities.
    • Prevented stretched or distorted visuals when resizing the display.
    • Updated overlay layers to remain aligned with the visualizer canvas.
  • Chores

    • Updated the highway 3D plugin version to 3.31.5.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Highway 3D sizing now applies a capped device-pixel ratio once when initializing and resizing the Butterchurn canvas, renderer, and overlays. The plugin manifest version is updated from 3.31.4 to 3.31.5.

Changes

Highway 3D sizing

Layer / File(s) Summary
Visualizer buffer initialization
plugins/highway_3d/screen.js
Canvas and Butterchurn initialization now use matching DPR-scaled dimensions with pixelRatio: 1.
Centralized resize flow
plugins/highway_3d/screen.js, plugins/highway_3d/plugin.json
_bcApplySize centralizes canvas, overlay, and renderer sizing, while resize paths use it and the manifest version becomes 3.31.5.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: fixing highway_3d Butterchurn canvas sizing to fill the highway.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/highway-3d-butterchurn-scaling

Comment @coderabbitai help to get the list of available commands.

ChrisBeWithYou and others added 2 commits July 10, 2026 13:07
…ghway

The 3D-highway Butterchurn background set only the output canvas CSS size
and called setRendererSize(), but never sized the canvas DRAWING BUFFER
(canvas.width/height). Butterchurn does not size the output canvas itself
(renderToScreen viewports to the reported size into the default
framebuffer), so the buffer stayed at the browser default 300x150 while the
viewport was the full highway. Only the bottom-left ~300x150 of the pattern
was drawn, then CSS-stretched across the whole highway -- zoomed, soft, and
aspect-wrong, worse the larger the panel.

Add _bcApplySize(cssW, cssH): set the drawing buffer to the device-pixel
render size (round(css * min(DPR, 1.5))), confine every layer (canvas,
backdrop, scrim, tint) to the highway rect, and report the same device px
to setRendererSize so buffer == on-screen viewport. Seed the buffer at
create and switch createVisualizer to pixelRatio:1, textureRatio:1 (DPR is
now folded into the reported size, so buffer == viewport == internal
texsize, no double-counting). render() and resize() both route through it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
Rebased onto main; #823 already shipped 3.31.4 (per-panel camera), so this
Butterchurn buffer-sizing fix advances to 3.31.5.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@byrongamatos
byrongamatos marked this pull request as ready for review July 10, 2026 11:08
@byrongamatos
byrongamatos force-pushed the fix/highway-3d-butterchurn-scaling branch from 2adb2ca to 283d535 Compare July 10, 2026 11:08

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@plugins/highway_3d/screen.js`:
- Around line 645-648: Update the sizing check near _bcApplySize to also detect
changes in the backing canvas dimensions, not only sz.w and sz.h or the device
pixel ratio. Reapply sizing when canvas.width or canvas.height differs from the
dimensions tracked by the control state, including changes made outside
renderer.resize(), and update those tracked values after applying the size.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8dae0f2d-6919-4a84-9111-cb1f29472ee7

📥 Commits

Reviewing files that changed from the base of the PR and between 751209b and 283d535.

📒 Files selected for processing (2)
  • plugins/highway_3d/plugin.json
  • plugins/highway_3d/screen.js

Comment on lines 645 to 648
const sz = sizeProvider && sizeProvider();
if (sz && sz.w > 0 && sz.h > 0 && (sz.w !== ctrl.lastW || sz.h !== ctrl.lastH)) {
ctrl.lastW = sz.w; ctrl.lastH = sz.h;
const wpx = sz.w + 'px', hpx = sz.h + 'px';
// Confine ALL layers to exactly the highway-canvas rect so the opaque
// backdrop can't bleed over the transport bar above the highway.
[ctrl.canvas, ctrl.backdrop, ctrl.scrim, ctrl.tint].forEach((el) => {
if (el) { el.style.width = wpx; el.style.height = hpx; el.style.right = 'auto'; el.style.bottom = 'auto'; }
});
try { ctrl.viz.setRendererSize(sz.w, sz.h); } catch (e) {}
_bcApplySize(sz.w, sz.h);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reapply sizing when the DPR or backing buffer changes.

Line [647] only detects CSS-size changes. If devicePixelRatio changes, or another path modifies canvas.width/canvas.height while sz.w and sz.h remain unchanged, _bcApplySize() is skipped and Butterchurn keeps stale dimensions.

Proposed fix
                 const sz = sizeProvider && sizeProvider();
-                if (sz && sz.w > 0 && sz.h > 0 && (sz.w !== ctrl.lastW || sz.h !== ctrl.lastH)) {
-                    _bcApplySize(sz.w, sz.h);
+                if (sz && sz.w > 0 && sz.h > 0) {
+                    const ratio = Math.min(window.devicePixelRatio || 1, 1.5);
+                    const bw = Math.max(1, Math.round(sz.w * ratio));
+                    const bh = Math.max(1, Math.round(sz.h * ratio));
+                    if (sz.w !== ctrl.lastW || sz.h !== ctrl.lastH ||
+                        canvas.width !== bw || canvas.height !== bh) {
+                        _bcApplySize(sz.w, sz.h);
+                    }
                 }

As per coding guidelines, re-run applySize() when the backing-store canvas size changes because the splitscreen plugin may bypass renderer.resize().

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const sz = sizeProvider && sizeProvider();
if (sz && sz.w > 0 && sz.h > 0 && (sz.w !== ctrl.lastW || sz.h !== ctrl.lastH)) {
ctrl.lastW = sz.w; ctrl.lastH = sz.h;
const wpx = sz.w + 'px', hpx = sz.h + 'px';
// Confine ALL layers to exactly the highway-canvas rect so the opaque
// backdrop can't bleed over the transport bar above the highway.
[ctrl.canvas, ctrl.backdrop, ctrl.scrim, ctrl.tint].forEach((el) => {
if (el) { el.style.width = wpx; el.style.height = hpx; el.style.right = 'auto'; el.style.bottom = 'auto'; }
});
try { ctrl.viz.setRendererSize(sz.w, sz.h); } catch (e) {}
_bcApplySize(sz.w, sz.h);
}
const sz = sizeProvider && sizeProvider();
if (sz && sz.w > 0 && sz.h > 0) {
const ratio = Math.min(window.devicePixelRatio || 1, 1.5);
const bw = Math.max(1, Math.round(sz.w * ratio));
const bh = Math.max(1, Math.round(sz.h * ratio));
if (sz.w !== ctrl.lastW || sz.h !== ctrl.lastH ||
canvas.width !== bw || canvas.height !== bh) {
_bcApplySize(sz.w, sz.h);
}
}
🤖 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 `@plugins/highway_3d/screen.js` around lines 645 - 648, Update the sizing check
near _bcApplySize to also detect changes in the backing canvas dimensions, not
only sz.w and sz.h or the device pixel ratio. Reapply sizing when canvas.width
or canvas.height differs from the dimensions tracked by the control state,
including changes made outside renderer.resize(), and update those tracked
values after applying the size.

Source: Coding guidelines

@byrongamatos
byrongamatos merged commit e134f5c into main Jul 10, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants