Skip to content

Commit

Permalink
Fix two crashes from NativeAnimatedExamples (#3400)
Browse files Browse the repository at this point in the history
* merge in basic slider

* Fix two issues:
1) you cannot animated 2 subchannels of the same property with different animations. to fix this we animated yet another property set for translation and scale owned by the props nodes and use one animation to animate all of the subchannels for the uiElement.
2) Reference parameter names which started with a multi digit number are unsupported so i added an n to the start of each name, which was previously just the node's tag.

* Change files

* yarn format

* Fix an issue with spring animations that did not start from 0, causing them to double count the start value.

* respond to feedback.

* yarn format
  • Loading branch information
StephenLPeters committed Oct 17, 2019
1 parent 04e07f7 commit 4d9e944
Show file tree
Hide file tree
Showing 17 changed files with 225 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"type": "prerelease",
"comment": "Fix two issues: 1) you cannot animated 2 subchannels of the same property with different animations. to fix this we animated yet another property set for translation and scale owned by the props nodes and use one animation to animate all of the subchannels for the uiElement. 2) Reference parameter names which started with a multi digit number are unsupported so i added an n to the start of each name, which was previously just the node's tag.",
"packageName": "react-native-windows",
"email": "stpete@microsoft.com",
"commit": "62049fdbd667fc71ae09b09f074446d8593d826d",
"date": "2019-10-11T20:21:32.900Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"type": "patch",
"comment": "Fix two issues: 1) you cannot animated 2 subchannels of the same property with different animations. to fix this we animated yet another property set for translation and scale owned by the props nodes and use one animation to animate all of the subchannels for the uiElement. 2) Reference parameter names which started with a multi digit number are unsupported so i added an n to the start of each name, which was previously just the node's tag.",
"packageName": "react-native-windows-extended",
"email": "stpete@microsoft.com",
"commit": "62049fdbd667fc71ae09b09f074446d8593d826d",
"date": "2019-10-11T20:21:24.946Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"type": "patch",
"comment": "Fix two issues: 1) you cannot animated 2 subchannels of the same property with different animations. to fix this we animated yet another property set for translation and scale owned by the props nodes and use one animation to animate all of the subchannels for the uiElement. 2) Reference parameter names which started with a multi digit number are unsupported so i added an n to the start of each name, which was previously just the node's tag.",
"packageName": "rnpm-plugin-windows",
"email": "stpete@microsoft.com",
"commit": "62049fdbd667fc71ae09b09f074446d8593d826d",
"date": "2019-10-11T20:20:58.182Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"type": "none",
"comment": "Fix two issues: 1) you cannot animated 2 subchannels of the same property with different animations. to fix this we animated yet another property set for translation and scale owned by the props nodes and use one animation to animate all of the subchannels for the uiElement. 2) Reference parameter names which started with a multi digit number are unsupported so i added an n to the start of each name, which was previously just the node's tag.",
"packageName": "rnpm-plugin-wpf",
"email": "stpete@microsoft.com",
"commit": "62049fdbd667fc71ae09b09f074446d8593d826d",
"date": "2019-10-11T20:21:05.364Z"
}
2 changes: 2 additions & 0 deletions vnext/ReactUWP/Base/UwpReactInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include <Views/RootViewManager.h>
#include <Views/ScrollContentViewManager.h>
#include <Views/ScrollViewManager.h>
#include <Views/SliderViewManager.h>
#include <Views/SwitchViewManager.h>
#include <Views/TextInputViewManager.h>
#include <Views/TextViewManager.h>
Expand Down Expand Up @@ -124,6 +125,7 @@ CreateUIManager(
viewManagers.push_back(std::make_unique<RawTextViewManager>(instance));
viewManagers.push_back(std::make_unique<RootViewManager>(instance));
viewManagers.push_back(std::make_unique<ScrollContentViewManager>(instance));
viewManagers.push_back(std::make_unique<SliderViewManager>(instance));
viewManagers.push_back(std::make_unique<ScrollViewManager>(instance));
viewManagers.push_back(std::make_unique<SwitchViewManager>(instance));
viewManagers.push_back(std::make_unique<TextViewManager>(instance));
Expand Down
4 changes: 2 additions & 2 deletions vnext/ReactUWP/Modules/Animated/AdditionAnimatedNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ AdditionAnimatedNode::AdditionAnimatedNode(
int64_t tag,
const folly::dynamic &config,
const std::shared_ptr<NativeAnimatedNodeManager> &manager)
: ValueAnimatedNode(tag, config, manager) {
: ValueAnimatedNode(tag, manager) {
for (const auto &inputNode : config.find(s_inputName).dereference().second) {
m_inputNodes.insert(static_cast<int64_t>(inputNode.asDouble()));
}
Expand All @@ -24,7 +24,7 @@ AdditionAnimatedNode::AdditionAnimatedNode(
anim.Expression([nodes, manager, anim]() {
winrt::hstring expr = L"0";
for (const auto tag : nodes) {
const auto identifier = std::to_wstring(tag);
const auto identifier = L"n" + std::to_wstring(tag);
anim.SetReferenceParameter(
identifier, manager->GetValueAnimatedNode(tag)->PropertySet());
expr = expr + L" + " + identifier + L"." + s_valueName + L" + " +
Expand Down
2 changes: 1 addition & 1 deletion vnext/ReactUWP/Modules/Animated/DivisionAnimatedNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ DivisionAnimatedNode::DivisionAnimatedNode(
L"." + s_valueName + L" + " + s_baseName + L"." + s_offsetName +
L")";
for (const auto tag : nodes) {
const auto identifier = std::to_wstring(tag);
const auto identifier = L"n" + std::to_wstring(tag);
anim.SetReferenceParameter(
identifier, manager->GetValueAnimatedNode(tag)->PropertySet());
expr = expr + L" / (" + identifier + L"." + s_valueName + L" " +
Expand Down
2 changes: 1 addition & 1 deletion vnext/ReactUWP/Modules/Animated/ModulusAnimatedNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ ModulusAnimatedNode::ModulusAnimatedNode(
int64_t tag,
const folly::dynamic &config,
const std::shared_ptr<NativeAnimatedNodeManager> &manager)
: ValueAnimatedNode(tag, config, manager) {
: ValueAnimatedNode(tag, manager) {
m_inputNodeTag = static_cast<int64_t>(
config.find(s_inputName).dereference().second.asDouble());
m_modulus = static_cast<int64_t>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ MultiplicationAnimatedNode::MultiplicationAnimatedNode(
int64_t tag,
const folly::dynamic &config,
const std::shared_ptr<NativeAnimatedNodeManager> &manager)
: ValueAnimatedNode(tag, config, manager) {
: ValueAnimatedNode(tag, manager) {
for (const auto &inputNode : config.find(s_inputName).dereference().second) {
m_inputNodes.insert(static_cast<int64_t>(inputNode.asDouble()));
}
Expand All @@ -24,7 +24,7 @@ MultiplicationAnimatedNode::MultiplicationAnimatedNode(
anim.Expression([nodes, manager, anim]() {
winrt::hstring expr = L"1";
for (const auto tag : nodes) {
auto identifier = std::to_wstring(tag);
auto identifier = L"n" + std::to_wstring(tag);
anim.SetReferenceParameter(
identifier, manager->GetValueAnimatedNode(tag)->PropertySet());
expr = expr + L" * (" + identifier + L"." + s_valueName + L" + " +
Expand Down
37 changes: 36 additions & 1 deletion vnext/ReactUWP/Modules/Animated/PropsAnimatedNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,27 @@ PropsAnimatedNode::PropsAnimatedNode(
m_propMapping.insert({entry.first.getString(),
static_cast<int64_t>(entry.second.asDouble())});
}

m_subchannelPropertySet =
winrt::Window::Current().Compositor().CreatePropertySet();
m_subchannelPropertySet.InsertScalar(L"TranslationX", 0.0f);
m_subchannelPropertySet.InsertScalar(L"TranslationY", 0.0f);
m_subchannelPropertySet.InsertScalar(L"ScaleX", 1.0f);
m_subchannelPropertySet.InsertScalar(L"ScaleY", 1.0f);

m_translationCombined =
winrt::Window::Current().Compositor().CreateExpressionAnimation(
L"Vector3(subchannels.TranslationX, subchannels.TranslationY, 0.0)");
m_translationCombined.SetReferenceParameter(
L"subchannels", m_subchannelPropertySet);
m_translationCombined.Target(L"Translation");

m_scaleCombined =
winrt::Window::Current().Compositor().CreateExpressionAnimation(
L"Vector3(subchannels.ScaleX, subchannels.ScaleY, 1.0)");
m_scaleCombined.SetReferenceParameter(
L"subchannels", m_subchannelPropertySet);
m_scaleCombined.Target(L"Scale");
}

void PropsAnimatedNode::ConnectToView(int64_t viewTag) {
Expand Down Expand Up @@ -89,7 +110,21 @@ void PropsAnimatedNode::StartAnimations() {
if (const auto uiElement = GetUIElement()) {
uiElement.RotationAxis(m_rotationAxis);
for (const auto anim : m_expressionAnimations) {
uiElement.StartAnimation(anim.second);
if (anim.second.Target() == L"Translation.X") {
m_subchannelPropertySet.StartAnimation(L"TranslationX", anim.second);
uiElement.StartAnimation(m_translationCombined);
} else if (anim.second.Target() == L"Translation.Y") {
m_subchannelPropertySet.StartAnimation(L"TranslationY", anim.second);
uiElement.StartAnimation(m_translationCombined);
} else if (anim.second.Target() == L"Scale.X") {
m_subchannelPropertySet.StartAnimation(L"ScaleX", anim.second);
uiElement.StartAnimation(m_translationCombined);
} else if (anim.second.Target() == L"Scale.Y") {
m_subchannelPropertySet.StartAnimation(L"ScaleY", anim.second);
uiElement.StartAnimation(m_translationCombined);
} else {
uiElement.StartAnimation(anim.second);
}
}
if (m_needsCenterPointAnimation) {
if (!m_centerPointAnimation) {
Expand Down
3 changes: 3 additions & 0 deletions vnext/ReactUWP/Modules/Animated/PropsAnimatedNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ class PropsAnimatedNode : public AnimatedNode {
nullptr};
winrt::Numerics::float3 m_rotationAxis{0, 0, 1};
bool m_needsCenterPointAnimation{false};
winrt::CompositionPropertySet m_subchannelPropertySet{nullptr};
winrt::CompositionAnimation m_translationCombined{nullptr};
winrt::CompositionAnimation m_scaleCombined{nullptr};

static constexpr int64_t s_connectedViewTagUnset{-1};
};
Expand Down
11 changes: 7 additions & 4 deletions vnext/ReactUWP/Modules/Animated/SpringAnimationDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ SpringAnimationDriver::MakeAnimation(const folly::dynamic &config) {
compositor.CreateLinearEasingFunction());
}();

const auto startValue = GetAnimatedValue()->RawValue();
const auto startValue = GetAnimatedValue()->Value();
std::vector<float> keyFrames = [this, startValue]() {
std::vector<float> keyFrames;
bool done = false;
Expand All @@ -80,12 +80,15 @@ SpringAnimationDriver::MakeAnimation(const folly::dynamic &config) {
animation.Duration(duration);

auto normalizedProgress = 0.0f;
animation.InsertKeyFrame(
normalizedProgress, static_cast<float>(startValue), easingFunction);
// We are animating the values offset property which should start at 0.
animation.InsertKeyFrame(normalizedProgress, 0.0f, easingFunction);
for (const auto keyFrame : keyFrames) {
normalizedProgress =
std::min(normalizedProgress + 1.0f / keyFrames.size(), 1.0f);
animation.InsertKeyFrame(normalizedProgress, keyFrame, easingFunction);
animation.InsertKeyFrame(
normalizedProgress,
keyFrame - static_cast<float>(startValue),
easingFunction);
}

if (m_iterations == -1) {
Expand Down
4 changes: 2 additions & 2 deletions vnext/ReactUWP/Modules/Animated/SubtractionAnimatedNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ SubtractionAnimatedNode::SubtractionAnimatedNode(
int64_t tag,
const folly::dynamic &config,
const std::shared_ptr<NativeAnimatedNodeManager> &manager)
: ValueAnimatedNode(tag, config, manager) {
: ValueAnimatedNode(tag, manager) {
for (const auto &inputNode : config.find(s_inputName).dereference().second) {
if (m_firstInput == s_firstInputUnset) {
m_firstInput = static_cast<int64_t>(inputNode.asDouble());
Expand All @@ -34,7 +34,7 @@ SubtractionAnimatedNode::SubtractionAnimatedNode(
L"." + s_valueName + L" + " + s_baseName + L"." + s_offsetName +
L")";
for (const auto tag : nodes) {
const auto identifier = std::to_wstring(tag);
const auto identifier = L"n" + std::to_wstring(tag);
anim.SetReferenceParameter(
identifier, manager->GetValueAnimatedNode(tag)->PropertySet());
expr = expr + L" - (" + identifier + L"." + s_valueName + L" + " +
Expand Down
2 changes: 2 additions & 0 deletions vnext/ReactUWP/ReactUWP.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@
<ClInclude Include="Views\ScrollContentViewManager.h" />
<ClInclude Include="Views\ScrollViewManager.h" />
<ClInclude Include="Views\SwitchViewManager.h" />
<ClInclude Include="Views\SliderViewManager.h" />
<ClInclude Include="Views\TextInputViewManager.h" />
<ClInclude Include="Views\TextViewManager.h" />
<ClInclude Include="Views\TouchEventHandler.h" />
Expand Down Expand Up @@ -350,6 +351,7 @@
<ClCompile Include="Views\RootViewManager.cpp" />
<ClCompile Include="Views\ScrollContentViewManager.cpp" />
<ClCompile Include="Views\ScrollViewManager.cpp" />
<ClCompile Include="Views\SliderViewManager.cpp" />
<ClCompile Include="Views\ShadowNodeBase.cpp" />
<ClCompile Include="Views\SwitchViewManager.cpp" />
<ClCompile Include="Views\TextInputViewManager.cpp" />
Expand Down
8 changes: 7 additions & 1 deletion vnext/ReactUWP/ReactUWP.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,9 @@
<ClCompile Include="Modules\StatusBarModule.cpp">
<Filter>Modules</Filter>
</ClCompile>
<ClCompile Include="Views\SliderViewManager.cpp">
<Filter>Views</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<ClInclude Include="Modules\DeviceInfoModule.h">
Expand Down Expand Up @@ -633,9 +636,12 @@
<ClInclude Include="Utils\CppWinrtLessExceptions.h">
<Filter>Utils</Filter>
</ClInclude>
<ClInclude Include="Views\SliderViewManager.h">
<Filter>Views</Filter>
</ClInclude>
<ClInclude Include="Modules\StatusBarModule.h">
<Filter>Modules</Filter>
</ClInclude>
</ClCompile>
</ItemGroup>
<ItemGroup>
<None Include="EndPoints\dll\react-native-uwp.arm.def">
Expand Down
95 changes: 95 additions & 0 deletions vnext/ReactUWP/Views/SliderViewManager.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

#include "pch.h"

#include <Views/ShadowNodeBase.h>
#include "SliderViewManager.h"

#include <Utils/ValueUtils.h>

#include <IReactInstance.h>

#include <winrt/Windows.UI.Xaml.Controls.Primitives.h>

namespace winrt {
using ToggleButton = Windows::UI::Xaml::Controls::Primitives::ToggleButton;
}

namespace react {
namespace uwp {

class SliderShadowNode : public ShadowNodeBase {
using Super = ShadowNodeBase;

public:
SliderShadowNode() = default;
void createView() override;
void updateProperties(const folly::dynamic &&props) override;
};

void SliderShadowNode::createView() {
Super::createView();
}

void SliderShadowNode::updateProperties(const folly::dynamic &&props) {
m_updating = true;
Super::updateProperties(std::move(props));
m_updating = false;
}

SliderViewManager::SliderViewManager(
const std::shared_ptr<IReactInstance> &reactInstance)
: Super(reactInstance) {}

const char *SliderViewManager::GetName() const {
return "RCTSlider";
}

folly::dynamic SliderViewManager::GetNativeProps() const {
auto props = Super::GetNativeProps();

props.update(
folly::dynamic::object("value", "integer")("disabled", "boolean"));

return props;
}

facebook::react::ShadowNode *SliderViewManager::createShadow() const {
return new SliderShadowNode();
}

XamlView SliderViewManager::CreateViewCore(int64_t tag) {
auto slider = winrt::Slider();
return slider;
}

void SliderViewManager::UpdateProperties(
ShadowNodeBase *nodeToUpdate,
const folly::dynamic &reactDiffMap) {
auto slider = nodeToUpdate->GetView().as<winrt::Slider>();
if (slider == nullptr)
return;

for (const auto &pair : reactDiffMap.items()) {
const std::string &propertyName = pair.first.getString();
const folly::dynamic &propertyValue = pair.second;

if (propertyName == "disabled") {
if (propertyValue.isBool())
slider.IsEnabled(!propertyValue.asBool());
else if (pair.second.isNull())
slider.ClearValue(winrt::Control::IsEnabledProperty());
} else if (propertyName == "value") {
if (propertyValue.isNumber())
slider.Value(propertyValue.asDouble());
else if (pair.second.isNull())
slider.Value(0);
}
}

Super::UpdateProperties(nodeToUpdate, reactDiffMap);
}

} // namespace uwp
} // namespace react
33 changes: 33 additions & 0 deletions vnext/ReactUWP/Views/SliderViewManager.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

#pragma once

#include <Views/ControlViewManager.h>

namespace react {
namespace uwp {

class SliderViewManager : public ControlViewManager {
using Super = ControlViewManager;

public:
SliderViewManager(const std::shared_ptr<IReactInstance> &reactInstance);

const char *GetName() const override;
folly::dynamic GetNativeProps() const override;

facebook::react::ShadowNode *createShadow() const override;

void UpdateProperties(
ShadowNodeBase *nodeToUpdate,
const folly::dynamic &reactDiffMap) override;

protected:
XamlView CreateViewCore(int64_t tag) override;

friend class SliderShadowNode;
};

} // namespace uwp
} // namespace react

0 comments on commit 4d9e944

Please sign in to comment.