[vtk-developers] vtkCheckerboardRepresentation fixes?

[dean.inglis at camris.ca]

three bugs(?) with vtkCheckerboardRepresentation,
first two relate to how companion sliders are updated in
void vtkCheckerboardRepresentation::SliderValueChanged(int sliderNum)

1)when interacting with the sliders, one slider's partner
appears to lag behind since its value is being updated as
an integer (cast from a double) and not as a double

2) the sliders only update for an x-y image plane configuration

Since this widget uses a ref to a vtkImageActor, which can
only assume x-y,y-z, x-z planar configuration, we can keep an ivar 
for "orthoAxis" defined in BuildRepresentation 

<snip>
  double bounds[6];
  double o[3];
  vtkImageData *image = this->ImageActor->GetInput();
  image->Update();
  image->GetBounds(bounds);
  image->GetOrigin(o);
  if ( image->GetDataDimension() != 2 )
    {
    vtkErrorMacro(<<" requires a 2D image");
    return;
    }
  double t0 = bounds[1]-bounds[0];
  double t1 = bounds[3]-bounds[2];
  double t2 = bounds[5]-bounds[4];
  this->OrthoAxis = ( t0 < t1 ? (t0 < t2 ? 0 : 2) : (t1 < t2 ? 1 : 2) )
<snip>

and revise SliderValueChanged to:

//----------------------------------------------------------------------
void vtkCheckerboardRepresentation::SliderValueChanged(int sliderNum)
{
  int *numDivisions = this->Checkerboard->GetNumberOfDivisions();
  int value;
  int div[] = {1,1,1};

  if ( sliderNum == vtkCheckerboardRepresentation::TopSlider )
    {
    value = static_cast<int>(this->TopRepresentation->GetValue());
    this->BottomRepresentation->SetValue(this->TopRepresentation->GetValue()); // fixes lagging following
    switch ( this->OrthoAxis )
      {
      case 0: div[1] = value; div[2] = numDivisions[2]; break;
      case 1: div[0] = value; div[2] = numDivisions[2]; break;
      case 2: div[0] = value; div[1] = numDivisions[1]; break;
      }

    this->Checkerboard->SetNumberOfDivisions(div);
    }
  else if ( sliderNum == vtkCheckerboardRepresentation::RightSlider )
    {
    value = static_cast<int>(this->RightRepresentation->GetValue());
    this->LeftRepresentation->SetValue(this->RightRepresentation->GetValue());
    switch ( this->OrthoAxis )
      {
      case 0: div[1] = numDivisions[1]; div[2] = value; break;
      case 1: div[0] = numDivisions[0]; div[2] = value; break;
      case 2: div[0] = numDivisions[0]; div[1] = value; break;
      }

    this->Checkerboard->SetNumberOfDivisions(div);
    }
  else if ( sliderNum == vtkCheckerboardRepresentation::BottomSlider )
    {
    value = static_cast<int>(this->BottomRepresentation->GetValue());
    this->TopRepresentation->SetValue(this->BottomRepresentation->GetValue());
    switch ( this->OrthoAxis )
      {
      case 0: div[1] = value; div[2] = numDivisions[2]; break;
      case 1: div[0] = value; div[2] = numDivisions[2]; break;
      case 2: div[0] = value; div[1] = numDivisions[1]; break;
      }

    this->Checkerboard->SetNumberOfDivisions(div);
    }
  else if ( sliderNum == vtkCheckerboardRepresentation::LeftSlider )
    {
    value = static_cast<int>(this->LeftRepresentation->GetValue());
    this->RightRepresentation->SetValue(this->LeftRepresentation->GetValue());
    switch ( this->OrthoAxis )
      {
      case 0: div[1] = numDivisions[1]; div[2] = value; break;
      case 1: div[0] = numDivisions[0]; div[2] = value; break;
      case 2: div[0] = numDivisions[0]; div[1] = value; break;
      }

    this->Checkerboard->SetNumberOfDivisions(div);
    }
}


Third bug deals with BuildRepresentation.  The widget, I think,
should wrap the sliders around/along the boundary of the image actor.
If so, then the image actor bounds should be used, at least
to get the right z-offset of an x-y plane, for example, rather than the 
image origin to define the Point1 and Point2 Coordinates of the constituent sliders, since for
slices other than those at the origin (e.g., clip a slice from a 3D volume)
the sliders may appear offset normal from the image actor plane:

<snip>
  if ( this->OrthoAxis == 0 ) //x-axis
    {
    this->TopRepresentation->GetPoint1Coordinate()->SetValue(bounds[0], o[1]+o1, o[2]+t2);  // use bounds instead of image origin
    this->TopRepresentation->GetPoint2Coordinate()->SetValue(bounds[0], o[1]+t1-o1, o[2]+t2);
    this->TopRepresentation->SetValue(numDivisions[1]);
    this->RightRepresentation->GetPoint1Coordinate()->SetValue(bounds[0], o[1]+t1, o[2]+o2);
    this->RightRepresentation->GetPoint2Coordinate()->SetValue(bounds[0], o[1]+t1, o[2]+t2-o2);
    this->RightRepresentation->SetValue(numDivisions[2]);
    this->BottomRepresentation->GetPoint1Coordinate()->SetValue(bounds[0], o[1]+o1, o[2]);
    this->BottomRepresentation->GetPoint2Coordinate()->SetValue(bounds[0], o[1]+t1-o1, o[2]);
    this->BottomRepresentation->SetValue(numDivisions[1]);
    this->LeftRepresentation->GetPoint1Coordinate()->SetValue(bounds[0], o[1], o[2]+o2);
    this->LeftRepresentation->GetPoint2Coordinate()->SetValue(bounds[0], o[1], o[2]+t2-o2);
    this->LeftRepresentation->SetValue(numDivisions[2]);
    }
<snip>

Any thoughts? Are there objections to committing these changes?

Dean