Center button to center color scale range by cmorency1 · Pull Request #25 · SEATStandards/ncvis

cmorency1 · 2026-02-18T19:46:26Z

This is a new button next to range. Open ncvis, click Center and verify that the range is now centered on 0.

Copilot

Pull request overview

Adds a “Center” control to the data range UI in NcVis to quickly set the color scale range symmetric about 0, improving visualization of signed fields.

Changes:

Added a new “Center” button next to the range controls and wired it into the event table.
Implemented SetDataRangeCenteredOnZero() to compute min/max (respecting missing values) and set a symmetric [-M, M] range.
Added the OnRangeCenterMinMax callback and corresponding declarations.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

File	Description
src/wxNcVisFrame.h	Declares the new centering range method and the new button callback.
src/wxNcVisFrame.cpp	Implements centered-on-zero range logic, adds button + event wiring, and hooks callback into the UI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-18T22:45:34Z

src/wxNcVisFrame.cpp

+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];
+        }
+    }


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 · 2026-02-18T22:45:34Z

src/wxNcVisFrame.cpp

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


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 · 2026-02-18T22:45:34Z

src/wxNcVisFrame.h

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


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 · 2026-02-18T22:45:34Z

src/wxNcVisFrame.h

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


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 · 2026-02-18T22:45:35Z

src/wxNcVisFrame.cpp

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

+	SetDataRangeCenteredOnZero(false);


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 · 2026-02-18T22:45:35Z

src/wxNcVisFrame.cpp

+    // 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)) {


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 · 2026-02-18T22:45:35Z

src/wxNcVisFrame.cpp

+    if (!(M > 0.0f) || std::isnan(M)) {
+        return;
+    }
+
+    m_imagepanel->SetDataRange(-M, M, fRedraw);


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);

Add Center button to center color scale range

146577f

rljacob requested a review from Copilot February 18, 2026 22:41

Copilot started reviewing on behalf of rljacob February 18, 2026 22:42 View session

Copilot AI reviewed Feb 18, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Center button to center color scale range#25

Center button to center color scale range#25
cmorency1 wants to merge 1 commit intoSEATStandards:mainfrom
cmorency1:center-color-scale

cmorency1 commented Feb 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

-    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);

Comments

Conversation

cmorency1 commented Feb 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant