[vtk-developers] BUG: vtkImageActor - set of ComputedDisplayExtent variable.

Wed Jul 14 10:45:06 EDT 2010

Hello vtk-devs,

sorry for this whole misunderstanding. The description of Karthik makes
sense for me now. Maybe we should at least add some comment for this
lines.

Thanks for clarification!

________________________________

Von: Dave Partyka [mailto:dave.partyka at kitware.com] 
Gesendet: Mittwoch, 14. Juli 2010 16:28
An: Karthik Krishnan
Cc: David Gobbi; David Cole; vtk-developers at vtk.org; Pokutnev, Pavel (GE
EntSol, SensInsp)
Betreff: Re: [vtk-developers] BUG: vtkImageActor - set of
ComputedDisplayExtent variable.

I apologize for hastily committing this. I have reverted it back.

On Wed, Jul 14, 2010 at 10:23 AM, Karthik Krishnan
<karthik.krishnan at kitware.com> wrote:

	This behaviour has always existed in vtkImageActor - due to the
way VTK deals with point data and cell data and the fact that the tips
of the render window delineate the available screen pixels for display.
What this means is that the tips of the clipped rectangle you are seeing
(and the centers of any other rectangle) correspond to the positions of
the centers of vtkImageData voxels. Because the display extent, in word
coordinates runs from the Word position of the left edge voxel center to
the world position of the right edge voxel center, you end up with
clipping.

	This has been the topic of discussion on the list before.

	I don't believe that the fix you proposed can resolve that. Does
it ? It would simply cause one to update the whole input needlessly
leading to slowness when one renders a vtkImageActor for the very first
time, without setting the DisplayExtent.

	Thanks
	--
	karthik 

	On Wed, Jul 14, 2010 at 7:43 PM, Dave Partyka
<dave.partyka at kitware.com> wrote:

		Hi David Gobbi, 

		It appears from git blame that you are the author, or at
least, the last person to modify this code. What are your thoughts? Is
this a bug or intended as Karthik described?

		The blobs of code are:

		 // if the display extent has not been set, then compute
one
		  int *wExtent = input->GetWholeExtent();
		  if (this->DisplayExtent[0] == -1)
		    {
		    this->ComputedDisplayExtent[0] = wExtent[0];
		    this->ComputedDisplayExtent[1] = wExtent[1];
		    this->ComputedDisplayExtent[2] = wExtent[2];
		    this->ComputedDisplayExtent[3] = wExtent[3];
		    this->ComputedDisplayExtent[4] = wExtent[4];
		    this->ComputedDisplayExtent[5] = wExtent[4];
		    }

		and

		 // if the display extent has not been set, then compute
one
		  int *wExtent = this->Input->GetWholeExtent();
		  if (this->DisplayExtent[0] == -1)
		    {
		    this->ComputedDisplayExtent[0] = wExtent[0];
		    this->ComputedDisplayExtent[1] = wExtent[1];
		    this->ComputedDisplayExtent[2] = wExtent[2];
		    this->ComputedDisplayExtent[3] = wExtent[3];
		    this->ComputedDisplayExtent[4] = wExtent[4];
		    this->ComputedDisplayExtent[5] = wExtent[4];
		    }

		On Wed, Jul 14, 2010 at 9:54 AM, Karthik Krishnan
<karthik.krishnan at kitware.com> wrote:

			Dave:

			I would recommend holding off that commit. I'm
not convinced that its a bug. Specifically, the fragment of code reads :

			int
vtkImageActor::RenderOpaqueGeometry(vtkViewport* viewport)
			{
			....

			 int *wExtent = input->GetWholeExtent();
			  if (this->DisplayExtent[0] == -1)
			    {
			    this->ComputedDisplayExtent[0] = wExtent[0];
			    this->ComputedDisplayExtent[1] = wExtent[1];
			    this->ComputedDisplayExtent[2] = wExtent[2];
			    this->ComputedDisplayExtent[3] = wExtent[3];
			    this->ComputedDisplayExtent[4] = wExtent[4];

			    this->ComputedDisplayExtent[5] = wExtent[4];
			    }

input->SetUpdateExtent(this->ComputedDisplayExtent);
			  ...
			}

			An ImageActor should have flattened extents
along some dimension. It does not make sense for the ImageActor to
update the WHOLE input. It appears that with the fix,
"this->ComputedDisplayExtent[5] = wExtent[4];", it will lead to updating
the entire image. 

			I think the author of the class simply intended
to set the display extent to the first axial slice if they haven't been
set.

			Pavel: I'm also interested in knowing how you
uncovered/ concluded that this is a bug. We use vtkImageActor quite
heavily and haven't had any issues; Maybe we haven't covered this "if"
block, but still, its good to know..

			Thanks
			--
			karthik 

			On Wed, Jul 14, 2010 at 7:09 PM, Dave Partyka
<dave.partyka at kitware.com> wrote:

				Hi Pavel, I am curious how you
discovered this bug. Were you just browsing through the file or do you
have code that is affected by this? If so would you mind sharing it? I
would like to make a test out of it if possible. 

				On Wed, Jul 14, 2010 at 9:26 AM, Dave
Partyka <dave.partyka at kitware.com> wrote:

				Those lines are definitely covered.
Though it appears the only are used when the extents haven't been set
yet. 

http://www.cdash.org/CDash/viewCoverageFile.php?buildid=663956&fileid=10
980615 

				On Wed, Jul 14, 2010 at 9:18 AM, David
Cole <david.cole at kitware.com> wrote:

				This sort of error looks like it should
produce some sort of seriously incorrect results. 

				Do we not have a test that covers this
code? Is the coverage for the lines in question "0" or do we just not
have a *good* test that runs through that code...?

				I would think vtkImageActor is a heavily
used class.

				?

				On Wed, Jul 14, 2010 at 8:51 AM, Dave
Partyka <dave.partyka at kitware.com> wrote:

				Thanks for the fix! I have just
committed it.

				On Wed, Jul 14, 2010 at 4:19 AM,
Pokutnev, Pavel (GE EntSol, SensInsp) <Pavel.Pokutnev at ge.com> wrote:

				Hello vtk-devs,

				it looks like there is a copy & paste
bug in vtkImageActor line 250 and 293:

				The line:
				  this->ComputedDisplayExtent[5] =
wExtent[4];
				should be:
				  this->ComputedDisplayExtent[5] =
wExtent[5];

				Best regards!

_______________________________________________
				Powered by www.kitware.com

				Visit other Kitware open-source projects
at http://www.kitware.com/opensource/opensource.html

				Follow this link to
subscribe/unsubscribe:

http://www.vtk.org/mailman/listinfo/vtk-developers

_______________________________________________
				Powered by www.kitware.com

				Visit other Kitware open-source projects
at http://www.kitware.com/opensource/opensource.html

				Follow this link to
subscribe/unsubscribe:

http://www.vtk.org/mailman/listinfo/vtk-developers

_______________________________________________
				Powered by www.kitware.com

				Visit other Kitware open-source projects
at http://www.kitware.com/opensource/opensource.html

				Follow this link to
subscribe/unsubscribe:

http://www.vtk.org/mailman/listinfo/vtk-developers

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20100714/fddacefe/attachment.html>