Skip to content

Center button to center color scale range #25

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.

Sign up for GitHub

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

Open
cmorency1 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Center button to center color scale range #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 78 additions & 1 deletion src/wxNcVisFrame.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ enum {
ID_SAMPLER = 12,
ID_EXPORT = 13,
ID_COLORMAPINVERT = 14,
ID_RANGECENTERMINMAX = 15,
ID_VARSELECTOR = 100,
ID_DIMEDIT = 200,
ID_DIMDOWN = 300,
Expand All @@ -64,6 +65,7 @@ wxBEGIN_EVENT_TABLE(wxNcVisFrame, wxFrame)
EVT_TEXT_ENTER(ID_RANGEMIN, wxNcVisFrame::OnRangeChanged)
EVT_TEXT_ENTER(ID_RANGEMAX, wxNcVisFrame::OnRangeChanged)
EVT_BUTTON(ID_RANGERESETMINMAX, wxNcVisFrame::OnRangeResetMinMax)
EVT_BUTTON(ID_RANGECENTERMINMAX, wxNcVisFrame::OnRangeCenterMinMax)
EVT_BUTTON(ID_COLORMAPINVERT, wxNcVisFrame::OnColorMapInvertClicked)
EVT_COMBOBOX(ID_COLORMAP, wxNcVisFrame::OnColorMapCombo)
EVT_COMBOBOX(ID_GRIDLINES, wxNcVisFrame::OnGridLinesCombo)
Expand Down Expand Up @@ -1441,6 +1443,70 @@ void wxNcVisFrame::SetDataRangeByMinMax(

////////////////////////////////////////////////////////////////////////////////

void wxNcVisFrame::SetDataRangeCenteredOnZero(bool fRedraw) {
if (m_data.size() == 0) {
return;
}

float dDataMin = 0.0f;
float dDataMax = 0.0f;

int i;

if ((!m_fDataHasMissingValue) || (std::isnan(m_dMissingValueFloat))) {
for (i = 0; i < (int)m_data.size(); i++) {
if (!std::isnan(m_data[i])) {
break;
}
}
if (i == (int)m_data.size()) {
return; // no valid data
}

dDataMin = m_data[i];
dDataMax = m_data[i];

for (i++; i < (int)m_data.size(); i++) {
if (std::isnan(m_data[i])) {
continue;
}
if (m_data[i] < dDataMin) dDataMin = m_data[i];
if (m_data[i] > dDataMax) dDataMax = m_data[i];
}

} else {
for (i = 0; i < (int)m_data.size(); i++) {
if ((m_data[i] != m_dMissingValueFloat) && (!std::isnan(m_data[i]))) {
break;
}
}
if (i == (int)m_data.size()) {
return; // no valid data
}

dDataMin = m_data[i];
dDataMax = m_data[i];

for (i++; i < (int)m_data.size(); i++) {
if ((m_data[i] == m_dMissingValueFloat) || (std::isnan(m_data[i]))) {
continue;
}
if (m_data[i] < dDataMin) dDataMin = m_data[i];
if (m_data[i] > dDataMax) dDataMax = m_data[i];
}
}
Comment on lines +1446 to +1497
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

SetDataRangeCenteredOnZero() largely duplicates the min/max scan logic from SetDataRangeByMinMax(), but with slightly different edge-case handling. To reduce maintenance burden (and avoid the two implementations drifting), consider extracting a shared helper that computes (min,max) over valid data and reusing it from both functions.

Copilot uses AI. Check for mistakes.

// Center on 0: make range symmetric around zero
const float M = std::max(std::fabs(dDataMin), std::fabs(dDataMax));
if (!(M > 0.0f) || std::isnan(M)) {
Comment on lines +1499 to +1501
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

SetDataRangeCenteredOnZero() uses std::max but this file doesn't include directly. Relying on transitive includes can break portability; please add the appropriate standard header include for std::max (and keep math includes consistent with std::fabs/std::isnan usage).

Copilot uses AI. Check for mistakes.
return;
}

m_imagepanel->SetDataRange(-M, M, fRedraw);
Comment on lines +1501 to +1505
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

SetDataRangeCenteredOnZero() returns early when M == 0, which means for an all-zero field the "Center" button will do nothing and the range may remain non-centered (e.g., the default 0..1). Consider explicitly handling the degenerate case (min==max==0) by setting a small symmetric range around zero (or a fixed fallback range) instead of returning.

Suggested change
if (!(M > 0.0f) || std::isnan(M)) {
return;
}
m_imagepanel->SetDataRange(-M, M, fRedraw);
// If M is NaN, there is no valid magnitude to use.
if (std::isnan(M)) {
return;
}
// For valid data with zero magnitude (e.g., all-zero field), use a small
// symmetric fallback range around zero instead of doing nothing.
float rangeMax = M;
if (!(M > 0.0f)) {
rangeMax = 1.0f; // fallback half-range for degenerate all-zero fields
}
m_imagepanel->SetDataRange(-rangeMax, rangeMax, fRedraw);

Copilot uses AI. Check for mistakes.
}

////////////////////////////////////////////////////////////////////////////////

void wxNcVisFrame::SetDisplayedDimensionValue(
long lDim,
long lValue
Expand Down Expand Up @@ -1887,14 +1953,15 @@ void wxNcVisFrame::GenerateDimensionControls() {
varboundsminmax->Add(m_vecwxRange[0], 1, wxEXPAND | wxALL, 0);
varboundsminmax->Add(m_vecwxRange[1], 1, wxEXPAND | wxALL, 0);

m_vardimsizer->Add(new wxStaticText(this, -1, _T("")), 0, wxEXPAND | wxALL, 0);
m_vardimsizer->Add(new wxButton(this, ID_RANGECENTERMINMAX, _T("Center")), 0, wxEXPAND | wxALL, 0);
m_vardimsizer->Add(new wxStaticText(this, -1, _T("range"), wxDefaultPosition, wxDefaultSize, wxALIGN_CENTRE_HORIZONTAL | wxALIGN_CENTER_VERTICAL), 1, wxALIGN_CENTER_VERTICAL | wxEXPAND | wxALL, 4);
m_vardimsizer->Add(varboundsminmax, 0, wxEXPAND | wxALL, 2);
m_vardimsizer->Add(new wxButton(this, ID_RANGERESETMINMAX, _T("Reset"), wxDefaultPosition, wxSize(3*nCtrlHeight,nCtrlHeight)), 0, wxEXPAND | wxALL, 0);

m_vecwxRange[0]->Enable(true);
m_vecwxRange[1]->Enable(true);

SetDataRangeCenteredOnZero(false);
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

GenerateDimensionControls() calls SetDataRangeCenteredOnZero(false) and then immediately calls SetDataRangeByMinMax(false), which overwrites the centered range and scans the dataset twice. Since centering is triggered by the new "Center" button, the first call appears redundant; consider removing it (or, if centering is meant to be the default, remove/adjust the subsequent SetDataRangeByMinMax(false) so the centered range is preserved).

Suggested change
SetDataRangeCenteredOnZero(false);

Copilot uses AI. Check for mistakes.
SetDataRangeByMinMax(false);

// Resize window if needed
Expand Down Expand Up @@ -2261,6 +2328,16 @@ void wxNcVisFrame::OnRangeResetMinMax(

////////////////////////////////////////////////////////////////////////////////

////////////////////////////////////////////////////////////////////////////////

void wxNcVisFrame::OnRangeCenterMinMax(
wxCommandEvent & event
) {
SetDataRangeCenteredOnZero(true);
}
Comment on lines +2331 to +2337
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

The new OnRangeCenterMinMax block has inconsistent indentation and duplicate separator lines (two consecutive "////////////////////////////////////////////////////////////////////////////////" plus trailing whitespace). Please match the existing formatting style in this file (tabs/indentation and a single separator) to keep diffs clean and consistent.

Suggested change
////////////////////////////////////////////////////////////////////////////////
void wxNcVisFrame::OnRangeCenterMinMax(
wxCommandEvent & event
) {
SetDataRangeCenteredOnZero(true);
}
void wxNcVisFrame::OnRangeCenterMinMax(
wxCommandEvent & event
) {
SetDataRangeCenteredOnZero(true);
}

Copilot uses AI. Check for mistakes.

////////////////////////////////////////////////////////////////////////////////

//TODO: Verify that timer is actually able to execute with the desired frequency
void wxNcVisFrame::OnDimTimer(wxTimerEvent & event) {
if (m_fVerbose) {
Expand Down
12 changes: 12 additions & 0 deletions src/wxNcVisFrame.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,13 @@ class wxNcVisFrame : public wxFrame {
bool fRedraw = false
);

/// <summary>
/// Set the data range centered on zero.
/// </summary>
Comment on lines +276 to +278
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

The newly added XML doc comments use different indentation/alignment than the surrounding header (which consistently uses tab-indented "///\t

"). Please reformat these comments to match the established style in this file for consistency.

Suggested change
/// <summary>
/// Set the data range centered on zero.
/// </summary>
/// <summary>
/// Set the data range centered on zero.
/// </summary>

Copilot uses AI. Check for mistakes.
void SetDataRangeCenteredOnZero(
bool fRedraw = false
);

/// <summary>
/// Set the dimension value displayed.
/// </summary>
Expand Down Expand Up @@ -373,6 +380,11 @@ class wxNcVisFrame : public wxFrame {
/// </summary>
void OnRangeResetMinMax(wxCommandEvent & event);

/// <summary>
/// Callback triggered when the range center min/max button is pressed.
/// </summary>
Comment on lines +383 to +385
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

The newly added XML doc comments for OnRangeCenterMinMax use different indentation/alignment than the surrounding header comment blocks. Please align them with the existing tab-indented comment style used throughout this header for consistency.

Suggested change
/// <summary>
/// Callback triggered when the range center min/max button is pressed.
/// </summary>
/// <summary>
/// Callback triggered when the range center min/max button is pressed.
/// </summary>

Copilot uses AI. Check for mistakes.
void OnRangeCenterMinMax(wxCommandEvent & event);

/// <summary>
/// Callback triggered when the dimension timer is triggered.
/// </summary>
Expand Down