Baseline support #317

Closed
woehrl01 wants to merge 33 commits from baseline-support into master
16 changed files with 1612 additions and 6 deletions
Showing only changes of commit 45f0011065 - Show all commits

View File

@@ -7,57 +7,57 @@
* of patent rights can be found in the PATENTS file in the same directory.
*/
#include <yoga/Yoga.h>
#include <gtest/gtest.h>
#include <yoga/Yoga.h>
static float _baseline(YGNodeRef node) {
float *baseline = (float*) YGNodeGetContext(node);
float *baseline = (float *) YGNodeGetContext(node);
return *baseline;
}
TEST(YogaTest, align_baseline_customer_func) {
const YGNodeRef root = YGNodeNew();
YGNodeStyleSetFlexDirection(root, YGFlexDirectionRow);
YGNodeStyleSetAlignItems(root, YGAlignBaseline);
YGNodeStyleSetWidth(root, 100);
YGNodeStyleSetHeight(root, 100);
const YGNodeRef root = YGNodeNew();
YGNodeStyleSetFlexDirection(root, YGFlexDirectionRow);
YGNodeStyleSetAlignItems(root, YGAlignBaseline);
YGNodeStyleSetWidth(root, 100);
YGNodeStyleSetHeight(root, 100);
const YGNodeRef root_child0 = YGNodeNew();
YGNodeStyleSetWidth(root_child0, 50);
YGNodeStyleSetHeight(root_child0, 50);
YGNodeInsertChild(root, root_child0, 0);
const YGNodeRef root_child0 = YGNodeNew();
YGNodeStyleSetWidth(root_child0, 50);
YGNodeStyleSetHeight(root_child0, 50);
YGNodeInsertChild(root, root_child0, 0);
const YGNodeRef root_child1 = YGNodeNew();
YGNodeStyleSetWidth(root_child1, 50);
YGNodeStyleSetHeight(root_child1, 20);
YGNodeInsertChild(root, root_child1, 1);
const YGNodeRef root_child1 = YGNodeNew();
YGNodeStyleSetWidth(root_child1, 50);
YGNodeStyleSetHeight(root_child1, 20);
YGNodeInsertChild(root, root_child1, 1);
float baselineValue = 10;
const YGNodeRef root_child1_child0 = YGNodeNew();
YGNodeSetContext(root_child1_child0, &baselineValue);
YGNodeStyleSetWidth(root_child1_child0, 50);
YGNodeSetBaselineFunc(root_child1_child0, _baseline);
YGNodeStyleSetHeight(root_child1_child0, 20);
YGNodeInsertChild(root_child1, root_child1_child0, 0);
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
float baselineValue = 10;
const YGNodeRef root_child1_child0 = YGNodeNew();
YGNodeSetContext(root_child1_child0, &baselineValue);
YGNodeStyleSetWidth(root_child1_child0, 50);
YGNodeSetBaselineFunc(root_child1_child0, _baseline);
YGNodeStyleSetHeight(root_child1_child0, 20);
YGNodeInsertChild(root_child1, root_child1_child0, 0);
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetHeight(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetHeight(root_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetLeft(root_child1));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetTop(root_child1));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetWidth(root_child1));
ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child1));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetLeft(root_child1));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetTop(root_child1));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetWidth(root_child1));
ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child1));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child1_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child1_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetWidth(root_child1_child0));
ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child1_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child1_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child1_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetWidth(root_child1_child0));
ASSERT_FLOAT_EQ(20, YGNodeLayoutGetHeight(root_child1_child0));
}

View File

@@ -335,7 +335,6 @@ YGMeasureFunc YGNodeGetMeasureFunc(const YGNodeRef node) {
return node->measure;
emilsjolander commented 2017-01-05 02:25:21 -08:00 (Migrated from github.com)
Review

node->baseline = baselineFunc; is enough

`node->baseline = baselineFunc;` is enough
}
void YGNodeSetBaselineFunc(const YGNodeRef node, YGBaselineFunc baselineFunc) {
node->baseline = baselineFunc;
}
@@ -951,61 +950,55 @@ static inline YGDirection YGNodeResolveDirection(const YGNodeRef node,
}
emilsjolander commented 2017-01-05 05:43:58 -08:00 (Migrated from github.com)
Review

YGNodeBaseline

`YGNodeBaseline`
}
static float YGBaselineOfFirstLine(const YGNodeRef node, const YGFlexDirection mainAxis, const float parentWidth)
{
if(node->baseline != NULL)
{
static float YGBaselineOfFirstLine(const YGNodeRef node,
const YGFlexDirection mainAxis,
const float parentWidth) {
emilsjolander commented 2017-01-05 11:06:20 -08:00 (Migrated from github.com)
Review

This will always be height as baseline is undefined for column flex direction. right? in that case I think it is better to be clear and use YGDimensionHeight instead of dim[crossAxis]

This will always be height as baseline is undefined for column flex direction. right? in that case I think it is better to be clear and use `YGDimensionHeight` instead of `dim[crossAxis]`
emilsjolander commented 2017-01-05 11:07:40 -08:00 (Migrated from github.com)
Review

Is there a good reason to allow undefined return value for baseline? I would prefer asserting and crashing as it would probably indicate a bug. What do you think?

Is there a good reason to allow undefined return value for baseline? I would prefer asserting and crashing as it would probably indicate a bug. What do you think?
emilsjolander commented 2017-01-05 11:11:39 -08:00 (Migrated from github.com)
Review

Why child->lineIndex > 0? I understand that it skips multiline children but i'm not sure why.

Why `child->lineIndex > 0`? I understand that it skips multiline children but i'm not sure why.
emilsjolander commented 2017-01-05 11:12:21 -08:00 (Migrated from github.com)
Review

same as above regarding dim[crossAxis]

same as above regarding `dim[crossAxis]`
emilsjolander commented 2017-01-05 11:14:02 -08:00 (Migrated from github.com)
Review

The baseline of a container is defined by its first baseline aligned child, but when there is no baseline aligned child then the container's baseline is defined by it's last child. This seems odd. Are you sure we have test cases covering edge cases here to make sure this is how it works on the web?

The baseline of a container is defined by its first baseline aligned child, but when there is no baseline aligned child then the container's baseline is defined by it's last child. This seems odd. Are you sure we have test cases covering edge cases here to make sure this is how it works on the web?
emilsjolander commented 2017-01-05 11:14:19 -08:00 (Migrated from github.com)
Review

same as above regarding pos[crossAxis]

same as above regarding `pos[crossAxis]`
woehrl01 commented 2017-01-05 11:25:14 -08:00 (Migrated from github.com)
Review

I'm not sure my self about this. I thought this could be a kind of "feature", so if you return undefined, you simply use the nodes height. But crashing would be fine too for me. What I'm not sure about is if we explicitly add the padding-top here or if the implementation of the custom function needs to consider this.

I'm not sure my self about this. I thought this could be a kind of "feature", so if you return undefined, you simply use the nodes height. But crashing would be fine too for me. What I'm not sure about is if we explicitly add the padding-top here or if the implementation of the custom function needs to consider this.
woehrl01 commented 2017-01-05 11:26:59 -08:00 (Migrated from github.com)
Review

we use only the first line of the children for base layout alignment. At least this is how chrome handles it.

we use only the first line of the children for base layout alignment. At least this is how chrome handles it.
woehrl01 commented 2017-01-05 11:27:56 -08:00 (Migrated from github.com)
Review

no it's defined by its first child on the first line or the first baseline aligned child if there is one (one the first line). I'll add a test for this.

no it's defined by its first child on the first line or the first baseline aligned child if there is one (one the first line). I'll add a test for this.
emilsjolander commented 2017-01-05 11:40:10 -08:00 (Migrated from github.com)
Review

The custom function should not take padding into account. We don't expect this for the measure function so I would like to preserve that here if possible.

Let's crash for now. If we find a valid reason to have this feature we can implement it later.

The custom function should not take padding into account. We don't expect this for the measure function so I would like to preserve that here if possible. Let's crash for now. If we find a valid reason to have this feature we can implement it later.
emilsjolander commented 2017-01-05 11:41:06 -08:00 (Migrated from github.com)
Review

yes, but this looks like we are skipping over children with multiple lines instead of just looking at their first line?

yes, but this looks like we are skipping over children with multiple lines instead of just looking at their first line?
emilsjolander commented 2017-01-05 11:43:16 -08:00 (Migrated from github.com)
Review

So the current code is wrong i think. Right? As it will pick the last child in the case no child is baseline aligned?

So the current code is wrong i think. Right? As it will pick the last child in the case no child is baseline aligned?
woehrl01 commented 2017-01-05 11:46:36 -08:00 (Migrated from github.com)
Review

oh, yep. We should break instead of continue here!

oh, yep. We should break instead of continue here!
woehrl01 commented 2017-01-05 11:47:30 -08:00 (Migrated from github.com)
Review

no as we only use the child if baselineChild is still NULL. Which is false as seen as we find the first one. We still need to iterate to take any baseline aligned child into account.

no as we only use the child if ```baselineChild``` is still ```NULL```. Which is false as seen as we find the first one. We still need to iterate to take any baseline aligned child into account.
if (node->baseline != NULL) {
emilsjolander commented 2017-01-05 02:26:21 -08:00 (Migrated from github.com)
Review

uint32_t

uint32_t
emilsjolander commented 2017-01-05 02:26:32 -08:00 (Migrated from github.com)
Review

i++

i++
return node->baseline(node);
}
YGNodeRef baselineChild = NULL;
for(uint32_t i = 0; i < YGNodeGetChildCount(node); i++)
{
for (uint32_t i = 0; i < YGNodeGetChildCount(node); i++) {
const YGNodeRef child = YGNodeGetChild(node, i);
if(child->style.positionType == YGPositionTypeAbsolute || child->lineIndex > 0)
{
if (child->style.positionType == YGPositionTypeAbsolute || child->lineIndex > 0) {
continue;
}
if(YGNodeAlignItem(node, child) == YGAlignBaseline)
{
if (YGNodeAlignItem(node, child) == YGAlignBaseline) {
baselineChild = child;
break;
}
if(baselineChild == NULL)
{
if (baselineChild == NULL) {
baselineChild = child;
}
}
if(baselineChild == NULL)
{
if (baselineChild == NULL) {
return YGUndefined;
}
float baseline = YGBaselineOfFirstLine(baselineChild, node->style.flexDirection, node->layout.measuredDimensions[YGDimensionWidth]);
if(YGFloatIsUndefined(baseline))
{
baseline = YGNodeLeadingPaddingAndBorder(baselineChild, mainAxis, parentWidth)
+ baselineChild->layout.measuredDimensions[dim[mainAxis]];
float baseline = YGBaselineOfFirstLine(baselineChild,
node->style.flexDirection,
node->layout.measuredDimensions[YGDimensionWidth]);
if (YGFloatIsUndefined(baseline)) {
baseline = YGNodeLeadingPaddingAndBorder(baselineChild, mainAxis, parentWidth) +
baselineChild->layout.measuredDimensions[dim[mainAxis]];
}
return baseline + baselineChild->layout.position[YGEdgeTop];
}
static inline float YGBaselineForNode(const YGNodeRef node, const YGFlexDirection crossAxis, const float parentWidth)
{
static inline float YGBaselineForNode(const YGNodeRef node,
const YGFlexDirection crossAxis,
const float parentWidth) {
float baseline = YGBaselineOfFirstLine(node, crossAxis, parentWidth);
if(YGFloatIsUndefined(baseline))
{
if (YGFloatIsUndefined(baseline)) {
return node->layout.measuredDimensions[dim[crossAxis]];
}
return baseline;
}
static inline YGFlexDirection YGFlexDirectionResolve(const YGFlexDirection flexDirection,
const YGDirection direction) {
if (direction == YGDirectionRTL) {
@@ -1623,8 +1616,6 @@ static bool YGNodeFixedSizeSetMeasuredDimensions(const YGNodeRef node,
// * Margins cannot be specified as 'auto'. They must be specified in terms of
// pixel
// values, and the default value is 0.
// * The 'baseline' value is not supported for alignItems and alignSelf
// properties.
// * Values of width, maxWidth, minWidth, height, maxHeight and minHeight must
// be
// specified as pixel values, not as percentages.
@@ -2487,7 +2478,8 @@ static void YGNodelayoutImpl(const YGNodeRef node,
}
// STEP 8: MULTI-LINE CONTENT ALIGNMENT
if ((node->style.alignItems == YGAlignBaseline || lineCount > 1) && performLayout && !YGFloatIsUndefined(availableInnerCrossDim)) {
if ((node->style.alignItems == YGAlignBaseline || lineCount > 1) && performLayout &&
!YGFloatIsUndefined(availableInnerCrossDim)) {
const float remainingAlignContentDim = availableInnerCrossDim - totalLineCrossDim;
float crossDimLead = 0;
@@ -2519,7 +2511,7 @@ static void YGNodelayoutImpl(const YGNodeRef node,
// compute the line's height and find the endIndex
float lineHeight = 0;
float maxAscentForCurrentLine = 0;
float maxDescentForCurrentLine = 0;
float maxDescentForCurrentLine = 0;
for (ii = startIndex; ii < childCount; ii++) {
const YGNodeRef child = YGNodeListGet(node->children, ii);
@@ -2532,12 +2524,12 @@ static void YGNodelayoutImpl(const YGNodeRef node,
child->layout.measuredDimensions[dim[crossAxis]] +
YGNodeMarginForAxis(child, crossAxis, availableInnerWidth));
}
if (performLayout && YGNodeAlignItem(node, child) == YGAlignBaseline)
{
if (performLayout && YGNodeAlignItem(node, child) == YGAlignBaseline) {
const float ascent = YGBaselineForNode(child, crossAxis, availableInnerWidth) +
YGNodeLeadingMargin(child, crossAxis, availableInnerWidth);
YGNodeLeadingMargin(child, crossAxis, availableInnerWidth);
const float descent = child->layout.measuredDimensions[dim[crossAxis]] +
YGNodeMarginForAxis(child, crossAxis, availableInnerWidth) - ascent;
YGNodeMarginForAxis(child, crossAxis, availableInnerWidth) -
ascent;
maxAscentForCurrentLine = fmaxf(maxAscentForCurrentLine, ascent);
maxDescentForCurrentLine = fmaxf(maxDescentForCurrentLine, descent);
lineHeight = fmaxf(lineHeight, maxAscentForCurrentLine + maxDescentForCurrentLine);
@@ -2580,8 +2572,8 @@ static void YGNodelayoutImpl(const YGNodeRef node,
}
case YGAlignBaseline: {
child->layout.position[pos[crossAxis]] =
emilsjolander commented 2017-01-05 12:07:18 -08:00 (Migrated from github.com)
Review

Could pos[crossAxis] be hardcoded? Same with crossAxis a couple lines below

Could `pos[crossAxis]` be hardcoded? Same with `crossAxis` a couple lines below
woehrl01 commented 2017-01-05 12:19:27 -08:00 (Migrated from github.com)
Review

sure, I just wanted to be the same like the others, to not cause confusion. But I'll replace them, to be explicit that it is only used on YGEdgeTop

sure, I just wanted to be the same like the others, to not cause confusion. But I'll replace them, to be explicit that it is only used on ```YGEdgeTop```
currentLead + maxAscentForCurrentLine -
YGBaselineForNode(child, crossAxis, availableInnerWidth);
currentLead + maxAscentForCurrentLine -
YGBaselineForNode(child, crossAxis, availableInnerWidth);
break;
}
case YGAlignAuto:

View File

@@ -50,7 +50,7 @@ typedef YGSize (*YGMeasureFunc)(YGNodeRef node,
float height,
YGMeasureMode heightMode);
typedef float(*YGBaselineFunc)(YGNodeRef node);
typedef float (*YGBaselineFunc)(YGNodeRef node);
typedef void (*YGPrintFunc)(YGNodeRef node);
typedef int (*YGLogger)(YGLogLevel level, const char *format, va_list args);