CesiumGS / cesium

An open-source JavaScript library for world-class 3D globes and maps :earth_americas:
https://cesium.com/cesiumjs/
Apache License 2.0
13.03k stars 3.51k forks source link

In CustomShader , normalized values are wrong when the componentType is "UINT8". #11092

Open insar-dev opened 1 year ago

insar-dev commented 1 year ago

I have a points cloud tileset which has a property named "velocity".
I normalized it and I set the property normalized to true. image

But In CustomShader, I got the wrong values. It seems that the normalized data is not divided by 255.0 before Cesium return result to user;

I got the velocity which was not between min and max;

image

I tested the velocity value; Its value was original scale + offset; But it should be original / 255.0 scale + offset;

test data: 3dtiles.zip

shader code:

float velocity = fsInput.metadata.velocity;

  if(velocity < 25.0)
  {
    material.diffuse = vec3(1.0, 0.0, 0.0);
    material.alpha = 0.9;
  }
  else
 {
    material.diffuse = vec3(0.0, 1.0, 0.0);
    material.alpha = 0.9;
 }

Sandcastle example:

Browser: edge

Operating System: win10

insar-dev commented 1 year ago

if I set "scale" to scale / 255.0, the result is right. And at this situation, Even if I set normalized to false, the result is still right.

ggetz commented 1 year ago

I believe normalized specifies that the data in the tiles is normalized, not if the values should be normalized at runtime.

insar-dev commented 1 year ago

@ggetz The data in the tile is normalized, but the denormalization in customshader is NOT right. The spec says " transformedValue = offset + scale * normalize(value) ". image

For UINT8 type, normalize(value) = value / 255.0. So transformedValue = offset + scale (value / 255.0); But actually transformedValue in customshader is : offset + scale value.

lilleyse commented 1 year ago

Yes, this seems like a bug.

One workaround (untested) is to set "normalized": true on the glTF accessor storing the velocity values.

For the actual fix, it would most likely go here. If the vertex attribute is already normalized then we don't need to do anything; otherwise we need to add some normalization code to the generated GLSL.

insar-dev commented 1 year ago

@lilleyse I use the first method. And it works. Thank you very much.

ggetz commented 1 year ago

Thanks @lilleyse and @insar-dev!

@insar-dev Would you have the bandwidth to contribute a Pull Request for the fix as @lilleyse suggested?

insar-dev commented 1 year ago

@ggetz I 'm sorry that I am not good at js.