glTF 2.0 models dark on Android of Qualcomm platform

[verybigzhouhai]

Hi，
Recently I used cesium to work on the mobile side. I found that my 3dtile model will become very dark on the Android platform.
I found similar problems in issues. #6622 and #7403 ，But it seems that I have not found a solution.
I have done a lot of testing on this problem and found that the problem comes from the value of ellipsoid.radii.x in processPbrMaterials.js, but I found this to be a fixed value of 6378137.0 in the czm_getWgs84EllipsoidEC method. But I replaced ellipsoid.radii.x with a specific value, and the program works fine on mobile devices, so I don't think this is a problem caused by float precision. I found some relevant information here, struct will have problems when initializing on Android devices, KhronosGroup/WebGL#1302.

I tried to make the following modifications,Can display my model correctly on my phone。
I am not sure that this modification is the best. If there is a better way, please let me know, I am willing to continue working for it and contribute my code.

I wrote a test code that will display a green square on the pc, but will display a red square on Android, the phone processor is Snapdragon 845, the browser is chrome, Both tests failed on Xiaomi and Samsung phones.. But I tested it on Huawei phones.
pc：

xiaomi：

this is my test code：

<title>Struct constructor/initializers highp bug.</title> <script type="application/javascript"> "use strict"; var canvas = document.getElementById('canvas'); var gl = canvas.getContext('webgl'); gl.viewport(0, 0, canvas.width, canvas.height); gl.clearColor(0, 0, 1, 1); gl.clear(gl.COLOR_BUFFER_BIT);

    var vs = gl.createShader(gl.VERTEX_SHADER);
    var vs_code =`
        attribute vec4 pos;
        void main(){
            gl_Position = pos;
        }
    `;
    gl.shaderSource(vs, vs_code);
    gl.compileShader(vs);

    var fs = gl.createShader(gl.FRAGMENT_SHADER);
    var fs_code =`
        precision highp float;
        struct Test {
             vec3 color;
         };
         void main() {
             vec3 color = vec3( 6378137.0, 0.0, 0.0 );
             Test test;
             test.color = color;
             if (test.color.x == 6000000.0) {
                gl_FragColor = vec4( 1.0, 0.0, 0.0, 1.0 );
             } else {
                gl_FragColor = vec4( 0.0, 1.0, 0.0, 1.0 );
             }
         }
    `;
    gl.shaderSource(fs, fs_code);
    gl.compileShader(fs);

    var prg = gl.createProgram();
    gl.attachShader(prg, vs);
    gl.attachShader(prg, fs);
    gl.linkProgram(prg);
    gl.useProgram(prg);

    var posLocation = gl.getAttribLocation(prg, 'pos');
    gl.enableVertexAttribArray(posLocation);

    var vertex = new Float32Array([
        -0.8, -0.8, 0,
        -0.8, 0.8, 0,
        0.8, 0.8, 0,
        0.8, -0.8, 0
    ]);

    var index = new Uint8Array([0, 2, 1, 0, 3, 2]);

    var vertexBuffer = gl.createBuffer();
    gl.bindBuffer(gl.ARRAY_BUFFER, vertexBuffer);
    gl.bufferData(gl.ARRAY_BUFFER, vertex, gl.STATIC_DRAW);
    gl.vertexAttribPointer(posLocation, 3, gl.FLOAT, false, 0, 0);

    var indexBuffer = gl.createBuffer();
    gl.bindBuffer(gl.ELEMENT_ARRAY_BUFFER, indexBuffer);
    gl.bufferData(gl.ELEMENT_ARRAY_BUFFER, index, gl.STATIC_DRAW);
    gl.drawElements(gl.TRIANGLES, index.length, gl.UNSIGNED_BYTE, 0);

</script>

[hpinkos]

@lilleyse ?

[OmarShehata]

@verybigzhouhai thanks a lot of digging into this! I recently ran into this as well. It sounds like this is a driver issue (from Ken's comment KhronosGroup/WebGL#1302 (comment)) and we can at best work around it by avoiding the use of structs. I think this might be a good PR to open, and do the same thing you did for the ellipsoid radii but for all the struct parameters too and just remove the struct. Then we can confirm if it fixes the other linked issues as well.

I wonder if this affects other things too. Like the ground atmosphere structs:

https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Shaders/GroundAtmosphere.glsl#L55-L59

@bagnell does this sound like a good approach?

[verybigzhouhai]

@OmarShehata Are you now experiencing something that is affected by the struct, except for the model?

[OmarShehata]

@verybigzhouhai for my case it was specifically this New York 3D Tileset appearing darker on my Pixel 3 phone:

https://cesiumjs.org/Cesium/Build/Apps/Sandcastle/?src=3D%20Tiles%20Feature%20Picking.html

These buildings are Batched 3D Models, which are glTF models.

I was just suggesting that if it is indeed the struct issue I would expect it to occur anywhere else we use structs in our shaders, but I haven't found an example of this yet.

[verybigzhouhai]

@OmarShehata I hope to find some other examples.

[verybigzhouhai]

@OmarShehata There is another problem. Although this is a problem with structs, it is not a problem at all times. For example, if you change the void main method to the following image, the square with the correct color will be displayed on the Qualcomm platform, but I am No rules were found that would lead to errors, such as when the initialized value is greater or less than a certain value, it seems to be erratic.

[OmarShehata]

@verybigzhouhai this does seem bizarre. The only difference there is your vec3 has a 2.0 instead of 6378137.0 right?

I am planning on opening a pull request this week to see if removing the use of structs solves the issue in Cesium, unless you're planning on opening one, in which case I'd be happy to review it.

[verybigzhouhai]

@OmarShehata yes,using 2.0 instead of 6378137.0 will give different results.
Do you mean to remove all the structs, as I did for the ellipsoid radii? but this will add a lot of variables.

[OmarShehata]

Do you mean to remove all the structs, as I did for the ellipsoid radii? but this will add a lot of variables.

I think it would make sense to just remove this struct first, since it sounds like it's not necessarily an issue with all structs. Let me know if you open a pull request for this and I'd be happy to review or provide feedback.

[verybigzhouhai]

@OmarShehata Ok, I will work on it as soon as possible.

[OmarShehata]

This just came up again on the forum. It's an easy fix that seems to be impacting a lot of people so I'm marking it as next release.

[hpinkos]

@OmarShehata are you planning on submitting a PR for this?

[OmarShehata]

@hpinkos yes.

[verybigzhouhai]

@OmarShehata I am sorry for my promise. For some reason I have delayed my work. In order to make up for my loss of trust, I have started my work immediately. I feel sorry again.I will submit a PR today.

[verybigzhouhai]

@OmarShehata I have submitted a request for this issue. #7911
For the use of other structures, no bad effects have been found yet. Do you think it is necessary to modify the use of all structs?

[verybigzhouhai]

new pr at #7944

[tfili]

@OmarShehata Can you take a look at this before the release?

[OmarShehata]

Yeah the PR referenced above does solve this issue, I'm just reviewing for code style and tests now, I expect it to be merged end of this week/weekend.

[OmarShehata]

This should be fixed with #7944, and will be out with CesiumJS 1.59 coming out next week.

Thanks again @verybigzhouhai !!

[cesium-concierge]

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/d/msg/cesium-dev/qYjFtc3Ktg8/cjc_JId9BwAJ

If this issue affects any of these threads, please post a comment like the following:

The issue at #7651 has just been closed and may resolve your issue. Look for the change in the next stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.