From ff7adf310236559faed0ed757e78c62bc0e1588d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Victor=20Glind=C3=A5s?= Date: Sun, 18 Feb 2018 13:53:51 +0100 Subject: [PATCH 1/6] Refactors ApplicationStateManager.detach to use the early retur pattern. --- .../mini/app/state/ApplicationStateManager.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/Engine/src/mini/app/state/ApplicationStateManager.java b/Engine/src/mini/app/state/ApplicationStateManager.java index 91f38d2..89f9009 100644 --- a/Engine/src/mini/app/state/ApplicationStateManager.java +++ b/Engine/src/mini/app/state/ApplicationStateManager.java @@ -83,13 +83,15 @@ public boolean detach(ApplicationState state) { states.remove(state); terminating.add(state); return true; - } else if (initializing.contains(state)) { + } + + if (initializing.contains(state)) { state.stateDetached(this); initializing.remove(state); return true; - } else { - return false; } + + return false; } /** @@ -118,9 +120,7 @@ public void update(float tpf) { * @param renderManager The RenderManager */ public void render(RenderManager renderManager) { - Arrays.stream(getStates()) - .filter(ApplicationState::isEnabled) - .forEach(state -> state.render(renderManager)); + Arrays.stream(getStates()).filter(ApplicationState::isEnabled).forEach(state -> state.render(renderManager)); } /** @@ -129,9 +129,7 @@ public void render(RenderManager renderManager) { * @param renderManager The RenderManager */ public void postRender() { - Arrays.stream(getStates()) - .filter(ApplicationState::isEnabled) - .forEach(ApplicationState::postRender); + Arrays.stream(getStates()).filter(ApplicationState::isEnabled).forEach(ApplicationState::postRender); } private void initializePending() { From 007fe8beb30e7f28d10035ea86286110e9806b54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Victor=20Glind=C3=A5s?= Date: Sun, 18 Feb 2018 14:01:16 +0100 Subject: [PATCH 2/6] Bit of refactor/format WeakrefCloneAssetCache. --- .../asset/cache/WeakRefCloneAssetCache.java | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/Engine/src/mini/asset/cache/WeakRefCloneAssetCache.java b/Engine/src/mini/asset/cache/WeakRefCloneAssetCache.java index a1272e4..952e663 100644 --- a/Engine/src/mini/asset/cache/WeakRefCloneAssetCache.java +++ b/Engine/src/mini/asset/cache/WeakRefCloneAssetCache.java @@ -28,10 +28,9 @@ public void registerAssetClone(AssetKey key, T clone) { @Override public boolean deleteFromCache(AssetKey key) { List loadStack = assetLoadStack.get(); + if (!loadStack.isEmpty()) { - throw new UnsupportedOperationException( - "Cache cannot be modified while assets are being" - + " loaded"); + throw new UnsupportedOperationException("Cache cannot be modified while assets are being loaded"); } return smartCache.remove(key) != null; @@ -42,8 +41,7 @@ public void clearCache() { List loadStack = assetLoadStack.get(); if (!loadStack.isEmpty()) { - throw new UnsupportedOperationException("Cache cannot be modified" - + "while assets are being loaded"); + throw new UnsupportedOperationException("Cache cannot be modified while assets are being loaded"); } smartCache.clear(); @@ -51,11 +49,10 @@ public void clearCache() { private final ConcurrentMap smartCache = new ConcurrentHashMap<>(); private final ReferenceQueue referenceQueue = new ReferenceQueue<>(); - private final ThreadLocal> assetLoadStack = - ThreadLocal.withInitial(ArrayList::new); + private final ThreadLocal> assetLoadStack = ThreadLocal.withInitial(ArrayList::new); private void removeCollectedAssets() { - for (KeyRef ref; (ref = (KeyRef) referenceQueue.poll()) != null; ) { + for (KeyRef ref; (ref = (KeyRef) referenceQueue.poll()) != null;) { smartCache.remove(ref.clonedKey); } } @@ -87,17 +84,17 @@ public T getFromCache(AssetKey key) { if (smartInfo == null) { return null; - } else { - AssetKey keyForTheClone = smartInfo.get(); - if (keyForTheClone == null) { - // was collected by GC between here and smartCache.get - return null; - } - - List loadStack = assetLoadStack.get(); - loadStack.add(keyForTheClone); - return (T) smartInfo.asset; } + + AssetKey keyForTheClone = smartInfo.get(); + if (keyForTheClone == null) { + // Was collected by GC between here and smartCache.get + return null; + } + + List loadStack = assetLoadStack.get(); + loadStack.add(keyForTheClone); + return (T) smartInfo.asset; } private static final class KeyRef extends PhantomReference { From e914100cd132cc66e8a8ae66b481cb9da22a6a08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Victor=20Glind=C3=A5s?= Date: Sun, 18 Feb 2018 14:06:13 +0100 Subject: [PATCH 3/6] Refactor/format UrlAssetInfo. --- .../src/mini/asset/plugins/UrlAssetInfo.java | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/Engine/src/mini/asset/plugins/UrlAssetInfo.java b/Engine/src/mini/asset/plugins/UrlAssetInfo.java index 1210e32..99cd8aa 100644 --- a/Engine/src/mini/asset/plugins/UrlAssetInfo.java +++ b/Engine/src/mini/asset/plugins/UrlAssetInfo.java @@ -22,8 +22,7 @@ private UrlAssetInfo(AssetManager assetManager, AssetKey key, URL url, InputStre this.in = in; } - public static UrlAssetInfo create(AssetManager assetManager, AssetKey key, URL url) throws - IOException { + public static UrlAssetInfo create(AssetManager assetManager, AssetKey key, URL url) throws IOException { // Check if URL can be reached. This will throw // IOException which calling code will handle. URLConnection conn = url.openConnection(); @@ -33,9 +32,9 @@ public static UrlAssetInfo create(AssetManager assetManager, AssetKey key, URL u // For some reason url cannot be reached? if (in == null) { return null; - } else { - return new UrlAssetInfo(assetManager, key, url, in); } + + return new UrlAssetInfo(assetManager, key, url, in); } @Override @@ -45,15 +44,15 @@ public InputStream openStream() { InputStream in2 = in; in = null; return in2; - } else { - // Create a new stream for subsequent invocations. - try { - URLConnection conn = url.openConnection(); - conn.setUseCaches(false); - return conn.getInputStream(); - } catch (IOException ex) { - throw new RuntimeException("Failed to read URL " + url, ex); - } + } + + // Create a new stream for subsequent invocations. + try { + URLConnection connection = url.openConnection(); + connection.setUseCaches(false); + return connection.getInputStream(); + } catch (IOException ex) { + throw new RuntimeException("Failed to read URL " + url, ex); } } From c565bd2a6232550da2eedec939d2ee9c3c6d2c01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Victor=20Glind=C3=A5s?= Date: Sun, 18 Feb 2018 14:58:49 +0100 Subject: [PATCH 4/6] Refactors BoundingBox. --- Engine/src/mini/bounding/BoundingBox.java | 129 +++++++++++++--------- 1 file changed, 75 insertions(+), 54 deletions(-) diff --git a/Engine/src/mini/bounding/BoundingBox.java b/Engine/src/mini/bounding/BoundingBox.java index 29947c5..fb9b7f6 100644 --- a/Engine/src/mini/bounding/BoundingBox.java +++ b/Engine/src/mini/bounding/BoundingBox.java @@ -174,11 +174,13 @@ public Plane.Side whichSide(Plane plane) { if (distance < -radius) { return Plane.Side.Negative; - } else if (distance > radius) { + } + + if (distance > radius) { return Plane.Side.Positive; - } else { - return Plane.Side.None; } + + return Plane.Side.None; } @Override @@ -200,10 +202,13 @@ public void computeFromPoints(FloatBuffer points) { float[] tmpArray = vars.skinPositions; - float minX = Float.POSITIVE_INFINITY, minY = Float.POSITIVE_INFINITY, minZ - = Float.POSITIVE_INFINITY; - float maxX = Float.NEGATIVE_INFINITY, maxY = Float.NEGATIVE_INFINITY, maxZ - = Float.NEGATIVE_INFINITY; + float minX = Float.POSITIVE_INFINITY; + float minY = Float.POSITIVE_INFINITY; + float minZ = Float.POSITIVE_INFINITY; + + float maxX = Float.NEGATIVE_INFINITY; + float maxY = Float.NEGATIVE_INFINITY; + float maxZ = Float.NEGATIVE_INFINITY; int iterations = (int) FastMath.ceil(points.limit() / ((float) tmpArray.length)); // TODO: Parallel iterations through the array could get all 6 values. Using streaming on the array @@ -292,7 +297,15 @@ public BoundingVolume mergeLocal(BoundingVolume volume) { */ private BoundingVolume mergeLocal(Vector3f center, float xExtent, float yExtent, float zExtent) { - if (this.xExtent == Float.POSITIVE_INFINITY || xExtent == Float.POSITIVE_INFINITY) { + mergeLocalX(center, xExtent); + mergeLocalY(center, yExtent); + mergeLocalZ(center, zExtent); + + return this; + } + + private void mergeLocalX(Vector3f center, float xExtent) { + if (this.xExtent == Float.POSITIVE_INFINITY || xExtent == Float.POSITIVE_INFINITY) { this.center.x = 0; this.xExtent = Float.POSITIVE_INFINITY; } else { @@ -307,24 +320,10 @@ private BoundingVolume mergeLocal(Vector3f center, float xExtent, float yExtent, this.center.x = (low + high) / 2; this.xExtent = high - this.center.x; } + } - if (this.yExtent == Float.POSITIVE_INFINITY || yExtent == Float.POSITIVE_INFINITY) { - this.center.y = 0; - this.yExtent = Float.POSITIVE_INFINITY; - } else { - float low = this.center.y - this.yExtent; - if (low > center.y - yExtent) { - low = center.y - yExtent; - } - float high = this.center.y + this.yExtent; - if (high < center.y + yExtent) { - high = center.y + yExtent; - } - this.center.y = (low + high) / 2; - this.yExtent = high - this.center.y; - } - - if (this.zExtent == Float.POSITIVE_INFINITY || zExtent == Float.POSITIVE_INFINITY) { + private void mergeLocalZ(Vector3f center, float zExtent) { + if (this.zExtent == Float.POSITIVE_INFINITY || zExtent == Float.POSITIVE_INFINITY) { this.center.z = 0; this.zExtent = Float.POSITIVE_INFINITY; } else { @@ -339,9 +338,25 @@ private BoundingVolume mergeLocal(Vector3f center, float xExtent, float yExtent, this.center.z = (low + high) / 2; this.zExtent = high - this.center.z; } + } - return this; - } + private void mergeLocalY(Vector3f center, float yExtent) { + if (this.yExtent == Float.POSITIVE_INFINITY || yExtent == Float.POSITIVE_INFINITY) { + this.center.y = 0; + this.yExtent = Float.POSITIVE_INFINITY; + } else { + float low = this.center.y - this.yExtent; + if (low > center.y - yExtent) { + low = center.y - yExtent; + } + float high = this.center.y + this.yExtent; + if (high < center.y + yExtent) { + high = center.y + yExtent; + } + this.center.y = (low + high) / 2; + this.yExtent = high - this.center.y; + } + } @Override public float distanceToEdge(Vector3f point) { @@ -466,30 +481,36 @@ && clip(+direction.z, -diff.z - zExtent, t) */ private boolean clip(float denominator, float numerator, float t[]) { if (denominator > 0.0f) { - float newT = numerator / denominator; - if (newT > t[1]) { - return false; - } - - if (newT > t[0]) { - t[0] = newT; - } - return true; - } else if (denominator < 0.0f) { - float newT = numerator / denominator; - if (newT < t[0]) { - return false; - } + return clipPositive(denominator, numerator, t); + } + if (denominator < 0.0f) { + return clipNegative(denominator, numerator, t); + } + return numerator <= 0.0f; + } - if (newT < t[1]) { - t[1] = newT; - } - return true; - } else { - return numerator <= 0.0f; + private boolean clipPositive(float denominator, float numerator, float[] t) { + float newT = numerator / denominator; + if (newT > t[1]) { + return false; + } + if (newT > t[0]) { + t[0] = newT; } + return true; } + private boolean clipNegative(float denominator, float numerator, float[] t) { + float newT = numerator / denominator; + if (newT < t[0]) { + return false; + } + if (newT < t[1]) { + t[1] = newT; + } + return true; + } + /** * Query extent * @@ -530,13 +551,13 @@ public Vector3f getMax(Vector3f store) { */ public BoundingVolume clone(BoundingVolume store) { if (store != null && store.getType() == Type.AABB) { - BoundingBox rVal = (BoundingBox) store; - rVal.center.set(center); - rVal.xExtent = xExtent; - rVal.yExtent = yExtent; - rVal.zExtent = zExtent; - rVal.checkPlane = checkPlane; - return rVal; + BoundingBox clone = (BoundingBox) store; + clone.center.set(center); + clone.xExtent = xExtent; + clone.yExtent = yExtent; + clone.zExtent = zExtent; + clone.checkPlane = checkPlane; + return clone; } return new BoundingBox(center.clone(), xExtent, yExtent, zExtent); From f5d1a447be20bdc854b7fa3817660bba11bc2e6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Victor=20Glind=C3=A5s?= Date: Sun, 18 Feb 2018 15:29:46 +0100 Subject: [PATCH 5/6] Refactors BoundingSphere. --- Engine/src/mini/bounding/BoundingSphere.java | 103 +++++++++++-------- 1 file changed, 58 insertions(+), 45 deletions(-) diff --git a/Engine/src/mini/bounding/BoundingSphere.java b/Engine/src/mini/bounding/BoundingSphere.java index 201dada..17f0317 100644 --- a/Engine/src/mini/bounding/BoundingSphere.java +++ b/Engine/src/mini/bounding/BoundingSphere.java @@ -109,11 +109,13 @@ public Plane.Side whichSide(Plane plane) { if (distance <= -radius) { return Plane.Side.Negative; - } else if (distance >= radius) { + } + + if (distance >= radius) { return Plane.Side.Positive; - } else { - return Plane.Side.None; } + + return Plane.Side.None; } @Override @@ -190,21 +192,24 @@ private void setSphere(Vector3f O, Vector3f A, Vector3f B, Vector3f C) { Vector3f b = B.subtract(O); Vector3f c = C.subtract(O); - float denominator = 2.0f * (a.x * (b.y * c.z - c.y * b.z) - b.x * (a.y * c.z - c.y * a.z) - + c.x * (a.y * b.z - b.y - a.z)); + float ax = a.x * (b.y * c.z - c.y * b.z); + float bx = b.x * (a.y * c.z - c.y * a.z); + float cx = c.x * (a.y * b.z - b.y - a.z); + float denominator = 2.0f * (ax - bx + cx); if (denominator == 0) { center.set(0, 0, 0); radius = 0; - } else { - Vector3f o = a.cross(b).multLocal(c.lengthSquared()) - .addLocal(c.cross(a).multLocal(b.lengthSquared())) - .addLocal(b.cross(c).multLocal(a.lengthSquared())) - .divideLocal(denominator); - - radius = o.length() * RADIUS_EPSILON; - O.add(o, center); + return; } + + Vector3f o = a.cross(b).multLocal(c.lengthSquared()) + .addLocal(c.cross(a).multLocal(b.lengthSquared())) + .addLocal(b.cross(c).multLocal(a.lengthSquared())) + .divideLocal(denominator); + + radius = o.length() * RADIUS_EPSILON; + O.add(o, center); } private void setSphere(Vector3f O, Vector3f A, Vector3f B) { @@ -217,19 +222,21 @@ private void setSphere(Vector3f O, Vector3f A, Vector3f B) { if (denominator == 0) { center.set(0, 0, 0); radius = 0; - } else { - Vector3f o = aCrossB.cross(a).multLocal(b.lengthSquared()) - .addLocal(b.cross(aCrossB).multLocal(a.lengthSquared())) - .divideLocal(denominator); - radius = o.length() * RADIUS_EPSILON; - O.add(o, center); + return; } + + Vector3f o = aCrossB.cross(a).multLocal(b.lengthSquared()) + .addLocal(b.cross(aCrossB).multLocal(a.lengthSquared())) + .divideLocal(denominator); + radius = o.length() * RADIUS_EPSILON; + O.add(o, center); } private void setSphere(Vector3f O, Vector3f A) { - radius = FastMath.sqrt( - ((A.x - O.x) * (A.x - O.x) + (A.y - O.y) * (A.y - O.y) + (A.z - O.z) * (A.z - O.z)) - / 4f) + RADIUS_EPSILON - 1f; + float x = (A.x - O.x) * (A.x - O.x); + float y = (A.y - O.y) * (A.y - O.y); + float z = (A.z - O.z) * (A.z - O.z); + radius = FastMath.sqrt((a + b + c) / 4f) + RADIUS_EPSILON - 1f; center.interpolateLocal(O, A, .5f); } @@ -317,27 +324,29 @@ public BoundingVolume mergeLocal(BoundingVolume volume) { } switch (volume.getType()) { - case Sphere: { - BoundingSphere sphere = (BoundingSphere) volume; - float tempRadius = sphere.radius; - Vector3f tempCenter = sphere.center; - return merge(tempRadius, tempCenter, this); - } - case AABB: { - BoundingBox box = (BoundingBox) volume; - TempVars vars = TempVars.get(); - Vector3f radVect = vars.vect1; - radVect.set(box.getXExtent(), box.getYExtent(), box.getZExtent()); - Vector3f tempCenter = box.center; - float length = radVect.length(); - vars.release(); // TODO: Is this better than defining new Vector3f? - return merge(length, tempCenter, this); - } + case Sphere: + return mergeLocalSphere(volume); + case AABB: + return mergeLocalBox(volume); default: return null; } } + private BoundingVolume mergeLocalSphere(BoundingSphere sphere) { + return merge(sphere.radius, sphere.center, this); + } + + private BoundingVolume mergeLocalBox(BoundingBox box) { + TempVars vars = TempVars.get(); + Vector3f radVect = vars.vect1; + radVect.set(box.getXExtent(), box.getYExtent(), box.getZExtent()); + Vector3f tempCenter = box.center; + float length = radVect.length(); + vars.release(); // TODO: Is this better than defining new Vector3f? + return merge(length, tempCenter, this); + } + /** * clone creates a new BoundingSphere object containing the same data as this one. * @@ -377,12 +386,14 @@ public int collideWith(Collidable other, CollisionResults results) { CollisionResult result = new CollisionResult(); results.addCollision(result); return 1; - } else if (other instanceof Ray) { + } + + if (other instanceof Ray) { Ray ray = (Ray) other; return collideWithRay(ray, results); - } else { - throw new UnsupportedOperationException(); } + + throw new UnsupportedOperationException(); } private int collideWithRay(Ray ray, CollisionResults results) { @@ -403,7 +414,9 @@ private int collideWithRay(Ray ray, CollisionResults results) { if (discr < 0.0) { return 0; - } else if (discr >= FastMath.ZERO_TOLERANCE) { + } + + if (discr >= FastMath.ZERO_TOLERANCE) { root = FastMath.sqrt(discr); float distance = -a1 - root; @@ -412,10 +425,10 @@ private int collideWithRay(Ray ray, CollisionResults results) { distance = -a1 + root; addCollision(ray, results, distance); return 2; - } else { - addCollision(ray, results, -a1); - return 1; } + + addCollision(ray, results, -a1); + return 1; } private void addCollision(Ray ray, CollisionResults results, float distance) { From 4e6bb3a0a591afe62536925ba6607b24a5808abe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Victor=20Glind=C3=A5s?= Date: Sun, 18 Feb 2018 15:41:44 +0100 Subject: [PATCH 6/6] Refactors Intersection. --- Engine/src/mini/bounding/Intersection.java | 29 +++++++++++----------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/Engine/src/mini/bounding/Intersection.java b/Engine/src/mini/bounding/Intersection.java index 603c7b5..3f66d0e 100644 --- a/Engine/src/mini/bounding/Intersection.java +++ b/Engine/src/mini/bounding/Intersection.java @@ -33,24 +33,23 @@ public static boolean intersect(BoundingBox boundingbox, Vector3f center, float float minZ = boundingbox.center.z - boundingbox.getZExtent(); float maxZ = boundingbox.center.z + boundingbox.getZExtent(); - if (center.x < minX) { - distSqr -= FastMath.sqr(center.x - minX); - } else if (center.x > maxX) { - distSqr -= FastMath.sqr(center.x - maxX); - } + distSqr -= centerOffset(center.x, minX, maxX); + distSqr -= centerOffset(center.y, minY, maxY); + distSqr -= centerOffset(center.z, minZ, maxZ); + + return distSqr > 0; + } - if (center.y < minY) { - distSqr -= FastMath.sqr(center.y - minY); - } else if (center.y > maxY) { - distSqr -= FastMath.sqr(center.y - maxY); + // FIXME: Needs a better name. + private static float centerOffset(float center, float min, float max) { + if (center < min) { + return FastMath.sqr(center - min); } - if (center.z < minZ) { - distSqr -= FastMath.sqr(center.z - minZ); - } else if (center.z > maxZ) { - distSqr -= FastMath.sqr(center.z - maxZ); + if (center > max) { + return FastMath.sqr(center - max); } - return distSqr > 0; - } + return 0; + } }