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

Specializes DigitalSurface::facesAroundVertex in the 3D case #1377

Merged

Conversation

JacquesOlivierLachaud
Copy link
Member

@JacquesOlivierLachaud JacquesOlivierLachaud commented Jan 10, 2019

PR Description

This PR specializes the method DigitalSurface::facesAroundVertex in the 3D case, such that faces (ie pointels) are ordered counterclockwise with respect of the vertex (ie surfel) seen from the exterior.

Checklist

  • Unit-test of your feature by updating topology/testDigitalSurface
  • Doxygen documentation of the code completed (classes, methods, types, members...)
  • Documentation module page added or updated.
  • New entry in the ChangeLog.md added.
  • No warning raised in Debug cmake mode (otherwise, Travis C.I. will fail).
  • All continuous integration tests pass (Travis & appveyor)

@JacquesOlivierLachaud JacquesOlivierLachaud changed the title Add test Specializes DigitalSurface::facesAroundVertex in the 3D case Jan 10, 2019
Copy link
Member

@dcoeurjo dcoeurjo left a comment

Choose a reason for hiding this comment

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

Thanks ;)
Just few comments to clarify the doc

ChangeLog.md Outdated Show resolved Hide resolved

@param order_ccw_in_3d when 'true', orders faces
counterclockwise around vertex (solely in 3d). It corresponds
to ordering the pointels counterclockwise around the given
Copy link
Member

@dcoeurjo dcoeurjo Jan 16, 2019

Choose a reason for hiding this comment

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

counterclockwise wrt to what ?

Copy link
Member Author

Choose a reason for hiding this comment

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

it means around the normal vector (I think this is the usual definition).

Copy link
Member Author

Choose a reason for hiding this comment

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

I have updated the doc here and in class DigitalSurface

Copy link
Member

Choose a reason for hiding this comment

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

Perfect. Many thanks

@dcoeurjo
Copy link
Member

ping @JacquesOlivierLachaud , I'd like to merge this PR ;)

@dcoeurjo
Copy link
Member

All good. Let me see if I can fix the conflict for you...

@dcoeurjo
Copy link
Member

Conflict ok but I don’t know why the tests are not launched...

@dcoeurjo
Copy link
Member

👌 merging.thx

@dcoeurjo dcoeurjo merged commit 7e351c4 into DGtal-team:master Jan 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants