-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Harmonize default representation in PCLVisualizer #1132
Conversation
I agree that both |
Friendly pi(n)g 🐷 @jspricke |
Victor, I'm not sure I understand the logic behind changing the default representation of all shapes. As far as I understand, the original complaint was only about different plane functions using different representations. Why do we need to change everything else? |
The idea is to be consistent; either a shape should be added as wireframe or surface by default. At the moment one version of the So we should change the |
Still don't get why we need consistency between different shapes. Planes are better off with surfaces, yes, but why everything else should be the same? |
We don't need consistency, but it would be better. |
Ok, what about cubes? I never used it myself, but I would expect that people may use it to outline a detected object, in which case wireframe is more appropriate... |
+1 for switching all to surface. |
Ok, I'll merge then. |
It really depends on the usage; I used the cube to model a real cube in my robot workspace for example. setShapeRenderingProperties is available for those who want to change the rendering anyway. |
Harmonize default representation in PCLVisualizer
All shapes are rendered as wire-frame by default; I see no reason why one of the
addPlane
should not follow that rule.Reported by Silex: http://www.pcl-users.org/pcl-visualization-PCLVisualizer-addPlane-function-draws-only-boundary-of-the-plane-tp4036992p4037214.html
EDIT: as Sergey suggested; I switched them all to surface, for the line I see no change between wireframe/surface.