Fix widget selection on high-DPI displays; remove 'hack' shader toggle

The widget-selection render pass drew into an FBO sized in Qt logical
(widget) coordinates but inherited whatever viewport was last set. Qt
sets the viewport to the physical framebuffer size (logical size times
devicePixelRatio) before each paintGL, so on high-DPI displays the
selection scene was rendered at devicePixelRatio scale relative to the
FBO and mouse coordinates, breaking shift-click widget picking.

The 'Viewer widget selection correction' preference compensated by
scaling NDC by 0.5 in the selection vertex shader (a uniform literally
named "hack"), which only worked for devicePixelRatio == 2 (e.g. Retina
Macs). It was wrong for standard displays (hence the toggle) and for
fractional scale factors (e.g. 125%/150% on Windows, common with Qt 6),
where neither setting worked.

Fix the root cause instead: explicitly set the viewport to the
selection FBO's size while rendering the selection pass, so clip space
maps onto the FBO 1:1 regardless of display scale, and restore the
previous viewport afterward. Remove the hack uniform, the
widgetSelectionCorrection preference, its settings entry, and the
preferences-dialog checkbox.

Fixes #2558

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: dcwhite

src/Core/Application/Preferences/Preferences.cc

@@ -51,7 +51,6 @@ Preferences::Preferences() :
   highlightPorts("highlightPorts", false),
   autoNotes("autoNotes", false),
   highDPIAdjustment("highDPIAdjustment", false),
-  widgetSelectionCorrection("widgetSelectionCorrection", false),
   autoRotateViewerOnMouseRelease("autoRotateViewerOnMouseRelease", false),
   moduleExecuteDownstreamOnly("moduleExecuteDownstreamOnly", true),
   forceGridBackground("forceGridBackground", false),

src/Core/Application/Preferences/Preferences.h

@@ -86,7 +86,6 @@ namespace SCIRun
       BooleanVariable highlightPorts;
       BooleanVariable autoNotes;
       BooleanVariable highDPIAdjustment;
-      BooleanVariable widgetSelectionCorrection;
       BooleanVariable autoRotateViewerOnMouseRelease;
       TrackedVariable<BooleanVariable> moduleExecuteDownstreamOnly;
       TrackedVariable<BooleanVariable> forceGridBackground;

src/Interface/Application/Preferences.ui

@@ -412,13 +412,6 @@
         </widget>
        </item>
        <item row="1" column="0">
-        <widget class="QCheckBox" name="viewerWidgetSelectionCorrectionCheckbox_">
-         <property name="text">
-          <string>Viewer widget selection correction--enable if widgets don't respond to shift-click actions</string>
-         </property>
-        </widget>
-       </item>
-       <item row="2" column="0">
         <widget class="QGroupBox" name="groupBox_2">
          <property name="minimumSize">
           <size>

src/Interface/Application/PreferencesWindow.cc

@@ -46,18 +46,12 @@ PreferencesWindow::PreferencesWindow(NetworkEditor* editor, std::function<void()
   connect(errorGraphicItemsCheckBox_, &QCheckBox::stateChanged, this, &PreferencesWindow::updateModuleErrorInlineMessagesOption);
   connect(highDPIAdjustCheckBox_, &QCheckBox::stateChanged, this, &PreferencesWindow::updateHighDPIAdjust);
   connect(forceGridBackgroundCheckBox_, &QCheckBox::stateChanged, this, &PreferencesWindow::updateForceGridBackground);
-  connect(viewerWidgetSelectionCorrectionCheckbox_, &QCheckBox::stateChanged, this, &PreferencesWindow::updateWidgetSelectionCorrection);
   connect(autoRotateViewerOnMouseReleaseCheckbox_, &QCheckBox::stateChanged, this, &PreferencesWindow::updateAutoRotateViewer);
   connect(moduleExecuteDownstreamOnlyCheckBox_, &QCheckBox::stateChanged, this, &PreferencesWindow::updateModuleExecuteDownstream);
   connect(toolBarPopupShowDelaySpinBox_, qOverload<int>(&QSpinBox::valueChanged), this, &PreferencesWindow::updateToolBarPopupShowDelay);
   connect(toolBarPopupHideDelaySpinBox_, qOverload<int>(&QSpinBox::valueChanged), this, &PreferencesWindow::updateToolBarPopupHideDelay);
 }
 
-void PreferencesWindow::updateWidgetSelectionCorrection(int state)
-{
-  SCIRun::Core::Preferences::Instance().widgetSelectionCorrection.setValue(state != 0);
-}
-
 void PreferencesWindow::updateToolBarPopupShowDelay(int delay)
 {
   SCIRun::Core::Preferences::Instance().toolBarPopupShowDelay.setValueWithSignal(delay);
@@ -133,12 +127,6 @@ void PreferencesWindow::setToolBarPopupHideDelay(int delay)
   updateToolBarPopupHideDelay(delay);
 }
 
-void PreferencesWindow::setWidgetSelectionCorrection(bool mode)
-{
-  updateWidgetSelectionCorrection(mode ? 1 : 0);
-  viewerWidgetSelectionCorrectionCheckbox_->setChecked(mode);
-}
-
 void PreferencesWindow::setHighDPIAdjustment(bool highDPI)
 {
   updateHighDPIAdjust(highDPI ? 1 : 0);

src/Interface/Application/PreferencesWindow.h

@@ -57,7 +57,6 @@ class PreferencesWindow : public QDialog, public Ui::PreferencesDialog
   void setHighDPIAdjustment(bool highDPI);
   void setModuleExecuteDownstreamOnly(bool mode);
   void setAutoRotateViewerOnMouseRelease(bool mode);
-  void setWidgetSelectionCorrection(bool mode);
   void setToolBarPopupShowDelay(int delay);
   void setToolBarPopupHideDelay(int delay);
 
@@ -68,7 +67,6 @@ public Q_SLOTS:
   void updateAutoNotesState(int state);
   void updateHighDPIAdjust(int state);
   void updateForceGridBackground(int state);
-  void updateWidgetSelectionCorrection(int state);
   void updateAutoRotateViewer(int state);
   void updateToolBarPopupShowDelay(int delay);
   void updateToolBarPopupHideDelay(int delay);

src/Interface/Application/Settings.cc

@@ -301,9 +301,6 @@ void SCIRunMainWindow::readSettings()
     makeSetting("invertMouseZoom", toBool,
       [](bool b) { prefs.invertMouseZoom.setValue(b); },
       []() { return prefs.invertMouseZoom.val(); }),
-    makeSetting("widgetSelectionCorrection", toBool,
-      [this](bool b) { prefsWindow_->setWidgetSelectionCorrection(b); },
-      []() { return prefs.widgetSelectionCorrection.val(); }),
     makeSetting("autoRotateViewerOnMouseRelease", toBool,
       [this](bool b) { prefsWindow_->setAutoRotateViewerOnMouseRelease(b); },
       []() { return prefs.autoRotateViewerOnMouseRelease.val(); }),

src/Interface/Modules/Render/ES/SRInterface.cc

@@ -431,6 +431,16 @@ void SRInterface::runGCOnNextExecution()
       widgetSelectFboId_ = fboMan->getOrCreateFBO(mCore, GL_TEXTURE_2D, screen_.width, screen_.height, 1, widgetSelectFboName);
       fboMan->bindFBO(*widgetSelectFboId_);
 
+      // The selection FBO matches the logical widget size that mouse coordinates
+      // are reported in, but the inherited viewport is the on-screen framebuffer's,
+      // which Qt scales by devicePixelRatio on high-DPI displays. Point the
+      // viewport at the selection FBO so clip space maps onto it 1:1.
+      GLint priorViewport[4];
+      GL(glGetIntegerv(GL_VIEWPORT, priorViewport));
+      GL(glViewport(0, 0, static_cast<GLsizei>(screen_.width), static_cast<GLsizei>(screen_.height)));
+      ScopedLambdaExecutor restoreViewport([&priorViewport]()
+        { GL(glViewport(priorViewport[0], priorViewport[1], priorViewport[2], priorViewport[3])); });
+
       //a map from selection id to name
       std::map<uint32_t, std::string> selMap;
       std::vector<uint64_t> entityList;
@@ -514,13 +524,11 @@ void SRInterface::runGCOnNextExecution()
           const char* vs =
             "uniform mat4 uModelViewProjection;\n"
             "uniform vec4 uColor;\n"
-            "uniform bool hack;\n"
             "attribute vec3 aPos;\n"
             "varying vec4 fColor;\n"
             "void main()\n"
             "{\n"
             "  gl_Position = uModelViewProjection * vec4(aPos, 1.0);\n"
-            "  if(hack) gl_Position.xy = ((gl_Position.xy/gl_Position.w) * vec2(0.5) - vec2(0.5)) * gl_Position.w;\n"
             "  fColor = uColor;\n"
             "}\n";
           const char* fs =
@@ -555,7 +563,6 @@ void SRInterface::runGCOnNextExecution()
         mCore.addComponent(entityID, commonUniforms);
 
         applyUniform(entityID, SpireSubPass::Uniform("uColor", selCol));
-        applyUniform(entityID, SpireSubPass::Uniform("hack", Preferences::Instance().widgetSelectionCorrection));
 
         // Add components associated with entity. We just need a base class which
         // we can pass in an entity ID, then a derived class which bundles