Commandar Rick - hey @Techmannih is this still ...
hey @Techmannih is this still open? https://github.com/tscircuit/jscad-electronics/issues/152
97 Replies
Yes
cool ill try to fix it
can i get a repro for this?
@Techmannih
So the pads intersecting here is the issue right?
@Techmannih
is this in the right direction?
also bun test failed for this, but it failed before i made any changes, this does not seem be related to the fix
here are the changes made, If you’d like, I can push a tiny clearance (e.g., 0.001) above z=0 for bodies that start exactly at 0, but you may find flush alignment acceptable for visualization.
@Techmannih
make sure you have latest deps running
bun i, update the snapshots using BUN_UPDATE_SNAPSHOTS=1sure lemme try that
@shibo so the smc test did pass but the rest two still fail and they seem to be unlreated to the changes made
even after undoing the changes by git reset these still seem to fail
https://github.com/tscircuit/jscad-electronics/pull/175
i have put up a draft pr for the same, all tests seem to work, please see @shibo @Techmannih
include a screenshot
sure wait
the screenshot should show the change + the name of the test file you're showcasing
the bun test passes in the ci, which means it's an issue from your end
what comes to mind is upgrading the bun version, deleting the node modules and rerunning
bun iokay lemme try that too
meanwhile updated the screenshot and the same video used here
No more issues!!
what was the fix?
updating bun and building it again
thats all
please lemme know if any other change is required
can you explain/investigate why you had to translate in the first place? why was not in that position by default?
Pads were originally centered at z=0 (0.01 thick), so they extended from −0.005 to +0.005, while most bodies assume z=0 is the board top and start building upward from there—causing a slight overlap. Translating pads down by −0.005 places their top exactly at z=0, aligning copper with the board-top convention and removing intersections without per-package tweaks.
cool, since you also changed the plated hole pad, please include a screenshot of it too
Hey since the issue was merged before I could post that is it still required or is it okay?
I checked it and it looks fine, just keep in mind next time to create tests for any change you made, explain the changes in the PR body and showcase any visual change with a screenshot or video
since you already did the checking yourself why make the reviewer go though it again? when a reviewer is doing many reviews it adds up, I haven't done so many reviews myself but that's how I see it
Okay sure sure, i did put them up on discord but forgot to do so on github
yeah makes sense
also i noticed an issue w the sponsrships, the sponsorships csv that is being generated this month didnt reflect contributions of this week , this week me and rishabcodes had 3 contribution stars this week but the sponsorship csv generated afterwards seems to have it 0
you have 3 points not 3 stars
this should be in the readme but:
oh no thats not what i meant
i checked the generate-sponsorship-csv.ts script and that uses the contribution score (3 in my case) and uses that info from the json to calculate sponsorships based on that right?
no, it calculates the sponsorship based on number of stars
this should be clearer in the readme tbh
oh okay understood
thanks for clearing that up
ill start with https://github.com/tscircuit/pcb-viewer/issues/398
Hey @shibo cool if I take this https://github.com/tscircuit/contribution-tracker/issues/248?
is this good?
or should it be more detailed
https://github.com/tscircuit/contribution-tracker/pull/251
looks good, one thing is that ai evaluates 1-3 star rating per PR, 4 and 5 stars must be manually given by staff
make it directly under this section
and a reference to it under "Contributor Overview"
Cool will do this
Good?
nice, I'll request Seve's review now
Okay thanks
hey @shibo back in July 2024, someone manually added a "3 major sections" list with navigation links to the README.md. However, this was never part of the code generation logic in generateMarkdown.ts. This meant that every time the weekly overview script ran, it would regenerate the README and potentially overwrite those helpful navigation links. The sections themselves were being generated. i have manually added the sections in the script itself, hope thats okay
does the generate markdown script replace the README.md file or modify it?
The generate markdown script does not replace the entire README.md file - it only modifies a specific section of it that is marked with special HTML comments
also tried my hand at https://github.com/tscircuit/pcb-viewer/issues/398
fixed it seems like issue was two problems in src/lib/Drawer.ts: (1) a missing inner3 color mapping in LAYER_NAME_TO_COLOR, and (2) an incorrect DEFAULT_DRAW_ORDER that placed inner layers at the end of the rendering sequence, causing inner5 and inner6 to have the lowest z-index values and appear hidden behind other layers. The fix added the missing inner3 entry and reordered the layers from bottom-to-top (board → inner6 through inner1 → bottom → top) so all 8 layers now render correctly with proper visibility and stacking.
before and after here
I had a doubt. Top layer disappears when other layers are selected, like in this image. the others remain at 0.5 opacity, is this behavior expected?
I have observed the same thing in other circuits too that when you click any other layer, top layer (red) disappears, so it might be the expected behaviour but wasnt sure so thought its better to ask
@shibo please update me whenever you can, thanks
It shouldn't disappear, the selected layer is at full oppacity and the rest are at 0.5
Also the selected layer should appear above all other layers
okay lemme look into that, just verified on https://pcb-viewer.vercel.app/?fixture={%22path%22%3A%22src%2Fexamples%2Fmulti-layer%2Fsix-layer-circuit.fixture.tsx%22}
this seems to be an issolated issue at my end
? what does this mean i didnt quite understand
if elements of different layers are stacked on top of each other, the selected layer element should be above the others(higher z index)
The top layer was disappearing when other layers were selected because the orderAndFadeLayers() function in src/lib/Drawer.ts was placing the selected layer (foregroundLayer) early in the rendering order array, causing it to receive a lower z-index than layers that came later in the array.
The Fix: Restructured the layer ordering logic to place the foregroundLayer at the END of the order array, ensuring it always gets the highest z-index. The z-index calculation was updated so the selected layer gets zIndexMap.topLayer (highest), its associated silkscreen gets zIndexMap.topLayer + 1, and all other layers get progressively lower z-index values. This ensures the selected layer always appears on top with full opacity while other layers remain visible at 50% opacity.
Verification: Created a new test fixture (overlapping-layers-test.fixture.tsx) with all 8 layers having pads at the exact same position (0, 0) with decreasing sizes. This creates a visual "bullseye" effect where you can clearly see which layer is rendered on top. By selecting different layers using hotkeys 1-8 or the layer dropdown, you can verify that the selected layer's pad always appears fully opaque on top while all other layers remain visible beneath at 50% opacity, confirming the z-index stacking works correctly.
cool, please link thepr
lemme first share the video of it working,
if it seems fine will put the pr
here
also the eight-layer-circuit.fixture.tsx was taken directly from the orignal issue and json provided within https://github.com/tscircuit/pcb-viewer/issues/398
share a PR and add the video to the PR body
okay sure will add a small description of the fix too
https://github.com/tscircuit/pcb-viewer/pull/440
instead of "(1 ⭐ = 1 star, 👑 = 3 stars)" do "(⭐ = 1 star, ⭐⭐⭐ = 3 stars)" we don't care too much about crowns currently
I merged your PR, you can make a new one
okay cool cool will do this rn
updated the readme as well as the snapshot? this is what you meant right?
"(1 ⭐" > "(⭐"
as requested
Pushed please review
https://github.com/tscircuit/contribution-tracker/pull/252
Hey @shibo can you also check https://github.com/tscircuit/pcb-viewer/pull/440
Thanks for the review @shibo
🥳
nice work, pretty good for first time contributions
thanks man
if its okay id like to try this next https://github.com/tscircuit/contribution-tracker/issues/139
seems like an old issue but still hasnt been fixed yet
@Commandar Rick turns out the issue was not really fixed: please watch the vid in this PR, review it and then review the PR before it which I also made: https://github.com/tscircuit/pcb-viewer/pull/445
you will notice that my first PR simplified the layer ordering logic and put the drill layer in it's correct order
the second PR put the board layer in it's correct order, but that reintroduced the issue, which means there is a bug that has nothing to do with layer ordering
I guess it's displaying a maximum number of layers,
the last 2 layers in the order are not drawn
if we have 2 more layers then the last 4 layers will not be drawn
that's why when the board and drill layers where pushed to the end of the order they disappeared and layer5 + layer6 appeared
Okay let me look into that
So i tried creating a ten layer circuit but only 2 were not displayed, if the max number of layers was the issue shouldn't there be 4 layers missing
it was just a guess, try to some debugging where those layers are drawn and see if the missing layers behave differently
i might have fixed it wait lemme send you the summary
I'm going to be afk now, I'll review when I can
okay okay
@shibo I identified that the topLayer value in z-index-map.ts was set to 10, which was too low for PCBs with many layers. The z-index calculation formula topLayer - (order.length - i) would produce negative z-indexes when you have more than 10 layers in the rendering order, causing those layers to either not render or render incorrectly behind other elements. increased topLayer from 10 to 20 to support up to 20 layers without negative z-indexes, and also bumped up the other overlay z-indexes proportionally to ensure they stay above the PCB layers.
on the safer side if there are any other examples you'd like me to create and test before putting the pr please let me know
also this seems unrelated but didnt se any issues for this anywhere, thought id bring this to your attention
this is live btw, same issue while running locally dont know what its about
Hey @shibo did you check this out?
I will asap, you may wany to start on another issue meanwhile, I personally work on multile issues at the same time so that I'm not blocked on reviews
Okay, have started on the issue for the fonts repo you asked me about in the morning
it's because the file is exporting circuitJson not a pcbViewer
I'd like to see a PR, it's better if you give me the full context even if the PR is not ready
Sure will put up a pr soon
hey @shibo , sorry for the dealay
https://github.com/tscircuit/pcb-viewer/pull/447
please review the changes
the type check error isnt related to the fix, rather the ten layer multi circuit, will try solving it
added the description and the video
hey @shibo do you want me to remove inner7 and inner8 or the entire ten multi circuit file?
hey @Techmannih do you want me to remove inner7 and inner8 or the entire ten multi circuit file
yes you need to remove them, these don’t exist. see comments
okay but without them the fixture becomes an 8 layer one (inner1-inner6, top and bottom)
and that already exists so there wont be any need for the 10 layer file if i am on the right track
in that case wont it be better to remove the file
@Techmannih
or do you want to duplicate some pads on inner5 or inner6 to test this?
as a ten layer fixture
I don’t think you need to add support for 10 layers — just confirm with Shibo.
okay will do once he sees this @shibo
meawhile is it okay if i remove the ten layer file or should i keep as is?
from the pr
yeah just remove it, it was good for debugging and figuring out what's going on, but no need to add it to a test
Okay cool, will do and update rest looks good? Or do you need me to make any changes or do any other tests to make sure this doesn't break any other circuit?
Hey I also had a doubt say someone didn't have a stripe account can they recieve sponsorship amount in any other way? Or algora bounty any other way, my friend also wanted to contribute but she can't make a stripe account since india doesn't support individual account holders to recieve individual payments?
I believe you must use stripe
Is there no other way?
I believe not
Okay thanks anyways
Do lemme know I'm removing the ten layer circuit
done @shibo please review
removed and updated
https://github.com/tscircuit/pcb-viewer/pull/447
@Exceluyi see how @Commandar Rick does it — he always asks his doubts first, confirms what the issue actually is, and then decides how to approach it. I think that’s a really good practice.