Skip to content

Conversation

@noacoohen
Copy link
Contributor

Tracked by RSDSO-19396

@noacoohen noacoohen requested a review from OhadMeir June 29, 2025 19:50
}
}

for( auto & sub : subdevices )
Copy link
Contributor

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.

Copy link
Contributor

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

std::vector<std::shared_ptr<processing_block_model>> const_effects;

bool uvmapping_calib_full = false;
hdr_model _hdr_model;
Copy link
Contributor

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;
Copy link
Contributor

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 } };
Copy link
Contributor

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)

ImGui::SetNextItemWidth( 100 );
int g = control.DepthGain;
if( ImGui::InputInt( "##gain", &g ) )
control.DepthGain = std::max( 1, std::min( g, 16384 ) );
Copy link
Contributor

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;
Copy link
Contributor

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?

Copy link
Contributor

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

Copy link
Contributor

@OhadMeir OhadMeir left a 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.

@OhadMeir OhadMeir merged commit 1b62d62 into realsenseai:development Jul 13, 2025
25 of 26 checks passed
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