Two ReactART TODOs implemented on Android

[tepamid]

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.

[ghost]

By analyzing the blame information on this pull request, we identified @spicyj and @emilsjolander to be potential reviewers.

[ghost]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

[ghost]

@tepamid updated the pull request - view changes

[ghost]

@tepamid updated the pull request - view changes

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[ghost]

@tepamid updated the pull request - view changes

[ghost]

@tepamid updated the pull request - view changes

[robclouth]

Hey, what's the status of this?

[robclouth]

I'm trying this code, and it seems that despite calling markChildrenUpdatesSeen, onCollectExtraUpdates is only called once for each surface at startup before the Surface has been created, resulting in nothing being drawn. Any ideas?

[facebook-github-bot]

@tepamid updated the pull request - view changes

[facebook-github-bot]

@tepamid updated the pull request - view changes

[foghina]

@tepamid did you get rid of the random IllegalArgumentExceptions?

[facebook-github-bot]

@tepamid updated the pull request - view changes

[tepamid]

@foghina I couldn't reproduce IllegalArgumentException myself, the code works fine. However in our production exception trackers (Raygun and Google Analytics) I rarely see that some users do face IllegalArgumentExceptions. Even more strange is that there are cases when the Surface is valid and can be successfully locked before drawing, but after drawing while unlocking the Surface IllegalArgumentException is thrown.

I googled this case and found a "holy-war" in stackoverflow whether adding try-catch around Surface locking-unlocking is a dirty hack or a reliability necessity. I personally tend to think it's a hack, but otherwise the app crashes which is even worse.

Finally, I ended with adding try-catch to handle random IllegalArgumentExceptions.

[foghina]

@tepamid nice, thanks for the summary. Do you know what happens when you catch? Does the app end up in a bad state, or does the next draw go through correctly?

[tepamid]

@foghina before I had a synthetic testcase which reproduced IllegalArgumentException in drawing on ARTSurface. An exception happened when I navigated between app screens and the screen containing ARTSurfare was unmounted. I suspect that native Android TextureView can be unmounted during drawing process putting surface handle (i.e. Canvas or Surface) into an invalid state even after the surface had been previously locked. So there won't be any more render cycles for this unmounted component and therefore lockCanvas attempts.

Also exceptions occur either in lockCanvas or unlockCanvas methods. And not during drawing shapes on a canvas.

[foghina]

Cool, thanks for the explanation! I'll go ahead and try to land this.

[facebook-github-bot]

Thanks for importing. If you are a Facebook employee, you can view this diff on Phabricator.