Math.floor operation on MeasureOutput for Java and C# #296

New Issue

2016-12-22T10:42:16-08:00

rozele commented

2016-12-22 10:42:16 -08:00

(Migrated from github.com)

I just wanted to check if this behavior was by-design before I made any PR. In react-native-windows, we have to do Math.Ceiling on all of our measure function outputs because casting from float/double to int always does a floor operation:

https://github.com/facebook/yoga/blob/master/csharp/Facebook.Yoga/MeasureOutput.cs#L16
https://github.com/facebook/yoga/blob/master/java/com/facebook/yoga/YogaMeasureOutput.java#L18

I imagine this isn't a problem for react-native on Android because they don't encounter fractional pixel dimensions?

I just wanted to check if this behavior was by-design before I made any PR. In react-native-windows, we have to do Math.Ceiling on all of our measure function outputs because casting from float/double to int always does a floor operation: https://github.com/facebook/yoga/blob/master/csharp/Facebook.Yoga/MeasureOutput.cs#L16 https://github.com/facebook/yoga/blob/master/java/com/facebook/yoga/YogaMeasureOutput.java#L18 I imagine this isn't a problem for react-native on Android because they don't encounter fractional pixel dimensions?

woehrl01 commented

2016-12-22 11:30:35 -08:00

(Migrated from github.com)

Nice catch! But I think we should not use any rounding logic as those values are treated as float in the c part.

For Java we could instead use:

  public static long make(float width, float height) {
    return make(Float.floatToRawIntBits(width), Float.floatToRawIntBits(height));
  }

In C#:

        public static long Make(double width, double height)
        {
            int widthInt = BitConverter.ToInt32(BitConverter.GetBytes((float)width), 0);
            int heightInt = BitConverter.ToInt32(BitConverter.GetBytes((float)height), 0);
            return Make(widthInt, heightInt);
        }

As this is a little cumbersome on C# maybe its better to use directly a struct with two floats (see YGSize)?

This additionally prevents us from a bug if we calculate a size bigger than 16777216 as this would overflow into the exponent part.

Nice catch! But I think we should not use any rounding logic as those values are treated as float in the c part. For Java we could instead use: ```java public static long make(float width, float height) { return make(Float.floatToRawIntBits(width), Float.floatToRawIntBits(height)); } ``` In C#: ```csharp public static long Make(double width, double height) { int widthInt = BitConverter.ToInt32(BitConverter.GetBytes((float)width), 0); int heightInt = BitConverter.ToInt32(BitConverter.GetBytes((float)height), 0); return Make(widthInt, heightInt); } ``` As this is a little cumbersome on C# maybe its better to use directly a struct with two floats (see [YGSize](https://github.com/facebook/yoga/blob/master/yoga/Yoga.h#L36))? This additionally prevents us from a bug if we calculate a size bigger than 16777216 as this would overflow into the exponent part.

woehrl01 commented

2016-12-25 02:05:59 -08:00

(Migrated from github.com)

@emilsjolander Java still needs to be changed/fixed.

emilsjolander commented

2016-12-25 02:45:06 -08:00

(Migrated from github.com)

@woehrl01 putting up an internal diff for java now. thanks!

emilsjolander commented

2016-12-29 05:03:06 -08:00

(Migrated from github.com)

352f592767