Permalink
Browse files

Two ReactART TODOs implemented on Android

Summary:
Implemented 2 TODOs from ReactART for Android:
- TODO(7255985): Use TextureView and pass Surface from the view to draw on it asynchronously instead of passing the bitmap (which is inefficient especially in terms of memory usage)
- TODO(6352067): Support dashes in ARTShape

We use ReactNativeART in our Android project.
1. Our app crashes sometimes on large screen smartphones with OutOfMemoryError. Crashes happen in ARTSurfaceShadowNode where TODO(7255985) was suggested in a comment in order to use memory more efficiently.
2. We needed dashes for drawing on ARTSurface.

**Test plan (required)**

I attach a screenshot of our app which shows dashed-lines and two ARTSurfaces on top of each other rendering exactly the same as in the pervious implementation of ARTSurface.
![screenshot_2016-08-19-16-45-43](https://cloud.githubusercontent.com/assets/18415611/17811741/cafc35c4-662c-11e6-8a63-7c35ef1c5ba9.png)
Closes #9486

Differential Revision: D4021303

Pulled By: foghina

fbshipit-source-id: 880175e841e3c598013982a7748b6fc691c7e8d6
  • Loading branch information...
1 parent 8acf1a0 commit d294e15c43a6222f25953dae8189d0e220dbdf7e @tepamid tepamid committed with Facebook Github Bot Oct 18, 2016
@@ -15,6 +15,7 @@
import android.graphics.Paint;
import android.graphics.Path;
import android.graphics.RectF;
+import android.graphics.DashPathEffect;
import com.facebook.common.logging.FLog;
import com.facebook.react.bridge.JSApplicationIllegalArgumentException;
@@ -158,8 +159,7 @@ protected boolean setupStrokePaint(Paint paint, float opacity) {
(int) (mStrokeColor[1] * 255),
(int) (mStrokeColor[2] * 255));
if (mStrokeDash != null && mStrokeDash.length > 0) {
- // TODO(6352067): Support dashes
- FLog.w(ReactConstants.TAG, "ART: Dashes are not supported yet!");
+ paint.setPathEffect(new DashPathEffect(mStrokeDash, 0));
}
return true;
}
@@ -9,37 +9,15 @@
package com.facebook.react.views.art;
-import javax.annotation.Nullable;
-
import android.content.Context;
-import android.graphics.Bitmap;
-import android.graphics.Canvas;
-import android.view.View;
+import android.view.TextureView;
/**
* Custom {@link View} implementation that draws an ARTSurface React view and its children.
*/
-public class ARTSurfaceView extends View {
-
- private @Nullable Bitmap mBitmap;
-
+public class ARTSurfaceView extends TextureView {
public ARTSurfaceView(Context context) {
super(context);
- }
-
- public void setBitmap(Bitmap bitmap) {
- if (mBitmap != null) {
- mBitmap.recycle();
- }
- mBitmap = bitmap;
- invalidate();
- }
-
- @Override
- protected void onDraw(Canvas canvas) {
- super.onDraw(canvas);
- if (mBitmap != null) {
- canvas.drawBitmap(mBitmap, 0, 0, null);
- }
+ setOpaque(false);
}
}
@@ -9,8 +9,6 @@
package com.facebook.react.views.art;
-import android.graphics.Bitmap;
-
import com.facebook.csslayout.CSSMeasureMode;
import com.facebook.csslayout.CSSNodeAPI;
import com.facebook.csslayout.MeasureOutput;
@@ -63,6 +61,6 @@ protected ARTSurfaceView createViewInstance(ThemedReactContext reactContext) {
@Override
public void updateExtraData(ARTSurfaceView root, Object extraData) {
- root.setBitmap((Bitmap) extraData);
+ root.setSurfaceTextureListener((ARTSurfaceViewShadowNode) extraData);
}
}
@@ -9,17 +9,29 @@
package com.facebook.react.views.art;
-import android.graphics.Bitmap;
+import javax.annotation.Nullable;
+
import android.graphics.Canvas;
import android.graphics.Paint;
+import android.graphics.Color;
+import android.view.Surface;
+import android.graphics.PorterDuff;
+import android.graphics.SurfaceTexture;
+import android.view.TextureView;
+import com.facebook.common.logging.FLog;
+import com.facebook.react.common.ReactConstants;
import com.facebook.react.uimanager.LayoutShadowNode;
import com.facebook.react.uimanager.UIViewOperationQueue;
+import com.facebook.react.uimanager.ReactShadowNode;
/**
* Shadow node for ART virtual tree root - ARTSurfaceView
*/
-public class ARTSurfaceViewShadowNode extends LayoutShadowNode {
+public class ARTSurfaceViewShadowNode extends LayoutShadowNode
+ implements TextureView.SurfaceTextureListener {
+
+ private @Nullable Surface mSurface;
@Override
public boolean isVirtual() {
@@ -34,23 +46,61 @@ public boolean isVirtualAnchor() {
@Override
public void onCollectExtraUpdates(UIViewOperationQueue uiUpdater) {
super.onCollectExtraUpdates(uiUpdater);
- uiUpdater.enqueueUpdateExtraData(getReactTag(), drawOutput());
+ drawOutput();
+ uiUpdater.enqueueUpdateExtraData(getReactTag(), this);
}
- private Object drawOutput() {
- // TODO(7255985): Use TextureView and pass Surface from the view to draw on it asynchronously
- // instead of passing the bitmap (which is inefficient especially in terms of memory usage)
- Bitmap bitmap = Bitmap.createBitmap(
- (int) getLayoutWidth(),
- (int) getLayoutHeight(),
- Bitmap.Config.ARGB_8888);
- Canvas canvas = new Canvas(bitmap);
- Paint paint = new Paint();
- for (int i = 0; i < getChildCount(); i++) {
- ARTVirtualNode child = (ARTVirtualNode) getChildAt(i);
- child.draw(canvas, paint, 1f);
+ private void drawOutput() {
+ if (mSurface == null || !mSurface.isValid()) {
+ markChildrenUpdatesSeen(this);
+ return;
+ }
+
+ try {
+ Canvas canvas = mSurface.lockCanvas(null);
+ canvas.drawColor(Color.TRANSPARENT, PorterDuff.Mode.CLEAR);
+
+ Paint paint = new Paint();
+ for (int i = 0; i < getChildCount(); i++) {
+ ARTVirtualNode child = (ARTVirtualNode) getChildAt(i);
+ child.draw(canvas, paint, 1f);
+ child.markUpdateSeen();
+ }
+
+ if (mSurface == null) {
+ return;
+ }
+
+ mSurface.unlockCanvasAndPost(canvas);
+ } catch (IllegalArgumentException | IllegalStateException e) {
+ FLog.e(ReactConstants.TAG, e.getClass().getSimpleName() + " in Surface.unlockCanvasAndPost");
+ }
+ }
+
+ private void markChildrenUpdatesSeen(ReactShadowNode shadowNode) {
+ for (int i = 0; i < shadowNode.getChildCount(); i++) {
+ ReactShadowNode child = shadowNode.getChildAt(i);
child.markUpdateSeen();
+ markChildrenUpdatesSeen(child);
}
- return bitmap;
}
+
+ @Override
+ public void onSurfaceTextureAvailable(SurfaceTexture surface, int width, int height) {
+ mSurface = new Surface(surface);
+ drawOutput();
@magicismight
magicismight Nov 14, 2016 Contributor

ARTSurfaceView will be re-rendered every time it is scrolled out and into the sight.
If put the ART icons into a ListView it will make the ListView drop frames.
Is there anyway to prevent this?
Do not destroy the surface every time it is outside the screen.

@tepamid
tepamid Nov 14, 2016 Contributor

@magicismight is the problem caused by this pull request?

@magicismight
magicismight Nov 14, 2016 Contributor

Yep.
You can put some ART icons into a ListView.
And scroll the ListView will drop frames badly.
Because every time the TextureView is outside the screen it will call the onSurfaceTextureDestroyed,
and if theTextureViews are scrolled into the screen it will call onSurfaceTextureAvailable method to re-calculate the output again.

@tepamid
tepamid Nov 15, 2016 Contributor

@magicismight, looks like the issue need some research. I see the following open questions:

  1. Is SurfaceTexture destroyed because the corresponding React component is unmounted or it's a native Android TextureView's behaviour (but at the same time the very TextureView is still mounted)?
  2. "ART icons" is some popular react module, is it?
  3. I personally believe ARTSurfaceView is supposed be used for high FPS rendering of moving shapes (as I do in my application). In your case you render a static picture on ARTSurfaceView. In your place I'd better render Icons in a vector font like FontAwesome.
@magicismight
magicismight Nov 16, 2016 Contributor
  1. The SurfaceTexture is destroyed when it is outside the screen.The React component is still mounted and so is the TextureView. I think it's a native Android behavior.
  2. "ART icons" I meant the <Surface></Surface> components with some <Shape /> components, I just used them as vector icons.
  3. In spite of the usage of the ARTSurfaceView. I'm not sure if this is a high-FPS way to re-render the Surface every time it is reappeared on the screen.
@tepamid
tepamid Nov 16, 2016 Contributor

@magicismight Ok, I'll see what can be done to speedup re-initialization.

@tepamid
tepamid Dec 7, 2016 edited Contributor

@magicismight, I did a fix to speed up ART surface initialisation. It helped to fix 2 issues in my app.

In meantime can you try to build React Native from sources together with this commit: tepamid@e361e9b ?

@magicismight
magicismight Jan 5, 2017 Contributor

@tepamid Everything works fine in 0.40.0

+ }
+
+ @Override
+ public boolean onSurfaceTextureDestroyed(SurfaceTexture surface) {
+ surface.release();
+ mSurface = null;
+ return true;
+ }
+
+ @Override
+ public void onSurfaceTextureSizeChanged(SurfaceTexture surface, int width, int height) {}
+
+ @Override
+ public void onSurfaceTextureUpdated(SurfaceTexture surface) {}
}

0 comments on commit d294e15

Please sign in to comment.