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

Fewer batches to sketch a cube #2049

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

adamchalmers
Copy link
Collaborator

@adamchalmers adamchalmers commented Apr 9, 2024

part of #1969

Previously, sketching a cube took 4 batches:

[MakePlane, SketchModeEnable, StartPath, MovePathPen, ExtendPathx3, ClosePath, SketchModeDisable]
[Extrude]
[ObjectBringToFront]
[Solid3DGetExtrusionFaceInfo]

This means each cube takes 4 batches at 16ms each, so, 66ms per cube.

Now, there's only 2 batches (33ms per cube):

[MakePlane, SketchModeEnable, StartPath, MovePathPen, ExtendPathx3, ClosePath, SketchModeDisable]
[Extrude, ObjectBringToFront, Solid3DGetExtrusionFaceInfo]

This can definitely be improved further, but I'm breaking into small PRs.

Copy link

vercel bot commented Apr 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview Apr 11, 2024 10:42pm

Copy link

codecov bot commented Apr 9, 2024

Codecov Report

Attention: Patch coverage is 96.77419% with 1 lines in your changes are missing coverage. Please review.

Files Patch % Lines
src/wasm-lib/kcl/src/engine/mod.rs 95.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

… calls into one batch

This means sketching a cube takes 2 batches, not 1.
@lf94
Copy link
Contributor

lf94 commented Apr 9, 2024

LGTM. I thought by now we could make it all 1 batch request.

@adamchalmers
Copy link
Collaborator Author

adamchalmers commented Apr 9, 2024

Hmm, this PR has a bug -- my "draw three cubes" program works fine in a Rust unit test, but doesn't actually work when I run the app. My theory is that something is broken in the WASM connection but not the normal Rust connection.

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.

None yet

3 participants