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 event handling #1319

Merged
merged 3 commits into from
Oct 27, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Common/cpp/NativeModules/NativeReanimatedModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,11 @@ void NativeReanimatedModule::onEvent(std::string eventName, std::string eventAsS
}
}

bool NativeReanimatedModule::isAnyHandlerWaitingForEvent(std::string eventName) {
return eventHandlerRegistry->isAnyHandlerWaitingForEvent(eventName);
}


void NativeReanimatedModule::maybeRequestRender()
{
if (!renderRequested)
Expand Down
8 changes: 8 additions & 0 deletions Common/cpp/Registries/EventHandlerRegistry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,21 @@ static jsi::Value eval(jsi::Runtime &rt, const char *code) {
}

void EventHandlerRegistry::registerEventHandler(std::shared_ptr<EventHandler> eventHandler) {
const std::lock_guard<std::mutex> lock(instanceMutex);
eventMappings[eventHandler->eventName][eventHandler->id] = eventHandler;
eventHandlers[eventHandler->id] = eventHandler;
}

void EventHandlerRegistry::unregisterEventHandler(unsigned long id) {
const std::lock_guard<std::mutex> lock(instanceMutex);
auto handlerIt = eventHandlers.find(id);
if (handlerIt != eventHandlers.end()) {
eventMappings[handlerIt->second->eventName].erase(id);
}
}

void EventHandlerRegistry::processEvent(jsi::Runtime &rt, std::string eventName, std::string eventPayload) {
const std::lock_guard<std::mutex> lock(instanceMutex);
auto handlersIt = eventMappings.find(eventName);
if (handlersIt != eventMappings.end()) {
// TODO: use jsi::Value::createFromJsonUtf8
Expand All @@ -31,4 +34,9 @@ void EventHandlerRegistry::processEvent(jsi::Runtime &rt, std::string eventName,
}
}

bool EventHandlerRegistry::isAnyHandlerWaitingForEvent(std::string eventName) {
auto it = eventMappings.find(eventName);
return (it != eventMappings.end()) and (!(it->second).empty());
}

}
1 change: 1 addition & 0 deletions Common/cpp/headers/NativeModules/NativeReanimatedModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class NativeReanimatedModule : public NativeReanimatedModuleSpec

void onRender(double timestampMs);
void onEvent(std::string eventName, std::string eventAsString);
bool isAnyHandlerWaitingForEvent(std::string eventName);

void maybeRequestRender();

Expand Down
3 changes: 3 additions & 0 deletions Common/cpp/headers/Registries/EventHandlerRegistry.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <unordered_map>
#include <string>
#include <jsi/jsi.h>
#include <mutex>

using namespace facebook;

Expand All @@ -16,12 +17,14 @@ class EventHandler;
class EventHandlerRegistry {
std::map<std::string, std::unordered_map<unsigned long, std::shared_ptr<EventHandler>>> eventMappings;
std::map<unsigned long, std::shared_ptr<EventHandler>> eventHandlers;
std::mutex instanceMutex;

public:
void registerEventHandler(std::shared_ptr<EventHandler> eventHandler);
void unregisterEventHandler(unsigned long id);

void processEvent(jsi::Runtime &rt, std::string eventName, std::string eventPayload);
bool isAnyHandlerWaitingForEvent(std::string eventName);
};

}
8 changes: 7 additions & 1 deletion android/src/main/cpp/NativeProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#include "NativeProxy.h"
#include "AndroidErrorHandler.h"
#include "NativeReanimatedModule.h"
#include "AndroidScheduler.h"
#include <android/log.h>
#include "PlatformDepMethodsHolder.h"
Expand Down Expand Up @@ -91,6 +90,8 @@ void NativeProxy::installJSIBindings()
propObtainer,
platformDepMethodsHolder);

_nativeReanimatedModule = module;

this->registerEventHandler([module](std::string eventName, std::string eventAsString) {
module->onEvent(eventName, eventAsString);
});
Expand All @@ -101,11 +102,16 @@ void NativeProxy::installJSIBindings()
jsi::Object::createFromHostObject(*runtime_, module));
}

bool NativeProxy::isAnyHandlerWaitingForEvent(std::string s) {
return _nativeReanimatedModule->isAnyHandlerWaitingForEvent(s);
}

void NativeProxy::registerNatives()
{
registerHybrid({
makeNativeMethod("initHybrid", NativeProxy::initHybrid),
makeNativeMethod("installJSIBindings", NativeProxy::installJSIBindings),
makeNativeMethod("isAnyHandlerWaitingForEvent", NativeProxy::isAnyHandlerWaitingForEvent)
});
}

Expand Down
4 changes: 4 additions & 0 deletions android/src/main/cpp/headers/NativeProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
#include <react/jni/CxxModuleWrapper.h>
#include <react/jni/JMessageQueueThread.h>
#include <react/jni/WritableNativeMap.h>
#include "NativeReanimatedModule.h"
#include <ReactCommon/CallInvokerHolder.h>
#include <memory>
#include <unordered_map>


#include "Scheduler.h"
#include "AndroidScheduler.h"

Expand Down Expand Up @@ -89,9 +91,11 @@ class NativeProxy : public jni::HybridClass<NativeProxy> {
jni::global_ref<NativeProxy::javaobject> javaPart_;
jsi::Runtime *runtime_;
std::shared_ptr<facebook::react::CallInvoker> jsCallInvoker_;
std::shared_ptr<NativeReanimatedModule> _nativeReanimatedModule;
std::shared_ptr<Scheduler> scheduler_;

void installJSIBindings();
bool isAnyHandlerWaitingForEvent(std::string);
void requestRender(std::function<void(double)> onRender);
void registerEventHandler(std::function<void(std::string,std::string)> handler);
void updateProps(jsi::Runtime &rt, int viewTag, const jsi::Object &props);
Expand Down
42 changes: 42 additions & 0 deletions android/src/main/java/com/swmansion/reanimated/CopiedEvent.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.swmansion.reanimated;

import com.facebook.react.bridge.WritableArray;
import com.facebook.react.bridge.WritableMap;
import com.facebook.react.uimanager.events.Event;
import com.facebook.react.uimanager.events.RCTEventEmitter;

import androidx.annotation.Nullable;

public class CopiedEvent {
private int targetTag;
private String eventName;
private WritableMap payload;

CopiedEvent(Event event) {
event.dispatch(new RCTEventEmitter() {
@Override
public void receiveEvent(int targetTag, String eventName, @Nullable WritableMap event) {
CopiedEvent.this.targetTag = targetTag;
CopiedEvent.this.eventName = eventName;
CopiedEvent.this.payload = event;
}

@Override
public void receiveTouches(String eventName, WritableArray touches, WritableArray changedIndices) {
//noop
}
});
}

public int getTargetTag() {
return targetTag;
}

public String getEventName() {
return eventName;
}

public WritableMap getPayload() {
return payload;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ public NativeProxy(ReactApplicationContext context) {
private native HybridData initHybrid(long jsContext, CallInvokerHolderImpl jsCallInvokerHolder, Scheduler scheduler);
private native void installJSIBindings();

public native boolean isAnyHandlerWaitingForEvent(String eventName);

@DoNotStrip
private void requestRender(AnimationFrameCallback callback) {
mNodesManager.postOnAnimation(callback);
Expand Down
30 changes: 27 additions & 3 deletions android/src/main/java/com/swmansion/reanimated/NodesManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@
import java.util.concurrent.Semaphore;
import java.util.concurrent.atomic.AtomicBoolean;

import javax.annotation.Nullable;

public class NodesManager implements EventDispatcherListener {

private static final Double ZERO = Double.valueOf(0);
Expand Down Expand Up @@ -104,7 +106,7 @@ public interface OnAnimationFrame {

private RCTEventEmitter mCustomEventHandler;
private List<OnAnimationFrame> mFrameCallbacks = new ArrayList<>();
private ConcurrentLinkedQueue<Event> mEventQueue = new ConcurrentLinkedQueue<>();
private ConcurrentLinkedQueue<CopiedEvent> mEventQueue = new ConcurrentLinkedQueue<>();
private boolean mWantRunUpdates;

public double currentFrameTimeMs;
Expand Down Expand Up @@ -228,7 +230,8 @@ private void onAnimationFrame(long frameTimeNanos) {
currentFrameTimeMs = frameTimeNanos / 1000000.;

while (!mEventQueue.isEmpty()) {
handleEvent(mEventQueue.poll());
CopiedEvent copiedEvent = mEventQueue.poll();
handleEvent(copiedEvent.getTargetTag(), copiedEvent.getEventName(), copiedEvent.getPayload());
jakub-gonet marked this conversation as resolved.
Show resolved Hide resolved
}

if (!mFrameCallbacks.isEmpty()) {
Expand Down Expand Up @@ -445,7 +448,13 @@ public void onEventDispatch(Event event) {
handleEvent(event);
performOperations();
} else {
mEventQueue.offer(event);
boolean shouldSaveEvent = false;
String eventName = mCustomEventNamesResolver.resolveCustomEventName(event.getEventName());
int viewTag = event.getViewTag();
String key = viewTag + eventName;

shouldSaveEvent |= (mCustomEventHandler != null && mNativeProxy.isAnyHandlerWaitingForEvent(key));
mEventQueue.offer(new CopiedEvent(event));
Copy link
Member

Choose a reason for hiding this comment

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

Copied from #1149

Maybe we could also pool CopiedEvent objects same way we have pools of other events. In this case this should be rather efficient as there typically won't be more than 2-3 events in use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see a need for a CopiedEvent pool.

Copy link
Member

Choose a reason for hiding this comment

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

Not using the pool will strain GC, event objects will be created and removed frequently if events will be coming from other source than the UI thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't make sense for me because we still have to create and remove events' payloads.

Copy link
Member

Choose a reason for hiding this comment

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

So maybe @kmagiera will chime in since he suggested this change in the original PR.

startUpdatingOnAnimationFrame();
}
}
Expand All @@ -468,6 +477,21 @@ private void handleEvent(Event event) {
}
}

private void handleEvent(int targetTag, String eventName, @Nullable WritableMap event) {
if (mCustomEventHandler != null) {
mCustomEventHandler.receiveEvent(targetTag, eventName, event);
}

String key = targetTag + eventName;

if (!mEventMapping.isEmpty()) {
EventNode node = mEventMapping.get(key);
if (node != null) {
node.receiveEvent(targetTag, eventName, event);
}
}
}

public UIManagerModule.CustomEventNamesResolver getEventNameResolver() {
return mCustomEventNamesResolver;
}
Expand Down