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

Exaggerate clip plane size #490

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou commented Oct 16, 2024

The clipplane mesh is not fully filling the gap left by the clipping effect. This PR fixes it.

Before:

Screencast.from.2024-10-16.20-46-22.mp4

After:

Screencast.from.2024-10-16.20-44-32.mp4

@martinRenou martinRenou marked this pull request as ready for review October 16, 2024 18:47
Copy link
Contributor

Preview PR at appsharing.space

Copy link
Contributor

Integration tests repot: appsharing.space

Copy link
Member

@arjxn-py arjxn-py left a comment

Choose a reason for hiding this comment

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

Super 🚀
Thanks @martinRenou

@arjxn-py
Copy link
Member

After:

How do you turn these controls to the sphere thing? That's soo coool

@SylvainCorlay
Copy link
Member

Could this be computed base on the e.g. the bounding box of the whole jcad document, or would that be too expensive? (I guess bounding boxes of individual objects could be cached)

@trungleduc
Copy link
Member

Could this be computed base on the e.g. the bounding box of the whole jcad document, or would that be too expensive? (I guess bounding boxes of individual objects could be cached)

this._refLength is the size of the bounding box of the whole jcad document.

@trungleduc trungleduc merged commit 0723f22 into jupytercad:main Oct 16, 2024
11 of 12 checks passed
@martinRenou martinRenou deleted the exaggerate_clipplane branch October 17, 2024 07:18
@martinRenou
Copy link
Member Author

How do you turn these controls to the sphere thing? That's soo coool

That's the "R" keyboard shortcut :)

@martinRenou
Copy link
Member Author

this._refLength is the size of the bounding box of the whole jcad document.

By the way, testing out this morning, I see that refLength is quite small and is not updating properly.

I'm looking into it.

Multiplying by 1000 here still makes sense, we just want to make sure the plane is bigger, if I could make an infinite plane I would.

@SylvainCorlay
Copy link
Member

SylvainCorlay commented Oct 17, 2024

By the way, testing out this morning, I see that refLength is quite small and is not updating properly.

Is this still required after #493?

(just reacting to the magicall x1000 constant, which might not even be good if one uses a completely different unit...)

@martinRenou
Copy link
Member Author

I'm not sure why you're saying it might not be good?

@martinRenou
Copy link
Member Author

What we need is an infinite plane, but there is no such thing in WebGL. Multiplying the reflength by a big number allows us to make sure the size of the plane is big enough to cover the shapes.

@martinRenou martinRenou added the bug PR that fixes a bug label Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug PR that fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants