Skip to content

Fix plated_holes showing wrong color on bottom layer in PCB viewer#698

Closed
rushabhcodes wants to merge 1 commit intotscircuit:mainfrom
rushabhcodes:fix/plated-holes-bottom-layer-color
Closed

Fix plated_holes showing wrong color on bottom layer in PCB viewer#698
rushabhcodes wants to merge 1 commit intotscircuit:mainfrom
rushabhcodes:fix/plated-holes-bottom-layer-color

Conversation

@rushabhcodes
Copy link
Copy Markdown
Contributor

@rushabhcodes rushabhcodes commented Mar 12, 2026

Summary

Fixed a bug where plated_holes (through-hole pads) were displaying with the wrong color when the bottom copper layer was selected in the PCB viewer.

This replicated the behavior of kicad

Changes

  • Added selectedLayer parameter to drawPlatedHolePads function
  • Added BOTTOM_COPPER_COLOR_MAP color override that maps copper.top to the bottom copper color (blue)
  • Applied color override when selectedLayer === "bottom" to make plated holes match the color of bottom traces and SMT pads
image image

Copilot AI review requested due to automatic review settings March 12, 2026 22:09
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 12, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
pcb-viewer Ready Ready Preview, Comment Mar 12, 2026 10:09pm

Request Review

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes incorrect plated-hole (through-hole pad) coloring when viewing the bottom copper layer in the PCB viewer by introducing a bottom-copper color override for plated-hole rendering.

Changes:

  • Added a selectedLayer parameter to drawPlatedHolePads and passed it from CanvasPrimitiveRenderer.
  • Introduced BOTTOM_COPPER_COLOR_MAP to remap copper.top to the bottom copper color when appropriate.
  • Applied the override for both normal and highlighted plated-hole drawing paths.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/lib/draw-plated-hole.ts Adds bottom-copper color override logic and threads selectedLayer into plated-hole pad rendering.
src/components/CanvasPrimitiveRenderer.tsx Passes the current selectedLayer through to plated-hole rendering.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +79 to +81
if (selectedLayer === "bottom") {
drawer.configure({ colorOverrides: BOTTOM_COPPER_COLOR_MAP })
}
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

The bottom-copper color override is currently keyed off selectedLayer === "bottom", which will also recolor plated-hole pads drawn on the top canvas when the user is viewing the bottom layer (non-foreground layers are still rendered at 0.5 opacity). This makes top-layer plated-hole pads appear blue while top traces/SMT pads remain the top color, causing a visible mismatch/regression. Consider applying the override based on the layer(s) being drawn (e.g., when layers includes "bottom_copper") or by passing an explicit copper-side flag / colorOverrides for the specific canvas, rather than using the global selected layer.

Copilot uses AI. Check for mistakes.
Comment on lines +88 to +97
if (selectedLayer === "bottom") {
highlightDrawer.configure({
colorOverrides: {
...HOVER_COLOR_MAP,
copper: {
...HOVER_COLOR_MAP.copper,
top: HOVER_COLOR_MAP.copper.bottom,
},
},
})
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

The highlighted-path bottom override is duplicated inline here (merging HOVER_COLOR_MAP and overriding copper.top). To reduce duplication and keep behavior consistent with the non-highlighted override, consider extracting a dedicated BOTTOM_HOVER_COLOR_MAP constant (similar to BOTTOM_COPPER_COLOR_MAP) and reusing it.

Copilot uses AI. Check for mistakes.
Comment on lines 47 to 49
drawSoldermask?: boolean
selectedLayer?: string
}) {
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

selectedLayer is typed as a plain string, but callers pass useGlobalStore((s) => s.selected_layer) which is a LayerRef from circuit-json. Typing this parameter as LayerRef (or a narrower union like "top" | "bottom" if that's all you support) will prevent accidentally passing incompatible layer names and make the intent clearer.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@seveibar seveibar left a comment

Choose a reason for hiding this comment

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

Do you see how this is confusing?

@rushabhcodes
Copy link
Copy Markdown
Contributor Author

i think i can simplify this by making some changes in circuit-to-canvas

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.

3 participants