Inconsistency in Switch implementation and usage

We’ve got an application with a pretty substantial scenegraph. We make extensive use of the Switch node type to control visibility.

I noticed it is easy to put Switch in a state that will throw IndexOutOfBoundsException from View.renderNode(). Around line 1008 of View in Xith3D_2004-05-03_cvs.tar.gz, there is


BitSet bs = sn.getChildMask();
List cl = sn.getChildren();
for(int i=bs.nextSetBit(0); i>=0; i=bs.nextSetBit(i+1)) {
        Node n = (Node) cl.get(i);
        renderNode(n);
}

This calls renderNode() for the children of the Switch node based solely on the contents of Switch.getChildMask(). In other words, I can easily create a Switch node with a 5-bit BitSet, all bits set to true, but with only two children. When this loop tries to call renderNode() on children 2-4, it will throw IndexOutOfBoundsException.

I can think of two ways to solve this problem. Either require Switch to match its BitSet size to its number of children, or modify that loop to verify the child exists before trying to hand it off to renderNode:


BitSet bs = sn.getChildMask();
List cl = sn.getChildren();
int listSize = cl.size();
for(int i=bs.nextSetBit(0); i>=0; i=bs.nextSetBit(i+1)) {
if (i < listSize) {
                Node n = (Node) cl.get(i);
                renderNode(n);
    }
}

Are there any possible solutions I’m missing to this?

Thanks,
Frank

Hi,

You are right, there is a problem here.

The only suggestion I can make is to break the loop after we discover that we are out of list.

Yuri

P.S. Please submit a patch to IssueZilla and I will apply it.

— orig/View.java Fri Aug 20 14:42:31 2004
+++ View.java Fri Aug 20 14:46:09 2004
@@ -1001,9 +1001,12 @@

                 BitSet bs = sn.getChildMask();
                 List cl = sn.getChildren();
  •                                   int listSize = cl.size();
                   for(int i=bs.nextSetBit(0); i>=0; i=bs.nextSetBit(i+1)) {
    
  •                    Node n = (Node) cl.get(i);
    
  •                    renderNode(n);
    
  •                                           if (i < listSize) { // Only try to render the node if the node exists in the children list
    
  •                                                   Node n = (Node) cl.get(i);
    
  •                                                   renderNode(n);
    
  •                                           }
                   }
    
               } else {

Hi,

Thanks for patch, it is applied to CVS.

Yuri