Hi David Gobbi,<div><br></div><div>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?</div><div><br>
</div><div>The blobs of code are:</div><div><br></div><div><div> // if the display extent has not been set, then compute one</div><div> int *wExtent = input->GetWholeExtent();</div><div> if (this->DisplayExtent[0] == -1)</div>
<div> {</div><div> this->ComputedDisplayExtent[0] = wExtent[0];</div><div> this->ComputedDisplayExtent[1] = wExtent[1];</div><div> this->ComputedDisplayExtent[2] = wExtent[2];</div><div> this->ComputedDisplayExtent[3] = wExtent[3];</div>
<div> this->ComputedDisplayExtent[4] = wExtent[4];</div><div> this->ComputedDisplayExtent[5] = wExtent[4];</div><div> }</div></div><div><br></div><div>and</div><div><br></div><div><div> // if the display extent has not been set, then compute one</div>
<div> int *wExtent = this->Input->GetWholeExtent();</div><div> if (this->DisplayExtent[0] == -1)</div><div> {</div><div> this->ComputedDisplayExtent[0] = wExtent[0];</div><div> this->ComputedDisplayExtent[1] = wExtent[1];</div>
<div> this->ComputedDisplayExtent[2] = wExtent[2];</div><div> this->ComputedDisplayExtent[3] = wExtent[3];</div><div> this->ComputedDisplayExtent[4] = wExtent[4];</div><div> this->ComputedDisplayExtent[5] = wExtent[4];</div>
<div> }</div><div><br></div><br><div class="gmail_quote">On Wed, Jul 14, 2010 at 9:54 AM, Karthik Krishnan <span dir="ltr"><<a href="mailto:karthik.krishnan@kitware.com">karthik.krishnan@kitware.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Dave:<br><br>I would recommend holding off that commit. I'm not convinced that its a bug. Specifically, the fragment of code reads :<br>
<br><br>int vtkImageActor::RenderOpaqueGeometry(vtkViewport* viewport)<br>{<br>....<br>
<br> int *wExtent = input->GetWholeExtent();<br> if (this->DisplayExtent[0] == -1)<br> {<br> this->ComputedDisplayExtent[0] = wExtent[0];<br> this->ComputedDisplayExtent[1] = wExtent[1];<br> this->ComputedDisplayExtent[2] = wExtent[2];<br>
this->ComputedDisplayExtent[3] = wExtent[3];<br> this->ComputedDisplayExtent[4] = wExtent[4];<div class="im"><br> this->ComputedDisplayExtent[5] = wExtent[4];<br> }<br></div> input->SetUpdateExtent(this->ComputedDisplayExtent);<br>
...<br>}<br><br>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. <br>
<br>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.<br><br>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..<br>
<br>Thanks<br>--<br><font color="#888888">karthik</font><div><div></div><div class="h5"><br><br><div class="gmail_quote">On Wed, Jul 14, 2010 at 7:09 PM, Dave Partyka <span dir="ltr"><<a href="mailto:dave.partyka@kitware.com" target="_blank">dave.partyka@kitware.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204);margin:0pt 0pt 0pt 0.8ex;padding-left:1ex">
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.<div>
<div></div><div><br>
<br><div class="gmail_quote">On Wed, Jul 14, 2010 at 9:26 AM, Dave Partyka <span dir="ltr"><<a href="mailto:dave.partyka@kitware.com" target="_blank">dave.partyka@kitware.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204);margin:0pt 0pt 0pt 0.8ex;padding-left:1ex">
Those lines are definitely covered. Though it appears the only are used when the extents haven't been set yet.<div><br></div><div><a href="http://www.cdash.org/CDash/viewCoverageFile.php?buildid=663956&fileid=10980615" target="_blank">http://www.cdash.org/CDash/viewCoverageFile.php?buildid=663956&fileid=10980615</a><div>
<div></div><div><br>
<br><div class="gmail_quote">On Wed, Jul 14, 2010 at 9:18 AM, David Cole <span dir="ltr"><<a href="mailto:david.cole@kitware.com" target="_blank">david.cole@kitware.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204);margin:0pt 0pt 0pt 0.8ex;padding-left:1ex">
This sort of error looks like it should produce some sort of seriously incorrect results.<div><br></div><div>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...?</div>
<div><br></div><div>I would think vtkImageActor is a heavily used class.</div><div><br></div><div><br>?</div><div><div></div><div><div><br></div><div><br><div class="gmail_quote">On Wed, Jul 14, 2010 at 8:51 AM, Dave Partyka <span dir="ltr"><<a href="mailto:dave.partyka@kitware.com" target="_blank">dave.partyka@kitware.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204);margin:0pt 0pt 0pt 0.8ex;padding-left:1ex">Thanks for the fix! I have just committed it.<br><br><div class="gmail_quote"><div><div></div>
<div>On Wed, Jul 14, 2010 at 4:19 AM, Pokutnev, Pavel (GE EntSol, SensInsp) <span dir="ltr"><<a href="mailto:Pavel.Pokutnev@ge.com" target="_blank">Pavel.Pokutnev@ge.com</a>></span> wrote:<br>
</div></div><blockquote class="gmail_quote" style="border-left:1px solid rgb(204, 204, 204);margin:0pt 0pt 0pt 0.8ex;padding-left:1ex"><div><div></div><div>
<div>
<div><font face="Arial" size="2"><span>Hello
vtk-devs,</span></font></div>
<div><font face="Arial" size="2"><span></span></font> </div>
<div><font face="Arial" size="2"><span>it looks like there
is a copy & paste bug in vtkImageActor line 250 and
293:</span></font></div>
<div><font face="Arial" size="2"><span></span></font> </div>
<div><font face="Arial" size="2"><span>The
line:</span></font></div>
<div><font face="Arial" size="2"><span>
this->ComputedDisplayExtent[5] = wExtent[4];</span></font></div>
<div><font face="Arial" size="2"><span>should
be:</span></font></div>
<div><font face="Arial" size="2"><span>
this->ComputedDisplayExtent[5] = wExtent[5];</span></font></div>
<div><font face="Arial" size="2"><span></span></font> </div>
<div><font face="Arial" size="2"><span>Best
regards!</span></font></div>
<div><font face="Arial" size="2"><span></span></font> </div>
<div><font face="Arial" size="2"></font> </div><font face="Arial" size="2">
<div align="left"><br></div></font>
<div><font face="Arial" size="2"></font> </div></div>
<br></div></div>_______________________________________________<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
<br>
<br></blockquote></div><br>
<br>_______________________________________________<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
<br>
<br></blockquote></div><br></div>
</div></div></blockquote></div><br></div></div></div>
</blockquote></div><br>
</div></div><br>_______________________________________________<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
<br>
<br></blockquote></div><br>
</div></div></blockquote></div><br></div>