[vtkusers] Fwd: Bug in vtkPolyData

Thu Dec 4 11:27:50 EST 2014

Hi Andy,

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?)

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.

Thanks,
Paul

On 3 December 2014 at 19:52, Andy Bauer <andy.bauer at kitware.com> wrote:

> Hi Paul,
>
> I looked at your fix for this and it looks better than mine. There a
> gerrit change set at http://review.source.kitware.com/#/c/18235/. 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.
>
> Regards,
> Andy
>
> On Mon, Dec 1, 2014 at 12:00 PM, Andy Bauer <andy.bauer at kitware.com>
> wrote:
>
>> Hi Paul,
>>
>> Thanks for sending your patch. I hope to be able to take a look at it
>> later this week.
>>
>> 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.
>>
>> Regards,
>> Andy
>>
>> On Thu, Nov 27, 2014 at 6:52 AM, Paul Edwards <paul.m.edwards at gmail.com>
>> wrote:
>>
>>> BTW there would be a problem with your implementation if
>>> vtkPolyData::InsertNextCell was called after a call to
>>> vtkPolyData::DeleteCell (and before vtkPolyData::RemoveDeletedCells)
>>> as it would not append to vtkPolyData::CellsCopy.  I'm not sure if
>>> that is allowed but your implementation would result in a index out of
>>> bounds.
>>>
>>> On 27 November 2014 at 11:23, Paul Edwards <paul.m.edwards at gmail.com>
>>> wrote:
>>> > Hi Andy,
>>> >
>>> > I've attached a patch that uses a slightly different approach which
>>> > only changes vtkPolyData::RemoveDeletedCells.  The code looks simpler
>>> > and it passes your new test although I am assuming it is ok to
>>> > ShallowCopy the vtkPolyData at the top of the function.  I'll let you
>>> > decide if it is better!
>>> >
>>> > Note: the patch is for the VTK with the current ParaView master
>>> > (v6.1.0-2506-g2e5a3c9)
>>> >
>>> > Best regards,
>>> > Paul
>>> >
>>> > On 23 November 2014 at 17:37, Andy Bauer <andy.bauer at kitware.com>
>>> wrote:
>>> >> Hi Paul,
>>> >>
>>> >> I have a potential fix for this bug at
>>> >> http://review.source.kitware.com/#/t/5040/. It feels like a hack but
>>> I
>>> >> couldn't figure out a better way to do it yet. I also modified a test
>>> to
>>> >> verify that the proper cells were getting removed, based on the cell's
>>> >> points.
>>> >>
>>> >> If you see a better way to do this, please let me know.
>>> >>
>>> >> Thanks,
>>> >> Andy
>>> >>
>>> >> On Mon, Nov 17, 2014 at 5:37 PM, Andy Bauer <andy.bauer at kitware.com>
>>> wrote:
>>> >>>
>>> >>> Hi Paul,
>>> >>>
>>> >>> Thanks for the input. I'm at SC14 right now so I won't be able to
>>> look at
>>> >>> this until I get back.
>>> >>>
>>> >>> Cheers,
>>> >>> Andy
>>> >>>
>>> >>> On Mon, Nov 17, 2014 at 4:26 AM, Paul Edwards <
>>> paul.m.edwards at gmail.com>
>>> >>> wrote:
>>> >>>>
>>> >>>> Hi Andy,
>>> >>>>
>>> >>>> I've attached a sample program demonstrating the issue.  The example
>>> >>>> just loads the sphere.vtk file, removes all cells where the "Result"
>>> >>>> value is not 0 and writes the result to output.vtk.  The tar file
>>> also
>>> >>>> contains the correct and incorrect output.
>>> >>>>
>>> >>>> Best regards,
>>> >>>> Paul
>>> >>>>
>>> >>>> On 15 November 2014 02:59, Andy Bauer <andy.bauer at kitware.com>
>>> wrote:
>>> >>>> > Hi Paul,
>>> >>>> >
>>> >>>> > I was part of that fix and I think it's correct. There may have
>>> been
>>> >>>> > more to
>>> >>>> > that fix though since it took a couple of shots before getting it
>>> >>>> > correct so
>>> >>>> > the git history may not be as pristine as desired. If you can
>>> give me
>>> >>>> > more
>>> >>>> > specifics on the bug though I can try to see what's going on with
>>> it.
>>> >>>> > If I
>>> >>>> > remember correctly though, that fix had to do with the fact that
>>> some
>>> >>>> > places
>>> >>>> > thought that the order of iterating through cells in a
>>> vtkPolyData was
>>> >>>> > first
>>> >>>> > through all vertex cells, then all line cells, then all polys and
>>> >>>> > finally
>>> >>>> > through all triangle strips, irregardless of the order that cells
>>> got
>>> >>>> > inserted in (i.e. using InsertNextCell()). The reality is that for
>>> >>>> > polydata
>>> >>>> > that cells are indeed iterated through in their insertion order.
>>> The
>>> >>>> > test
>>> >>>> > that was added for those changes hopefully shows the proper
>>> >>>> > functionality
>>> >>>> > along with associated cell data manipulations as well.
>>> >>>> >
>>> >>>> > Regards,
>>> >>>> > Andy
>>> >>>> >
>>> >>>> >
>>> >>>> >
>>> >>>> > On Thu, Nov 13, 2014 at 1:55 PM, Paul Edwards
>>> >>>> > <paul.m.edwards at gmail.com>
>>> >>>> > wrote:
>>> >>>> >>
>>> >>>> >> Hi,
>>> >>>> >>
>>> >>>> >> I've just noticed an issue with vtkPolyData.  I had been calling
>>> >>>> >> vtkPolyData::DeleteCell to remove the unwanted cells, followed
>>> by a
>>> >>>> >> call to vtkPolyData::RemoveDeletedCells.  The result is not as it
>>> >>>> >> should be in the latest version.  To fix I just started reverting
>>> >>>> >> previous commits.  I tracked it down to the following:
>>> >>>> >>
>>> >>>> >> bugfix 14459: Iterating through polydata cells incorrectly.
>>> >>>> >> commit: 44131c6d05375692af8b6b38cf4291983bdd8f45
>>> >>>> >>
>>> >>>> >> Sorry but I do not have time to provide a test case or track
>>> where in
>>> >>>> >> this commit lies the problem.  I just thought I would give
>>> details
>>> >>>> >> here in case anyone else notices the same problem and is
>>> searching for
>>> >>>> >> a solution.
>>> >>>> >>
>>> >>>> >> Best regards,
>>> >>>> >> Paul
>>> >>>> >> _______________________________________________
>>> >>>> >> Powered by www.kitware.com
>>> >>>> >>
>>> >>>> >> Visit other Kitware open-source projects at
>>> >>>> >> http://www.kitware.com/opensource/opensource.html
>>> >>>> >>
>>> >>>> >> Please keep messages on-topic and check the VTK FAQ at:
>>> >>>> >> http://www.vtk.org/Wiki/VTK_FAQ
>>> >>>> >>
>>> >>>> >> Follow this link to subscribe/unsubscribe:
>>> >>>> >> http://public.kitware.com/mailman/listinfo/vtkusers
>>> >>>> >
>>> >>>> >
>>> >>>
>>> >>>
>>> >>
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtkusers/attachments/20141204/0f6dec69/attachment.html>