Skip to content

Conversation

@mdkus
Copy link
Contributor

@mdkus mdkus commented Feb 9, 2015

Correction of #1129.

@mdkus
Copy link
Contributor Author

mdkus commented Feb 11, 2015

To see the bug, please link against vtk6.x.
 

Gesendet: Mittwoch, 11. Februar 2015 um 12:02 Uhr
Von: "Victor Lamoine" notifications@github.com
An: PointCloudLibrary/pcl pcl@noreply.github.com
Cc: mdkus mdkus@web.de
Betreff: Re: [pcl] fix: wrong position and scale of context items when resizing ImageViewer (vtk6) (#1144)

I tried to test but I can't see the bug;

#include <pcl/visualization/image_viewer.h>

int
main (int argc,
      char** argv)
{
  pcl::visualization::ImageViewer viewer;
  viewer.addRectangle (100, 200, 50, 250);
  viewer.spin ();
  return (0);
}

Nothing is wrong when I resize the viewer window; what am I doing wrong?
PCL latest trunk linked against VTK 5.8


Reply to this email directly or view it on GitHub.

 

@mdkus
Copy link
Contributor Author

mdkus commented Mar 2, 2015

@VictorLamoine Victor?

@mdkus
Copy link
Contributor Author

mdkus commented Mar 9, 2015

@taketwo Sergey?

@VictorLamoine
Copy link
Contributor

Back in business, will take a look at this in the week. Sorry for the delay

@taketwo
Copy link
Member

taketwo commented Mar 9, 2015

Sorry.

Actually, for me this example also works fine when resized (vtk6.1):

#include <pcl/visualization/image_viewer.h>

int
main (int argc, char** argv)
{
  pcl::visualization::ImageViewer viewer;
  viewer.addRectangle (100, 200, 50, 250);
  viewer.spin ();
  return (0);
}

@taketwo
Copy link
Member

taketwo commented Mar 9, 2015

Nevertheless, the patch gives a certain improvement on vtk6.1. Here is how "select_object_plane" app behaves before and after the patch.

Before the patch

The window looks like this:

image

When I resize it to half the height, it looks like this:

image

It's not so clear from the screenshots, but the image is scaled, whereas the overlay stays exactly the same.

After the patch

The window looks like this:

image

When I resize it to half the height, it looks like this:

image

Now the overlay is also resized, though it's still off and not to scale.

@VictorLamoine
Copy link
Contributor

I'm not sure how I can test this, you deleted your repository; unknown repository

@taketwo
Copy link
Member

taketwo commented Mar 11, 2015

@VictorLamoine Should be available in our repository. See this for instructions.
@mdkus Why are you deleting your fork all the time? :)

@VictorLamoine
Copy link
Contributor

Thank you Sergey I never faced this situation!

@VictorLamoine
Copy link
Contributor

I made short videos because it's hard showing this with images; all is tested with the patch

VTK 6.3 (trunk)

The resizing looks fine but the shapes goes out of the window scope when the window is resized small.
Video

VTK 5.8

Nothing is resized, is that the expected behavior? I don't think so
Video

@mdkus
Copy link
Contributor Author

mdkus commented Mar 12, 2015

@taketwo I renew the forks because I haven't found a way to make a new pull request when an older one is not closed. If I take the same fork, the new commit is added to the open pull request and doesn't create a new one. Can you help me to avoid this?

@mdkus
Copy link
Contributor Author

mdkus commented Mar 12, 2015

@VictorLamoine With VTK 5.8 nothing is resized, that is the expected behaviour! Everything is alright. Images don't resize as well, because they are drawn with another vtk/opengl technics: 'glDrawPixels'

The introduction of 'vtkImageSlice' changes this. For VTK it is a textured entity such as a sphere, a line and so on. The price to pay are resizable images, because other vtk classes are involved. So the 2D context items have to be resizable too, which is very unuseful in vtk. I try to open your videos. There seems to be still an error in the patch.

If the resizing behaviour of the context items can't be customized, a withdrawal of 'vtkImageSlice' should be discussed.

@VictorLamoine
Copy link
Contributor

Use branches, each pull request should refer to a branch of a fork.
That way, you just have to delete branches when the pull request is merged/closed and create a new branch for a pull request.

Note that if you closed a pull request, you can open a new pull request against the same branch (without having to delete/create branches)

The videos are very lightweight, you should be able to open them easily with a good player such as VLC.

@mdkus
Copy link
Contributor Author

mdkus commented Mar 12, 2015

@VictorLamoine Thanks a lot, Victor! Good advice to use branches with fork. I'll use it accordingly in the future. And great idea to use videos for analyzing.

I saw your video 'vtk_6.3': The behaviour can be still correct. You can determine it better with the tool 'pcd_select_object_plane' in the pcl trunk 'app' directory. Load the file 'milk_cartoon_all_small_clorox.pcd' in the 'test' directory and click on the objects to produce 2d context items. Then risize the window. To see the background image, reduce opacity of the context items (especially 'PlanarPolygon', or everything is blue).

I created a Video as you made. I tested with vtk 6.2, the behaviour seems correct. But Sergey had a wrong behaviour with vtk 6.1. What is your result?

@VictorLamoine
Copy link
Contributor

With VTK 6.1 (and the patch) I get the same behavior as with VTK 6.3
I also got error messages (with the short code - rectangle test) but the viewer doesn't crash

$ ./vtk_scale 
ERROR: In /home/dell/libraries/VTK-6.1.0/Rendering/Context2D/vtkOpenGLContextDevice2D.cxx, line 144
vtkOpenGL2ContextDevice2D (0x2143880): failed after Begin 1 OpenGL errors detected
  0 : (1281) Invalid value


ERROR: In /home/dell/libraries/VTK-6.1.0/Rendering/Context2D/vtkOpenGLContextDevice2D.cxx, line 144
vtkOpenGL2ContextDevice2D (0x2143880): failed after Begin 1 OpenGL errors detected
  0 : (1281) Invalid value


ERROR: In /home/dell/libraries/VTK-6.1.0/Rendering/Context2D/vtkOpenGLContextDevice2D.cxx, line 144
vtkOpenGL2ContextDevice2D (0x2143880): failed after Begin 1 OpenGL errors detected
  0 : (1281) Invalid value

@taketwo
Copy link
Member

taketwo commented Mar 12, 2015

Here is how it looks on my side: video.

@VictorLamoine
Copy link
Contributor

I can't test with the pcl_pcd_select_object_plane, I never get to see the point cloud!
I press R and turn the view but all I can see is the coordinate system.

Video

I got this working before, don't know why it doesn't work now.

@taketwo
Copy link
Member

taketwo commented Mar 13, 2015

It's because one of the recent patches. The point cloud with milk carton has wrong alpha values, and it completely transparent. Before the transparency was not handled, but then you filed that issue, I made that patch... And here we go, correct behavior, but "mysteriously" broken applications. Keep that in mind, when proposing changes ;)

@mdkus
Copy link
Contributor Author

mdkus commented Mar 18, 2015

@taketwo I think it could be a problem of the initial viewport dimensions. I tested with purely vtk windows, both 'pcl_visualizer' and 'image_viewer'. Initial settings: 'cloud_->width' and 'cloud_->height' (640 x 480). In your video the vtk viewports seem to be embedded in another app. Could you test it with pure vtk windows? By the way, the peformance in the video is amazing, do you you use any parallel computing or cuda modules?

@taketwo
Copy link
Member

taketwo commented Mar 20, 2015

No, in my video vtk is not embedded anywhere. It's just that I use a tiling window manager. It does not respect the size set by the application and coerces it to fit a tile. I'm not sure if I can avoid this behavior for testing purposes... but I'll see what I can do.

No, don't use anything special. It's an i7 with discrete video.

@SergioRAgostinho
Copy link
Member

@mdkus I was thinking of including this for 1.9.0. Did you manage to see the issue through the end or did you just drop it at some point? :x

@SergioRAgostinho SergioRAgostinho added needs: more work Specify why not closed/merged yet needs: author reply Specify why not closed/merged yet labels Aug 22, 2016
@stale
Copy link

stale bot commented Dec 14, 2017

This pull request has been automatically marked as stale because it hasn't had
any activity in the past 60 days. Commenting or adding a new commit to the
pull request will revert this.

Come back whenever you have time. We look forward to your contribution.

@stale stale bot added the status: stale label Dec 14, 2017
@SergioRAgostinho SergioRAgostinho removed this from the pcl-1.9.0 milestone Jan 25, 2018
@stale stale bot removed the status: stale label Jan 25, 2018
@stale
Copy link

stale bot commented Mar 26, 2018

This pull request has been automatically marked as stale because it hasn't had
any activity in the past 60 days. Commenting or adding a new commit to the
pull request will revert this.

Come back whenever you have time. We look forward to your contribution.

@stale stale bot added the status: stale label Mar 26, 2018
@stale
Copy link

stale bot commented Feb 21, 2020

This pull request has been automatically marked as stale because it hasn't had
any activity in the past 60 days. Commenting or adding a new commit to the
pull request will revert this.

Come back whenever you have time. We look forward to your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs: author reply Specify why not closed/merged yet needs: more work Specify why not closed/merged yet status: stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants