Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly handle POINTS for highlight logic #552

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

jdavidberger
Copy link

Public API Changes
None

Description
This adds POINTS buffers to the visibility logic for rendering highlights.

Likely this isn't noticeable typically outside of performance impacts, but it is noticeable when you are using a custom ShaderMaterial.

@jdavidberger
Copy link
Author

It's possible this fixes #466

@MatthijsBurgh
Copy link
Contributor

Please drop the changes in the build files. CI will update them.

Copy link
Contributor

@MatthijsBurgh MatthijsBurgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Highlighter.js on line 88/89, the docstring needs to be updated.

@MatthijsBurgh
Copy link
Contributor

Do we have any test/example which shows that this indeed needed. I don't see why it fixes #466, could you please elaborate?

@jdavidberger
Copy link
Author

The code that triggered this is in a private repo but I can paste in the snippets that cause it. This was from a while ago so I might be misrecalling details but here goes:

IIRC the highlight logic attempts to highlight point renders but doesn't actually show on screen most of the time with the default shader.

However, if you have custom shader -- especially one that discards points -- the highlight shows up and is sorta jarring. I think it's somewhat hardware dependent too and that is why it was visible on ios.

Here is the shader that I used to trigger the issue:

var vertexShader = `
    uniform float size;
    uniform float max_depth;
    varying float color_key;
    void main() {
      color_key = position.z / max_depth;
      gl_PointSize = clamp(size, 0., 128.);
      gl_Position = projectionMatrix * modelViewMatrix * vec4(position,1.0);
    }
  `;
var fragmentShader = `
    float colormap_red(float x) {
    if (x < 0.7) {
        return 4.0 * x - 1.5;
    } else {
        return -4.0 * x + 4.5;
    }
}

float colormap_green(float x) {
    if (x < 0.5) {
        return 4.0 * x - 0.5;
    } else {
        return -4.0 * x + 3.5;
    }
}

float colormap_blue(float x) {
    if (x < 0.3) {
       return 4.0 * x + 0.5;
    } else {
       return -4.0 * x + 2.5;
    }
}

vec4 colormap(float y) {
    float x = clamp(y, 0.0, 1.0);
    float r = clamp(colormap_red(x), 0.0, 1.0);
    float g = clamp(colormap_green(x), 0.0, 1.0);
    float b = clamp(colormap_blue(x), 0.0, 1.0);
    return vec4(r, g, b, 1.0);
}
    varying float color_key;
    void main() {
      if (dot(gl_PointCoord-0.5,gl_PointCoord-0.5)>0.25 || color_key <= 0.0) {
        discard;       
      } else {
        gl_FragColor = colormap(color_key);
      }
    }
  `;

 this.shaderMaterial = new THREE.ShaderMaterial( {
            uniforms: {max_depth: {value: 3.0} , size: {value: 3}},
            vertexShader:   vertexShader,
            fragmentShader: fragmentShader,
            transparent:  true
        });
 this.sub = new PointCloud2({
            ros:ros,
            tfClient: this.tfClient,
            rootObject: this.viewer.scene,
            topic: this.props.topic,
            max_pts: 1000000,
            material: this.shaderMaterial// {size: 0.01, color: 0xFF00FF, vertexShader: vertexShader, fragmentShader: fragmentShader }
        });

So this applies a jet coloring and if the point is too far away it drops it. Dropped points get rendered by the highlighter and show up.

I'm preparing to travel right now and don't have time to put together a full example, but hopefully this makes it clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants