changeset 5300:ab9489903f7b

RT-26587: 3D Data validation AND RT-30451: FX 8 3D: Need to validate Mesh's data size Summary: Validate input data during FX->NG sync stage. AND Need to validate the size and range of these arrays. Reviewed-by: kcr and cyang
author Yao Wang <yao.w.wang@oracle.com>
date Tue, 08 Oct 2013 09:30:23 -0700
parents 093ea25436c7
children e6331b4daf15
files modules/graphics/src/main/java/com/sun/javafx/sg/prism/NGShape3D.java modules/graphics/src/main/java/com/sun/javafx/sg/prism/NGTriangleMesh.java modules/graphics/src/main/java/com/sun/prism/impl/BaseMesh.java modules/graphics/src/main/java/javafx/scene/shape/Box.java modules/graphics/src/main/java/javafx/scene/shape/Cylinder.java modules/graphics/src/main/java/javafx/scene/shape/Sphere.java modules/graphics/src/main/java/javafx/scene/shape/TriangleMesh.java
diffstat 7 files changed, 154 insertions(+), 55 deletions(-) [+]
line wrap: on
line diff
--- a/modules/graphics/src/main/java/com/sun/javafx/sg/prism/NGShape3D.java	Tue Oct 08 09:26:09 2013 -0700
+++ b/modules/graphics/src/main/java/com/sun/javafx/sg/prism/NGShape3D.java	Tue Oct 08 09:30:23 2013 -0700
@@ -83,12 +83,10 @@
             setCullFace(cullFace);
         }
 
-        if (meshView == null) {
+        if (meshView == null || !mesh.validate()) {
             return;
         }
 
-        mesh.validate();
-
         Material mtl =  material.createMaterial(rf);
         if (materialDirty) {
             meshView.setMaterial(mtl);
--- a/modules/graphics/src/main/java/com/sun/javafx/sg/prism/NGTriangleMesh.java	Tue Oct 08 09:26:09 2013 -0700
+++ b/modules/graphics/src/main/java/com/sun/javafx/sg/prism/NGTriangleMesh.java	Tue Oct 08 09:30:23 2013 -0700
@@ -54,17 +54,20 @@
             mesh = rf.createMesh();
             meshDirty = true;
         }
-        validate();
         return mesh;        
     }
 
-    void validate() {
+    boolean validate() {
+        if (points == null || texCoords == null || faces == null || faceSmoothingGroups == null) {
+            return false;
+        }
         if (meshDirty) {
             if (!mesh.buildGeometry(points, texCoords, faces, faceSmoothingGroups)) {
                 throw new RuntimeException("NGTriangleMesh: buildGeometry failed");
             }
             meshDirty = false;
         }
+        return true;
     }
 
     // Note: This method is intentionally made package scope for security
@@ -101,22 +104,22 @@
 
     public void syncPoints(FloatArraySyncer array) {
         meshDirty = true;
-        points = array.syncTo(points);
+        points = array != null ? array.syncTo(points) : null;
     }
     
     public void syncTexCoords(FloatArraySyncer array) {
         meshDirty = true;
-        texCoords = array.syncTo(texCoords);
+        texCoords = array != null ? array.syncTo(texCoords) : null;
     }
 
     public void syncFaces(IntegerArraySyncer array) {
         meshDirty = true;
-        faces = array.syncTo(faces);    
+        faces = array != null ? array.syncTo(faces) : null;
     }
 
     public void syncFaceSmoothingGroups(IntegerArraySyncer array) {
         meshDirty = true;
-        faceSmoothingGroups = array.syncTo(faceSmoothingGroups);
+        faceSmoothingGroups = array != null ? array.syncTo(faceSmoothingGroups) : null;
     }
 
     // NOTE: This method is used for unit test purpose only.
--- a/modules/graphics/src/main/java/com/sun/prism/impl/BaseMesh.java	Tue Oct 08 09:26:09 2013 -0700
+++ b/modules/graphics/src/main/java/com/sun/prism/impl/BaseMesh.java	Tue Oct 08 09:30:23 2013 -0700
@@ -78,7 +78,7 @@
         this.pos = pos;
         this.uv = uv;
         this.faces = faces;
-        this.smoothing = smoothing != null && smoothing.length >= nFaces ? smoothing : null;
+        this.smoothing = smoothing.length == nFaces ? smoothing : null;
 
         MeshTempState instance = MeshTempState.getInstance();
         // big pool for all possible vertices
@@ -326,28 +326,14 @@
         if ((face == null) || (face.length < FACE_MEMBERS_SIZE)) {
             face = new int[FACE_MEMBERS_SIZE];
         }
-        if (faces[index] < nVerts
-                && faces[index + 2] < nVerts
-                && faces[index + 4] < nVerts
-                && faces[index + 1] < nTVerts
-                && faces[index + 3] < nTVerts
-                && faces[index + 5] < nTVerts) {
-            // Note: Order matter, [0, 5] == FaceMembers' points and texcoords
-            for (int i = 0; i < 6; i++) {
-                face[i] = faces[index + i];
-            }
-            // Note: Order matter, 6 == FaceMembers.SMOOTHING_GROUP.ordinal()
-            // There is a total of 32 smoothing groups.
-            // Assign to 1st smoothing group if smoothing is null.
-            face[6] = smoothing != null ? smoothing[fIdx] : 1;
-        } else {
-            // Note: Order matter, [0, 5] == FaceMembers' points and texcoords
-            for (int i = 0; i < 6; i++) {
-                face[i] = 0;
-            }
-            // Note: Order matter, 6 == FaceMembers.SMOOTHING_GROUP.ordinal()
-            face[6] = 1;
+        // Note: Order matter, [0, 5] == FaceMembers' points and texcoords
+        for (int i = 0; i < 6; i++) {
+            face[i] = faces[index + i];
         }
+        // Note: Order matter, 6 == FaceMembers.SMOOTHING_GROUP.ordinal()
+        // There is a total of 32 smoothing groups.
+        // Assign to 1st smoothing group if smoothing is null.
+        face[6] = smoothing != null ? smoothing[fIdx] : 1;
         return face;
     }
 }
--- a/modules/graphics/src/main/java/javafx/scene/shape/Box.java	Tue Oct 08 09:26:09 2013 -0700
+++ b/modules/graphics/src/main/java/javafx/scene/shape/Box.java	Tue Oct 08 09:30:23 2013 -0700
@@ -430,7 +430,7 @@
             1, 2, 4, 0, 0, 3,
         };
 
-        TriangleMesh mesh = new TriangleMesh();
+        TriangleMesh mesh = new TriangleMesh(true);
         mesh.getPoints().setAll(points);
         mesh.getTexCoords().setAll(texCoords);
         mesh.getFaces().setAll(faces);
--- a/modules/graphics/src/main/java/javafx/scene/shape/Cylinder.java	Tue Oct 08 09:26:09 2013 -0700
+++ b/modules/graphics/src/main/java/javafx/scene/shape/Cylinder.java	Tue Oct 08 09:30:23 2013 -0700
@@ -531,7 +531,7 @@
             smoothing[i] = 2;
         }
 
-        TriangleMesh m = new TriangleMesh();
+        TriangleMesh m = new TriangleMesh(true);
         m.getPoints().setAll(points);
         m.getTexCoords().setAll(tPoints);
         m.getFaces().setAll(faces);
--- a/modules/graphics/src/main/java/javafx/scene/shape/Sphere.java	Tue Oct 08 09:26:09 2013 -0700
+++ b/modules/graphics/src/main/java/javafx/scene/shape/Sphere.java	Tue Oct 08 09:30:23 2013 -0700
@@ -414,7 +414,7 @@
             fIndex += 6;
         }
 
-        TriangleMesh m = new TriangleMesh();
+        TriangleMesh m = new TriangleMesh(true);
         m.getPoints().setAll(points);
         m.getTexCoords().setAll(tPoints);
         m.getFaces().setAll(faces);
--- a/modules/graphics/src/main/java/javafx/scene/shape/TriangleMesh.java	Tue Oct 08 09:26:09 2013 -0700
+++ b/modules/graphics/src/main/java/javafx/scene/shape/TriangleMesh.java	Tue Oct 08 09:30:23 2013 -0700
@@ -45,6 +45,7 @@
 import javafx.scene.transform.Affine;
 import javafx.scene.transform.NonInvertibleTransformException;
 import javafx.scene.transform.Rotate;
+import sun.util.logging.PlatformLogger;
 
 /**
  * Defines a 3D geometric object contains separate arrays of points, 
@@ -95,9 +96,6 @@
     public static final int NUM_COMPONENTS_PER_TEXCOORD = 2;
     public static final int NUM_COMPONENTS_PER_FACE = 6;
 
-    // TODO: 3D - Need to validate the size and range of these arrays.
-    // A warning will be recorded to the logger and the mesh will have an empty
-    // bounds if the validation failed. (RT-30451)
     // The values in faces must be within range and the length of points,
     // texCoords and faces must be divisible by 3, 2 and 6 respectively.
     private final ObservableFloatArray points = FXCollections.observableFloatArray();
@@ -109,7 +107,9 @@
     private final Listener texCoordsSyncer = new Listener(texCoords);
     private final Listener facesSyncer = new Listener(faces);
     private final Listener faceSmoothingGroupsSyncer = new Listener(faceSmoothingGroups);
-
+    private final boolean isPredefinedShape;
+    private boolean isValidDirty = true;
+    private boolean isPointsValid, isTexCoordsValid, isFacesValid, isFaceSmoothingGroupValid;
     private int refCount = 1;
 
     private BaseBounds cachedBounds;
@@ -118,8 +118,23 @@
      * Creates a new instance of {@code TriangleMesh} class.
      */
     public TriangleMesh() {
+        this(false);
     }
 
+    TriangleMesh(boolean isPredefinedShape) {
+        this.isPredefinedShape = isPredefinedShape;
+        if (isPredefinedShape) {
+            isPointsValid = true;
+            isTexCoordsValid = true;
+            isFacesValid = true;
+            isFaceSmoothingGroupValid = true;
+        } else {
+            isPointsValid = false;
+            isTexCoordsValid = false;
+            isFacesValid = false;
+            isFaceSmoothingGroupValid = false;
+        }
+    }
     /**
      * Gets the {@code ObservableFloatArray} of points of this {@code TriangleMesh}.
      *
@@ -224,6 +239,100 @@
         return impl_getPGTriangleMesh();
     }
 
+    private boolean validatePoints() {
+        if (points.size() == 0) { // Valid but meaningless for picking or rendering.
+            return false;
+        }
+        
+        if ((points.size() % NUM_COMPONENTS_PER_POINT) != 0) {
+            String logname = TriangleMesh.class.getName();
+            PlatformLogger.getLogger(logname).warning("points.length has "
+                    + "to be divisible by NUM_COMPONENTS_PER_POINT. It is to"
+                    + " store multiple x, y, and z coordinates of this mesh");
+            return false;
+        }
+        return true;
+    }
+
+    private boolean validateTexCoords() {
+        if (texCoords.size() == 0) { // Valid but meaningless for picking or rendering.
+            return false;
+        }
+
+        if ((texCoords.size() % NUM_COMPONENTS_PER_TEXCOORD) != 0) {
+            String logname = TriangleMesh.class.getName();
+            PlatformLogger.getLogger(logname).warning("texCoords.length "
+                    + "has to be divisible by NUM_COMPONENTS_PER_TEXCOORD."
+                    + " It is to store multiple u and v texture coordinates"
+                    + " of this mesh");
+            return false;
+        }
+        return true;
+    }
+
+    private boolean validateFaces() {
+        if (faces.size() == 0) { // Valid but meaningless for picking or rendering.
+            return false;
+        }
+        
+        String logname = TriangleMesh.class.getName();
+        if ((faces.size() % NUM_COMPONENTS_PER_FACE) != 0) {
+            PlatformLogger.getLogger(logname).warning("faces.length has "
+                    + "to be divisible by NUM_COMPONENTS_PER_FACE.");
+            return false;
+        }
+
+        int nVerts = points.size() / NUM_COMPONENTS_PER_POINT;
+        int nTVerts = texCoords.size() / NUM_COMPONENTS_PER_TEXCOORD;
+        for (int i = 0; i < faces.size(); i++) {
+            if (i % 2 == 0 && (faces.get(i) >= nVerts || faces.get(i) < 0)
+                    || (i % 2 != 0 && (faces.get(i) >= nTVerts || faces.get(i) < 0))) {
+                PlatformLogger.getLogger(logname).warning("The values in the "
+                        + "faces array must be within the range of the number "
+                        + "of vertices in the points array (0 to points.length / 3 - 1) "
+                        + "for the point indices and within the range of the "
+                        + "number of the vertices in the texCoords array (0 to "
+                        + "texCoords.length / 2 - 1) for the texture coordinate indices.");
+                return false;
+            }
+        }
+        return true;
+    }
+
+    private boolean validateFaceSmoothingGroups() {
+        if (faceSmoothingGroups.size() != 0
+                && faceSmoothingGroups.size() != (faces.size() / NUM_COMPONENTS_PER_FACE)) {
+            String logname = TriangleMesh.class.getName();
+            PlatformLogger.getLogger(logname).warning("faceSmoothingGroups.length"
+                    + " has to equal to number of faces.");
+            return false;
+        }
+        return true;
+    }
+
+    private boolean validate() {
+        if (isPredefinedShape) {
+            return true;
+        }
+        
+        if (isValidDirty) {
+            if (pointsSyncer.dirtyInFull) {
+                isPointsValid = validatePoints();
+            }
+            if (texCoordsSyncer.dirtyInFull) {
+                isTexCoordsValid = validateTexCoords();
+            }
+            if (facesSyncer.dirty || pointsSyncer.dirtyInFull || texCoordsSyncer.dirtyInFull) {
+                isFacesValid = (isPointsValid && isTexCoordsValid) ? validateFaces() : false;
+            }
+            if (faceSmoothingGroupsSyncer.dirtyInFull || facesSyncer.dirtyInFull) {
+                isFaceSmoothingGroupValid = validateFaceSmoothingGroups();
+            }
+            isValidDirty = false;
+        }
+        return isPointsValid && isTexCoordsValid && isFaceSmoothingGroupValid && isFacesValid;
+    }
+
     /**
      * @treatAsPrivate implementation detail
      * @deprecated This is an internal API that is not intended for use and will be removed in the next version
@@ -235,19 +344,19 @@
             return;
         }
 
+        validate();
         final NGTriangleMesh pgTriMesh = impl_getPGTriangleMesh();
-        // sync points 
         if (pointsSyncer.dirty) {
-            pgTriMesh.syncPoints(pointsSyncer);
+            pgTriMesh.syncPoints(isPointsValid ? pointsSyncer : null);
         }
         if (texCoordsSyncer.dirty) {
-            pgTriMesh.syncTexCoords(texCoordsSyncer);
+            pgTriMesh.syncTexCoords(isTexCoordsValid ? texCoordsSyncer : null);
         }
         if (facesSyncer.dirty) {
-            pgTriMesh.syncFaces(facesSyncer);
+            pgTriMesh.syncFaces(isFacesValid ? facesSyncer : null);
         }
         if (faceSmoothingGroupsSyncer.dirty) {
-            pgTriMesh.syncFaceSmoothingGroups(faceSmoothingGroupsSyncer);
+            pgTriMesh.syncFaceSmoothingGroups(isFaceSmoothingGroupValid ? faceSmoothingGroupsSyncer : null);
         }
         setDirty(false);
     }
@@ -256,10 +365,11 @@
     BaseBounds computeBounds(BaseBounds bounds) {
         if (isDirty() || cachedBounds == null) {
             cachedBounds = new BoxBounds();
-
-            final double len = points.size();
-            for (int i = 0; i < len; i += NUM_COMPONENTS_PER_POINT) {
-                cachedBounds.add(points.get(i), points.get(i + 1), points.get(i + 2));
+            if (validate()) {
+                final double len = points.size();
+                for (int i = 0; i < len; i += NUM_COMPONENTS_PER_POINT) {
+                    cachedBounds.add(points.get(i), points.get(i + 1), points.get(i + 2));
+                }
             }
         }
         return bounds.deriveWithNewBounds(cachedBounds);
@@ -532,19 +642,20 @@
             Node candidate, CullFace cullFace, boolean reportFace) {
 
         boolean found = false;
-        final int size = faces.size();
+        if (validate()) {
+            final int size = faces.size();
 
-        final Vec3d o = pickRay.getOriginNoClone();
+            final Vec3d o = pickRay.getOriginNoClone();
 
-        final Vec3d d = pickRay.getDirectionNoClone();
+            final Vec3d d = pickRay.getDirectionNoClone();
 
-        for (int i = 0; i < size; i += NUM_COMPONENTS_PER_FACE) {
-            if (computeIntersectsFace(pickRay, o, d, i, cullFace, candidate, 
-                    reportFace, pickResult)) {
-                found = true;
+            for (int i = 0; i < size; i += NUM_COMPONENTS_PER_FACE) {
+                if (computeIntersectsFace(pickRay, o, d, i, cullFace, candidate,
+                        reportFace, pickResult)) {
+                    found = true;
+                }
             }
         }
-
         return found;
     }
 
@@ -597,6 +708,7 @@
             } else {
                 addDirtyRange(from, to - from);
             }
+            isValidDirty = true;
         }
 
         /**