[vtkusers] Fwd: Bug in vtkPolyData

[Andy Bauer]

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/20141201/bd14e67b/attachment.html>