<div dir="ltr">Hi Andy,<div><br></div><div>The page shows the files as completely new - not sure why it might be difficult for anyone to review as they will not see the differences (or am I looking at the wrong thing in gerrit?)</div><div><br></div><div>Anyway, thanks for looking at the patch and don't worry about the author - you did all the hard work creating the test, finding the bug and putting it on gerrit!  It's much appreciated.</div><div><br></div><div>Thanks,</div><div>Paul</div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On 3 December 2014 at 19:52, Andy Bauer <span dir="ltr"><<a href="mailto:andy.bauer@kitware.com" target="_blank">andy.bauer@kitware.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div>Hi Paul,<br><br></div>I looked at your fix for this and it looks better than mine. There a gerrit change set at <a href="http://review.source.kitware.com/#/c/18235/" target="_blank">http://review.source.kitware.com/#/c/18235/</a>. I apologize that it currently lists me as the author of this fix instead of you -- I'm trying to get this done quickly and overlooked that before I pushed it to gerrit. If everyone is happy with this fix I'll make sure to have the author changed to you.<br><br></div>Regards,<br>Andy<br></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Dec 1, 2014 at 12:00 PM, Andy Bauer <span dir="ltr"><<a href="mailto:andy.bauer@kitware.com" target="_blank">andy.bauer@kitware.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div>Hi Paul,<br><br></div>Thanks for sending your patch. I hope to be able to take a look at it later this week. <br><br></div>And yes, my patch does make some assumptions that maybe shouldn't be made. It's definitely needs more looking over if it were to get merged into VTK master, which I'm not sure it even should.<br><br>Regards,<br>Andy<br></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Nov 27, 2014 at 6:52 AM, Paul Edwards <span dir="ltr"><<a href="mailto:paul.m.edwards@gmail.com" target="_blank">paul.m.edwards@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">BTW there would be a problem with your implementation if<br>
vtkPolyData::InsertNextCell was called after a call to<br>
vtkPolyData::DeleteCell (and before vtkPolyData::RemoveDeletedCells)<br>
as it would not append to vtkPolyData::CellsCopy.  I'm not sure if<br>
that is allowed but your implementation would result in a index out of<br>
bounds.<br>
<div><div><br>
On 27 November 2014 at 11:23, Paul Edwards <<a href="mailto:paul.m.edwards@gmail.com" target="_blank">paul.m.edwards@gmail.com</a>> wrote:<br>
> Hi Andy,<br>
><br>
> I've attached a patch that uses a slightly different approach which<br>
> only changes vtkPolyData::RemoveDeletedCells.  The code looks simpler<br>
> and it passes your new test although I am assuming it is ok to<br>
> ShallowCopy the vtkPolyData at the top of the function.  I'll let you<br>
> decide if it is better!<br>
><br>
> Note: the patch is for the VTK with the current ParaView master<br>
> (v6.1.0-2506-g2e5a3c9)<br>
><br>
> Best regards,<br>
> Paul<br>
><br>
> On 23 November 2014 at 17:37, Andy Bauer <<a href="mailto:andy.bauer@kitware.com" target="_blank">andy.bauer@kitware.com</a>> wrote:<br>
>> Hi Paul,<br>
>><br>
>> I have a potential fix for this bug at<br>
>> <a href="http://review.source.kitware.com/#/t/5040/" target="_blank">http://review.source.kitware.com/#/t/5040/</a>. It feels like a hack but I<br>
>> couldn't figure out a better way to do it yet. I also modified a test to<br>
>> verify that the proper cells were getting removed, based on the cell's<br>
>> points.<br>
>><br>
>> If you see a better way to do this, please let me know.<br>
>><br>
>> Thanks,<br>
>> Andy<br>
>><br>
>> On Mon, Nov 17, 2014 at 5:37 PM, Andy Bauer <<a href="mailto:andy.bauer@kitware.com" target="_blank">andy.bauer@kitware.com</a>> wrote:<br>
>>><br>
>>> Hi Paul,<br>
>>><br>
>>> Thanks for the input. I'm at SC14 right now so I won't be able to look at<br>
>>> this until I get back.<br>
>>><br>
>>> Cheers,<br>
>>> Andy<br>
>>><br>
>>> On Mon, Nov 17, 2014 at 4:26 AM, Paul Edwards <<a href="mailto:paul.m.edwards@gmail.com" target="_blank">paul.m.edwards@gmail.com</a>><br>
>>> wrote:<br>
>>>><br>
>>>> Hi Andy,<br>
>>>><br>
>>>> I've attached a sample program demonstrating the issue.  The example<br>
>>>> just loads the sphere.vtk file, removes all cells where the "Result"<br>
>>>> value is not 0 and writes the result to output.vtk.  The tar file also<br>
>>>> contains the correct and incorrect output.<br>
>>>><br>
>>>> Best regards,<br>
>>>> Paul<br>
>>>><br>
>>>> On 15 November 2014 02:59, Andy Bauer <<a href="mailto:andy.bauer@kitware.com" target="_blank">andy.bauer@kitware.com</a>> wrote:<br>
>>>> > Hi Paul,<br>
>>>> ><br>
>>>> > I was part of that fix and I think it's correct. There may have been<br>
>>>> > more to<br>
>>>> > that fix though since it took a couple of shots before getting it<br>
>>>> > correct so<br>
>>>> > the git history may not be as pristine as desired. If you can give me<br>
>>>> > more<br>
>>>> > specifics on the bug though I can try to see what's going on with it.<br>
>>>> > If I<br>
>>>> > remember correctly though, that fix had to do with the fact that some<br>
>>>> > places<br>
>>>> > thought that the order of iterating through cells in a vtkPolyData was<br>
>>>> > first<br>
>>>> > through all vertex cells, then all line cells, then all polys and<br>
>>>> > finally<br>
>>>> > through all triangle strips, irregardless of the order that cells got<br>
>>>> > inserted in (i.e. using InsertNextCell()). The reality is that for<br>
>>>> > polydata<br>
>>>> > that cells are indeed iterated through in their insertion order. The<br>
>>>> > test<br>
>>>> > that was added for those changes hopefully shows the proper<br>
>>>> > functionality<br>
>>>> > along with associated cell data manipulations as well.<br>
>>>> ><br>
>>>> > Regards,<br>
>>>> > Andy<br>
>>>> ><br>
>>>> ><br>
>>>> ><br>
>>>> > On Thu, Nov 13, 2014 at 1:55 PM, Paul Edwards<br>
>>>> > <<a href="mailto:paul.m.edwards@gmail.com" target="_blank">paul.m.edwards@gmail.com</a>><br>
>>>> > wrote:<br>
>>>> >><br>
>>>> >> Hi,<br>
>>>> >><br>
>>>> >> I've just noticed an issue with vtkPolyData.  I had been calling<br>
>>>> >> vtkPolyData::DeleteCell to remove the unwanted cells, followed by a<br>
>>>> >> call to vtkPolyData::RemoveDeletedCells.  The result is not as it<br>
>>>> >> should be in the latest version.  To fix I just started reverting<br>
>>>> >> previous commits.  I tracked it down to the following:<br>
>>>> >><br>
>>>> >> bugfix 14459: Iterating through polydata cells incorrectly.<br>
>>>> >> commit: 44131c6d05375692af8b6b38cf4291983bdd8f45<br>
>>>> >><br>
>>>> >> Sorry but I do not have time to provide a test case or track where in<br>
>>>> >> this commit lies the problem.  I just thought I would give details<br>
>>>> >> here in case anyone else notices the same problem and is searching for<br>
>>>> >> a solution.<br>
>>>> >><br>
>>>> >> Best regards,<br>
>>>> >> Paul<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<br>
>>>> >> <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
>>>> >><br>
>>>> >> Please keep messages on-topic and check the VTK FAQ at:<br>
>>>> >> <a href="http://www.vtk.org/Wiki/VTK_FAQ" target="_blank">http://www.vtk.org/Wiki/VTK_FAQ</a><br>
>>>> >><br>
>>>> >> Follow this link to subscribe/unsubscribe:<br>
>>>> >> <a href="http://public.kitware.com/mailman/listinfo/vtkusers" target="_blank">http://public.kitware.com/mailman/listinfo/vtkusers</a><br>
>>>> ><br>
>>>> ><br>
>>><br>
>>><br>
>><br>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>