[Solved] Updating spatial by replacing them in the update loop

TrashCaster · January 17, 2015, 1:12am

Edit as of solution:

The following code in this post is now outdated to what my problem is. For the solution, scroll to the bottom. I have several snippets there of my solution.

Thanks nehon and david_bernard_31 for the replies and solutions.

Alright, so basically my goal is to replace a geometry/spatial in my rootNode.

I’m doing a sort of chunk based system, where the chunk builds an optimized spatial node of all the geometry passed in by the blocks that region contains.

I have looked through the mutli-threading area as best as I could (I used Google’s cache of the wiki as it seems to be down).

I need it multi-threaded to build the model because I don’t want the game to freeze up when the user would be playing it.

This is what I’ve tried:

In my application:

@Override
public void simpleUpdate(float tpf) {
    for (Plot p : plots) {
        Spatial rebuiltSpatial = null;
        try {
            if (p.isDirty() && future == null) {
                future = executor.submit(p.getCallable());
            } else if (future != null) {
                if (future.isDone()) {
                    rebuiltSpatial = (Spatial)future.get();
                    future = null;
                } else if (future.isCancelled()) {
                    future = null;
                }
            }
        } catch (Exception e) {
            e.printStackTrace();
        }
        if (rebuiltSpatial != null) {
            if (p.getCurrentGeometry() != null) {
                rootNode.detachChild(p.getCurrentGeometry());
            }
            p.setCurrentGeoemtry(rebuiltSpatial);
            rootNode.attachChild(p.getCurrentGeometry());
        }
    }
}

In my Plot class:

private Callable<Spatial> rebuildGeometryCallable = new Callable<Spatial>() {
    public Spatial call() throws Exception {
        Spatial rebuilt = Main.getInstance().enqueue(new Callable<Spatial>() {
            public Spatial call() throws Exception {
                return rebuildPlotGeometry(null);
            }
        }).get();
        return rebuilt;
    }
};

private synchronized Spatial rebuildPlotGeometry(Material globalMaterial) {
    DecimalFormat df = new DecimalFormat("#.##");
    Node n = new Node(String.format("Plot_{0}_{1}", new Object[]{df.format((double) location.x), df.format((double) location.y)}));
    Spatial returnedSpatial = null;
    for (int y = 0; y < PLOT_YSIZE; y++) {
        for (int x = 0; x < PLOT_XSIZE; x++) {
            for (int z = 0; z < PLOT_ZSIZE; z++) {
                IBlock block = getBlockAt(x, y, z);
                if (block != null) {
                    EnumMap<Side, IBlock> adjacentBlocks = new EnumMap<Side, IBlock>(Side.class);
                    for (Side side : Side.values()) {
                        IBlock neighborBlock = getBlockOrNullAt(x + side.getOffsetX(), y + side.getOffsetY(), z + side.getOffsetZ());
                        adjacentBlocks.put(side, neighborBlock);
                    }
                    if (block.getBlockGeometry(adjacentBlocks) != null) {
                        Spatial g = block.getBlockGeometry(adjacentBlocks).clone(true);
                        Vector3f offset = new Vector3f(x, y, z).add(0.5f, 0f, 0.5f);
                        g.setLocalTranslation(offset);
                        g.updateGeometricState();
                        n.attachChild(g);
                    }
                }
            }
        }
    }
    returnedSpatial = GeometryBatchFactory.optimize(n);
    returnedSpatial.setLocalTranslation(this.location.x, 0f, this.location.y);
    this.isDirty = false;

    if (globalMaterial != null) {
        returnedSpatial.setMaterial(globalMaterial);
    }
    return returnedSpatial;
}

Basically, what I want to happen is that when I modify the plot’s blocks, it will rebuild the geometry/spatial and replace the already existing one that is attached to the rootNode.

I’m 100% sure that I’m either approaching this wrong, or I’m overthinking the process.

Any guidance in the right direction would be much helped.

I don’t expect anyone to spoon feed me code, as that doesn’t help me learn, but if someone could give me useful snippets with explanations or something, then I could adapt my current code to it.

TrashCaster · January 17, 2015, 1:14am

I should also note, I did had this working as intended besides the lag spikes until I made drastic code changes to accommodate the “callable” stuff.

nehon · January 17, 2015, 8:37am

What I usually do when I need multithreading :
Have a ScheduledThreadPoolExecutor (the size of it depends on your need, how many concurrent thread will you have) .

I use the execute method to submit a Runnable
In the run method of the Runnable I do the stuff I want parallelized, then at the end of the method I enqueue a Callable to the Application (you may need to pass along a reference to it).
the callable call() method contains all the operation that may modify the scene graph (even those that modify material params).
So basically in your case the run method should contain the rebuildPlotGeometry() body and the call method should contain this

        if (rebuiltSpatial != null) {
            if (p.getCurrentGeometry() != null) {
                rootNode.detachChild(p.getCurrentGeometry());
            }
            p.setCurrentGeoemtry(rebuiltSpatial);
            rootNode.attachChild(p.getCurrentGeometry());
        }

This should prevent the lag spikes.

EDIT : btw the wiki will hopefully be up again soon.

david_bernard_31 · January 17, 2015, 9:43am

I don’t understand you code rebuildGeometryCallable block (.get()) to return a Future of Callable :

why not simply ?

If Main.getInstance().enqueue is the enqueue of SimpleApplication or Application, then rebuildPlotGeometry will run in the jme Thread. If I understand your code right, then your Executor is useless :
jme thread (simpleUpdate) → Executor (only forward result + block) → jme Thread (rebuildPlotGeometry). you can call rebuildPlotGeometry from simpleUpdate directly for same result (without the blocking Executor).

IMHO, a better flow is with less Future (java Future aren’t chainable so there a near to useless). something like :

for (Plot p : plots) {
    try {
        if (p.isDirty() && p.needUpdate) {
            p.needUpdate = false;
            executor.submit(new Callable<Spatial>(){
              public Spatial call() throws Exception {
                final Spatial rebuiltSpatial = rebuildPlotGeometry(null);
                if (rebuiltSpatial != null) {
                  Application.this.enqueue(new Callable<Spatial>(){
                    public Spatial call() throws Exception {
                      if (p.getCurrentGeometry() != null) {
                        rootNode.detachChild(p.getCurrentGeometry());
                      }
                      p.setCurrentGeoemtry(rebuiltSpatial);
                      rootNode.attachChild(p.getCurrentGeometry());
                      return rebuiltSpatial;
                    }
                  });
               }
               return rebuiltSpatial;
            });
        }
    } catch (Exception e) {
        e.printStackTrace();
    }
}

PS: Sorry if the code doesn’t compile I wrote it directly in the forum, and I use java8 lambda a lot.
PS2: I introduce p.needUpdate as a guard to avoid call rebuild multiple time, may be part of the role of your previous “future”

TrashCaster · January 17, 2015, 2:34pm

Your solution worked perfectly, thank you.

Also, David the method rebuildGeometry(Material material) is now, and was previously (before I want on a overhaul spree getting confused and posting), a void type.

All the geometry is stored locally on the Plot object and retrieved and set globally with getters and setters.
Also, I have changed the “isDirty” boolean into a byte-flag called “buildFlag” with the possibility of “unbuilt”, “built”, “ready”.

I will attach my changed snippets for further critique, and for solution to those who had this same problem.

EDIT:

In my update method:

    for (final Plot p : plots) {
        if (p.getBuildFlag() == Plot.UNBUILT) {
            executor.submit(new Runnable() {
                public void run() {
                    p.rebuildPlotGeometry(null);
                }
            });
        }
        if (p.getBuildFlag() == Plot.BUILT) {
            enqueue(new Callable<Spatial>() {
                public Spatial call() throws Exception {
                    p.enqueueUpdate(rootNode);
                    return p.getCurrentGeometry();
                }
            });
        }
    }

In my Plot class:

// Flag indicating that the plot was just modified and needs a rebuild
public static final byte UNBUILT = 0x02;

// Flag indicating that the plot was just rebuilt, but needs to be submitted to the root node
public static final byte BUILT = 0x01;

// Flag indicating that the plot is submitted, and ready for further operation
public static final byte READY = 0x00;

private byte buildFlag = READY;

public void rebuildPlotGeometry(Material globalMaterial) {
    DecimalFormat df = new DecimalFormat("#.##");
    Node n = new Node(String.format("Plot_%s_%s", new Object[]{df.format((double) location.x), df.format((double) location.y)}));
    for (int y = 0; y < PLOT_YSIZE; y++) {
        for (int x = 0; x < PLOT_XSIZE; x++) {
            for (int z = 0; z < PLOT_ZSIZE; z++) {
                IBlock block = getBlockAt(x, y, z);
                if (block != null) {
                    EnumMap<Side, IBlock> adjacentBlocks = new EnumMap<Side, IBlock>(Side.class);
                    for (Side side : Side.values()) {
                        IBlock neighborBlock = getBlockOrNullAt(x + side.getOffsetX(), y + side.getOffsetY(), z + side.getOffsetZ());
                        adjacentBlocks.put(side, neighborBlock);
                    }
                    if (block.getBlockGeometry(adjacentBlocks) != null) {
                        Spatial g = block.getBlockGeometry(adjacentBlocks).clone(true);
                        Vector3f offset = new Vector3f(x, y, z).add(0.5f, 0f, 0.5f);
                        g.setLocalTranslation(offset);
                        g.updateGeometricState();
                        n.attachChild(g);
                    }
                }
            }
        }
    }
    this.rebuiltPlotGeometry = GeometryBatchFactory.optimize(n);
    this.rebuiltPlotGeometry.setLocalTranslation(this.location.x, 0f, this.location.y);
    this.buildFlag = Plot.BUILT;

    if (globalMaterial != null) {
        this.rebuiltPlotGeometry.setMaterial(globalMaterial);
    }
}

public void enqueueUpdate(Node rootNode) {
    if (this.rebuiltPlotGeometry != null && this.buildFlag == Plot.BUILT) {
        if (this.currentPlotGeometry != null) {
            rootNode.detachChild(this.currentPlotGeometry);
        }
        this.currentPlotGeometry = this.rebuiltPlotGeometry.clone(true);
        rootNode.attachChild(this.currentPlotGeometry);
        this.rebuiltPlotGeometry = null;
        this.buildFlag = Plot.READY;
    }
}

david_bernard_31 · January 17, 2015, 2:50pm

use enqueue in the update method is only to said “do this in the next frame”. Callable enqueued are executed just before simpleUpdate (see Application.update iirc)

TrashCaster:

    if (p.getBuildFlag() == Plot.BUILT) {
            enqueue(new Callable<Spatial>() {
                public Spatial call() throws Exception {
                    p.enqueueUpdate(rootNode);
                    return p.getCurrentGeometry();
                }
            });
        }

So if you don’t need to 1 frame delay you can simplify by

    if (p.getBuildFlag() == Plot.BUILT) {
        p.enqueueUpdate(rootNode);
    }

use enqueue when you’re not in the jme Thread (the thread where update, render are running), like in the Executor Thread eg :

    for (final Plot p : plots) {
        if (p.getBuildFlag() == Plot.UNBUILT) {
            executor.submit(new Runnable() {
                public void run() {
                    p.rebuildPlotGeometry(null);
                    enqueue(new Callable<Spatial>() {
                        public Spatial call() throws Exception {
                            p.enqueueUpdate(rootNode);
                            return p.getCurrentGeometry();
                       }
                    });
                }
            });
        }
    }

TrashCaster · January 17, 2015, 2:55pm

Thanks. I thought I wasn’t allowed to modify the scene graph without an enqueue, but that makes it so much simpler.

david_bernard_31 · January 17, 2015, 3:01pm

I forgot : Great you find the solution

Just for fun and info, if you used java8, my last code could rewrite to :

for (final Plot p : plots) {
    if (p.getBuildFlag() == Plot.UNBUILT) {
        executor.submit(()  -> {
            p.rebuildPlotGeometry(null);
            enqueue(() -> {
                p.enqueueUpdate(rootNode);
                return p.getCurrentGeometry();
            });
        });
    }
}