Skip to content

Commit

Permalink
Two ReactART TODOs implemented on Android
Browse files Browse the repository at this point in the history
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
tepamid authored and Facebook Github Bot committed Oct 18, 2016
1 parent 8acf1a0 commit d294e15
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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();

This comment has been minimized.

Copy link
@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.

This comment has been minimized.

Copy link
@magicismight

magicismight Nov 14, 2016

Contributor

This comment has been minimized.

Copy link
@tepamid

tepamid Nov 14, 2016

Author Contributor

@magicismight is the problem caused by this pull request?

This comment has been minimized.

Copy link
@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.

This comment has been minimized.

Copy link
@tepamid

tepamid Nov 15, 2016

Author 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.

This comment has been minimized.

Copy link
@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.

This comment has been minimized.

Copy link
@tepamid

tepamid Nov 16, 2016

Author Contributor

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

This comment has been minimized.

Copy link
@tepamid

tepamid Dec 7, 2016

Author 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 ?

This comment has been minimized.

Copy link
@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.