Merge pull request #2388 from jMonkeyEngine/sgold/issue/2333

solve issue #2333 (Camera viewport dimensions not checked)

jme3-core/src/main/java/com/jme3/renderer/Camera.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009-2021 jMonkeyEngine
+ * Copyright (c) 2009-2025 jMonkeyEngine
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -179,22 +179,22 @@ public class Camera implements Savable, Cloneable {
     //view port coordinates
     /**
      * Percent value on display where horizontal viewing starts for this camera.
-     * Default is 0.
+     * Default is 0. Must be less than {@code viewPortRight}.
      */
     protected float viewPortLeft;
     /**
      * Percent value on display where horizontal viewing ends for this camera.
-     * Default is 1.
+     * Default is 1. Must be greater than {@code viewPortLeft}.
      */
     protected float viewPortRight;
     /**
      * Percent value on display where vertical viewing ends for this camera.
-     * Default is 1.
+     * Default is 1. Must be greater than {@code viewPortBottom}.
      */
     protected float viewPortTop;
     /**
      * Percent value on display where vertical viewing begins for this camera.
-     * Default is 0.
+     * Default is 0. Must be less than {@code viewPortTop}.
      */
     protected float viewPortBottom;
     /**
@@ -1017,7 +1017,8 @@ public class Camera implements Savable, Cloneable {
     /**
      * Sets the left boundary of the viewport.
      *
-     * @param left the left boundary of the viewport
+     * @param left the left boundary of the viewport (<viewPortRight,
+     *     default: 0)
      */
     public void setViewPortLeft(float left) {
         viewPortLeft = left;
@@ -1036,7 +1037,8 @@ public class Camera implements Savable, Cloneable {
     /**
      * Sets the right boundary of the viewport.
      *
-     * @param right the right boundary of the viewport
+     * @param right the right boundary of the viewport (>viewPortLeft,
+     *     default: 1)
      */
     public void setViewPortRight(float right) {
         viewPortRight = right;
@@ -1055,7 +1057,8 @@ public class Camera implements Savable, Cloneable {
     /**
      * Sets the top boundary of the viewport.
      *
-     * @param top the top boundary of the viewport
+     * @param top the top boundary of the viewport (>viewPortBottom,
+     *     default: 1)
      */
     public void setViewPortTop(float top) {
         viewPortTop = top;
@@ -1074,7 +1077,8 @@ public class Camera implements Savable, Cloneable {
     /**
      * Sets the bottom boundary of the viewport.
      *
-     * @param bottom the bottom boundary of the viewport
+     * @param bottom the bottom boundary of the viewport (<viewPortTop,
+     *     default: 0)
      */
     public void setViewPortBottom(float bottom) {
         viewPortBottom = bottom;
@@ -1084,10 +1088,10 @@ public class Camera implements Savable, Cloneable {
     /**
      * Sets the boundaries of the viewport.
      *
-     * @param left   the left boundary of the viewport (default: 0)
-     * @param right  the right boundary of the viewport (default: 1)
-     * @param bottom the bottom boundary of the viewport (default: 0)
-     * @param top    the top boundary of the viewport (default: 1)
+     * @param left the left boundary of the viewport (<right, default: 0)
+     * @param right the right boundary of the viewport (>left, default: 1)
+     * @param bottom the bottom boundary of the viewport (<top, default: 0)
+     * @param top the top boundary of the viewport (>bottom, default: 1)
      */
     public void setViewPort(float left, float right, float bottom, float top) {
         this.viewPortLeft = left;
@@ -1283,6 +1287,15 @@ public class Camera implements Savable, Cloneable {
      * Called when the viewport has been changed.
      */
     public void onViewPortChange() {
+        if (!(viewPortBottom < viewPortTop)) {
+            throw new IllegalArgumentException(
+                    "Viewport must have bottom < top");
+        }
+        if (!(viewPortLeft < viewPortRight)) {
+            throw new IllegalArgumentException(
+                    "Viewport must have left < right");
+        }
+
         viewportChanged = true;
         setGuiBounding();
     }

jme3-core/src/test/java/com/jme3/renderer/Issue2333Test.java

@@ -0,0 +1,241 @@
+/*
+ * Copyright (c) 2025 jMonkeyEngine
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in the
+ *   documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of 'jMonkeyEngine' nor the names of its contributors
+ *   may be used to endorse or promote products derived from this software
+ *   without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package com.jme3.renderer;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Automated tests for "Camera Viewport Dimensions not Checked" (issue #2333 at
+ * GitHub).
+ *
+ * @author Stephen Gold [email protected]
+ */
+public class Issue2333Test {
+
+    /**
+     * Tests some basic functionality of the viewport settings.
+     */
+    @Test
+    public void testIssue2333() {
+        Camera c = new Camera(1, 1);
+
+        // Verify some Camera defaults:
+        Assert.assertEquals(0f, c.getViewPortBottom(), 0f);
+        Assert.assertEquals(0f, c.getViewPortLeft(), 0f);
+        Assert.assertEquals(1f, c.getViewPortRight(), 0f);
+        Assert.assertEquals(1f, c.getViewPortTop(), 0f);
+
+        // Try some valid settings:
+        new Camera(1, 1).setViewPort(0.5f, 0.7f, 0.1f, 0.3f);
+        new Camera(1, 1).setViewPortBottom(0.9f);
+        new Camera(1, 1).setViewPortLeft(0.99f);
+        new Camera(1, 1).setViewPortRight(0.01f);
+        new Camera(1, 1).setViewPortTop(0.1f);
+    }
+
+    /**
+     * Verifies that setViewPort() with left = right throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase01() {
+        new Camera(1, 1).setViewPort(0.5f, 0.5f, 0f, 1f);
+    }
+
+    /**
+     * Verifies that setViewPort() with left > right throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase02() {
+        new Camera(1, 1).setViewPort(0.7f, 0.5f, 0f, 1f);
+    }
+
+    /**
+     * Verifies that setViewPortLeft() resulting in left = right throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase03() {
+        new Camera(1, 1).setViewPortLeft(1f);
+    }
+
+    /**
+     * Verifies that setViewPortLeft() resulting in left > right throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase04() {
+        new Camera(1, 1).setViewPortLeft(1.1f);
+    }
+
+    /**
+     * Verifies that setViewPortRight() resulting in left = right throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase05() {
+        new Camera(1, 1).setViewPortRight(0f);
+    }
+
+    /**
+     * Verifies that setViewPortRight() resulting in left > right throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase06() {
+        new Camera(1, 1).setViewPortRight(-0.1f);
+    }
+
+    /**
+     * Verifies that setViewPort() with bottom = top throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase07() {
+        new Camera(1, 1).setViewPort(0f, 1f, 0.5f, 0.5f);
+    }
+
+    /**
+     * Verifies that setViewPort() with bottom > top throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase08() {
+        new Camera(1, 1).setViewPort(0f, 1f, 0.7f, 0.6f);
+    }
+
+    /**
+     * Verifies that setViewPortBottom() resulting in bottom = top throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase09() {
+        new Camera(1, 1).setViewPortBottom(1f);
+    }
+
+    /**
+     * Verifies that setViewPortBottom() resulting in bottom > top throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase10() {
+        new Camera(1, 1).setViewPortBottom(2f);
+    }
+
+    /**
+     * Verifies that setViewPortTop() resulting in bottom = top throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase11() {
+        new Camera(1, 1).setViewPortTop(0f);
+    }
+
+    /**
+     * Verifies that setViewPortTop() resulting in bottom > top throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase12() {
+        new Camera(1, 1).setViewPortTop(-1f);
+    }
+
+    /**
+     * Verifies that setViewPort() with left = NaN throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase13() {
+        new Camera(1, 1).setViewPort(Float.NaN, 1f, 0f, 1f);
+    }
+
+    /**
+     * Verifies that setViewPort() with right = NaN throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase14() {
+        new Camera(1, 1).setViewPort(0f, Float.NaN, 0f, 1f);
+    }
+
+    /**
+     * Verifies that setViewPort() with bottom = NaN throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase15() {
+        new Camera(1, 1).setViewPort(0f, 1f, Float.NaN, 1f);
+    }
+
+    /**
+     * Verifies that setViewPort() with top = NaN throws an
+     * IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase16() {
+        new Camera(1, 1).setViewPort(0f, 1f, 0f, Float.NaN);
+    }
+
+    /**
+     * Verifies that setViewPortBottom(NaN) throws an IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase17() {
+        new Camera(1, 1).setViewPortBottom(Float.NaN);
+    }
+
+    /**
+     * Verifies that setViewPortLeft(NaN) throws an IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase18() {
+        new Camera(1, 1).setViewPortLeft(Float.NaN);
+    }
+
+    /**
+     * Verifies that setViewPortRight(NaN) throws an IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase19() {
+        new Camera(1, 1).setViewPortRight(Float.NaN);
+    }
+
+    /**
+     * Verifies that setViewPortTop(NaN) throws an IllegalArgumentException.
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void iaeCase20() {
+        new Camera(1, 1).setViewPortTop(Float.NaN);
+    }
+}