-
Notifications
You must be signed in to change notification settings - Fork 6
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
Markups planes export #12
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this, this will be useful. I've added a few comments that should simplify things a bit.
b2520a7
to
1a46cf9
Compare
ENH: redefine createPlaneModelFromMarkupsPlane
@lassoan did I achieve the rebase you wanted? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, it looks good, I've just added a few nitpicky comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, this looks good!
I did not find a way to not duplicate some code on the
addModelsToRenderer
function because its recursive and I didn't wanted to analyze it too much (but I can do it if required).It works