Skip to content

Commit

Permalink
chore: Apply various native code cleanups (#6470)
Browse files Browse the repository at this point in the history
## Summary

Taken those changes from #6378 for more granularity. I applied various
suggestions coming from Android Studio and clangd.

## Test plan

GitHub Actions + :shipit:
  • Loading branch information
tjzel committed Sep 3, 2024
1 parent c7f4b5f commit 9bb6d12
Show file tree
Hide file tree
Showing 16 changed files with 86 additions and 210 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/reanimated-android-validation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ jobs:
steps:
- name: checkout
uses: actions/checkout@v4
- name: Setup Java 17
# Using older versions of Java might cause the lint step to throw
uses: actions/setup-java@v3
with:
distribution: 'zulu'
java-version: 17
- name: Use Node.js
uses: actions/setup-node@v4
with:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
#pragma once

#include <ReactCommon/CallInvoker.h>
#include <ReactCommon/TurboModule.h>

#include <memory>
#include <string>
#include <vector>

#ifdef ANDROID
#include "TurboModule.h"
#else
#include <ReactCommon/TurboModule.h>
#endif

#include <ReactCommon/CallInvoker.h>

using namespace facebook;
using namespace react;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#include "JSScheduler.h"

#include <utility>

using namespace facebook;
using namespace react;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include <jsi/jsi.h>

#include <memory>
#include <utility>

using namespace facebook;
using namespace react;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#include <jsi/jsi.h>
#include <memory>
#include <string>
#include <utility>

#include "Shareables.h"
#include "WorkletRuntime.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ cmake_minimum_required(VERSION 3.8)
file(GLOB_RECURSE REANIMATED_COMMON_CPP_SOURCES CONFIGURE_DEPENDS "${REANIMATED_COMMON_CPP_DIR}/*.cpp")
file(GLOB_RECURSE REANIMATED_ANDROID_CPP_SOURCES CONFIGURE_DEPENDS "${REANIMATED_ANDROID_CPP_DIR}/*.cpp")

find_package(ReactAndroid REQUIRED CONFIG)

add_library(
reanimated
SHARED
Expand Down Expand Up @@ -55,4 +57,5 @@ set_target_properties(
target_link_libraries(
reanimated
worklets
ReactAndroid::react_nativemodule_core
)

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,9 @@ public class AndroidUIScheduler {
private final AtomicBoolean mActive = new AtomicBoolean(true);

private final Runnable mUIThreadRunnable =
new Runnable() {
@Override
public void run() {
if (mActive.get()) {
triggerUI();
}
() -> {
if (mActive.get()) {
triggerUI();
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ public interface OnAnimationFrame {
private final AtomicBoolean mCallbackPosted = new AtomicBoolean();
private final ReactContext mContext;
private final UIManager mUIManager;
private ReactApplicationContext mReactApplicationContext;
private RCTEventEmitter mCustomEventHandler = new NoopEventHandler();
private List<OnAnimationFrame> mFrameCallbacks = new ArrayList<>();
private ConcurrentLinkedQueue<CopiedEvent> mEventQueue = new ConcurrentLinkedQueue<>();
Expand Down Expand Up @@ -135,7 +134,6 @@ public void invalidate() {

public void initWithContext(
ReactApplicationContext reactApplicationContext, String valueUnpackerCode) {
mReactApplicationContext = reactApplicationContext;
mNativeProxy = new NativeProxy(reactApplicationContext, valueUnpackerCode);
mAnimationManager.setAndroidUIScheduler(getNativeProxy().getAndroidUIScheduler());
compatibility = new ReaCompatibility(reactApplicationContext);
Expand Down Expand Up @@ -245,7 +243,7 @@ public void runGuarded() {
}
while (!copiedOperationsQueue.isEmpty()) {
NativeUpdateOperation op = copiedOperationsQueue.remove();
ReactShadowNode shadowNode = mUIImplementation.resolveShadowNode(op.mViewTag);
ReactShadowNode<?> shadowNode = mUIImplementation.resolveShadowNode(op.mViewTag);
if (shadowNode != null) {
((UIManagerModule) mUIManager)
.updateView(op.mViewTag, shadowNode.getViewClass(), op.mNativeProps);
Expand All @@ -261,7 +259,7 @@ public void runGuarded() {
});
if (trySynchronously) {
try {
semaphore.tryAcquire(16, TimeUnit.MILLISECONDS);
boolean ignored = semaphore.tryAcquire(16, TimeUnit.MILLISECONDS);
} catch (InterruptedException e) {
// if the thread is interrupted we just continue and let the layout update happen
// asynchronously
Expand Down Expand Up @@ -440,19 +438,25 @@ public String obtainProp(int viewTag, String propName) {
}

switch (propName) {
case "opacity":
case "opacity" -> {
return Float.toString(view.getAlpha());
case "zIndex":
}
case "zIndex" -> {
return Float.toString(view.getElevation());
case "width":
}
case "width" -> {
return Float.toString(PixelUtil.toDIPFromPixel(view.getWidth()));
case "height":
}
case "height" -> {
return Float.toString(PixelUtil.toDIPFromPixel(view.getHeight()));
case "top":
}
case "top" -> {
return Float.toString(PixelUtil.toDIPFromPixel(view.getTop()));
case "left":
}
case "left" -> {
return Float.toString(PixelUtil.toDIPFromPixel(view.getLeft()));
case "backgroundColor":
}
case "backgroundColor" -> {
Drawable background = view.getBackground();
int actualColor = -1;

Expand All @@ -471,15 +475,16 @@ public String obtainProp(int viewTag, String propName) {
return "Unable to resolve background color";
}

String invertedColor = String.format("%08x", (0xFFFFFFFF & actualColor));
String invertedColor = String.format("%08x", actualColor);
// By default transparency is first, color second
return "#" + invertedColor.substring(2, 8) + invertedColor.substring(0, 2);

default:
}
default -> {
throw new IllegalArgumentException(
"[Reanimated] Attempted to get unsupported property "
+ propName
+ " with function `getViewProp`");
}
}
}

Expand All @@ -494,26 +499,13 @@ private static WritableArray copyReadableArray(ReadableArray array) {
for (int i = 0; i < array.size(); i++) {
ReadableType type = array.getType(i);
switch (type) {
case Boolean:
copy.pushBoolean(array.getBoolean(i));
break;
case String:
copy.pushString(array.getString(i));
break;
case Null:
copy.pushNull();
break;
case Number:
copy.pushDouble(array.getDouble(i));
break;
case Map:
copy.pushMap(copyReadableMap(array.getMap(i)));
break;
case Array:
copy.pushArray(copyReadableArray(array.getArray(i)));
break;
default:
throw new IllegalStateException("[Reanimated] Unknown type of ReadableArray.");
case Boolean -> copy.pushBoolean(array.getBoolean(i));
case String -> copy.pushString(array.getString(i));
case Null -> copy.pushNull();
case Number -> copy.pushDouble(array.getDouble(i));
case Map -> copy.pushMap(copyReadableMap(array.getMap(i)));
case Array -> copy.pushArray(copyReadableArray(array.getArray(i)));
default -> throw new IllegalStateException("[Reanimated] Unknown type of ReadableArray.");
}
}
return copy;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static com.facebook.react.bridge.ReactMarkerConstants.CREATE_UI_MANAGER_MODULE_END;
import static com.facebook.react.bridge.ReactMarkerConstants.CREATE_UI_MANAGER_MODULE_START;

import androidx.annotation.NonNull;
import com.facebook.react.ReactApplication;
import com.facebook.react.ReactInstanceManager;
import com.facebook.react.ReactPackage;
Expand All @@ -21,16 +22,16 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;

@ReactModuleList(
nativeModules = {
ReanimatedModule.class,
ReanimatedUIManager.class,
})
public class ReanimatedPackage extends TurboReactPackage implements ReactPackage {

@Override
public NativeModule getModule(String name, ReactApplicationContext reactContext) {
public NativeModule getModule(String name, @NonNull ReactApplicationContext reactContext) {
if (name.equals(ReanimatedModule.NAME)) {
return new ReanimatedModule(reactContext);
}
Expand All @@ -49,7 +50,8 @@ public ReactModuleInfoProvider getReactModuleInfoProvider() {

final Map<String, ReactModuleInfo> reactModuleInfoMap = new HashMap<>();
for (Class<? extends NativeModule> moduleClass : moduleList) {
ReactModule reactModule = moduleClass.getAnnotation(ReactModule.class);
ReactModule reactModule =
Objects.requireNonNull(moduleClass.getAnnotation(ReactModule.class));

reactModuleInfoMap.put(
reactModule.name(),
Expand All @@ -58,17 +60,11 @@ public ReactModuleInfoProvider getReactModuleInfoProvider() {
moduleClass.getName(),
true,
reactModule.needsEagerInit(),
reactModule.hasConstants(),
reactModule.isCxxModule(),
BuildConfig.IS_NEW_ARCHITECTURE_ENABLED));
}

return new ReactModuleInfoProvider() {
@Override
public Map<String, ReactModuleInfo> getReactModuleInfos() {
return reactModuleInfoMap;
}
};
return () -> reactModuleInfoMap;
}

private UIManagerModule createUIManager(final ReactApplicationContext reactContext) {
Expand Down
Loading

0 comments on commit 9bb6d12

Please sign in to comment.