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

Load Android Gradle Plugin conditionally #354

Merged
merged 1 commit into from
Apr 8, 2020
Merged

Load Android Gradle Plugin conditionally #354

merged 1 commit into from
Apr 8, 2020

Conversation

SaeedZhiany
Copy link
Contributor

Summary

  • Load Android Gradle Plugin conditionally

This wraps the Android Gradle plugin dependency in the buildscripts section of android/build.gradle in a conditional:

if (project == rootProject) {
    // ... (dependency here)
}

The Android Gradle plugin is only required when opening the project stand-alone, not when it is included as a dependency. By doing this, the project opens correctly in Android Studio, and it can also be consumed as a native module dependency from an application project without affecting the app project (avoiding unnecessary downloads/conflicts/etc).

for more info, you can refer to this thread and especially this comment.

  • bumped default gradle configs versions

Test Plan

What's required for testing (prerequisites)?

What are the steps to reproduce (after prerequisites)?

Compatibility

OS Implemented
iOS
Android

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I mentioned this change in CHANGELOG.md
  • I updated the typed files (TS and Flow)
  • I added a sample use of the API in the example project (example/App.js)

Also, bumped default gradle configs versions
@msand msand merged commit 0ad8a53 into Kureev:master Apr 8, 2020
@SaeedZhiany SaeedZhiany deleted the patch-1 branch April 8, 2020 15:16
lxp-git pushed a commit to Flickering-AI/react-native-blur that referenced this pull request Mar 7, 2022
Motivation
When creating multiple dynamic animations with react-native-reanimated the number of nodes that needs to be transported over the bridge is a performance challenge. One of the limitations of the current primitives offered by reanimated is the lack of function nodes. Being able to define node trees that can be reused will offer better performance. A previous attempt has been done in this pull request: Kureev#42

Implementation
The suggested implementation tries to limit changes to the current source code by adding new nodes without changing the existing implementation too much. Three new nodes have been added:

ParamNode
CallFunctionNode
FunctionNode
A few changes to the NodeManager to be able to force reevaluation when calling functions have been added.

The implementation uses a simple proc function in Javascript to create function nodes. Following is an example of how a function is defined and used.

// Defining the function
const mulXY = proc((x, y) => multipy(x, y));

// Using the function
const myX = new Animated.Value(12);
const myY = new Animated.Value(14);
const myResult = mulXY(myX, myY);
Technical
The solution I have used in this implementation is to implement the ParamNode as a node that holds a reference to other nodes - returning the referenced node's value when evaluated and setting the referenced node's value when changed.

Calling the function node is done using the CallFuncNode that is created when calling the function (on the JS side). When the CallFuncNode is evaluated, it performs the following steps:

Start a new context
Copy current argument nodes to the param nodes (using a Stack in the param node)
Evaluate the function node's expression
Pop the current argument nodes from the param nodes
End context
I have also changed NodeManager's invalidate strategy so that it will force a reevaluation of nodes when we are in a function call to avoid returning the same value for multiple functions calls with different parameter references.

Using a stack in the ParamNode ensures that we can create recursive functions.

Memory
A single proc node should be created as a global constant in javascript since they are immediately attached and will never be removed.

Platforms
Both Android and iOS platform-specific code has been added in addition to the required JS code.

Further plans
Rewriting functionality in reanimated called often should be considered. Moving complex spring functions to functions could also be considered but is not necessarily part of this PR.

Testing
Convert built-in derived functionality to use proc nodes and verify that all examples work nicely. I have already done this for interpolate and easings and it seems to be working fine. Verify that no memory leaks exist.

I believe that the current implementation supports caching of function evaluation - would be happy if someone else could confirm this.

Co-authored-by: osdnk <[email protected]>
lxp-git pushed a commit to Flickering-AI/react-native-blur that referenced this pull request Mar 7, 2022
Why
fixes "test" example - all boxes animate across the screen
fixes "interpolate" example - now boxes don't jitter when you drag them
fixes "colors" example - now colors interpolate smoothly rather than snap to highest RGB
fixes "image viewer" example - no more jittering when the user stops panning
fixes snapping to edges in "chat heads" example - doesn't fix the bug where other chat heads don't follow the main one
changes "different spring configs" to go from jittery circles to not moving circles (¯\_(ツ)_/¯)
doesn't fix any of the interactable examples
How
Adding more JS support for the features added by this PR Kureev#354 we should be more careful in the future not to add native-only features.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants