[vtk-developers] ARB-Request: Blessing of vtkPolyData->IsEdge fix

lucantiga at softhome.net lucantiga at softhome.net
Mon Mar 17 09:56:17 EST 2003


Good Monday morning (sounds like a contradiction),
 I added Will's suggestions to IsEdge, as reported below. Rasums, if it's
ok for you to test it, just go ahead! 
 If retaining the old performance is crucial, the old method could be kept
inline, and should be modified as

inline int vtkPolyData::IsEdge(int p1, int p2)
{
  unsigned short int ncells;
  int i;
  vtkIdType *cells;
   
  this->GetPointCells(p1,ncells,cells);
  for (i=0; i < ncells; i++)
    {
    cellType = this->GetCellType(cells[i]);
    if (cellType == VTK_TRIANGLE)
      {
      if ( this->IsPointUsedByCell(p2,cells[i]) )
        {
        return 1;
        }
      }
    else
      {
      if ( this->IsEdgeGeneric(p1,p2) )
        {
        return 1;
        }
      }
    }
 
   return 0;
}

where IsEdgeGeneric is a non-inline protected method looking like the new 
IsEdge without the VTK_TRIANGLE case. cellType could be passed to 
IsEdgeGeneric, to avoid retrieving cellType twice.
 Let me know if this makes sense

luca

 
int vtkPolyData::IsEdge(vtkIdType p1, vtkIdType p2)
{
  unsigned short int ncells;
  vtkIdType cellType;
  vtkIdType npts;
  vtkIdType i, j;
  vtkIdType *cells, *pts;
   
  this->GetPointCells(p1,ncells,cells);
  for (i=0; i<ncells; i++)
    {
    cellType = this->GetCellType(cells[i]);
    switch (cellType)
      {
      case VTK_EMPTY_CELL: case VTK_VERTEX: case VTK_POLY_VERTEX: case 
VTK_LINE: case VTK_POLY_LINE:
	break;
      case VTK_TRIANGLE:
        if ( this->IsPointUsedByCell(p2,cells[i]) )
          {
          return 1;
          }
	break;
      case VTK_QUAD:
	this->GetCellPoints(cells[i],npts,pts);
	for (j=0; j<npts-1; j++)
	  {
	  if 
(((pts[j]==p1)&&(pts[j+1]==p2))||((pts[j]==p2)&&(pts[j+1]==p1)))
 	    {
	    return 1;
	    }
	  }
	if 
(((pts[0]==p1)&&(pts[npts-1]==p2))||((pts[0]==p2)&&(pts[npts-1]==p1)))
	  {
	  return 1;
	  }
	break;
      case VTK_TRIANGLE_STRIP:
	this->GetCellPoints(cells[i],npts,pts);
	for (j=0; j<npts-2; j++)
	  {
	  if 
((((pts[j]==p1)&&(pts[j+1]==p2))||((pts[j]==p2)&&(pts[j+1]==p1)))||
	      
(((pts[j]==p1)&&(pts[j+2]==p2))||((pts[j]==p2)&&(pts[j+2]==p1))))
 	    {
	    return 1;
	    }
	  }
	if 
(((pts[npts-2]==p1)&&(pts[npts-1]==p2))||((pts[npts-2]==p2)&&(pts[npts-1]==p1)))
	  {
	  return 1;
	  }
	break;
      default:
	this->GetCellPoints(cells[i],npts,pts);
	for (j=0; j<npts; j++)
	  {
	  if (p1==pts[j])
	    {
	    if ((pts[(j-1+npts)%npts]==p2)||(pts[(j+1)%npts]==p2))
	      {
	      return 1;
	      }
	    }
	  }
      }
    }

  return 0;
}


On Mon, 17 Mar 2003, Will Schroeder wrote:

> A historical note and problems with suggested code:
> 
> According to documentation (VTK Textbook, etc.), by definition PolyVertex 
> and PolyLine have no edges (1-dimensional edge is only found on cells 2D 
> and higher). Edges are defined as 1-D BOUNDARY entities to cells. For 
> example, the vtkLine::GetNumberOfEdges() returns 0, as does vtkPolyVertex 
> and vtkPolyLine. Hence the IsEdge method should always return false when 
> any type of cell dimension 1 or less is involved. If the meaning of what an 
> edge is changed, then these other methods must be changed for consistency. 
> I would not recommend going down this snake hole.
> 
> The reason the code was written this way was for speed. It is used heavily 
> in inner loops for obtaining boundary information and assumes a lot about 
> how it is used. I'm betting the new implementation is much slower although 
> I have not tested it. I think the problem is that the intended meaning of 
> the method has been interpreted differently. One meaning is "do these two 
> points form an edge"; the other meaning is "given two points that form an 
> edge on one cell, is it an edge in another cell?" Also I suspect that the 
> code was originally written for triangle meshes (where it works just fine) 
> and evolved into something more general.
> 
> Here's what I think should be done:
> 
> 1. All cells of dimension 1 or less should return false.
> 2. The separate cases for the various types are good (triangle strip and 
> default), however,
> 3. I would create two new cases, one for VTK_TRIANGLE (the old code works 
> just fine for a triangle, no modulo required) and another for VTK_QUAD. 
> Adding these two cases should regain most of the old performance.
> 
> Will
> 
> At 09:16 AM 3/12/2003 -0500, Ken Martin wrote:
> >I think there are a couple problems with the suggested code.
> >Specifically three cases come to mind that I think will break it.
> >
> >1) For a triangle strip with points p0, p1, p2, p3 ... the points p0 and
> >p2 do form an edge but would not be correctly reported by the code
> >below.
> >
> >2) for a poly vertex there are no edges but the function could say that
> >there was one
> >
> >3) for poly lines if someone checked the first and last point of a poly
> >line (in either order) it would match.
> >
> >And a minor formatting issue in that the "if" statements do not have {}
> >under them. Having said that, the original code was clearly screwed up.
> >It looks like a valid solution will have to know what type of cell is
> >being considered. Perhaps a switch (this->GetCellType(cell[i])) with the
> >current code in the default case but special cases for Tstrips,
> >polyvert, and polyline. The whole thing should probably be moved into
> >the cxx file as well, getting a bit long and complicated for an inline.
> >
> >Thanks
> >Ken
> >
> > > Luca Antiga and I would like to apply the following fix to
> > > vtkPolyData->IsEdge :
> > >
> > > inline int vtkPolyData::IsEdge(int p1, int p2)
> > > {
> > >   unsigned short int ncells;
> > >   int npts;
> > >   int i, j;
> > >   vtkIdType *cells, *pts;
> > >
> > >   this->GetPointCells(p1,ncells,cells);
> > >   for (i=0; i<ncells; i++)
> > >   {
> > >     this->GetCellPoints(cells[i],npts,pts);
> > >     for (j=0; j<npts; j++)
> > >     {
> > >       if (p1==pts[j])
> > >         if ((pts[(j-1+npts)%npts]==p2)||(pts[(j+1)%npts]==p2))
> > >           return 1;
> > >     }
> > >   }
> > >
> > >   return 0;
> > > }
> >
> >
> >
> >_______________________________________________
> >vtk-developers mailing list
> >vtk-developers at public.kitware.com
> >http://public.kitware.com/mailman/listinfo/vtk-developers
> 
> 
> 
> _______________________________________________
> vtk-developers mailing list
> vtk-developers at public.kitware.com
> http://public.kitware.com/mailman/listinfo/vtk-developers
> 

-- 
Luca Antiga, PhD
--------------------------------------
Imaging Research Laboratories
Robarts Research Institute
P.O. Box 5015, 100 Perth Drive
London, Ontario, Canada N6A 5K8
--------------------------------------
phone: (001) (519) 663-5777 ext.34028
email: lantiga at imaging.robarts.ca
web:   www.imaging.robarts.ca/~lantiga
--------------------------------------




More information about the vtk-developers mailing list