Skip to content

Extending igl::cat to work with std::vectors AND Allowing N-gons in writeOBJ#1108

Merged
jdumas merged 18 commits intolibigl:devfrom
MeshGeometry:dev
Apr 27, 2019
Merged

Extending igl::cat to work with std::vectors AND Allowing N-gons in writeOBJ#1108
jdumas merged 18 commits intolibigl:devfrom
MeshGeometry:dev

Conversation

@lawsonfulton
Copy link
Copy Markdown
Contributor

I've extended igl::cat to accept a std::vector of matrices. I've also added a test case.

Also in this pull request: Template instantiations and a bug fix for adjacency_list (it wasn't working with std::vector).

Check all that apply (change to [x])

  • All changes meet libigl style-guidelines.
  • Adds new .cpp file.
  • Adds corresponding unit test.
  • Adds corresponding python binding.
  • This is a minor change.

Copy link
Copy Markdown
Collaborator

@jdumas jdumas left a comment

Choose a reason for hiding this comment

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

Looks fine, but I don't understand the change from MatrixBase to PlainObjectBase (the FAQ says to use MatrixBase for inputs).

@jdumas jdumas added the feature label Mar 21, 2019
@jdumas jdumas mentioned this pull request Mar 21, 2019
3 tasks
@lawsonfulton lawsonfulton changed the title Extending igl::cat to work with std::vectors Extending igl::cat to work with std::vectors AND Allowing N-gons in writeOBJ Mar 22, 2019
@lawsonfulton
Copy link
Copy Markdown
Contributor Author

Thanks @jdumas I think this is all set. Is there anything I still need to do?

Copy link
Copy Markdown
Collaborator

@jdumas jdumas left a comment

Choose a reason for hiding this comment

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

Hi, sorry for the delay. I spotted two tinsy details that ought to be fixed, but then I believe it's good to go.

@lawsonfulton
Copy link
Copy Markdown
Contributor Author

Hopefully that does it :)

@jdumas jdumas merged commit 4509358 into libigl:dev Apr 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants