Recent updates to 3.0

cmake/CompilerFlags.cmake

@@ -11,7 +11,13 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang"
   include(CheckCXXCompilerFlag)
 
   # Additional warnings for GCC
-  set(CMAKE_CXX_FLAGS_WARN "-Wnon-virtual-dtor -Wno-long-long -Wcast-align -Wchar-subscripts -Wall -Wpointer-arith -Wformat-security -Woverloaded-virtual -fno-check-new -fno-common")
+  set(CMAKE_CXX_FLAGS_WARN "-Wnon-virtual-dtor -Wno-long-long -Wcast-align -Wchar-subscripts -Wall -Wpointer-arith -Wformat-security -Woverloaded-virtual -fno-common")
+
+  # -fno-check-new is a GCC-only flag; clang accepts but ignores it and then
+  # warns that the argument is unused, so only pass it to GCC.
+  if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+    set(CMAKE_CXX_FLAGS_WARN "${CMAKE_CXX_FLAGS_WARN} -fno-check-new")
+  endif()
 
   # This flag is useful as not returning from a non-void function is an error
   # with MSVC, but it is not supported on all GCC compiler versions

tests/cxx/CMakeLists.txt

@@ -56,6 +56,7 @@ set(_pythonpath "${_pythonpath}${_separator}$ENV{PYTHONPATH}")
 add_cxx_test(Variant)
 add_cxx_test(ScanID)
 add_cxx_test(Utilities)
+add_cxx_test(VolumeBricking)
 add_cxx_qtest(InterfaceBuilder)
 # AcquisitionClient needs the `bottle` Python package (acquisition/requirements-dev.txt)
 # to start its test server; not part of the conda build env, so disabled here.

tests/cxx/PipelineStripWidgetDemo.cxx

@@ -584,7 +584,7 @@ int main(int argc, char** argv)
   auto* srcLayout = new QVBoxLayout(srcGroup);
   auto* srcLabelEdit = new QLineEdit("New Source");
   srcLayout->addWidget(srcLabelEdit);
-  auto [srcIn, srcOut] = makePortGrid(srcLayout, false, true, 0, 1);
+  auto srcPorts = makePortGrid(srcLayout, false, true, 0, 1);
   auto* srcButton = new QPushButton("Add Source");
   srcLayout->addWidget(srcButton);
   centralLayout->addWidget(srcGroup);
@@ -594,7 +594,7 @@ int main(int argc, char** argv)
   auto* xfLayout = new QVBoxLayout(xfGroup);
   auto* xfLabelEdit = new QLineEdit("New Transform");
   xfLayout->addWidget(xfLabelEdit);
-  auto [xfIn, xfOut] = makePortGrid(xfLayout, true, true, 1, 1);
+  auto xfPorts = makePortGrid(xfLayout, true, true, 1, 1);
   auto* xfButton = new QPushButton("Add Transform");
   xfLayout->addWidget(xfButton);
   centralLayout->addWidget(xfGroup);
@@ -604,7 +604,7 @@ int main(int argc, char** argv)
   auto* sinkLayout = new QVBoxLayout(sinkGroup);
   auto* sinkLabelEdit = new QLineEdit("New Sink");
   sinkLayout->addWidget(sinkLabelEdit);
-  auto [sinkIn, sinkOut] = makePortGrid(sinkLayout, true, false, 1, 0);
+  auto sinkPorts = makePortGrid(sinkLayout, true, false, 1, 0);
   auto* sinkButton = new QPushButton("Add Sink");
   sinkLayout->addWidget(sinkButton);
   centralLayout->addWidget(sinkGroup);
@@ -641,7 +641,7 @@ int main(int argc, char** argv)
     if (!p) return;
     QString label = srcLabelEdit->text();
     if (label.isEmpty()) label = "Source";
-    auto outputs = collectPorts(srcOut, "out");
+    auto outputs = collectPorts(srcPorts.second, "out");
     auto* src = new SourceNode();
     src->setLabel(label);
     for (auto& [name, type] : outputs) {
@@ -655,8 +655,8 @@ int main(int argc, char** argv)
     if (!p) return;
     QString label = xfLabelEdit->text();
     if (label.isEmpty()) label = "Transform";
-    auto inputs = collectPorts(xfIn, "in");
-    auto outputs = collectPorts(xfOut, "out");
+    auto inputs = collectPorts(xfPorts.first, "in");
+    auto outputs = collectPorts(xfPorts.second, "out");
     auto* xform = new DemoTransform(label, inputs, outputs);
     p->addNode(xform);
   });
@@ -666,7 +666,7 @@ int main(int argc, char** argv)
     if (!p) return;
     QString label = sinkLabelEdit->text();
     if (label.isEmpty()) label = "Sink";
-    auto inputs = collectPorts(sinkIn, "in");
+    auto inputs = collectPorts(sinkPorts.first, "in");
     auto* sink = new DemoSink(label, inputs);
     p->addNode(sink);
   });

tests/cxx/VolumeBrickingTest.cxx

@@ -0,0 +1,176 @@
+/* This source file is part of the Tomviz project, https://tomviz.org/.
+   It is released under the 3-Clause BSD License, see "LICENSE". */
+
+#include <gtest/gtest.h>
+
+#include "sinks/VolumeBricking.h"
+
+#include <vtkFloatArray.h>
+#include <vtkImageData.h>
+#include <vtkMultiBlockDataSet.h>
+#include <vtkNew.h>
+#include <vtkPointData.h>
+#include <vtkSmartPointer.h>
+
+#include <array>
+#include <set>
+
+using tomviz::pipeline::brickVolume;
+using tomviz::pipeline::computeBlockCount;
+using tomviz::pipeline::exceedsTextureLimit;
+
+namespace {
+
+// Build a volume whose scalar at (x,y,z) is a unique, reproducible value so we
+// can check that bricks carry the right data at the right global indices.
+vtkSmartPointer<vtkImageData> makeRampVolume(int nx, int ny, int nz)
+{
+  auto image = vtkSmartPointer<vtkImageData>::New();
+  image->SetDimensions(nx, ny, nz);
+  image->SetSpacing(1.0, 1.0, 1.0);
+  image->SetOrigin(0.0, 0.0, 0.0);
+
+  vtkNew<vtkFloatArray> scalars;
+  scalars->SetName("ImageScalars");
+  scalars->SetNumberOfComponents(1);
+  scalars->SetNumberOfTuples(static_cast<vtkIdType>(nx) * ny * nz);
+  for (int z = 0; z < nz; ++z) {
+    for (int y = 0; y < ny; ++y) {
+      for (int x = 0; x < nx; ++x) {
+        vtkIdType id = static_cast<vtkIdType>(z) * ny * nx +
+                       static_cast<vtkIdType>(y) * nx + x;
+        scalars->SetValue(id, static_cast<float>(x * 1000000 + y * 1000 + z));
+      }
+    }
+  }
+  image->GetPointData()->SetScalars(scalars);
+  return image;
+}
+
+float expectedValue(int x, int y, int z)
+{
+  return static_cast<float>(x * 1000000 + y * 1000 + z);
+}
+
+} // namespace
+
+TEST(VolumeBrickingTest, BlockCount)
+{
+  // Fits exactly -> a single block, no split.
+  EXPECT_EQ(computeBlockCount(2048, 2048), 1);
+  EXPECT_EQ(computeBlockCount(1, 2048), 1);
+  EXPECT_EQ(computeBlockCount(2049, 2048), 2);
+  // One-voxel overlap means two 2048 bricks share a boundary plane and so
+  // cover only 2*2047 + 1 = 4095 voxels. 4095 fits in two; 4096 needs three.
+  EXPECT_EQ(computeBlockCount(4095, 2048), 2);
+  EXPECT_EQ(computeBlockCount(4096, 2048), 3);
+  EXPECT_EQ(computeBlockCount(5000, 2048), 3);
+}
+
+TEST(VolumeBrickingTest, ComputedBricksNeverExceedCap)
+{
+  // Whatever the count, each brick (step + 1 shared-boundary voxels) must fit.
+  for (int maxTex : { 4, 7, 16, 100, 2048 }) {
+    for (int length = 1; length < 6 * maxTex; ++length) {
+      int n = computeBlockCount(length, maxTex);
+      ASSERT_GE(n, 1);
+      // Largest brick under an even split.
+      int span = length - 1;
+      int maxStep = (n > 0) ? (span + n - 1) / n : span; // ceil(span / n)
+      EXPECT_LE(maxStep + 1, maxTex)
+        << "length=" << length << " maxTex=" << maxTex << " n=" << n;
+    }
+  }
+}
+
+TEST(VolumeBrickingTest, SingleBlockWhenItFits)
+{
+  auto image = makeRampVolume(8, 8, 8);
+  EXPECT_FALSE(exceedsTextureLimit(image, 2048));
+
+  auto blocks = brickVolume(image, 2048);
+  ASSERT_EQ(blocks->GetNumberOfBlocks(), 1u);
+
+  auto* brick = vtkImageData::SafeDownCast(blocks->GetBlock(0));
+  ASSERT_NE(brick, nullptr);
+  int dims[3];
+  brick->GetDimensions(dims);
+  EXPECT_EQ(dims[0], 8);
+  EXPECT_EQ(dims[1], 8);
+  EXPECT_EQ(dims[2], 8);
+}
+
+TEST(VolumeBrickingTest, SplitsLongAxisOnly)
+{
+  // Long in x only; y and z fit. Expect a 3 x 1 x 1 brick grid.
+  auto image = makeRampVolume(100, 10, 10);
+  const int maxTex = 40;
+  EXPECT_TRUE(exceedsTextureLimit(image, maxTex));
+
+  auto blocks = brickVolume(image, maxTex);
+  EXPECT_EQ(blocks->GetNumberOfBlocks(),
+            static_cast<unsigned>(computeBlockCount(100, maxTex)));
+
+  for (unsigned i = 0; i < blocks->GetNumberOfBlocks(); ++i) {
+    auto* brick = vtkImageData::SafeDownCast(blocks->GetBlock(i));
+    ASSERT_NE(brick, nullptr);
+    int dims[3];
+    brick->GetDimensions(dims);
+    EXPECT_LE(dims[0], maxTex);
+    EXPECT_EQ(dims[1], 10); // y not split
+    EXPECT_EQ(dims[2], 10); // z not split
+  }
+}
+
+TEST(VolumeBrickingTest, BricksCoverVolumeWithOverlapAndCorrectValues)
+{
+  auto image = makeRampVolume(100, 60, 5);
+  const int maxTex = 32;
+  auto blocks = brickVolume(image, maxTex);
+  ASSERT_GT(blocks->GetNumberOfBlocks(), 1u);
+
+  // Every global voxel index must be covered by at least one brick, and the
+  // scalar there must match the original. Track coverage to confirm no gaps.
+  std::set<std::array<int, 3>> covered;
+
+  for (unsigned i = 0; i < blocks->GetNumberOfBlocks(); ++i) {
+    auto* brick = vtkImageData::SafeDownCast(blocks->GetBlock(i));
+    ASSERT_NE(brick, nullptr);
+
+    int ext[6];
+    brick->GetExtent(ext);
+    // Each brick keeps its global extent indices and the shared origin/spacing.
+    auto* scalars = brick->GetPointData()->GetScalars();
+    ASSERT_NE(scalars, nullptr);
+
+    for (int z = ext[4]; z <= ext[5]; ++z) {
+      for (int y = ext[2]; y <= ext[3]; ++y) {
+        for (int x = ext[0]; x <= ext[1]; ++x) {
+          double val = brick->GetScalarComponentAsDouble(x, y, z, 0);
+          EXPECT_FLOAT_EQ(static_cast<float>(val), expectedValue(x, y, z))
+            << "brick " << i << " at (" << x << "," << y << "," << z << ")";
+          covered.insert({ x, y, z });
+        }
+      }
+    }
+  }
+
+  EXPECT_EQ(covered.size(), static_cast<size_t>(100 * 60 * 5));
+
+  // Confirm there is genuine overlap: summed brick voxel counts exceed the
+  // volume's voxel count (adjacent bricks share a boundary plane).
+  vtkIdType totalBrickVoxels = 0;
+  for (unsigned i = 0; i < blocks->GetNumberOfBlocks(); ++i) {
+    totalBrickVoxels +=
+      vtkImageData::SafeDownCast(blocks->GetBlock(i))->GetNumberOfPoints();
+  }
+  EXPECT_GT(totalBrickVoxels, static_cast<vtkIdType>(100 * 60 * 5));
+}
+
+TEST(VolumeBrickingTest, NullInputIsSafe)
+{
+  EXPECT_FALSE(exceedsTextureLimit(nullptr, 2048));
+  auto blocks = brickVolume(nullptr, 2048);
+  ASSERT_NE(blocks, nullptr);
+  EXPECT_EQ(blocks->GetNumberOfBlocks(), 0u);
+}

tests/python/utils.py

@@ -4,6 +4,8 @@
 import importlib.util
 import inspect
 import shutil
+import time
+import urllib.error
 import urllib.request
 import zipfile
 
@@ -82,7 +84,23 @@ def download_progress_hook(block_num, block_size, total_size):
             print(f"\rDownload Progress: {progress}% ({downloaded_mb:.2f}/{total_size_mb:.2f} MB)")
 
     print(f'Downloading "{url}" to "{filename}"')
-    urllib.request.urlretrieve(url, filename, reporthook=download_progress_hook)
+    # Retry on transient network errors (e.g. a 5xx from the data server) with
+    # exponential backoff, since the test fixtures are downloaded at run time.
+    attempts = 5
+    for attempt in range(1, attempts + 1):
+        try:
+            urllib.request.urlretrieve(url, filename,
+                                       reporthook=download_progress_hook)
+            break
+        except (urllib.error.URLError, TimeoutError) as e:
+            # Don't retry on 4xx (client) errors - those won't fix themselves.
+            code = getattr(e, 'code', None)
+            if (code is not None and 400 <= code < 500) or attempt == attempts:
+                raise
+            wait = 2 ** (attempt - 1)
+            print(f'Download failed ({e}); retrying in {wait}s '
+                  f'({attempt}/{attempts - 1})')
+            time.sleep(wait)
     print('\n')
 
     shutil.copy(filename, destination)

tomviz/AddPythonSourceReaction.cxx

@@ -40,7 +40,7 @@ void AddPythonSourceReaction::onTriggered()
 
   // Add the source to the pipeline before opening the dialog. This is
   // what gives us the cancel-rollback symmetry with transform
-  // insertion: NodeEditDialog::onCancel removes the node from the
+  // insertion: NodeEditDialog::reject() removes the node from the
   // pipeline (and deletes it). On OK we finish the standard source
   // scaffolding (sinks, color map, execute) via completeSourceSetup.
   LoadDataReaction::addSourceToPipeline(source);

tomviz/CMakeLists.txt

@@ -339,6 +339,7 @@ set(SOURCES
   pipeline/transforms/SnapshotTransform.cxx
   pipeline/transforms/SnapshotTransform.h
   pipeline/PortUtils.h
+  pipeline/ThreadUtils.h
   pipeline/pybind11/PybindVTKTypeCaster.h
   pipeline/DeferredLinkInfo.h
   pipeline/sinks/VolumeStatsSink.cxx
@@ -347,6 +348,8 @@ set(SOURCES
   pipeline/sinks/LegacyModuleSink.h
   pipeline/sinks/VolumeSink.cxx
   pipeline/sinks/VolumeSink.h
+  pipeline/sinks/VolumeBricking.cxx
+  pipeline/sinks/VolumeBricking.h
   pipeline/sinks/SliceSink.cxx
   pipeline/sinks/SliceSink.h
   pipeline/sinks/ContourSink.cxx
@@ -452,6 +455,13 @@ list(APPEND SOURCES
 )
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/loguru)
 
+# loguru is vendored third-party code; silence warnings we will not patch
+# upstream for (empty variadic macro args and a benign snprintf truncation).
+if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+  set_source_files_properties(loguru/loguru.cpp PROPERTIES
+    COMPILE_OPTIONS "-Wno-c++20-extensions;-Wno-format-truncation")
+endif()
+
 list(APPEND SOURCES
   legacy/modules/Module.cxx
   legacy/modules/Module.h
@@ -906,6 +916,11 @@ if(APPLE AND TOMVIZ_MACOSX_BUNDLE)
   list(APPEND exec_sources icons/tomviz.icns)
   set(MACOSX_BUNDLE_ICON_FILE tomviz.icns)
   set(MACOSX_BUNDLE_BUNDLE_VERSION "${tomviz_version}")
+  # A non-empty bundle identifier and name are required for macOS to present
+  # privacy-gated native panels (NSOpenPanel/NSSavePanel). Without them the
+  # native QFileDialog silently fails to appear. Match the packaged bundle id.
+  set(MACOSX_BUNDLE_GUI_IDENTIFIER "org.tomviz.tomviz")
+  set(MACOSX_BUNDLE_BUNDLE_NAME "tomviz")
   set_source_files_properties(icons/tomviz.icns PROPERTIES
     MACOSX_PACKAGE_LOCATION Resources)
 elseif(WIN32)
@@ -1006,7 +1021,6 @@ install(TARGETS tomvizlib
   ARCHIVE DESTINATION "${INSTALL_ARCHIVE_DIR}")
 
 if(tomviz_data_DIR)
-  add_definitions(-DTOMVIZ_DATA)
   install(DIRECTORY "${tomviz_data_DIR}"
     DESTINATION "${tomviz_data_install_dir}"
     USE_SOURCE_PERMISSIONS