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

Texture #13

Merged
merged 6 commits into from
Oct 11, 2016
Merged

Texture #13

merged 6 commits into from
Oct 11, 2016

Conversation

janzill
Copy link
Contributor

@janzill janzill commented Oct 6, 2016

Simple image loading via texture files. plotimage() takes a 4x3 matrix for the corners of the image and a path to an image file.

Overwrite points or lines with the same label

See plot3d for documentation
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

This doc should be updated for images :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And I should also add some documentation for the two functions above.


#-------------------------------------------------------------------------------
# Texture file
function write_ply(texturefile::String, filename::String, vertices::AbstractArray)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be called write_ply_texture?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, it's not a generic ply-writer, thanks.

@janzill
Copy link
Contributor Author

janzill commented Oct 10, 2016

Thanks for the review @JoshChristie , I fixed it up

@c42f
Copy link
Owner

c42f commented Oct 11, 2016

Thanks Jan, I just had a look this looks great. Also thanks a heap Josh for reviewing it.

Just one comment - perhaps plotimage should be exported?

@janzill
Copy link
Contributor Author

janzill commented Oct 11, 2016

Done, thanks Chris.

@c42f c42f merged commit 951690b into c42f:master Oct 11, 2016
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