[vtkusers] Fwd: Bug in vtkPolyData

[Andy Bauer]

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/20141203/5433215e/attachment.html>