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.

Sign up for GitHub

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

Jump to bottom

refactor: simplify isClipped #4661

Merged
merged 2 commits into from
Aug 12, 2024
Merged

refactor: simplify isClipped #4661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e400773
refactor: simplify isClipped
haakonflatval-cognite Jul 11, 2024
a33ab3e
Merge branch 'master' into hflatval/simplify-isclipped
haakonflatval-cognite Aug 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion viewer/packages/rendering/src/glsl/base/isClipped.glsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
uniform vec4 clippingPlanes[NUM_CLIPPING_PLANES];
#endif

bool isClipped(NodeAppearance nodeAppearance, vec3 point) {
bool isClipped(vec3 point) {
#if NUM_CLIPPING_PLANES > 0
vec3 pointFlipped = -point;
vec4 plane;
Copy link
Contributor

@nilscognite nilscognite Jul 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The lines below (in the unchanged code) sucks in my eyes! Why not simplify with early return and remove clipped? Cannot understand why this code has been approved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hehe, not sure why.. It could be that they were afraid that shader threads running in parallel would diverge (one returns early, another one doesn't), which could cause some unfortunate performance loss, but not sure if that's a real concern or not 😅

Expand Down
26 changes: 1 addition & 25 deletions viewer/packages/rendering/src/glsl/pointcloud/pointcloud.vert
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ precision highp float;
precision highp int;

#pragma glslify: import('../base/pointSizeRelativeToScreen.glsl');
#pragma glslify: import('../base/isClipped.glsl');

#define max_clip_boxes 30

Expand Down Expand Up @@ -45,31 +46,6 @@ uniform sampler2D gradient;
uniform sampler2D classificationLUT;
uniform sampler2D objectIdLUT;

#if NUM_CLIPPING_PLANES > 0
uniform vec4 clippingPlanes[NUM_CLIPPING_PLANES];
#endif

bool isClipped(vec3 point) {
#if NUM_CLIPPING_PLANES > 0
vec3 pointFlipped = -point;
vec4 plane;

for (int i = 0; i < UNION_CLIPPING_PLANES; i++) {
plane = clippingPlanes[i];
if (dot(pointFlipped, plane.xyz) > plane.w) return true;
}
#if UNION_CLIPPING_PLANES < NUM_CLIPPING_PLANES
bool clipped = true;
for ( int i = UNION_CLIPPING_PLANES; i < NUM_CLIPPING_PLANES; i ++ ) {
plane = clippingPlanes[ i ];
clipped = (dot(pointFlipped, plane.xyz) > plane.w) && clipped;
}
if ( clipped ) return true;
#endif
#endif
return false;
}

out vec3 vColor;

#if defined(weighted_splats)
Expand Down
2 changes: 1 addition & 1 deletion viewer/packages/rendering/src/glsl/sector/instancedMesh.frag
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ void main()
{
highp float treeIndex = unpackTreeIndex(v_treeIndexPacked);
NodeAppearance appearance = nodeAppearanceFromTexel(v_nodeAppearanceTexel);
if (isClipped(appearance, v_viewPosition)) {
if (isClipped(v_viewPosition)) {
discard;
}

Expand Down
2 changes: 1 addition & 1 deletion viewer/packages/rendering/src/glsl/sector/mesh.frag
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ void main()
{
highp float v_treeIndex = unpackTreeIndex(v_treeIndexPacked);
NodeAppearance appearance = nodeAppearanceFromTexel(v_nodeAppearanceTexel);
if (isClipped(appearance, v_viewPosition)) {
if (isClipped(v_viewPosition)) {
discard;
}

Expand Down
2 changes: 1 addition & 1 deletion viewer/packages/rendering/src/glsl/sector/primitives/circle.frag
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ void main()
discard;

NodeAppearance appearance = nodeAppearanceFromTexel(v_nodeAppearanceTexel);
if (isClipped(appearance, vViewPosition)) {
if (isClipped(vViewPosition)) {
discard;
}

Expand Down
4 changes: 2 additions & 2 deletions viewer/packages/rendering/src/glsl/sector/primitives/cone.frag
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ void main()
if (intersectionPoint.z <= 0.0 ||
intersectionPoint.z > height ||
theta > v_angle + v_arcAngle ||
isClipped(appearance, p) ||
isClipped(p) ||
rayTargetDist + dist < 0.0
) {
// Missed the first point, check the other point
Expand All @@ -124,7 +124,7 @@ void main()
if (intersectionPoint.z <= 0.0 ||
intersectionPoint.z > height ||
theta > v_angle + v_arcAngle ||
isClipped(appearance, p) ||
isClipped(p) ||
rayTargetDist + dist < 0.0
) {
// Missed the other point too
Expand Down
4 changes: 2 additions & 2 deletions viewer/packages/rendering/src/glsl/sector/primitives/eccentricCone.frag
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ void main()

if (intersectionPointZ <= 0.0 ||
intersectionPointZ >= L ||
isClipped(appearance, p) ||
isClipped(p) ||
rayTargetDist + dist < 0.0
) {
// Either intersection point is behind starting point (happens inside the cone),
Expand All @@ -115,7 +115,7 @@ void main()

if (intersectionPointZ <= 0.0 ||
intersectionPointZ >= L ||
isClipped(appearance, p) ||
isClipped(p) ||
rayTargetDist + dist < 0.0
) {
// Missed the other point too
Expand Down
4 changes: 2 additions & 2 deletions viewer/packages/rendering/src/glsl/sector/primitives/ellipsoidSegment.frag
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ void main()

if (intersectionPointZ <= vRadius - height ||
intersectionPointZ > vRadius ||
isClipped(appearance, p)
isClipped(p)
) {
// Missed the first point, check the other point

Expand All @@ -90,7 +90,7 @@ void main()
p = rayTarget + dist * rayDirection;
if (intersectionPointZ <= vRadius - height ||
intersectionPointZ > vRadius ||
isClipped(appearance, p)
isClipped(p)
) {
// Missed the other point too
discard;
Expand Down
4 changes: 2 additions & 2 deletions viewer/packages/rendering/src/glsl/sector/primitives/generalCylinder.frag
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ void main()
if (dot(intersectionPoint - planeACenter, planeANormal) > 0.0 ||
dot(intersectionPoint - planeBCenter, planeBNormal) > 0.0 ||
theta > v_angles[1] + v_angles[0] ||
isClipped(appearance, p) ||
isClipped(p) ||
rayTargetDist + dist < 0.0
) {
// Missed the first point, check the other point
Expand All @@ -102,7 +102,7 @@ void main()
p = rayTarget + dist*rayDirection;
if (dot(intersectionPoint - planeACenter, planeANormal) > 0.0 ||
dot(intersectionPoint - planeBCenter, planeBNormal) > 0.0 ||
theta > v_angles[1] + v_angles[0] || isClipped(appearance, p) ||
theta > v_angles[1] + v_angles[0] || isClipped(p) ||
rayTargetDist + dist < 0.0
) {
// Missed the other point too
Expand Down
2 changes: 1 addition & 1 deletion viewer/packages/rendering/src/glsl/sector/primitives/generalring.frag
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ void main()
// Redo appearance texture lookup from vertex shader due to limit in transferable attributes
NodeAppearance appearance = determineNodeAppearance(colorDataTexture, treeIndexTextureSize, v_treeIndex);

if (isClipped(appearance, vViewPosition)) {
if (isClipped(vViewPosition)) {
discard;
}

Expand Down
2 changes: 1 addition & 1 deletion viewer/packages/rendering/src/glsl/sector/primitives/torusSegment.frag
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ void main()
{
highp float v_treeIndex = unpackTreeIndex(v_treeIndexPacked);
NodeAppearance appearance = nodeAppearanceFromTexel(v_nodeAppearanceTexel);
if (isClipped(appearance, vViewPosition)) {
if (isClipped(vViewPosition)) {
discard;
}

Expand Down
2 changes: 1 addition & 1 deletion viewer/packages/rendering/src/glsl/sector/primitives/trapezium.frag
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ void main()
{
highp float v_treeIndex = unpackTreeIndex(v_treeIndexPacked);
NodeAppearance appearance = nodeAppearanceFromTexel(v_nodeAppearanceTexel);
if (isClipped(appearance, vViewPosition)) {
if (isClipped(vViewPosition)) {
discard;
}

Expand Down
Loading