Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Crashes during CSS transitions of complex properties #6966

Merged
merged 10 commits into from
Feb 4, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ namespace reanimated {
using namespace facebook;

using PropertyNames = std::vector<std::string>;
using PropertyValues = std::unique_ptr<jsi::Value>;
using PropertyPath = std::vector<std::string>;
/**
* If nullopt - all style properties can trigger transition
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ jsi::Value CSSTransition::run(
progressProvider_.runProgressProviders(
timestamp,
changedProps.changedPropertyNames,
styleInterpolator_.getReversedPropertyNames(rt, *changedProps.newProps));
styleInterpolator_.getReversedPropertyNames(rt, changedProps.newProps));
styleInterpolator_.updateInterpolatedProperties(
rt, changedProps, progressProvider_.getPropertyProgressProviders());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,33 @@ void ArrayPropertiesInterpolator::updateKeyframesFromStyleChange(
jsi::Runtime &rt,
const jsi::Value &oldStyleValue,
const jsi::Value &newStyleValue) {
const size_t valuesCount = newStyleValue.isObject()
? newStyleValue.asObject(rt).asArray(rt).size(rt)
: oldStyleValue.asObject(rt).asArray(rt).size(rt);
MatiPl01 marked this conversation as resolved.
Show resolved Hide resolved
auto getArrayFromStyle = [&rt](const jsi::Value &style) {
if (!style.isObject()) {
return jsi::Array(rt, 0);
}
auto obj = style.asObject(rt);
return obj.isArray(rt) ? obj.asArray(rt) : jsi::Array(rt, 0);
};

const auto oldStyleArray = getArrayFromStyle(oldStyleValue);
const auto newStyleArray = getArrayFromStyle(newStyleValue);

const size_t valuesCount =
std::max(oldStyleArray.size(rt), newStyleArray.size(rt));

resizeInterpolators(valuesCount);

for (size_t i = 0; i < valuesCount; ++i) {
interpolators_[i]->updateKeyframesFromStyleChange(
rt,
oldStyleValue.isObject()
? oldStyleValue.asObject(rt).asArray(rt).getValueAtIndex(rt, i)
: jsi::Value::undefined(),
newStyleValue.isObject()
? newStyleValue.asObject(rt).asArray(rt).getValueAtIndex(rt, i)
: jsi::Value::undefined());
// These index checks ensure that interpolation works between 2 arrays
// with different lengths
const auto oldValue = oldStyleArray.size(rt) > i
? oldStyleArray.getValueAtIndex(rt, i)
: jsi::Value::undefined();
const auto newValue = newStyleArray.size(rt) > i
? newStyleArray.getValueAtIndex(rt, i)
: jsi::Value::undefined();

interpolators_[i]->updateKeyframesFromStyleChange(rt, oldValue, newValue);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <reanimated/CSS/interpolation/groups/GroupPropertiesInterpolator.h>
#include <reanimated/CSS/util/interpolators.h>

#include <algorithm>
#include <memory>

namespace reanimated {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,20 +53,9 @@ void RecordPropertiesInterpolator::updateKeyframes(
propertyNames.getValueAtIndex(rt, i).asString(rt).utf8(rt);
const jsi::Value &propertyKeyframes = keyframesObject.getProperty(
rt, jsi::PropNameID::forUtf8(rt, propertyName));
auto interpolatorIt = interpolators_.find(propertyName);

if (interpolatorIt == interpolators_.end()) {
const auto newInterpolator = createPropertyInterpolator(
propertyName,
propertyPath_,
factories_,
progressProvider_,
viewStylesRepository_);
interpolatorIt =
interpolators_.emplace(propertyName, newInterpolator).first;
}

interpolatorIt->second->updateKeyframes(rt, propertyKeyframes);
maybeCreateInterpolator(propertyName);
interpolators_.at(propertyName)->updateKeyframes(rt, propertyKeyframes);
}
}

Expand All @@ -76,35 +65,34 @@ void RecordPropertiesInterpolator::updateKeyframesFromStyleChange(
const jsi::Value &newStyleValue) {
// TODO - maybe add a possibility to remove interpolators that are no longer
// used (for now, for simplicity, we only add new ones)
const jsi::Array propertyNames = newStyleValue.isObject()
? newStyleValue.asObject(rt).getPropertyNames(rt)
: oldStyleValue.asObject(rt).getPropertyNames(rt);
const size_t propertiesCount = propertyNames.size(rt);

for (size_t i = 0; i < propertiesCount; ++i) {
const std::string propertyName =
propertyNames.getValueAtIndex(rt, i).asString(rt).utf8(rt);
auto interpolatorIt = interpolators_.find(propertyName);

if (interpolatorIt == interpolators_.end()) {
const auto newInterpolator = createPropertyInterpolator(
propertyName,
propertyPath_,
factories_,
progressProvider_,
viewStylesRepository_);
interpolatorIt =
interpolators_.emplace(propertyName, newInterpolator).first;
const auto oldStyleObject =
oldStyleValue.isObject() ? oldStyleValue.asObject(rt) : jsi::Object(rt);
const auto newStyleObject =
newStyleValue.isObject() ? newStyleValue.asObject(rt) : jsi::Object(rt);

std::unordered_set<std::string> propertyNamesSet;
const jsi::Object *objects[] = {&oldStyleObject, &newStyleObject};
for (const auto *styleObject : objects) {
const auto propertyNames = styleObject->getPropertyNames(rt);
for (size_t i = 0; i < propertyNames.size(rt); ++i) {
propertyNamesSet.insert(
propertyNames.getValueAtIndex(rt, i).asString(rt).utf8(rt));
MatiPl01 marked this conversation as resolved.
Show resolved Hide resolved
}
}

interpolatorIt->second->updateKeyframesFromStyleChange(
rt,
oldStyleValue.isObject()
? oldStyleValue.asObject(rt).getProperty(rt, propertyName.c_str())
: jsi::Value::undefined(),
newStyleValue.isObject()
? newStyleValue.asObject(rt).getProperty(rt, propertyName.c_str())
: jsi::Value::undefined());
for (const auto &propertyName : propertyNamesSet) {
maybeCreateInterpolator(propertyName);

auto getValue = [&](const jsi::Object &obj) {
return obj.hasProperty(rt, propertyName.c_str())
? obj.getProperty(rt, jsi::PropNameID::forUtf8(rt, propertyName))
: jsi::Value::undefined();
};

interpolators_.at(propertyName)
->updateKeyframesFromStyleChange(
rt, getValue(oldStyleObject), getValue(newStyleObject));
}
}

Expand All @@ -121,6 +109,19 @@ jsi::Value RecordPropertiesInterpolator::mapInterpolators(
return result;
}

void RecordPropertiesInterpolator::maybeCreateInterpolator(
const std::string &propertyName) {
if (interpolators_.find(propertyName) == interpolators_.end()) {
const auto newInterpolator = createPropertyInterpolator(
propertyName,
propertyPath_,
factories_,
progressProvider_,
viewStylesRepository_);
interpolators_.emplace(propertyName, newInterpolator);
}
}

} // namespace reanimated

#endif // RCT_NEW_ARCH_ENABLED
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <memory>
#include <string>
#include <unordered_set>

namespace reanimated {

Expand Down Expand Up @@ -34,6 +35,8 @@ class RecordPropertiesInterpolator : public GroupPropertiesInterpolator {
const std::function<jsi::Value(PropertyInterpolator &)> &callback)
const override;

void maybeCreateInterpolator(const std::string &propertyName);

private:
const InterpolatorFactoriesRecord &factories_;
PropertyInterpolatorsRecord interpolators_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ void TransitionStyleInterpolator::updateInterpolatedProperties(
jsi::Runtime &rt,
const ChangedProps &changedProps,
const TransitionPropertyProgressProviders &progressProviders) {
const auto oldPropsObj = changedProps.oldProps->asObject(rt);
const auto newPropsObj = changedProps.newProps->asObject(rt);
const auto oldPropsObj = changedProps.oldProps.asObject(rt);
const auto newPropsObj = changedProps.newProps.asObject(rt);

for (const auto &propertyName : changedProps.changedPropertyNames) {
auto interpolatorIt = interpolators_.find(propertyName);
Expand Down
Loading
Loading