From ddd7a899ac46d566fe5a0366d4fa613e0ecafc66 Mon Sep 17 00:00:00 2001 From: Tom Mulcahy Date: Mon, 23 Mar 2015 17:49:47 +0000 Subject: [PATCH] Make Java measure thread-safe. --- src/JavaTranspiler.js | 1 + src/Layout.c | 1 + src/Layout.js | 5 ++- .../facebook/csslayout/CSSLayoutContext.java | 20 ++++++++++ .../src/com/facebook/csslayout/CSSNode.java | 18 ++++----- .../com/facebook/csslayout/LayoutEngine.java | 19 ++++++--- .../facebook/csslayout/LayoutCachingTest.java | 40 +++++++++++-------- .../facebook/csslayout/LayoutEngineTest.java | 3 +- src/transpile.js | 1 + 9 files changed, 72 insertions(+), 36 deletions(-) create mode 100644 src/java/src/com/facebook/csslayout/CSSLayoutContext.java diff --git a/src/JavaTranspiler.js b/src/JavaTranspiler.js index a4dddc77..8239bc1c 100644 --- a/src/JavaTranspiler.js +++ b/src/JavaTranspiler.js @@ -26,6 +26,7 @@ function __transpileToJavaCommon(code) { .replace(/pos\[([^\]]+)\]/g, 'getPos($1)') .replace(/dim\[([^\]]+)\]/g, 'getDim($1)') .replace(/isUndefined/g, 'CSSConstants.isUndefined') + .replace(/\/\*\(java\)!([^*]+)\*\//g, '$1') // Since Java doesn't store its attributes in arrays, we need to use setters/getters to access // the appropriate layout/style fields diff --git a/src/Layout.c b/src/Layout.c index 832c2d34..21980c5a 100644 --- a/src/Layout.c +++ b/src/Layout.c @@ -378,6 +378,7 @@ static void layoutNodeImpl(css_node_t *node, float parentMaxWidth) { if (isRowUndefined || isColumnUndefined) { css_dim_t measureDim = node->measure( node->context, + width ); if (isRowUndefined) { diff --git a/src/Layout.js b/src/Layout.js index d3e0bd40..d60e9e9c 100755 --- a/src/Layout.js +++ b/src/Layout.js @@ -278,6 +278,7 @@ var computeLayout = (function() { if (isRowUndefined || isColumnUndefined) { var/*css_dim_t*/ measureDim = node.style.measure( /*(c)!node->context,*/ + /*(java)!layoutContext.measureOutput,*/ width ); if (isRowUndefined) { @@ -397,7 +398,7 @@ var computeLayout = (function() { // This is the main recursive call. We layout non flexible children. if (alreadyComputedNextLayout === 0) { - layoutNode(child, maxWidth); + layoutNode(/*(java)!layoutContext, */child, maxWidth); } // Absolute positioned elements do not take part of the layout, so we @@ -472,7 +473,7 @@ var computeLayout = (function() { } // And we recursively call the layout algorithm for this child - layoutNode(child, maxWidth); + layoutNode(/*(java)!layoutContext, */child, maxWidth); } } diff --git a/src/java/src/com/facebook/csslayout/CSSLayoutContext.java b/src/java/src/com/facebook/csslayout/CSSLayoutContext.java new file mode 100644 index 00000000..8c93e0eb --- /dev/null +++ b/src/java/src/com/facebook/csslayout/CSSLayoutContext.java @@ -0,0 +1,20 @@ +/** + * Copyright (c) 2014, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ +package com.facebook.csslayout; + +/** + * A context for holding values local to a given instance of layout computation. + * + * This is necessary for making layout thread-safe. A separate instance should + * be used when {@link CSSNode#calculateLayout} is called concurrently on + * different node hierarchies. + */ +public class CSSLayoutContext { + /*package*/ final MeasureOutput measureOutput = new MeasureOutput(); +} diff --git a/src/java/src/com/facebook/csslayout/CSSNode.java b/src/java/src/com/facebook/csslayout/CSSNode.java index 2071cb3a..df8c3c63 100644 --- a/src/java/src/com/facebook/csslayout/CSSNode.java +++ b/src/java/src/com/facebook/csslayout/CSSNode.java @@ -39,10 +39,6 @@ public class CSSNode { UP_TO_DATE, } - // Only one copy kept around to keep from allocating a bunch of MeasureOutput objects - // NOT THREAD SAFE! NOT RE-ENTRANT SAFE! - private static final MeasureOutput MEASURE_OUTPUT = new MeasureOutput(); - public static interface MeasureFunction { /** @@ -114,22 +110,22 @@ public class CSSNode { return mMeasureFunction != null; } - /*package*/ MeasureOutput measure(float width) { + /*package*/ MeasureOutput measure(MeasureOutput measureOutput, float width) { if (!isMeasureDefined()) { throw new RuntimeException("Measure function isn't defined!"); } - MEASURE_OUTPUT.height = CSSConstants.UNDEFINED; - MEASURE_OUTPUT.width = CSSConstants.UNDEFINED; - Assertions.assertNotNull(mMeasureFunction).measure(this, width, MEASURE_OUTPUT); - return MEASURE_OUTPUT; + measureOutput.height = CSSConstants.UNDEFINED; + measureOutput.width = CSSConstants.UNDEFINED; + Assertions.assertNotNull(mMeasureFunction).measure(this, width, measureOutput); + return measureOutput; } /** * Performs the actual layout and saves the results in {@link #layout} */ - public void calculateLayout() { + public void calculateLayout(CSSLayoutContext layoutContext) { layout.resetResult(); - LayoutEngine.layoutNode(this, CSSConstants.UNDEFINED); + LayoutEngine.layoutNode(layoutContext, this, CSSConstants.UNDEFINED); } /** diff --git a/src/java/src/com/facebook/csslayout/LayoutEngine.java b/src/java/src/com/facebook/csslayout/LayoutEngine.java index 79e520f6..5b72b39c 100644 --- a/src/java/src/com/facebook/csslayout/LayoutEngine.java +++ b/src/java/src/com/facebook/csslayout/LayoutEngine.java @@ -261,13 +261,16 @@ public class LayoutEngine { !FloatUtil.floatsEqual(node.lastLayout.parentMaxWidth, parentMaxWidth); } - /*package*/ static void layoutNode(CSSNode node, float parentMaxWidth) { + /*package*/ static void layoutNode( + CSSLayoutContext layoutContext, + CSSNode node, + float parentMaxWidth) { if (needsRelayout(node, parentMaxWidth)) { node.lastLayout.requestedWidth = node.layout.width; node.lastLayout.requestedHeight = node.layout.height; node.lastLayout.parentMaxWidth = parentMaxWidth; - layoutNodeImpl(node, parentMaxWidth); + layoutNodeImpl(layoutContext, node, parentMaxWidth); node.lastLayout.copy(node.layout); } else { node.layout.copy(node.lastLayout); @@ -276,7 +279,10 @@ public class LayoutEngine { node.markHasNewLayout(); } - private static void layoutNodeImpl(CSSNode node, float parentMaxWidth) { + private static void layoutNodeImpl( + CSSLayoutContext layoutContext, + CSSNode node, + float parentMaxWidth) { for (int i = 0; i < node.getChildCount(); i++) { node.getChildAt(i).layout.resetResult(); @@ -324,7 +330,8 @@ public class LayoutEngine { // Let's not measure the text if we already know both dimensions if (isRowUndefined || isColumnUndefined) { MeasureOutput measureDim = node.measure( - width + layoutContext.measureOutput, + width ); if (isRowUndefined) { node.layout.width = measureDim.width + @@ -443,7 +450,7 @@ public class LayoutEngine { // This is the main recursive call. We layout non flexible children. if (alreadyComputedNextLayout == 0) { - layoutNode(child, maxWidth); + layoutNode(layoutContext, child, maxWidth); } // Absolute positioned elements do not take part of the layout, so we @@ -518,7 +525,7 @@ public class LayoutEngine { } // And we recursively call the layout algorithm for this child - layoutNode(child, maxWidth); + layoutNode(layoutContext, child, maxWidth); } } diff --git a/src/java/tests/com/facebook/csslayout/LayoutCachingTest.java b/src/java/tests/com/facebook/csslayout/LayoutCachingTest.java index a88a05d2..965d70a6 100644 --- a/src/java/tests/com/facebook/csslayout/LayoutCachingTest.java +++ b/src/java/tests/com/facebook/csslayout/LayoutCachingTest.java @@ -35,6 +35,7 @@ public class LayoutCachingTest { @Test public void testCachesFullTree() { + CSSLayoutContext layoutContext = new CSSLayoutContext(); CSSNode root = new CSSNode(); CSSNode c0 = new CSSNode(); CSSNode c1 = new CSSNode(); @@ -43,11 +44,11 @@ public class LayoutCachingTest { root.addChildAt(c1, 1); c0.addChildAt(c0c0, 0); - root.calculateLayout(); + root.calculateLayout(layoutContext); assertTreeHasNewLayout(true, root); markLayoutAppliedForTree(root); - root.calculateLayout(); + root.calculateLayout(layoutContext); assertTrue(root.hasNewLayout()); assertTreeHasNewLayout(false, c0); assertTreeHasNewLayout(false, c1); @@ -55,6 +56,7 @@ public class LayoutCachingTest { @Test public void testInvalidatesCacheWhenChildAdded() { + CSSLayoutContext layoutContext = new CSSLayoutContext(); CSSNode root = new CSSNode(); CSSNode c0 = new CSSNode(); CSSNode c1 = new CSSNode(); @@ -68,12 +70,12 @@ public class LayoutCachingTest { c0.addChildAt(c0c0, 0); c0c0.addChildAt(c1c0, 0); - root.calculateLayout(); + root.calculateLayout(layoutContext); markLayoutAppliedForTree(root); c0.addChildAt(c0c1, 1); - root.calculateLayout(); + root.calculateLayout(layoutContext); assertTrue(root.hasNewLayout()); assertTrue(c0.hasNewLayout()); assertTrue(c0c1.hasNewLayout()); @@ -86,6 +88,7 @@ public class LayoutCachingTest { @Test public void testInvalidatesCacheWhenEnumPropertyChanges() { + CSSLayoutContext layoutContext = new CSSLayoutContext(); CSSNode root = new CSSNode(); CSSNode c0 = new CSSNode(); CSSNode c1 = new CSSNode(); @@ -94,11 +97,11 @@ public class LayoutCachingTest { root.addChildAt(c1, 1); c0.addChildAt(c0c0, 0); - root.calculateLayout(); + root.calculateLayout(layoutContext); markLayoutAppliedForTree(root); c1.setAlignSelf(CSSAlign.CENTER); - root.calculateLayout(); + root.calculateLayout(layoutContext); assertTrue(root.hasNewLayout()); assertTrue(c1.hasNewLayout()); @@ -109,6 +112,7 @@ public class LayoutCachingTest { @Test public void testInvalidatesCacheWhenFloatPropertyChanges() { + CSSLayoutContext layoutContext = new CSSLayoutContext(); CSSNode root = new CSSNode(); CSSNode c0 = new CSSNode(); CSSNode c1 = new CSSNode(); @@ -117,11 +121,11 @@ public class LayoutCachingTest { root.addChildAt(c1, 1); c0.addChildAt(c0c0, 0); - root.calculateLayout(); + root.calculateLayout(layoutContext); markLayoutAppliedForTree(root); c1.setMargin(Spacing.LEFT, 10); - root.calculateLayout(); + root.calculateLayout(layoutContext); assertTrue(root.hasNewLayout()); assertTrue(c1.hasNewLayout()); @@ -132,6 +136,7 @@ public class LayoutCachingTest { @Test public void testInvalidatesFullTreeWhenParentWidthChanges() { + CSSLayoutContext layoutContext = new CSSLayoutContext(); CSSNode root = new CSSNode(); CSSNode c0 = new CSSNode(); CSSNode c1 = new CSSNode(); @@ -142,11 +147,11 @@ public class LayoutCachingTest { c0.addChildAt(c0c0, 0); c1.addChildAt(c1c0, 0); - root.calculateLayout(); + root.calculateLayout(layoutContext); markLayoutAppliedForTree(root); c0.setStyleWidth(200); - root.calculateLayout(); + root.calculateLayout(layoutContext); assertTrue(root.hasNewLayout()); assertTrue(c0.hasNewLayout()); @@ -158,6 +163,7 @@ public class LayoutCachingTest { @Test public void testDoesNotInvalidateCacheWhenPropertyIsTheSame() { + CSSLayoutContext layoutContext = new CSSLayoutContext(); CSSNode root = new CSSNode(); CSSNode c0 = new CSSNode(); CSSNode c1 = new CSSNode(); @@ -167,11 +173,11 @@ public class LayoutCachingTest { c0.addChildAt(c0c0, 0); root.setStyleWidth(200); - root.calculateLayout(); + root.calculateLayout(layoutContext); markLayoutAppliedForTree(root); root.setStyleWidth(200); - root.calculateLayout(); + root.calculateLayout(layoutContext); assertTrue(root.hasNewLayout()); assertTreeHasNewLayout(false, c0); @@ -180,6 +186,7 @@ public class LayoutCachingTest { @Test public void testInvalidateCacheWhenHeightChangesPosition() { + CSSLayoutContext layoutContext = new CSSLayoutContext(); CSSNode root = new CSSNode(); CSSNode c0 = new CSSNode(); CSSNode c1 = new CSSNode(); @@ -188,11 +195,11 @@ public class LayoutCachingTest { root.addChildAt(c1, 1); c1.addChildAt(c1c0, 0); - root.calculateLayout(); + root.calculateLayout(layoutContext); markLayoutAppliedForTree(root); c0.setStyleHeight(100); - root.calculateLayout(); + root.calculateLayout(layoutContext); assertTrue(root.hasNewLayout()); assertTrue(c0.hasNewLayout()); @@ -202,6 +209,7 @@ public class LayoutCachingTest { @Test public void testInvalidatesOnNewMeasureFunction() { + CSSLayoutContext layoutContext = new CSSLayoutContext(); CSSNode root = new CSSNode(); CSSNode c0 = new CSSNode(); CSSNode c1 = new CSSNode(); @@ -210,7 +218,7 @@ public class LayoutCachingTest { root.addChildAt(c1, 1); c0.addChildAt(c0c0, 0); - root.calculateLayout(); + root.calculateLayout(layoutContext); markLayoutAppliedForTree(root); c1.setMeasureFunction(new CSSNode.MeasureFunction() { @@ -221,7 +229,7 @@ public class LayoutCachingTest { } }); - root.calculateLayout(); + root.calculateLayout(layoutContext); assertTrue(root.hasNewLayout()); assertTrue(c1.hasNewLayout()); diff --git a/src/java/tests/com/facebook/csslayout/LayoutEngineTest.java b/src/java/tests/com/facebook/csslayout/LayoutEngineTest.java index b32dc12d..71b109af 100644 --- a/src/java/tests/com/facebook/csslayout/LayoutEngineTest.java +++ b/src/java/tests/com/facebook/csslayout/LayoutEngineTest.java @@ -50,7 +50,8 @@ public class LayoutEngineTest { } private static void test(String message, CSSNode style, CSSNode expectedLayout) { - style.calculateLayout(); + CSSLayoutContext layoutContext = new CSSLayoutContext(); + style.calculateLayout(layoutContext); assertLayoutsEqual(message, style, expectedLayout); } diff --git a/src/transpile.js b/src/transpile.js index 7fab697f..d649df3c 100644 --- a/src/transpile.js +++ b/src/transpile.js @@ -237,6 +237,7 @@ function transpileAnnotatedJStoC(jsCode) { .replace(/\n {2}/g, '\n') .replace(/\/\*\(c\)!([^*]+)\*\//g, '$1') .replace(/\/[*]!([^*]+)[*]\//g, '$1') + .replace(/\/\*\(java\)!([^*]+)\*\//g, '') .split('\n').slice(1, -1).join('\n'); }