-
Notifications
You must be signed in to change notification settings - Fork 51
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
Support setting horizontal and vertical resolution for GpuRays #229
Conversation
Signed-off-by: Nate Koenig <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #229 +/- ##
==========================================
+ Coverage 56.98% 57.01% +0.02%
==========================================
Files 151 151
Lines 15000 15010 +10
==========================================
+ Hits 8548 8558 +10
Misses 6452 6452
Continue to review full report at Codecov.
|
changes looks good to me. Waiting for Windows CI fix in #228 |
CI looks good. Just some windows warnings: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
I think that there needs to be some documentation of what the units of the resolution parameters are. I think it's supposed to be radians/increment, but I'm not 100% positive.
@osrf-jenkins retest this please |
Signed-off-by: Nate Koenig <[email protected]>
…robotics/ign-rendering into gpu_rays_resolution_fix_edifice
I added more documentation in f41df65. Resolution is a factor and unitless. |
Signed-off-by: Nate Koenig <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the doc, CI failure is unrelated.
This allows horizontal and vertical resolution to be set on GpuRays.
This problem was found in Citadel (see here), but applying a non-breaking change to Citadel proved difficult.
Signed-off-by: Nate Koenig [email protected]