Fix NullPointerException when calling getBBox

[mjmasn]

Summary

When calling getBBox() inside an onLayout callback of a <Rect />, we were getting a exception thrown:
NullPointerException: Attempt to read from field 'float android.graphics.RectF.left' on a null object reference.

Test Plan

What's required for testing (prerequisites)?

What are the steps to reproduce (after prerequisites)?

Compatibility

OS	Implemented
iOS	n/a
Android	✅

Checklist

• I have tested this on a device
• and a simulator
• I added the documentation in README.md
• I mentioned this change in CHANGELOG.md
• I updated the typed files (TS and Flow)
• I added a sample use of the API in the example project (example/App.js)

[mjmasn]

Added a fix for a crash when calling getBBox on <Text /> component in an onLayout callback

[msand]

Thanks for working on this! Have you validated the values from getBBox? Do they make sense to you? I've barely done any testing of that api so it's very much experimental:
https://github.com/react-native-community/react-native-svg/blob/099439ca9f7efb946f553ccc2eea8ae51feb1b62/src/elements/Shape.tsx#L267-L282

[mjmasn]

@msand I haven't, but I think because onLayout (and therefore getBBox) is called multiple times in our situation, the last value at least seems ok.

But actually the text fixes are causing a lot of crashes. Doesn't trigger the usual red screen in debug mode but adb logcat shows JNI DETECTED ERROR IN APPLICATION: field operation on NULL object: 0x0 which is making it a lot harder to debug. I'm looking into it now.

[mjmasn]

@msand just to follow on from the above, it looks like the crashes only occur if calling getBBox from a ref callback something along the lines of the example below. Looking at it I don't think it's necessary to call this.onTextLayout() from setTextRef*, and that does stop the crashing with my patch applied to react-native-svg. So this PR is still an improvement IMO, but of course my depth of knowledge here is pretty shallow so I can't say it definitely doesn't make anything worse.

* Although it would still be nice to understand why this crashes, I would have expected it to return 0.

class A extends React.Component {
  constructor(props) {
    super(props);

    this.state = {
      textWidth: 0,
    }
  }

  setTextRef = (node) => {
    this.textRef = node;

    if (node) {
      this.onTextLayout();
    } else {
      this.setState({
        textWidth: 0
      });
    }
  }

  onTextLayout = () => {
    if (this.textRef) {
      const {width} = this.textRef.getBBox();
      this.setState({
        textWidth: width
      });
    }
  }

  render() {
    return (
      <Text ref={this.setTextRef} onLayout={this.onTextLayout}>
        <TSpan>Some</TSpan> Text
      </Text>
    );
  }
}

[msand]

Have you tried returning new Path() instead of null in the text fixes?

[dlockwo]

I am encountering this issue as well. Is the PR waiting on further updates?

[mjmasn]

@msand I think I may have tried that at some point, may have had issues as the values are cached in some places. Can't remember exactly what I tried though.

@dlockwo It could probably be improved but we have been using this fix in production for just over a month and it seems pretty solid.

[msand]

I've made a slightly different fix in the develop branch, would you mind testing that?

[dlockwo]

I pulled and linked the develop branch to test. For my use case I call getBBox within the onLayout callback of the Text component. The following error is display onscreen:
"Attempt to invoke virtual method 'double java.lang.Double.doubleValue()' on a null object reference is display on screen."
The following error is thrown by getBBox():
:Error: Exception in HostFunction: java.lang.NullPointerException: Attempt to read from field 'float android.graphics.RectF.left' on a null object reference"

[msand]

At least for me using the develop branch and this code:

import React, { Component } from "react";
import { View, StyleSheet } from "react-native";
import { Svg, Text, TSpan } from "react-native-svg";

class A extends React.Component {
  constructor(props) {
    super(props);

    this.state = {
      textWidth: 0
    };
  }

  setTextRef = node => {
    this.textRef = node;

    if (node) {
      this.onTextLayout();
    } else {
      this.setState({
        textWidth: 0
      });
    }
  };

  onTextLayout = () => {
    if (this.textRef) {
      const { width } = this.textRef.getBBox();
      console.log({ width });
      this.setState({
        textWidth: width
      });
    }
  };

  render() {
    return (
      <Text y="20" ref={this.setTextRef} onLayout={this.onTextLayout}>
        <TSpan>Some</TSpan> Text
      </Text>
    );
  }
}

export default class App extends Component {
  render() {
    return (
      <View style={styles.container}>
        <Svg width={"100%"} height={"100%"}>
          <A />
        </Svg>
      </View>
    );
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: "center",
    backgroundColor: "#ecf0f1",
    padding: 8
  }
});

I get this output in android studio:

I/ReactNativeJS: Running "BareMinimum" with {"rootTag":1}
W/com.bareminimu: Accessing hidden method Landroid/text/SpannableStringInternal;->length()I (light greylist, linking)
W/com.bareminimu: Accessing hidden field Landroid/view/View;->mAccessibilityDelegate:Landroid/view/View$AccessibilityDelegate; (light greylist, reflection)
I/ReactNativeJS: { width: undefined }
D/EGL_emulation: eglMakeCurrent: 0xd5a329c0: ver 3 0 (tinfo 0xd5915a80)
I/ReactNativeJS: { width: 55.89713668823242 }
I/ReactNativeJS: { width: 55.89713668823242 }

[msand]

Alternatively you might want something like this: #1231 (comment)

[mjmasn]

@msand looks very promising in that case, the ref callback calling onTextLayout I think was still crashing with my PR. I'll hopefully get a chance to try your fix over the weekend.

I remember seeing the issue @dlockwo is seeing while developing my PR. Can't remember what was causing it though.

[msand]

Perhaps we can close this now? v11 has been released with the fix