Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Fix edge case in composite function interpolation #8613

Merged
merged 3 commits into from
Apr 3, 2017

Conversation

anandthakker
Copy link
Contributor

This fixes a bug where, for a zoom value greater than that of the highest zoom
stop, composite function interpolation would return nan. (Blocking a render
test over in #8593)

Anand Thakker added 2 commits April 1, 2017 08:54
The failing cases here are:
 - Should interpolate before the first stop
 - Should interpolate past the last stop
@anandthakker anandthakker requested review from lbud and jfirebaugh April 1, 2017 13:08
}), 0.0f)
.evaluate(2.0f, oneInteger, -1.0f)) << "Should interpolate between stops";

EXPECT_EQ(0.0, CompositeFunction<float>("property", CompositeExponentialStops<float>({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the expected result be 33.0? Functions of all types are supposed to be flat before their first stop.

}), 0.0f)
.evaluate(0.0f, oneInteger, -1.0f)) << "Should interpolate before the first stop";

EXPECT_EQ(99.0, CompositeFunction<float>("property", CompositeExponentialStops<float>({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, I'd expect 66.0.

@anandthakker
Copy link
Contributor Author

@jfirebaugh thanks for the review. Updated to hold function constant outside the min,max stop range.

Copy link
Contributor

@jfirebaugh jfirebaugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kkaefer, any hints about the CircleCI build failure here?

@anandthakker anandthakker merged commit b5b4549 into master Apr 3, 2017
@anandthakker anandthakker deleted the fix-composite-interpolation branch April 3, 2017 16:25
jfirebaugh added a commit that referenced this pull request Apr 20, 2017
b5b4549 / #8613 handled the edge case for layout properties, but not paint properties. Move the check for a degenerate range to interpolationFactor in order to handle both correctly.
jfirebaugh added a commit that referenced this pull request Apr 20, 2017
b5b4549 / #8613 handled the edge case for layout properties, but not paint properties. Move the check for a degenerate range to interpolationFactor in order to handle both correctly.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants