Skip to content

Commit

Permalink
Optimize combining light in ZoneView.getVisibleArea().
Browse files Browse the repository at this point in the history
- Remove the use of a granular lock when populating `allLightPathMap`.
- Avoid converting `Path2D` back to `Area` until after all lights have been combined.
- Create `allLightAreaMap` in the swing worker and return it from there. This avoids any future unintended concurrent
  interactions with `allLightAreaMap`, and makes it clear that it is only used in `ZoneView.getVisibleArea()`.
  • Loading branch information
kwvanderlinde committed Jan 19, 2022
1 parent dfb1346 commit c33deaa
Showing 1 changed file with 39 additions and 28 deletions.
67 changes: 39 additions & 28 deletions src/main/java/net/rptools/maptool/client/ui/zone/ZoneView.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ public class ZoneView implements ModelChangeListener {
private final Map<GUID, Map<String, Set<Area>>> brightLightCache = new Hashtable<>();
/** Map the PlayerView to its visible area. */
private final Map<PlayerView, VisibleAreaMeta> visibleAreaMap = new HashMap<>();
/** Hold all of our lights combined by lumens. Used for hard FoW reveal. */
private final SortedMap<Double, Area> allLightAreaMap = new ConcurrentSkipListMap<>();
/** Map each token to their personal bright light source area. */
private final Map<GUID, Set<Area>> personalBrightLightCache = new HashMap<>();
/** Map each token to their personal drawable lights. */
Expand Down Expand Up @@ -428,13 +426,14 @@ public Area getVisibleArea(Token token) {
CombineLightsSwingWorker workerThread =
new CombineLightsSwingWorker(token.getSightType(), lightSourceTokens);
workerThread.execute();
SortedMap<Double, Area> allLightAreaMap;
try {
workerThread
.get(); // Jamz: We need to wait for this thread (which spawns more threads) to finish
// before we go on
// Jamz: We need to wait for this thread (which spawns more threads) to finish before we go
// on
allLightAreaMap = workerThread.get();
} catch (InterruptedException | ExecutionException e) {
// TODO Auto-generated catch block
e.printStackTrace();
allLightAreaMap = Collections.emptySortedMap();
}

// log.info("CombineLightsSwingWorker: \t" + stopwatch);
Expand Down Expand Up @@ -477,19 +476,19 @@ public Area getVisibleArea(Token token) {
}
}
}
allLightAreaMap.clear(); // Dispose of object, only needed for the scope of this method

tokenVisibleArea = allLightArea;
}

allLightAreaMap.clear(); // Dispose of object, only needed for the scope of this method
tokenVisionCache.put(token.getId(), tokenVisibleArea);

// log.info("getVisibleArea: \t\t" + stopwatch);

return tokenVisibleArea;
}

private class CombineLightsSwingWorker extends SwingWorker<Void, List<Token>> {
private class CombineLightsSwingWorker extends SwingWorker<SortedMap<Double, Area>, List<Token>> {
private final String sightName;
private final List<Token> lightSourceTokens;
private final ExecutorService lightsThreadPool;
Expand All @@ -502,16 +501,26 @@ private CombineLightsSwingWorker(String sightName, List<Token> lightSourceTokens
}

@Override
protected Void doInBackground() throws Exception {
protected SortedMap<Double, Area> doInBackground() throws Exception {
/* Hold all of our lights combined by lumens. Used for hard FoW reveal. */
var allLightPathMap = new ConcurrentSkipListMap<Double, Path2D>();

for (Token lightSourceToken : lightSourceTokens) {
CombineLightsTask task = new CombineLightsTask(sightName, lightSourceToken);
CombineLightsTask task =
new CombineLightsTask(allLightPathMap, sightName, lightSourceToken);
lightsThreadPool.submit(task);
}

lightsThreadPool.shutdown();
lightsThreadPool.awaitTermination(3, TimeUnit.MINUTES);

return null;
// Convert the luminous paths to areas.
var allLightAreaMap = new TreeMap<Double, Area>();
for (var entry : allLightPathMap.entrySet()) {
allLightAreaMap.put(entry.getKey(), new Area(entry.getValue()));
}

return allLightAreaMap;
}

@Override
Expand All @@ -530,10 +539,13 @@ public void done() {
* task
*/
private final class CombineLightsTask implements Callable<Map<Double, Area>> {
private final Map<Double, Path2D> allLightPathMap;
private final String sightName;
private final Token lightSourceToken;

private CombineLightsTask(String sightName, Token lightSourceToken) {
private CombineLightsTask(
Map<Double, Path2D> allLightPathMap, String sightName, Token lightSourceToken) {
this.allLightPathMap = allLightPathMap;
this.sightName = sightName;
this.lightSourceToken = lightSourceToken;
}
Expand All @@ -543,22 +555,21 @@ public Map<Double, Area> call() {
Map<Double, Area> lightArea = getLightSourceArea(sightName, lightSourceToken);

for (Entry<Double, Area> light : lightArea.entrySet()) {
// Area tempArea = light.getValue();
Path2D path = new Path2D.Double();
path.append(light.getValue().getPathIterator(null, 1), false);

synchronized (allLightAreaMap) {
if (allLightAreaMap.containsKey(light.getKey())) {
// Area allLight = allLightAreaMap.get(light.getKey());
// tempArea.add(allLight);

// Path2D is faster than Area it looks like
path.append(allLightAreaMap.get(light.getKey()).getPathIterator(null, 1), false);
}

// allLightAreaMap.put(light.getKey(), tempArea);
allLightAreaMap.put(light.getKey(), new Area(path));
}
// Add the token's light area to the global area in `allLightPathMap`.
// Because the remapping function may be called multiple times in case a retry is required,
// we make sure to use a clean `Path2D` object for each invocation. We also need to make
// sure we don't modify `existingValue` as such a change may result in an inconsistent
// state.
allLightPathMap.compute(
light.getKey(),
(lumens, totalPath) -> {
Path2D path = new Path2D.Double();
path.append(light.getValue().getPathIterator(null, 1), false);
if (totalPath != null) {
path.append(totalPath.getPathIterator(null, 1), false);
}
return path;
});
}

return lightArea;
Expand Down

0 comments on commit c33deaa

Please sign in to comment.