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

sensor_orientation_ not set to identity #176

Closed
wants to merge 2 commits into from
Closed

sensor_orientation_ not set to identity #176

wants to merge 2 commits into from

Conversation

nfioraio
Copy link
Contributor

@nfioraio nfioraio commented Jul 8, 2013

LZFDepth16ImageReader and OpenNIGrabber do not leave to identity rotation the sensor_orientation_ field of the cloud, giving incorrect visualization when the rotation is applied (see pull request #92 and issue 116 #116).

@jspricke
Copy link
Member

jspricke commented Jul 8, 2013

Hi Nicola, thanks for the patch. Some comments:

  • Could you please remove the merge.
  • Please don't remove all the whitspaces. I know they are annoying, but your actual changes are hard to find in all these trivial ones. Also, it would result in attributing all the lines to you in a git blame. Removing them in the lines you are changing anyhow is fine, though.
  • As far as I can see, Radu originally added the initial values to fix compiler warnings. So I guess it would make sense to put identity values in instead. Could you please change that? Otherwise, please don't send patches with commented code, just remove it (we have git to get the old code back ;) ).

Thanks!

Jochen

@nfioraio nfioraio closed this Jul 8, 2013
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.

2 participants