| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Add UI for HDR configurations #14104
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
Conversation
| } | ||
| } | ||
|
|
||
| for( auto & sub : subdevices ) |
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.
When you open HDR configuration window than, while it stays open, you select the menu the config window gets messed up. All open items are dark (like disabled), new items open up, and they are connected. Minimizing the old "Preset Item 1" also minimizes the new and so on...
I say when HDR config window is open allow interactions only with it. User must "Apply" or "Cancel" before doing other things in the viewer.
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.
Than you can also remove these lines
common/subdevice-model.h
Outdated
| std::vector<std::shared_ptr<processing_block_model>> const_effects; | ||
|
|
||
| bool uvmapping_calib_full = false; | ||
| hdr_model _hdr_model; |
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.
Don't add public members. Should be private and declare public functions that hide implementation details. e.g. subdevice_model::supports_hdr()
| struct sub_preset | ||
| { | ||
| std::string id; | ||
| int iterations; |
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.
What is the use of this "iterations" field and what is the use of preset_item::iterations field?
| _default_config.SubPreset.iterations = 0; | ||
|
|
||
| const std::vector< std::pair< int, int > > defaults | ||
| = { { 1, 1 }, { 32, 1000 }, { 128, 100000 }, { 2, 2 }, { 232, 200 }, { 2128, 20000 } }; |
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.
Any explanation to the values? Do they suit every model? (At least any current D400 and D500)
common/hdr-model.cpp
Outdated
| ImGui::SetNextItemWidth( 100 ); | ||
| int g = control.DepthGain; | ||
| if( ImGui::InputInt( "##gain", &g ) ) | ||
| control.DepthGain = std::max( 1, std::min( g, 16384 ) ); |
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.
For D455 (and other models) gain range is 16-248. Can we get values from actual option?
| if( _hdr_supported ) | ||
| { | ||
| initialize_default_config(); | ||
| _current_config = _default_config; |
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.
Can we load actual device configuration?
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.
The comment was left pending, adding now so we can handle in future PR
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 like there are no bugs. Approving now, unresolved comments will be handled in a future PR.
Labels
2 participants
Tracked by RSDSO-19396