From 8bb2f6dec8ab731b07387b947715fa1959c680e4 Mon Sep 17 00:00:00 2001 From: Sven Göthel Date: Sat, 17 Feb 2024 20:26:10 +0100 Subject: Bug 1489: Lock-Free Double-Buffered 'renderedShapes' causes data-race between rendering & input-edt, use synchronized tripple-buffering Tripple-buffering _almost_ produces zero data-race collisions, however .. it still does rarely -> hence synchronize on the used ArrayList<>. This adds a minimal chance for blocking the input-EDT, but gives correct code & results. Double-buffered 'renderedShapes' was introduced to resolve Bug 1489 in commit 5f9fb7159fa33bc979e5050d384b6939658049bd This solution is tested by passing '-swapInterval 0' via CommandlineOptions for FontView01, UIMediaGrid01 .., i.e. rendering faster than picking and hence provoking the data-race condition. --- make/scripts/tests.sh | 1 - .../jogamp/opengl/demos/graph/ui/FontView01.java | 4 + .../opengl/demos/graph/ui/UIMediaGrid01.java | 4 + .../opengl/demos/graph/ui/UISceneDemo03.java | 4 + .../opengl/demos/graph/ui/UISceneDemo20.java | 4 + .../opengl/demos/util/CommandlineOptions.java | 6 +- src/graphui/classes/com/jogamp/graph/ui/Group.java | 85 +++++++++++++--------- src/graphui/classes/com/jogamp/graph/ui/Scene.java | 76 +++++++++++-------- src/graphui/classes/jogamp/graph/ui/TreeTool.java | 46 ++++++------ 9 files changed, 143 insertions(+), 87 deletions(-) diff --git a/make/scripts/tests.sh b/make/scripts/tests.sh index abb0b912c..4447847cf 100644 --- a/make/scripts/tests.sh +++ b/make/scripts/tests.sh @@ -1032,7 +1032,6 @@ testnoawt com.jogamp.opengl.demos.graph.ui.FontView01 $* #testawt com.jogamp.opengl.test.junit.jogl.acore.TestDestroyGLAutoDrawableNewtAWT $* #testnoawt com.jogamp.opengl.demos.av.MovieSimple $* #testnoawt com.jogamp.opengl.demos.av.MovieCube $* -#testnoawt com.jogamp.opengl.demos.graph.ui.UIMediaGrid00 $* #testnoawt com.jogamp.opengl.demos.graph.ui.UIMediaGrid01 $* #testnoawt com.jogamp.opengl.demos.graph.ui.FontView01 $* #testawt com.jogamp.opengl.demos.graph.ui.UISceneDemo20 $* diff --git a/src/demos/com/jogamp/opengl/demos/graph/ui/FontView01.java b/src/demos/com/jogamp/opengl/demos/graph/ui/FontView01.java index 770b1b724..5804c09c0 100644 --- a/src/demos/com/jogamp/opengl/demos/graph/ui/FontView01.java +++ b/src/demos/com/jogamp/opengl/demos/graph/ui/FontView01.java @@ -167,6 +167,10 @@ public class FontView01 { final Animator animator = new Animator(0 /* w/o AWT */); animator.setUpdateFPSFrames(1*60, null); // System.err); final GLWindow window = GLWindow.create(reqCaps); + window.invoke(false, (final GLAutoDrawable glad) -> { + glad.getGL().setSwapInterval(options.swapInterval); + return true; + } ); window.setSize(options.surface_width, options.surface_height); window.setTitle(FontView01.class.getSimpleName()+": "+font.getFullFamilyName()+", "+window.getSurfaceWidth()+" x "+window.getSurfaceHeight()); window.setVisible(true); diff --git a/src/demos/com/jogamp/opengl/demos/graph/ui/UIMediaGrid01.java b/src/demos/com/jogamp/opengl/demos/graph/ui/UIMediaGrid01.java index 29ee9ab2c..7b9053ab0 100644 --- a/src/demos/com/jogamp/opengl/demos/graph/ui/UIMediaGrid01.java +++ b/src/demos/com/jogamp/opengl/demos/graph/ui/UIMediaGrid01.java @@ -216,6 +216,10 @@ public class UIMediaGrid01 { final Animator animator = new Animator(0 /* w/o AWT */); animator.setUpdateFPSFrames(1*60, null); // System.err); final GLWindow window = GLWindow.create(reqCaps); + window.invoke(false, (final GLAutoDrawable glad) -> { + glad.getGL().setSwapInterval(options.swapInterval); + return true; + } ); window.setSize(options.surface_width, options.surface_height); window.setVisible(true); System.out.println("Chosen: " + window.getChosenGLCapabilities()); diff --git a/src/demos/com/jogamp/opengl/demos/graph/ui/UISceneDemo03.java b/src/demos/com/jogamp/opengl/demos/graph/ui/UISceneDemo03.java index 98ea52771..0e7ddbeb8 100644 --- a/src/demos/com/jogamp/opengl/demos/graph/ui/UISceneDemo03.java +++ b/src/demos/com/jogamp/opengl/demos/graph/ui/UISceneDemo03.java @@ -194,6 +194,10 @@ public class UISceneDemo03 { animator.setUpdateFPSFrames(1 * 60, null); // System.err); final GLWindow window = GLWindow.create(reqCaps); + window.invoke(false, (final GLAutoDrawable glad) -> { + glad.getGL().setSwapInterval(options.swapInterval); + return true; + } ); window.setSize(options.surface_width, options.surface_height); window.setTitle(UISceneDemo03.class.getSimpleName() + ": " + window.getSurfaceWidth() + " x " + window.getSurfaceHeight()); window.setVisible(true); diff --git a/src/demos/com/jogamp/opengl/demos/graph/ui/UISceneDemo20.java b/src/demos/com/jogamp/opengl/demos/graph/ui/UISceneDemo20.java index 47c84c914..6336bb2bc 100644 --- a/src/demos/com/jogamp/opengl/demos/graph/ui/UISceneDemo20.java +++ b/src/demos/com/jogamp/opengl/demos/graph/ui/UISceneDemo20.java @@ -155,6 +155,10 @@ public class UISceneDemo20 implements GLEventListener { System.out.println("Requested: " + caps); final GLWindow window = GLWindow.create(screen, caps); + window.invoke(false, (final GLAutoDrawable glad) -> { + glad.getGL().setSwapInterval(options.swapInterval); + return true; + } ); if( 0 == options.sceneMSAASamples ) { window.setCapabilitiesChooser(new NonFSAAGLCapsChooser(false)); } diff --git a/src/demos/com/jogamp/opengl/demos/util/CommandlineOptions.java b/src/demos/com/jogamp/opengl/demos/util/CommandlineOptions.java index 3a3f2f365..60de2c1f0 100644 --- a/src/demos/com/jogamp/opengl/demos/util/CommandlineOptions.java +++ b/src/demos/com/jogamp/opengl/demos/util/CommandlineOptions.java @@ -44,6 +44,7 @@ public class CommandlineOptions { public boolean wait_to_start = false; public boolean keepRunning = false; public boolean stayOpen = false; + public int swapInterval = -1; // auto public float total_duration = 0f; // [s] static { @@ -153,6 +154,9 @@ public class CommandlineOptions { stayOpen = true; } else if (args[idx[0]].equals("-stay")) { stayOpen = true; + } else if (args[idx[0]].equals("-swapInterval")) { + ++idx[0]; + swapInterval = MiscUtils.atoi(args[idx[0]], swapInterval); } else if (args[idx[0]].equals("-duration")) { ++idx[0]; total_duration = MiscUtils.atof(args[idx[0]], total_duration); @@ -180,7 +184,7 @@ public class CommandlineOptions { return "Options{surface[width "+surface_width+" x "+surface_height+"], glp "+glProfileName+ ", renderModes "+Region.getRenderModeString(renderModes)+", aa-q "+graphAAQuality+ ", gmsaa "+graphAASamples+", smsaa "+sceneMSAASamples+ - ", exclusiveContext "+exclusiveContext+", wait "+wait_to_start+", keep "+keepRunning+", stay "+stayOpen+", dur "+total_duration+"s"+ + ", exclusiveContext "+exclusiveContext+", wait "+wait_to_start+", keep "+keepRunning+", stay "+stayOpen+", swap-ival "+swapInterval+", dur "+total_duration+"s"+ "}"; } } diff --git a/src/graphui/classes/com/jogamp/graph/ui/Group.java b/src/graphui/classes/com/jogamp/graph/ui/Group.java index f482605ea..eeaabaa3f 100644 --- a/src/graphui/classes/com/jogamp/graph/ui/Group.java +++ b/src/graphui/classes/com/jogamp/graph/ui/Group.java @@ -84,7 +84,9 @@ public class Group extends Shape implements Container { private Shape[] drawShapeArray = new Shape[0]; // reduce memory re-alloc @ display private final List renderedShapesB0 = new ArrayList(); private final List renderedShapesB1 = new ArrayList(); + private final List renderedShapesB2 = new ArrayList(); private volatile List renderedShapes = renderedShapesB1; + private int renderedShapesIdx = 1; /** Enforced fixed size. In case z-axis is NaN, its 3D z-axis will be adjusted. */ private final Vec3f fixedSize = new Vec3f(); private Layout layouter; @@ -313,6 +315,9 @@ public class Group extends Shape implements Container { drawShapeArray = new Shape[0]; renderedShapesB0.clear(); renderedShapesB1.clear(); + renderedShapesB2.clear(); + renderedShapes = renderedShapesB1; + renderedShapesIdx = 1; } @Override @@ -326,6 +331,9 @@ public class Group extends Shape implements Container { drawShapeArray = new Shape[0]; renderedShapesB0.clear(); renderedShapesB1.clear(); + renderedShapesB2.clear(); + renderedShapes = renderedShapesB1; + renderedShapesIdx = 1; if( null != border ) { border.destroy(gl, renderer); border = null; @@ -401,9 +409,13 @@ public class Group extends Shape implements Container { Arrays.sort(shapeArray, 0, shapeCount, Shape.ZAscendingComparator); // TreeTool.cullShapes(shapeArray, shapeCount); - final List iShapes = renderedShapes == renderedShapesB0 ? renderedShapesB1 : renderedShapesB0; - iShapes.clear(); - + final List iShapes; + final int iShapeIdx; + switch(renderedShapesIdx) { + case 0: iShapeIdx = 1; iShapes = renderedShapesB1; break; + case 1: iShapeIdx = 2; iShapes = renderedShapesB2; break; + default: iShapeIdx = 0; iShapes = renderedShapesB0; break; + } final boolean useClipFrustum = null != clipFrustum; if( useClipFrustum || clipOnBounds ) { final Frustum origClipFrustum = renderer.getClipFrustum(); @@ -411,46 +423,53 @@ public class Group extends Shape implements Container { final Frustum frustumMv = useClipFrustum ? clipFrustum : tempC00.set( box ).transform( pmv.getMv() ).updateFrustumPlanes(tempF00); renderer.setClipFrustum( frustumMv ); - for(int i=0; i renderedShapesB0 = new ArrayList(); private final List renderedShapesB1 = new ArrayList(); + private final List renderedShapesB2 = new ArrayList(); private volatile List renderedShapes = renderedShapesB1; + private int renderedShapesIdx = 1; private final AtomicReference toolTipActive = new AtomicReference(); private final AtomicReference toolTipHUD = new AtomicReference(); private final List topLevel = new ArrayList(); @@ -490,37 +492,47 @@ public final class Scene implements Container, GLEventListener { Arrays.sort(shapeArray, 0, shapeCount, Shape.ZAscendingComparator); // TreeTool.cullShapes(shapeArray, shapeCount); - final List iShapes = renderedShapes == renderedShapesB0 ? renderedShapesB1 : renderedShapesB0; - iShapes.clear(); - final GL2ES2 gl = drawable.getGL().getGL2ES2(); + final PMVMatrix4f pmv = renderer.getMatrix(); + final List iShapes; + final int iShapeIdx; + switch(renderedShapesIdx) { + case 0: iShapeIdx = 1; iShapes = renderedShapesB1; break; + case 1: iShapeIdx = 2; iShapes = renderedShapesB2; break; + default: iShapeIdx = 0; iShapes = renderedShapesB0; break; + } if( null != clearColor ) { gl.glClearColor(clearColor[0], clearColor[1], clearColor[2], clearColor[3]); gl.glClear(clearMask); } - final PMVMatrix4f pmv = renderer.getMatrix(); - renderer.enable(gl, true); - for(int i=0; i shapes = cont.getRenderedShapes(); Shape picked = null; - - for(int i=shapes.size()-1; null == picked && i>=0; --i) { - final Shape s = shapes.get(i); - pmv.pushMv(); - s.applyMatToMv(pmv); - picked = v.visit(s, pmv); - if( s instanceof Container ) { - final Shape childPick = pickForAllRenderedDesc((Container)s, pmv, v); - if( null != childPick ) { - picked = childPick; // child picked overrides group parent! + final List shapes = cont.getRenderedShapes(); + synchronized( shapes ) { // tripple-buffering is just almost enough + for(int i=shapes.size()-1; null == picked && i>=0; --i) { + final Shape s = shapes.get(i); + pmv.pushMv(); + s.applyMatToMv(pmv); + picked = v.visit(s, pmv); + if( s instanceof Container ) { + final Shape childPick = pickForAllRenderedDesc((Container)s, pmv, v); + if( null != childPick ) { + picked = childPick; // child picked overrides group parent! + } } + pmv.popMv(); } - pmv.popMv(); } return picked; } diff --git a/src/graphui/classes/jogamp/graph/ui/TreeTool.java b/src/graphui/classes/jogamp/graph/ui/TreeTool.java index 1309775a0..d225ead1f 100644 --- a/src/graphui/classes/jogamp/graph/ui/TreeTool.java +++ b/src/graphui/classes/jogamp/graph/ui/TreeTool.java @@ -174,36 +174,38 @@ public class TreeTool { } } private static boolean forAllRenderedAscn(final Container cont, final PMVMatrix4f pmv, final Visitor2 v) { - final List shapes = cont.getRenderedShapes(); boolean res = false; - - for(int i=0; !res && i shapes = cont.getRenderedShapes(); + synchronized( shapes ) { // tripple-buffering is just almost enough + for(int i=0; !res && i shapes = cont.getRenderedShapes(); boolean res = false; - - for(int i=shapes.size()-1; !res && i>=0; --i) { - final Shape s = shapes.get(i); - pmv.pushMv(); - s.applyMatToMv(pmv); - res = v.visit(s, pmv); - if( !res && s instanceof Container ) { - final Container c = (Container)s; - res = forAllRenderedDesc(c, pmv, v); + final List shapes = cont.getRenderedShapes(); + synchronized( shapes ) { // tripple-buffering is just almost enough + for(int i=shapes.size()-1; !res && i>=0; --i) { + final Shape s = shapes.get(i); + pmv.pushMv(); + s.applyMatToMv(pmv); + res = v.visit(s, pmv); + if( !res && s instanceof Container ) { + final Container c = (Container)s; + res = forAllRenderedDesc(c, pmv, v); + } + pmv.popMv(); } - pmv.popMv(); } return res; } -- cgit v1.2.3