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

Add support for 8 bit thermal camera image format #235

Merged
merged 6 commits into from
Feb 10, 2021

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Feb 5, 2021

Summary

The existing thermal camera implementation produces image output in 16 bit format and that's being hard coded in various places including the shaders. This PR makes the implementation slightly more generic to support generating 8 bit data.

There are a few workarounds that had to be done so that this change does not break ABI. For example, the new thermal image event's function callback is hard coded to uint16_t and that can not be changed so unfortunately I had to workaround this by packing 8 bit data into 16 bit array.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)

Note to maintainers: Remember to use Squash-Merge


Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@iche033 iche033 added enhancement New feature or request 🔮 dome Ignition Dome labels Feb 5, 2021
@iche033 iche033 requested a review from adlarkin February 5, 2021 08:57
@osrf-triage osrf-triage added this to Inbox in Core development Feb 5, 2021
@codecov
Copy link

codecov bot commented Feb 5, 2021

Codecov Report

Merging #235 (8a5bf29) into ign-rendering4 (38d1ec0) will increase coverage by 0.12%.
The diff coverage is 100.00%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           ign-rendering4     #235      +/-   ##
==================================================
+ Coverage           52.75%   52.87%   +0.12%     
==================================================
  Files                 143      143              
  Lines               13493    13522      +29     
==================================================
+ Hits                 7118     7150      +32     
+ Misses               6375     6372       -3     
Impacted Files Coverage Δ
ogre2/src/Ogre2ThermalCamera.cc 90.70% <100.00%> (+0.82%) ⬆️
src/PixelFormat.cc 73.33% <0.00%> (+10.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 38d1ec0...8a5bf29. Read the comment docs.

Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Core development automation moved this from Inbox to In review Feb 5, 2021
ogre2/src/Ogre2ThermalCamera.cc Outdated Show resolved Hide resolved
ogre2/src/Ogre2ThermalCamera.cc Outdated Show resolved Hide resolved
ogre2/src/Ogre2ThermalCamera.cc Outdated Show resolved Hide resolved
test/integration/thermal_camera.cc Outdated Show resolved Hide resolved
Changelog.md Show resolved Hide resolved
ogre2/src/Ogre2ThermalCamera.cc Outdated Show resolved Hide resolved
ogre2/src/Ogre2ThermalCamera.cc Show resolved Hide resolved
ogre2/src/Ogre2ThermalCamera.cc Outdated Show resolved Hide resolved
ogre2/src/Ogre2ThermalCamera.cc Outdated Show resolved Hide resolved
ogre2/src/media/materials/programs/heat_signature_fs.glsl Outdated Show resolved Hide resolved
ogre2/src/media/materials/programs/thermal_camera_fs.glsl Outdated Show resolved Hide resolved
test/integration/thermal_camera.cc Outdated Show resolved Hide resolved
test/integration/thermal_camera.cc Show resolved Hide resolved
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@iche033 iche033 requested a review from ahcorde February 8, 2021 22:40
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Copy link
Contributor

@adlarkin adlarkin left a comment

Choose a reason for hiding this comment

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

All comments have been addressed, and all checks have passed - LGTM!

@nkoenig nkoenig merged commit 269f9ed into ign-rendering4 Feb 10, 2021
Core development automation moved this from In review to Done Feb 10, 2021
@nkoenig nkoenig deleted the thermal_8bit branch February 10, 2021 15:57
@adlarkin adlarkin mentioned this pull request Feb 10, 2021
4 tasks
@chapulina chapulina mentioned this pull request Feb 12, 2021
@j-rivero j-rivero removed this from Done in Core development May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔮 dome Ignition Dome enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants