splitLongitude produces NaN texture coordinates

[OmarShehata]

worldRectangle = scene.primitives.add(new Cesium.Primitive({
        geometryInstances : new Cesium.GeometryInstance({
            geometry : new Cesium.RectangleGeometry({
                rectangle : Cesium.Rectangle.fromDegrees(-180.0, -90.0, 180.0, 90.0),
                vertexFormat : Cesium.EllipsoidSurfaceAppearance.VERTEX_FORMAT
            })
        }),
        appearance : new Cesium.EllipsoidSurfaceAppearance({
            aboveGround : false
        }),
        show : false
    }));

I've discovered that the texture coordinates attribute has NaN's in it. If I put the following check in Source/Core/AttributeCompression.js everything works fine:

https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Core/AttributeCompression.js#L308-L316

if (isNaN(textureCoordinates.x) || isNaN(textureCoordinates.y)) {
       return 0;
}

[mramato]

May not be the same problem. I can run the tests, but the Materials demo crashes.

[likangning93]

Looks like under linux it just kind of freezes after a few frames.

[alx696]

Ubuntu 18.04 Chrome71 crash after the Globe show.

[mramato]

@lilleyse @bagnell this is a huge issue that we need to address ASAP. Who would be the best person to look at this?

[lilleyse]

@OmarShehata is going to look into this a bit today. It should be easy to identify which geometry or material is causing the problem in this demo.

[OmarShehata]

Preliminary results: I'm not sure why yet, but it seems to be having trouble with worldRectangle.

worldRectangle = scene.primitives.add(new Cesium.Primitive({
        geometryInstances : new Cesium.GeometryInstance({
            geometry : new Cesium.RectangleGeometry({
                rectangle : Cesium.Rectangle.fromDegrees(-180.0, -90.0, 180.0, 90.0),
                vertexFormat : Cesium.EllipsoidSurfaceAppearance.VERTEX_FORMAT
            })
        }),
        appearance : new Cesium.EllipsoidSurfaceAppearance({
            aboveGround : false
        }),
        show : false
    }));

If I change the line:

rectangle : Cesium.Rectangle.fromDegrees(-180.0, -90.0, 180.0, 90.0),

To

rectangle : Cesium.Rectangle.fromDegrees(-180.0, -80.0, 180.0, 80.0),

It works fine. Sandcastle with above modification.

[OmarShehata]

The other way to fix it is to comment out the appearance, so the following works:

worldRectangle = scene.primitives.add(new Cesium.Primitive({
        geometryInstances : new Cesium.GeometryInstance({
            geometry : new Cesium.RectangleGeometry({
                rectangle : Cesium.Rectangle.fromDegrees(-180.0, -90.0, 180.0, 90.0),
                vertexFormat : Cesium.EllipsoidSurfaceAppearance.VERTEX_FORMAT
            })
        }),
        //appearance : new Cesium.EllipsoidSurfaceAppearance({
        //    aboveGround : false
        //}),
        show : false
    }));

I've discovered that the texture coordinates attribute has NaN's in it. If I put the following check in Source/Core/AttributeCompression.js everything works fine:

https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Core/AttributeCompression.js#L308-L316

if (isNaN(textureCoordinates.x) || isNaN(textureCoordinates.y)) {
       return 0;
}

For some reason, this st attribute list contains NaN values:

https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Core/GeometryPipeline.js#L1403-L1412

[OmarShehata]

The issue seems specifically to be in the compressVertices step. Another way to make this Sandcastle work is simply to pass compressVertices : false to the worldPrimitive.

[hpinkos]

Cool, texture coordinate NaNs is a very fixable problem. I guess this is something previous versions of Chrome tolerated but this latest release does not.

[OmarShehata]

Yup! That's the good news. I was worried it was some very specific obscure Chrome 71 WebGL issue, but you can see the NaNs are present in Firefox as well, and it just doesn't crash there.

[hpinkos]

I can probably take a look at this next week if no one else gets to it first. The texture coordinate calculation happens here: https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Core/RectangleGeometryLibrary.js#L56-L68

[OmarShehata]

I was wrong about compressVertices. It gets NaN's before that, it's just that it only crashes during that step.

@hpinkos I just checked RectangleGeometryLibrary and the produces coordinates there are clean.

The NaNs are introduced in GeometryPipeline.splitLongitude(instances[i]); Commenting out this line (or just running the Sandcastle with scene3DOnly proves this is the culprit.

That also explains why it was fixed when we made it smaller (I think).

[OmarShehata]

@hpinkos if you're more familiar with that class, I can stop there and let you take it from here.

[hpinkos]

We have so many problems caused by that splitLongitude function. @OmarShehata keep poking around at it and let me know if you get stuck. My guess would be something's going weird in this function: https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Core/GeometryPipeline.js#L1919

[mramato]

They released a new minor version update for Chrome last night and it no longer crashes on my system. Maybe it was a regression on their end? Can others confirm?

I still maintain that Chrome 71 was garbage, this and #7388 seem to confirm that.

[OmarShehata]

@mramato it's still true that splitLongitude is spitting out NaN's that are being sent to the GPU in the texture coordinate attribute buffer. If it doesn't crash anymore it might not be as big of a priority but probably still something we should fix.

I'm guessing the crash was not intentionally on Chrome's side because the debug log says something like "Application tried to access memory and was denied" as opposed to "NaN values are not allowed here".

[hpinkos]

Re-targeted this issue to fix the texture coordinate computation in splitLongitude. You can see from the gif I posted in the first comment that it's doing something completely crazy. The numbers its generating aren't within the [0, 1] range.

[hpinkos]

Only seems to be a problem with GroundPrimitive. They didn't show up at all prior to 1.49. We fixed it for 1.49 with #6951 so they display but the texture coordinates are wrong.

[hpinkos]

Just kidding, the GroundPrimitive texture thing is unrelated to splitLongitude. I'm going to open a separate issue. We'll keep this one for fixing the NaN thing.