Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testing elkjs #528

Closed
wants to merge 1 commit into from
Closed

testing elkjs #528

wants to merge 1 commit into from

Conversation

ShiboSoftwareDev
Copy link
Contributor

@ShiboSoftwareDev ShiboSoftwareDev commented Jan 16, 2025

  • need to wait for elkjs to finish the layout before you start routing the traces
  • some ports have schematic labels yet they impact the components position to get closer to their connected port, this should be fixed
  • elkjs is highly customizable but it's poorly documented
  • schY and schX need to be reset to 0 before you start the auto layout in my current implementation
  • for now we make autoSchematiclayoutEnabled true by default
  • schX and schY should override the auto layout
  • try to get schematic drag and drop working on top of the auto layout after this is implemeneted

this is just a showcase; do not merge

@@ -279,7 +279,8 @@ export abstract class PrimitiveComponent<
* schematic components
*/
computeSchematicPropsTransform(): Matrix {
return compose(translate(this.props.schX ?? 0, this.props.schY ?? 0))
// need to find a way to make schX and schY 0 before autolayout and apply them after autolayout
return compose(translate(0, 0))
Copy link
Contributor

Choose a reason for hiding this comment

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

i talked about this on stream today at the 25m point, drew a diagram

https://x.com/seveibar/status/1879928186630443342

image

#529

this._queueAsyncEffect("schematic-layout", async () => {
try {
await layoutSchematicWithElk(db)
this._markDirty("SchematicLayout")
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

const ports = db.schematic_port.list()

const elk = new Elk({
workerUrl: "./node_modules/elkjs/lib/elk-worker.min.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

we'll need to load this via blobUrl probably (unfortunately)

will require bundling step, similar to @tscircuit/eval-webworker

I'm thinking maybe we create a separate package for this just to simplify where this code lives and to make as many test cases as possible, but not sure

Copy link
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.

this is great, so my thought on this is, we might not want to remove the old stuff which I know how to improve (the schematic autolayout algo) versus ElkJS which I don't know the limitations of we might wanna create an repo for a new node project that specifically test the elk JS layout.

We could have a repo that's just filled with tests that we can use to test lots of different layouts, kind of like this: https://github.com/tscircuit/schematic-autolayout

We could literally use the setup in this repo, but I would start anew using snapshot testing, i created a repo here: https://github.com/tscircuit/elkjs-autolayout

Copy link

This PR has been automatically marked as stale because it has had no recent activity. It will be closed if no further activity occurs.

Copy link

This PR was closed because it has been inactive for 1 day since being marked as stale.

@github-actions github-actions bot closed this Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants