(0027965)
Leo Liu (developer)
2012-01-24 17:22

As suggested, the fix implements vtkClipPlanesPainter::UpdateBounds.

(0030754)
xabi riobe (reporter)
2013-05-13 04:05

There is a transformation missing for the planes, leading to the incorrect culling of some props in the case they have a transformation matrix different than identity.

this->ClippingPlanes are expressed in world coordinates, after the use of the actor's matrix, whereas the bounds are in data coordinates

To clip the data, the planes are used in vtkOpenGLClipPlanesPainter::RenderInternal to be passed to glClipPlane after
beeing transformed from world to data coords with the use of the actor's matrix.
But in the rendering pipeline we have a call to vtkClipPlanesPainter::UpdateBounds where these planes are considered to be in the data coordinates.

Here is the calling stack:

vtkClipPlanesPainter::UpdateBounds ===> original data bounds,
but actor's world coordinates transformed planes
vtkPainterPolyDataMapper::ComputeBounds
vtkPolyDataMapper::GetBounds
vtkActor::GetBounds ===> actor's
transformation Matrix applied to the bounds
vtkFrustumCoverageCuller::Cull ===> Prop can be
incorrectly culled !!!!

So in the method vtkClipPlanesPainter::UpdateBounds, the planes should be transformed with the actor's matrix before changing the bounds.

(0030760)
xabi riobe (reporter)
2013-05-14 08:34

Here is a simple example to illustrate the issue, with attached screenshots of different situations:

clip.jpg: the mapper is clipped, the actor has no transformation
transform.jpg: the mapper is not clipped, the actor has a transformation
cliptransform.jpg: the mapper is clipped, the actor has a transformation ==> the actor is culled

#include <vtkActor.h>
#include <vtkAxesActor.h>
#include <vtkPolyDataMapper.h>
#include <vtkRenderer.h>
#include <vtkRenderWindow.h>
#include <vtkRenderWindowInteractor.h>
#include <vtkSmartPointer.h>
#include <vtkSphereSource.h>
#include <vtkPlanes.h>
#include <vtkTransform.h>

   vtkSmartPointer<vtkRenderer> ren1= vtkSmartPointer<vtkRenderer>::New();
   ren1->SetBackground(0.4, 0.4, 0.5);
   ren1->SetBackground2(0.2,0.2,0.3);
   ren1->SetGradientBackground(true);

   vtkSmartPointer<vtkRenderWindow> renWin= vtkSmartPointer<vtkRenderWindow>::New();
   renWin->SetSize( 972, 800 );
   renWin->AddRenderer(ren1);

   vtkSmartPointer<vtkRenderWindowInteractor> iren= vtkSmartPointer<vtkRenderWindowInteractor>::New();
   iren->SetRenderWindow(renWin);

   vtkSmartPointer<vtkSphereSource> sph= vtkSmartPointer<vtkSphereSource>::New();
   sph->SetPhiResolution(22);
   sph->SetThetaResolution(22);
   sph->SetRadius(2.0);
   sph->Update();

   vtkSmartPointer<vtkPlanes> planes= vtkSmartPointer<vtkPlanes>::New();
   planes->SetBounds(1.5,-1.5,1.5,-1.5,1.5,-1.5);

   vtkSmartPointer<vtkPolyDataMapper> Mapper= vtkSmartPointer<vtkPolyDataMapper>::New();
   Mapper->SetInputConnection(sph->GetOutputPort());
   Mapper->SetClippingPlanes(planes);

   vtkSmartPointer<vtkTransform> tr= vtkSmartPointer<vtkTransform>::New();
   tr->Translate(10,0,0);

   vtkSmartPointer<vtkActor> Actor= vtkSmartPointer<vtkActor>::New();
   Actor->SetMapper(Mapper);
   Actor->SetUserTransform(tr);

   vtkSmartPointer<vtkAxesActor> axes= vtkSmartPointer<vtkAxesActor>::New();
   axes->AxisLabelsOff();

   ren1->AddActor(Actor);
   ren1->AddActor(axes);

   ren1->ResetCamera();
   renWin->Render();
   iren->Start();

(0031309)
Dave DeMarle (administrator)
2013-07-22 20:37

Leo no longer works on the project. If these issues still exist in 6.0, please reopen them and assign to Dave DeMarle.

(0031669)
xabi riobe (reporter)
2013-10-01 03:45

This issue is still present in 6.0.0

(0032409)
David Gobbi (developer)
2014-02-25 16:23

The code in the vtkClipPlanesPainter::UpdateBounds() that was added on 2012-01-23 definitely looks wrong. It will only give the correct result when the actor transform matrix is the identity matrix. None of the tests for this method apply a transform to the actor (see SHA 6d3e4c83).

The mapper clipping planes are specified in world coordinates, so they cannot be used to directly adjust the bounds: they have to be transformed to data coordinates first. This is a problem, of course, because the UpdateBounds() method does not, and cannot, know what the actor transform is. Therefore, I think that it was a mistake to add UpdateBounds() to vtkClipPlanesPainter.

(0032412)
xabi riobe (reporter)
2014-02-26 04:40

Actually the given example is wrong since the planes are passed in data coordinate.
As David said, the UpdateBounds method is incorrect, but the initial problem of the ResetCamera still remains.

(0032427)
David Gobbi (developer)
2014-02-27 11:50

Another reason for removing vtkClipPlanesPainter::UpdateBounds() is that it introduced a discrepancy between the behavior of vtkPainterPolyDataMapper as compared to vtkPolyDataMapper.

Below is the ComputeBounds method for vtkPolyDataMapper. It does not take the clipping planes into account.

    void vtkPolyDataMapper::ComputeBounds()
    {
      this->GetInput()->GetBounds(this->Bounds);
    }

In other words, if someone had been using vtkPolyDataMapper, and then switched to vtkPainterPolyDataMapper in order to improve rendering speed, then ResetCamera might start giving different results! I think this is a bad thing. It is best for the two poly data mappers to have the same behavior.

(0032454)
xabi riobe (reporter)
2014-03-04 05:37

http://review.source.kitware.com/#/t/4024/ [^]

(0032458)
David Gobbi (developer)
2014-03-05 15:43

Adding UpdateBounds() in vtkClipPlanesPainter caused problems whenever the actor's matrix was not the identity matrix. This is because the Bounds are in data coordinates, but the clipping planes are in world coordinates. So the Planes can only be used to adjust the Bounds if the Planes are first converted to data coords, but it is not possible to do this transformation in UpdateBounds() because the painter does not have access to the actor's matrix during the Update phase.

Issue History
Date Modified	Username	Field	Change
2011-03-09 10:48	xabi riobe	New Issue
2011-06-16 13:12	Zack Galbreath	Category	Development => (No Category)
2012-01-17 09:23	Utkarsh Ayachit	Assigned To	=> Leo Liu
2012-01-17 09:23	Utkarsh Ayachit	Status	backlog => tabled
2012-01-24 17:22	Leo Liu	Note Added: 0027965
2012-01-24 17:22	Leo Liu	Status	tabled => customer review
2012-01-24 17:22	Leo Liu	Resolution	open => fixed
2013-04-05 20:31	Berk Geveci	Status	customer review => closed
2013-05-13 04:05	xabi riobe	Note Added: 0030754
2013-05-13 04:05	xabi riobe	Status	closed => backlog
2013-05-13 04:05	xabi riobe	Resolution	fixed => reopened
2013-05-14 08:34	xabi riobe	Note Added: 0030760
2013-05-14 08:34	xabi riobe	File Added: clip.jpg
2013-05-14 08:35	xabi riobe	File Added: transform.jpg
2013-05-14 08:35	xabi riobe	File Added: cliptransform.jpg
2013-07-22 20:37	Dave DeMarle	Status	backlog => expired
2013-07-22 20:37	Dave DeMarle	Note Added: 0031309
2013-10-01 03:45	xabi riobe	Note Added: 0031669
2013-10-01 03:45	xabi riobe	Status	expired => backlog
2014-02-25 16:23	David Gobbi	Note Added: 0032409
2014-02-26 04:40	xabi riobe	Note Added: 0032412
2014-02-27 11:50	David Gobbi	Note Added: 0032427
2014-03-04 05:37	xabi riobe	Note Added: 0032454
2014-03-04 05:37	xabi riobe	Status	backlog => gerrit review
2014-03-05 15:43	David Gobbi	Note Added: 0032458
2014-03-05 15:43	David Gobbi	Status	gerrit review => closed
2014-03-05 15:43	David Gobbi	Assigned To	Leo Liu => David Gobbi
2014-03-05 15:43	David Gobbi	Resolution	reopened => not fixable
2014-03-05 15:43	David Gobbi	Fixed in Version	=> 6.2.0

Notes
(0027965) Leo Liu (developer) 2012-01-24 17:22	As suggested, the fix implements vtkClipPlanesPainter::UpdateBounds.

(0030754) xabi riobe (reporter) 2013-05-13 04:05	There is a transformation missing for the planes, leading to the incorrect culling of some props in the case they have a transformation matrix different than identity. this->ClippingPlanes are expressed in world coordinates, after the use of the actor's matrix, whereas the bounds are in data coordinates To clip the data, the planes are used in vtkOpenGLClipPlanesPainter::RenderInternal to be passed to glClipPlane after beeing transformed from world to data coords with the use of the actor's matrix. But in the rendering pipeline we have a call to vtkClipPlanesPainter::UpdateBounds where these planes are considered to be in the data coordinates. Here is the calling stack: vtkClipPlanesPainter::UpdateBounds ===> original data bounds, but actor's world coordinates transformed planes vtkPainterPolyDataMapper::ComputeBounds vtkPolyDataMapper::GetBounds vtkActor::GetBounds ===> actor's transformation Matrix applied to the bounds vtkFrustumCoverageCuller::Cull ===> Prop can be incorrectly culled !!!! So in the method vtkClipPlanesPainter::UpdateBounds, the planes should be transformed with the actor's matrix before changing the bounds.

(0030760) xabi riobe (reporter) 2013-05-14 08:34	Here is a simple example to illustrate the issue, with attached screenshots of different situations: clip.jpg: the mapper is clipped, the actor has no transformation transform.jpg: the mapper is not clipped, the actor has a transformation cliptransform.jpg: the mapper is clipped, the actor has a transformation ==> the actor is culled #include <vtkActor.h> #include <vtkAxesActor.h> #include <vtkPolyDataMapper.h> #include <vtkRenderer.h> #include <vtkRenderWindow.h> #include <vtkRenderWindowInteractor.h> #include <vtkSmartPointer.h> #include <vtkSphereSource.h> #include <vtkPlanes.h> #include <vtkTransform.h> vtkSmartPointer<vtkRenderer> ren1= vtkSmartPointer<vtkRenderer>::New(); ren1->SetBackground(0.4, 0.4, 0.5); ren1->SetBackground2(0.2,0.2,0.3); ren1->SetGradientBackground(true); vtkSmartPointer<vtkRenderWindow> renWin= vtkSmartPointer<vtkRenderWindow>::New(); renWin->SetSize( 972, 800 ); renWin->AddRenderer(ren1); vtkSmartPointer<vtkRenderWindowInteractor> iren= vtkSmartPointer<vtkRenderWindowInteractor>::New(); iren->SetRenderWindow(renWin); vtkSmartPointer<vtkSphereSource> sph= vtkSmartPointer<vtkSphereSource>::New(); sph->SetPhiResolution(22); sph->SetThetaResolution(22); sph->SetRadius(2.0); sph->Update(); vtkSmartPointer<vtkPlanes> planes= vtkSmartPointer<vtkPlanes>::New(); planes->SetBounds(1.5,-1.5,1.5,-1.5,1.5,-1.5); vtkSmartPointer<vtkPolyDataMapper> Mapper= vtkSmartPointer<vtkPolyDataMapper>::New(); Mapper->SetInputConnection(sph->GetOutputPort()); Mapper->SetClippingPlanes(planes); vtkSmartPointer<vtkTransform> tr= vtkSmartPointer<vtkTransform>::New(); tr->Translate(10,0,0); vtkSmartPointer<vtkActor> Actor= vtkSmartPointer<vtkActor>::New(); Actor->SetMapper(Mapper); Actor->SetUserTransform(tr); vtkSmartPointer<vtkAxesActor> axes= vtkSmartPointer<vtkAxesActor>::New(); axes->AxisLabelsOff(); ren1->AddActor(Actor); ren1->AddActor(axes); ren1->ResetCamera(); renWin->Render(); iren->Start();

(0031309) Dave DeMarle (administrator) 2013-07-22 20:37	Leo no longer works on the project. If these issues still exist in 6.0, please reopen them and assign to Dave DeMarle.

(0031669) xabi riobe (reporter) 2013-10-01 03:45	This issue is still present in 6.0.0

(0032409) David Gobbi (developer) 2014-02-25 16:23	The code in the vtkClipPlanesPainter::UpdateBounds() that was added on 2012-01-23 definitely looks wrong. It will only give the correct result when the actor transform matrix is the identity matrix. None of the tests for this method apply a transform to the actor (see SHA 6d3e4c83). The mapper clipping planes are specified in world coordinates, so they cannot be used to directly adjust the bounds: they have to be transformed to data coordinates first. This is a problem, of course, because the UpdateBounds() method does not, and cannot, know what the actor transform is. Therefore, I think that it was a mistake to add UpdateBounds() to vtkClipPlanesPainter.

(0032412) xabi riobe (reporter) 2014-02-26 04:40	Actually the given example is wrong since the planes are passed in data coordinate. As David said, the UpdateBounds method is incorrect, but the initial problem of the ResetCamera still remains.

(0032427) David Gobbi (developer) 2014-02-27 11:50	Another reason for removing vtkClipPlanesPainter::UpdateBounds() is that it introduced a discrepancy between the behavior of vtkPainterPolyDataMapper as compared to vtkPolyDataMapper. Below is the ComputeBounds method for vtkPolyDataMapper. It does not take the clipping planes into account. void vtkPolyDataMapper::ComputeBounds() { this->GetInput()->GetBounds(this->Bounds); } In other words, if someone had been using vtkPolyDataMapper, and then switched to vtkPainterPolyDataMapper in order to improve rendering speed, then ResetCamera might start giving different results! I think this is a bad thing. It is best for the two poly data mappers to have the same behavior.

(0032454) xabi riobe (reporter) 2014-03-04 05:37	http://review.source.kitware.com/#/t/4024/ [^]

(0032458) David Gobbi (developer) 2014-03-05 15:43	Adding UpdateBounds() in vtkClipPlanesPainter caused problems whenever the actor's matrix was not the identity matrix. This is because the Bounds are in data coordinates, but the clipping planes are in world coordinates. So the Planes can only be used to adjust the Bounds if the Planes are first converted to data coords, but it is not possible to do this transformation in UpdateBounds() because the painter does not have access to the actor's matrix during the Update phase.

Relationships