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

Feature/reflectivity #341

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Feature/reflectivity #341

wants to merge 6 commits into from

Conversation

PiotrMrozik
Copy link
Collaborator

This pull request introduces the concept of reflectivity to the RaytraceNode, allowing for the calculation and configuration of reflectivity values in ray-tracing operations.

Reflectivity Feature Addition:

  • Added RGL_FIELD_REFLECTIVITY_F32 to the rgl_field_t.
  • Introduced the function rgl_node_raytrace_configure_reflectivity_alpha to configure reflectivity alpha values for RaytraceNode
  • Modified GPU kernels to handle reflectivity calculations and storage.

Testing:

  • Added a new test file reflectivityTest.cpp to cover the new reflectivity functionality.

@PiotrMrozik PiotrMrozik requested a review from prybicki December 4, 2024 17:28
/**
* Modifies RaytraceNode to set reflectivity alpha.
* Reflectivity alpha is used to calculate reflectivity of the hit point. This value is constant for every hit point.
* Default reflectivity alpha is set to 0.1f.
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the reason behind this 0.1f choice?

Comment on lines +130 to +132
if (ctx->reflectivityF32 != nullptr) {
ctx->reflectivityF32[returnPointIdx] = ctx->mrSamples.reflectivity[sampleIdx];
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here you can use selected sample to compute reflectivity. This way reflectivity can be removed from multi-return samples to save memory.
This suggestion was NOT COMPILED.

Suggested change
if (ctx->reflectivityF32 != nullptr) {
ctx->reflectivityF32[returnPointIdx] = ctx->mrSamples.reflectivity[sampleIdx];
}
if (ctx->reflectivityF32 != nullptr) {
auto distance2 = ctx->mrSamples.intensity[sampleIdx] * ctx->mrSamples.intensity[sampleIdx];
ctx->reflectivityF32[returnPointIdx] = reflectivityAlpha * ctx->mrSamples.intensity[sampleIdx] * distance2;
}

@@ -27,6 +27,7 @@ struct MultiReturnSamplesPointers
Field<IS_HIT_I32>::type* isHit;
Field<DISTANCE_F32>::type* distance;
Field<INTENSITY_F32>::type* intensity;
Field<REFLECTIVITY_F32>::type* reflectivity;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove


TEST_P(ReflectivityTest, read_value)
{
auto [alpha, value] = GetParam();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please parametrize also distance.

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