Skip to content

Commit

Permalink
Reland: Sanitize measure function results
Browse files Browse the repository at this point in the history
Summary:
D57285584 was reverted because we have service code with a faulty measure function, and adding logging to Yoga when invalid measurements were received was enough to spike error rate to elevated levels and block release.

This is a reland of the below change, with a couple modifications:
1. We log warnings instead of errors, which from what I heard, shouldn't block release, but should still make signal
2. We only zero the dimension which was NaN, to preserve exact behavior

## Original

We've started seeing assertion failures in Yoga where a `NaN` value makes its way to an `availableHeight` constraint when measuring Litho tree.

Because it's only happening on Litho, I have some suspicion this might be originating from a Litho-specific measure function. This adds sanitization in Yoga to measure function results, where we will log an error, and set size to zero, if either dimension ends up being negative of `NaN`.

This doesn't really help track down where the error was happening, but Yoga doesn't have great context to show this to begin with. If we see this is issue, next steps would be Litho internal intrumentation to find culprit.

Changelog: [Internal]

Reviewed By: sbuggay

Differential Revision: D57473295

fbshipit-source-id: 979f1b9a51f5550a8d3ca534276ec191a3cb7b9e
  • Loading branch information
NickGerleman authored and facebook-github-bot committed May 18, 2024
1 parent 24f0c56 commit fb53cb7
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 6 deletions.
28 changes: 24 additions & 4 deletions yoga/node/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <iostream>

#include <yoga/debug/AssertFatal.h>
#include <yoga/debug/Log.h>
#include <yoga/node/Node.h>
#include <yoga/numeric/Comparison.h>

Expand Down Expand Up @@ -49,12 +50,31 @@ Node::Node(Node&& node) noexcept
}

YGSize Node::measure(
float width,
float availableWidth,
MeasureMode widthMode,
float height,
float availableHeight,
MeasureMode heightMode) {
return measureFunc_(
this, width, unscopedEnum(widthMode), height, unscopedEnum(heightMode));
const auto size = measureFunc_(
this,
availableWidth,
unscopedEnum(widthMode),
availableHeight,
unscopedEnum(heightMode));

if (yoga::isUndefined(size.height) || size.height < 0 ||
yoga::isUndefined(size.width) || size.width < 0) {
yoga::log(
this,
LogLevel::Warn,
"Measure function returned an invalid dimension to Yoga: [width=%f, height=%f]",
size.width,
size.height);
return {
.width = maxOrDefined(0.0f, size.width),
.height = maxOrDefined(0.0f, size.height)};
}

return size;
}

float Node::baseline(float width, float height) const {
Expand Down
4 changes: 2 additions & 2 deletions yoga/node/Node.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ class YG_EXPORT Node : public ::YGNode {
}

YGSize measure(
float width,
float availableWidth,
MeasureMode widthMode,
float height,
float availableHeight,
MeasureMode heightMode);

bool hasBaselineFunc() const noexcept {
Expand Down

0 comments on commit fb53cb7

Please sign in to comment.