From 8c17d9f19c8e8b473dc3a19159b7ff92eb224697 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Mon, 11 May 2020 11:45:26 +1000 Subject: [PATCH 01/22] fixelconnectivity: Enhancements - Option -tck_weights_in now supported, allowing CFE to be performed using SIFT2. Generation of the fixel-fixel connectivity matrix will be done using floating-point if such weights are provided, integers otherwise. - New options -count and -extent export fixel data files corresponding to the number of connections, and sum of connection weights, of each fixel respectively. - Modify error message on pre-existing output directory, as output target is not a file. --- cmd/fixelconnectivity.cpp | 59 ++++- core/app.h | 2 +- core/fixel/helpers.h | 16 +- core/fixel/types.h | 2 + docs/reference/commands/fixelconnectivity.rst | 9 + src/dwi/tractography/weights.h | 1 - src/fixel/filter/smooth.h | 2 + src/fixel/matrix.cpp | 200 ++++++++++------ src/fixel/matrix.h | 214 +++++++++++++----- 9 files changed, 361 insertions(+), 144 deletions(-) diff --git a/cmd/fixelconnectivity.cpp b/cmd/fixelconnectivity.cpp index 19ea2e5d38..be2fd03dff 100644 --- a/cmd/fixelconnectivity.cpp +++ b/cmd/fixelconnectivity.cpp @@ -15,10 +15,12 @@ #include "command.h" #include "fixel/helpers.h" -#include "fixel/index_remapper.h" + +#include "dwi/tractography/weights.h" #include "fixel/matrix.h" + #define DEFAULT_ANGLE_THRESHOLD 45.0 #define DEFAULT_CONNECTIVITY_THRESHOLD 0.01 @@ -55,7 +57,17 @@ void usage () + Argument ("value").type_float (0.0, 90.0) + Option ("mask", "provide a fixel data file containing a mask of those fixels to be computed; fixels outside the mask will be empty in the output matrix") - + Argument ("file").type_image_in(); + + Argument ("file").type_image_in() + + + DWI::Tractography::TrackWeightsInOption + + + OptionGroup ("Options for additional outputs to be generated") + + + Option ("count", "export a fixel data file encoding the number of connections for each fixel") + + Argument ("path").type_image_out() + + + Option ("extent", "export a fixel data file encoding the extent of connectivity (sum of weights) for each fixel") + + Argument ("path").type_image_out(); } @@ -64,6 +76,20 @@ using value_type = float; using Fixel::index_type; + +template +void set_optional_outputs (WriterType& writer) +{ + auto opt = get_options ("count"); + if (opt.size()) + writer.set_count_path (opt[0][0]); + opt = get_options ("extent"); + if (opt.size()) + writer.set_extent_path (opt[0][0]); +} + + + void run() { const value_type connectivity_threshold = get_option_value ("connectivity", value_type(DEFAULT_CONNECTIVITY_THRESHOLD)); @@ -91,14 +117,29 @@ void run() fixel_mask.value() = true; } - auto connectivity_matrix = Fixel::Matrix::generate (argument[1], - index_image, - fixel_mask, - angular_threshold); + if (get_options ("tck_weights_in").size()) { + + auto connectivity_matrix = Fixel::Matrix::generate_weighted (argument[1], + index_image, + fixel_mask, + angular_threshold); - Fixel::Matrix::normalise_and_write (connectivity_matrix, - connectivity_threshold, - argument[2]); + Fixel::Matrix::Writer writer (connectivity_matrix, connectivity_threshold); + set_optional_outputs (writer); + writer.save (argument[2]); + + } else { + + auto connectivity_matrix = Fixel::Matrix::generate_unweighted (argument[1], + index_image, + fixel_mask, + angular_threshold); + + Fixel::Matrix::Writer writer (connectivity_matrix, connectivity_threshold); + set_optional_outputs (writer); + writer.save (argument[2]); + + } } diff --git a/core/app.h b/core/app.h index 10442aa90a..71aec4cba4 100644 --- a/core/app.h +++ b/core/app.h @@ -171,7 +171,7 @@ namespace MR if (check_overwrite_files_func) check_overwrite_files_func (name); else - throw Exception ("output file \"" + name + "\" already exists (use -force option to force overwrite)"); + throw Exception ("output path \"" + name + "\" already exists (use -force option to force overwrite)"); } } diff --git a/core/fixel/helpers.h b/core/fixel/helpers.h index 116194a682..8e973d47c2 100644 --- a/core/fixel/helpers.h +++ b/core/fixel/helpers.h @@ -296,14 +296,14 @@ namespace MR return header; } - //! Generate a header for a sparse data file (Nx1x1) using an index image as a template - template - FORCE_INLINE Header data_header_from_index (IndexHeaderType& index) { - Header header (index); + //! Generate a header for a sparse data file (Nx1x1) with a known number of fixels + FORCE_INLINE Header make_data_header (const size_t num_fixels) { + Header header; header.ndim() = 3; - header.size(0) = get_number_of_fixels (index); + header.size(0) = num_fixels; header.size(1) = 1; header.size(2) = 1; + header.spacing(0) = header.spacing(1) = header.spacing(2) = 1.0; header.stride(0) = 1; header.stride(1) = 2; header.stride(2) = 3; @@ -313,6 +313,12 @@ namespace MR return header; } + //! Generate a header for a sparse data file (Nx1x1) using an index image as a template + template + FORCE_INLINE Header data_header_from_index (IndexHeaderType& index) { + return make_data_header (get_number_of_fixels (index)); + } + //! Generate a header for a fixel directions data file (Nx3x1) using an index image as a template template FORCE_INLINE Header directions_header_from_index (IndexHeaderType& index) { diff --git a/core/fixel/types.h b/core/fixel/types.h index 81a06d618e..54434eb7d5 100644 --- a/core/fixel/types.h +++ b/core/fixel/types.h @@ -16,6 +16,8 @@ #ifndef __fixel_types_h__ #define __fixel_types_h__ +#include + namespace MR { namespace Fixel diff --git a/docs/reference/commands/fixelconnectivity.rst b/docs/reference/commands/fixelconnectivity.rst index e98540ced3..eac6f2123c 100644 --- a/docs/reference/commands/fixelconnectivity.rst +++ b/docs/reference/commands/fixelconnectivity.rst @@ -36,6 +36,15 @@ Options that influence generation of the connectivity matrix / matrices - **-mask file** provide a fixel data file containing a mask of those fixels to be computed; fixels outside the mask will be empty in the output matrix +- **-tck_weights_in path** specify a text scalar file containing the streamline weights + +Options for additional outputs to be generated +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +- **-count path** export a fixel data file encoding the number of connections for each fixel + +- **-extent path** export a fixel data file encoding the extent of connectivity (sum of weights) for each fixel + Standard options ^^^^^^^^^^^^^^^^ diff --git a/src/dwi/tractography/weights.h b/src/dwi/tractography/weights.h index d55a2f878d..e77b884d57 100644 --- a/src/dwi/tractography/weights.h +++ b/src/dwi/tractography/weights.h @@ -23,7 +23,6 @@ namespace MR { namespace DWI { - namespace Tractography { diff --git a/src/fixel/filter/smooth.h b/src/fixel/filter/smooth.h index 25cabe4203..5c3ba91683 100644 --- a/src/fixel/filter/smooth.h +++ b/src/fixel/filter/smooth.h @@ -16,6 +16,8 @@ #ifndef __fixel_filter_smooth_h__ #define __fixel_filter_smooth_h__ +#include "fixel/types.h" + #include "fixel/matrix.h" #include "fixel/filter/base.h" diff --git a/src/fixel/matrix.cpp b/src/fixel/matrix.cpp index 0ae2f6279b..47cfc0885e 100644 --- a/src/fixel/matrix.cpp +++ b/src/fixel/matrix.cpp @@ -37,19 +37,12 @@ namespace MR - void InitFixel::add (const vector& indices) + template + void InitFixelBase::add (const MappedTrack& mapped_track) { - if ((*this).empty()) { - (*this).reserve (indices.size()); - for (auto i : indices) - (*this).emplace_back (InitElement (i)); - track_count = 1; - return; - } - ssize_t self_index = 0, in_index = 0; - // For anything in indices that doesn't yet appear in *this, + // For anything in mapped_track that doesn't yet appear in *this, // add to this list; once completed, extend *this by the appropriate // amount, and insert these into the appropriate locations // Need to continue making use of the existing allocated memory @@ -60,15 +53,15 @@ namespace MR // - On second pass, from back to front, move elements from previous back of vector to new back, // inserting new elements at appropriate locations to retain sortedness of list const ssize_t old_size = (*this).size(); - const ssize_t in_count = indices.size(); + const ssize_t in_count = mapped_track.size(); size_t intersection = 0; while (self_index < old_size && in_index < in_count) { - if ((*this)[self_index].index() == indices[in_index]) { - ++(*this)[self_index]; + if ((*this)[self_index].index() == mapped_track[in_index]) { + increment ((*this)[self_index], mapped_track); ++self_index; ++in_index; ++intersection; - } else if ((*this)[self_index].index() > indices[in_index]) { + } else if ((*this)[self_index].index() > mapped_track[in_index]) { ++in_index; } else { ++self_index; @@ -76,58 +69,53 @@ namespace MR } self_index = old_size - 1; - in_index = indices.size() - 1; + in_index = mapped_track.size() - 1; // It's possible that a resize() call may always result in requesting // a re-assignment of memory that exactly matches the size, which may in turn // lead to memory bloat due to inability to return the old memory // If this occurs, iteratively calling push_back() may instead engage the // memory-reservation-doubling behaviour - while ((*this).size() < old_size + indices.size() - intersection) - (*this).push_back (InitElement()); + while ((*this).size() < old_size + mapped_track.size() - intersection) + (*this).push_back (ElementType()); ssize_t out_index = (*this).size() - 1; // For each output vector location, need to determine whether it should come from copying an existing entry, // or creating a new one while (out_index > self_index && self_index >= 0 && in_index >= 0) { - if ((*this)[self_index].index() == indices[in_index]) { + if ((*this)[self_index].index() == mapped_track[in_index]) { (*this)[out_index] = (*this)[self_index]; --self_index; --in_index; - } else if ((*this)[self_index].index() > indices[in_index]) { + } else if ((*this)[self_index].index() > mapped_track[in_index]) { (*this)[out_index] = (*this)[self_index]; --self_index; } else { - (*this)[out_index] = InitElement (indices[in_index]); + (*this)[out_index] = ElementType (mapped_track[in_index], mapped_track); --in_index; } --out_index; } if (self_index < 0) { while (in_index >= 0 && out_index >= 0) - (*this)[out_index--] = InitElement (indices[in_index--]); + (*this)[out_index--] = ElementType (mapped_track[in_index--], mapped_track); } - // Track total number of streamlines intersecting this fixel, + // Track total number of streamlines / sum of streamline weights intersecting this fixel, // independently of the extent of fixel-fixel connectivity - ++track_count; + increment (mapped_track); } + template class InitFixelBase; + template class InitFixelBase; - - - init_matrix_type generate ( - const std::string& track_filename, - Image& index_image, - Image& fixel_mask, - const float angular_threshold) + namespace { - class TrackProcessor { MEMALIGN(TrackProcessor) public: @@ -143,7 +131,7 @@ namespace MR angular_threshold_dp (std::cos (angular_threshold * (Math::pi/180.0))) { } bool operator() (const DWI::Tractography::Streamline<>& tck, - vector& out) const + MappedTrack& out) const { using direction_type = Eigen::Vector3; using SetVoxelDir = DWI::Tractography::Mapping::SetVoxelDir; @@ -151,9 +139,10 @@ namespace MR SetVoxelDir in; mapper (tck, in); - // For each voxel tract tangent, assign to a fixel + // For each voxel intersection, assign to a fixel out.clear(); out.reserve (in.size()); + out.set_weight (tck.weight); for (const auto& i : in) { assign_pos_of (i).to (fixel_indexer); fixel_indexer.index(3) = 0; @@ -196,33 +185,77 @@ namespace MR }; - auto directions_image = Fixel::find_directions_header (Path::dirname (index_image.name())).template get_image().with_direct_io ({+2,+1}); - DWI::Tractography::Properties properties; - DWI::Tractography::Reader track_file (track_filename, properties); - const uint32_t num_tracks = properties["count"].empty() ? 0 : to(properties["count"]); - DWI::Tractography::Mapping::TrackLoader loader (track_file, num_tracks, "computing fixel-fixel connectivity matrix"); - DWI::Tractography::Mapping::TrackMapperBase mapper (index_image); - mapper.set_upsample_ratio (DWI::Tractography::Mapping::determine_upsample_ratio (index_image, properties, 0.333f)); - mapper.set_use_precise_mapping (true); + + template + class Receiver + { NOMEMALIGN + public: + Receiver (MatrixType& data) : + data (data) { } + bool operator() (const MappedTrack& in) const + { + try { + for (auto f : in) + data[f].add (in); + return true; + } catch (...) { + throw Exception ("Error assigning memory for CFE connectivity matrix"); + return false; + } + } + private: + MatrixType& data; + }; + } + + + +#define FIXEL_MATRIX_GENERATE_SHARED \ + auto directions_image = Fixel::find_directions_header (Path::dirname (index_image.name())).template get_image().with_direct_io ({+2,+1}); \ + DWI::Tractography::Properties properties; \ + DWI::Tractography::Reader track_file (track_filename, properties); \ + const uint32_t num_tracks = properties["count"].empty() ? 0 : to(properties["count"]); \ + DWI::Tractography::Mapping::TrackLoader loader (track_file, num_tracks, "computing fixel-fixel connectivity matrix"); \ + DWI::Tractography::Mapping::TrackMapperBase mapper (index_image); \ + mapper.set_upsample_ratio (DWI::Tractography::Mapping::determine_upsample_ratio (index_image, properties, 0.333f)); \ + mapper.set_use_precise_mapping (true); \ TrackProcessor track_processor (mapper, index_image, directions_image, fixel_mask, angular_threshold); - init_matrix_type connectivity_matrix (Fixel::get_number_of_fixels (index_image)); + + + + InitMatrixUnweighted generate_unweighted ( + const std::string& track_filename, + Image& index_image, + Image& fixel_mask, + const float angular_threshold) + { + FIXEL_MATRIX_GENERATE_SHARED + InitMatrixUnweighted connectivity_matrix (Fixel::get_number_of_fixels (index_image)); + Receiver receiver (connectivity_matrix); + Thread::run_queue (loader, + Thread::batch (DWI::Tractography::Streamline()), + track_processor, + Thread::batch (MappedTrack()), + receiver); + return connectivity_matrix; + } + + + + InitMatrixWeighted generate_weighted ( + const std::string& track_filename, + Image& index_image, + Image& fixel_mask, + const float angular_threshold) + { + FIXEL_MATRIX_GENERATE_SHARED + InitMatrixWeighted connectivity_matrix (Fixel::get_number_of_fixels (index_image)); + Receiver receiver (connectivity_matrix); Thread::run_queue (loader, Thread::batch (DWI::Tractography::Streamline()), track_processor, - Thread::batch (vector()), - // Inline lambda function for receiving streamline fixel visitations and - // updating the connectivity matrix - [&] (const vector& fixels) - { - try { - for (auto f : fixels) - connectivity_matrix[f].add (fixels); - return true; - } catch (...) { - throw Exception ("Error assigning memory for CFE connectivity matrix"); - return false; - } - }); + Thread::batch (MappedTrack()), + receiver); return connectivity_matrix; } @@ -230,14 +263,34 @@ namespace MR - void normalise_and_write (init_matrix_type& matrix, - const connectivity_value_type threshold, - const std::string& path, - const KeyValues& keyvals) + + template + void Writer::set_count_path (const std::string& path) + { + assert (!count_image.valid()); + count_image = Image::create (path, MR::Fixel::make_data_header (matrix.size())); + } + + template + void Writer::set_extent_path (const std::string& path) { + assert (!extent_image.valid()); + extent_image = Image::create (path, MR::Fixel::make_data_header (matrix.size())); + } + + + template + void Writer::save (const std::string& path) const + { if (Path::exists (path)) { - if (!Path::is_dir (path)) { + if (Path::is_dir (path)) { + if (!App::overwrite_files && (Path::is_file (Path::join (path, "index.mif")) || + Path::is_file (Path::join (path, "fixels.mif")) || + Path::is_file (Path::join (path, "values.mif")))) + throw Exception ("Cannot create fixel-fixel connectivity matrix \"" + path + "\": " + "one or more files already exists (use -force to override)"); + } else { if (App::overwrite_files) { File::remove (path); } else { @@ -263,6 +316,7 @@ namespace MR index_header.keyval() = keyvals; index_header.keyval()["nfixels"] = str(matrix.size()); index_header.datatype() = DataType::from(); + index_header.datatype().set_byte_order_native(); Image index_image = Image::create (Path::join (path, "index.mif"), index_header); // Can't use function write_mrtrix_header() as the file offset of the @@ -287,7 +341,7 @@ namespace MR if (stream_index) stream << DataType::from().specifier(); else - stream << DataType::from().specifier(); + stream << DataType::from().specifier(); stream << transform_type::Identity().matrix().topLeftCorner(3,4).format(fmt) << "\n"; stream << "scaling: 0,1\n"; stream << "nfixels: " + str(matrix.size()) + "\n"; @@ -301,7 +355,7 @@ namespace MR ProgressBar progress ("Normalising and writing fixel-fixel connectivity matrix to directory \"" + path + "\"", matrix.size()); size_t data_count = 0; - vector fixel_buffer; + vector fixel_buffer; vector value_buffer; for (size_t fixel_index = 0; fixel_index != matrix.size(); ++fixel_index) { @@ -310,12 +364,14 @@ namespace MR fixel_buffer.reserve (matrix[fixel_index].size()); value_buffer.reserve (matrix[fixel_index].size()); - const connectivity_value_type normalisation_factor = connectivity_value_type(1) / connectivity_value_type (matrix[fixel_index].count()); + const connectivity_value_type normalisation_factor = matrix[fixel_index].norm_factor(); + connectivity_value_type sum_connectivity = connectivity_value_type(0); for (auto& it : matrix[fixel_index]) { const connectivity_value_type connectivity = normalisation_factor * it.value(); if (connectivity >= threshold) { fixel_buffer.push_back (it.index()); value_buffer.push_back (connectivity); + sum_connectivity += connectivity; } } @@ -323,13 +379,22 @@ namespace MR index_image.index (3) = 0; index_image.value() = uint64_t(fixel_buffer.size()); index_image.index (3) = 1; index_image.value() = fixel_buffer.size() ? data_count : uint64_t(0); - fixel_stream.write (reinterpret_cast(fixel_buffer.data()), fixel_buffer.size() * sizeof (index_type)); + fixel_stream.write (reinterpret_cast(fixel_buffer.data()), fixel_buffer.size() * sizeof (fixel_index_type)); value_stream.write (reinterpret_cast(value_buffer.data()), value_buffer.size() * sizeof (connectivity_value_type)); data_count += fixel_buffer.size(); + if (count_image.valid()) { + count_image.index(0) = fixel_index; + count_image.value() = fixel_buffer.size(); + } + if (extent_image.valid()) { + extent_image.index(0) = fixel_index; + extent_image.value() = sum_connectivity; + } + // Force deallocation of memory used for this fixel in the generated matrix - InitFixel().swap (matrix[fixel_index]); + typename MatrixType::value_type().swap (matrix[fixel_index]); ++progress; } @@ -345,6 +410,9 @@ namespace MR } + template class Writer; + template class Writer; + diff --git a/src/fixel/matrix.h b/src/fixel/matrix.h index 323b80d20d..a721c37b1b 100644 --- a/src/fixel/matrix.h +++ b/src/fixel/matrix.h @@ -19,66 +19,158 @@ #include "image.h" #include "types.h" #include "file/ofstream.h" -#include "fixel/index_remapper.h" +#include "fixel/types.h" namespace MR { namespace Fixel { - - namespace Matrix { + using index_image_type = uint64_t; - using fixel_index_type = uint32_t; + using fixel_index_type = MR::Fixel::index_type; using count_type = uint32_t; using connectivity_value_type = float; - // Classes for dealing with dynamic multi-threaded construction of the - // fixel-fixel connectivity matrix - class InitElement + class MappedTrack : public vector + { NOMEMALIGN + public: + using BaseType = vector; + default_type get_weight() const { return weight; } + void set_weight (const default_type w) { weight = w; } + private: + default_type weight; + }; + + + + class InitElementBase + { NOMEMALIGN + public: + InitElementBase() : + fixel_index (std::numeric_limits::max()) { } + InitElementBase (const fixel_index_type fixel_index) : + fixel_index (fixel_index) { } + InitElementBase (const InitElementBase&) = default; + FORCE_INLINE InitElementBase& operator= (const InitElementBase& that) { fixel_index = that.fixel_index; return *this; } + FORCE_INLINE fixel_index_type index() const { return fixel_index; } + FORCE_INLINE bool operator< (const InitElementBase& that) const { return fixel_index < that.fixel_index; } + private: + fixel_index_type fixel_index; + }; + + + + class InitElementUnweighted : private InitElementBase { NOMEMALIGN public: - using ValueType = fixel_index_type; - InitElement() : - fixel_index (std::numeric_limits::max()), + using BaseType = InitElementBase; + using BaseType::operator<; + using ValueType = count_type; + InitElementUnweighted() : track_count (0) { } - InitElement (const fixel_index_type fixel_index) : - fixel_index (fixel_index), + InitElementUnweighted (const fixel_index_type fixel_index) : + BaseType (fixel_index), track_count (1) { } - InitElement (const fixel_index_type fixel_index, const ValueType track_count) : - fixel_index (fixel_index), - track_count (track_count) { } - InitElement (const InitElement&) = default; - FORCE_INLINE InitElement& operator++() { track_count++; return *this; } - FORCE_INLINE InitElement& operator= (const InitElement& that) { fixel_index = that.fixel_index; track_count = that.track_count; return *this; } - FORCE_INLINE fixel_index_type index() const { return fixel_index; } + InitElementUnweighted (const fixel_index_type fixel_index, const MappedTrack& all_data) : + BaseType (fixel_index), + track_count (1) { } + InitElementUnweighted (const InitElementUnweighted&) = default; + FORCE_INLINE fixel_index_type index() const { return BaseType::index(); } + FORCE_INLINE InitElementUnweighted& operator++() { track_count++; return *this; } + FORCE_INLINE InitElementUnweighted& operator= (const InitElementUnweighted& that) { BaseType::operator= (that); track_count = that.track_count; return *this; } FORCE_INLINE ValueType value() const { return track_count; } - FORCE_INLINE bool operator< (const InitElement& that) const { return fixel_index < that.fixel_index; } private: - fixel_index_type fixel_index; ValueType track_count; }; - class InitFixel : public vector - { MEMALIGN(InitFixel) + class InitElementWeighted : private InitElementBase + { NOMEMALIGN + public: + using BaseType = InitElementBase; + using BaseType::operator<; + using ValueType = connectivity_value_type; + InitElementWeighted() : + sum_weights (connectivity_value_type(0)) { } + InitElementWeighted (const fixel_index_type fixel_index) = delete; + InitElementWeighted (const fixel_index_type fixel_index, const MappedTrack& all_data) : + BaseType (fixel_index), + sum_weights (all_data.get_weight()) { } + InitElementWeighted (const InitElementWeighted&) = default; + FORCE_INLINE fixel_index_type index() const { return BaseType::index(); } + FORCE_INLINE InitElementWeighted& operator+= (const connectivity_value_type increment) { sum_weights += increment; return *this; } + FORCE_INLINE InitElementWeighted& operator= (const InitElementWeighted& that) { BaseType::operator= (that); sum_weights = that.sum_weights; return *this; } + FORCE_INLINE ValueType value() const { return sum_weights; } + private: + connectivity_value_type sum_weights; + }; + + + + template + class InitFixelBase : public vector + { NOMEMALIGN + public: + using BaseType = vector; + virtual ~InitFixelBase() { } + void add (const MappedTrack& mapped_track); + virtual default_type norm_factor() const = 0; + protected: + virtual void increment (const MappedTrack& data) = 0; + virtual void increment (ElementType& element, const MappedTrack& data) = 0; + }; + + class InitFixelUnweighted : public InitFixelBase + { NOMEMALIGN public: - using ElementType = InitElement; - using BaseType = vector; - InitFixel() : + using BaseType = InitFixelBase; + InitFixelUnweighted() : track_count (0) { } - void add (const vector& indices); - count_type count() const { return track_count; } + default_type norm_factor() const override { return 1.0 / default_type(track_count); } private: count_type track_count; + void increment (const MappedTrack& data) override { ++track_count; } + void increment (InitElementUnweighted& element, const MappedTrack& data) override { ++element; } }; + class InitFixelWeighted : public InitFixelBase + { NOMEMALIGN + public: + using BaseType = InitFixelBase; + InitFixelWeighted() : + sum_weights (default_type (0)) { } + default_type norm_factor() const override { return 1.0 / sum_weights; } + private: + default_type sum_weights; + void increment (const MappedTrack& data) override { sum_weights += data.get_weight(); } + void increment (InitElementWeighted& element, const MappedTrack& data) override { element += data.get_weight(); } + }; + + + + using InitMatrixUnweighted = vector; + using InitMatrixWeighted = vector; + + + + + + + + + + + + + + @@ -88,16 +180,16 @@ namespace MR { NOMEMALIGN public: using ValueType = connectivity_value_type; - NormElement (const index_type fixel_index, + NormElement (const fixel_index_type fixel_index, const ValueType connectivity_value) : fixel_index (fixel_index), connectivity_value (connectivity_value) { } - FORCE_INLINE index_type index() const { return fixel_index; } + FORCE_INLINE fixel_index_type index() const { return fixel_index; } FORCE_INLINE ValueType value() const { return connectivity_value; } FORCE_INLINE void exponentiate (const ValueType C) { connectivity_value = std::pow (connectivity_value, C); } FORCE_INLINE void normalise (const ValueType norm_factor) { connectivity_value *= norm_factor; } private: - index_type fixel_index; + fixel_index_type fixel_index; connectivity_value_type connectivity_value; }; @@ -136,10 +228,7 @@ namespace MR - // Different types are used depending on whether the connectivity matrix - // is in the process of being built, or whether it has been normalised - // TODO Revise - using init_matrix_type = vector; + @@ -147,7 +236,13 @@ namespace MR // Generate a fixel-fixel connectivity matrix - init_matrix_type generate ( + InitMatrixUnweighted generate_unweighted ( + const std::string& track_filename, + Image& index_image, + Image& fixel_mask, + const float angular_threshold); + + InitMatrixWeighted generate_weighted ( const std::string& track_filename, Image& index_image, Image& fixel_mask, @@ -155,35 +250,30 @@ namespace MR + template + class Writer + { MEMALIGN(Writer) + public: + Writer (MatrixType& matrix, + const connectivity_value_type threshold) : + matrix (matrix), + threshold (threshold) { } + void set_keyvals (KeyValues& kv) { keyvals = kv; } + void set_count_path (const std::string& path); + void set_extent_path (const std::string& path); + void save (const std::string& path) const; + private: + MatrixType& matrix; + const connectivity_value_type threshold; + KeyValues keyvals; + mutable Image count_image; + mutable Image extent_image; + }; + + - // New code for handling load/save of fixel-fixel connectivity matrix - // Use something akin to the fixel directory format, with a sub-directory - // within an existing fixel directory containing the following data: - // - index.mif: Similar to the index image in the fixel directory format, - // but has dimensions Nx1x1x2, where: - // - N is the number of fixels in the fixel template - // - The first volume contains the number of connected fixels for that fixel - // - The second volume contains the offset of the first connected fixel for that fixel - // - connectivity.mif: Floating-point image of dimension Cx1x1, where C is the total - // number of fixel-fixel connections stored in the entire matrix. Each value should be - // between 0.0 and 1.0, corresponding to the fraction of streamlines passing through - // the fixel that additionally pass through some other fixel. - // - fixel.mif: Unsigned integer image of dimension Cx1x1, where C is the total number of - // fixel-fixel connections stored in the entire matrix. Each value indexes the - // fixel to which the fixel-fixel connection refers. - // - // In order to avoid duplication of memory usage, the writing function should - // perform, for each fixel in turn: - // - Normalisation of the matrix - // - Writing to the three images - // - Erasing the memory used for that matrix in the initial building - - void normalise_and_write (init_matrix_type& matrix, - const connectivity_value_type threshold, - const std::string& path, - const KeyValues& keyvals = KeyValues()); From 9978009d18f982dfe0d8a39b4c6976704b8073d0 Mon Sep 17 00:00:00 2001 From: J-Donald Tournier Date: Thu, 19 Jan 2023 15:07:07 +0000 Subject: [PATCH 02/22] mrview: change option name -orientationlabel to avoid conflict with -orientation --- src/gui/mrview/window.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/gui/mrview/window.cpp b/src/gui/mrview/window.cpp index 7bfcb5374c..cec6454c1b 100644 --- a/src/gui/mrview/window.cpp +++ b/src/gui/mrview/window.cpp @@ -2084,12 +2084,12 @@ namespace MR return; } - if (opt.opt->is ("orientationlabel")) { + if (opt.opt->is ("orientlabel")) { try { show_orientation_labels_action->setChecked (to (opt[0])); } catch (Exception& E) { - throw Exception ("-orientationlabel option expects a boolean"); + throw Exception ("-orientlabel option expects a boolean"); } glarea->update(); return; @@ -2206,7 +2206,7 @@ namespace MR + Option ("voxelinfo", "Show or hide voxel information overlay.").allow_multiple() + Argument ("boolean").type_bool () - + Option ("orientationlabel", "Show or hide orientation label overlay.").allow_multiple() + + Option ("orientlabel", "Show or hide orientation label overlay.").allow_multiple() + Argument ("boolean").type_bool () + Option ("colourbar", "Show or hide colourbar overlay.").allow_multiple() From 52f5bb5baf81e62dcb782df7a46eb61485286004 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Fri, 20 Jan 2023 10:42:57 +1100 Subject: [PATCH 03/22] =?UTF-8?q?Docs=20update=20for=20#2569=20/=20=C2=969?= =?UTF-8?q?978009d?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/reference/commands/mrview.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/commands/mrview.rst b/docs/reference/commands/mrview.rst index 5cfc207cb3..21a3bc81dc 100644 --- a/docs/reference/commands/mrview.rst +++ b/docs/reference/commands/mrview.rst @@ -70,7 +70,7 @@ View options - **-voxelinfo boolean** *(multiple uses permitted)* Show or hide voxel information overlay. -- **-orientationlabel boolean** *(multiple uses permitted)* Show or hide orientation label overlay. +- **-orientlabel boolean** *(multiple uses permitted)* Show or hide orientation label overlay. - **-colourbar boolean** *(multiple uses permitted)* Show or hide colourbar overlay. From eea0db901bbd03301d30537eaa07a5561d4829c7 Mon Sep 17 00:00:00 2001 From: J-Donald Tournier Date: Fri, 3 Feb 2023 10:15:34 +0000 Subject: [PATCH 04/22] Revert "mrview: change option name -orientationlabel to avoid conflict with -orientation" This reverts PR #2569 --- docs/reference/commands/mrview.rst | 2 +- src/gui/mrview/window.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/reference/commands/mrview.rst b/docs/reference/commands/mrview.rst index 21a3bc81dc..5cfc207cb3 100644 --- a/docs/reference/commands/mrview.rst +++ b/docs/reference/commands/mrview.rst @@ -70,7 +70,7 @@ View options - **-voxelinfo boolean** *(multiple uses permitted)* Show or hide voxel information overlay. -- **-orientlabel boolean** *(multiple uses permitted)* Show or hide orientation label overlay. +- **-orientationlabel boolean** *(multiple uses permitted)* Show or hide orientation label overlay. - **-colourbar boolean** *(multiple uses permitted)* Show or hide colourbar overlay. diff --git a/src/gui/mrview/window.cpp b/src/gui/mrview/window.cpp index cec6454c1b..7bfcb5374c 100644 --- a/src/gui/mrview/window.cpp +++ b/src/gui/mrview/window.cpp @@ -2084,12 +2084,12 @@ namespace MR return; } - if (opt.opt->is ("orientlabel")) { + if (opt.opt->is ("orientationlabel")) { try { show_orientation_labels_action->setChecked (to (opt[0])); } catch (Exception& E) { - throw Exception ("-orientlabel option expects a boolean"); + throw Exception ("-orientationlabel option expects a boolean"); } glarea->update(); return; @@ -2206,7 +2206,7 @@ namespace MR + Option ("voxelinfo", "Show or hide voxel information overlay.").allow_multiple() + Argument ("boolean").type_bool () - + Option ("orientlabel", "Show or hide orientation label overlay.").allow_multiple() + + Option ("orientationlabel", "Show or hide orientation label overlay.").allow_multiple() + Argument ("boolean").type_bool () + Option ("colourbar", "Show or hide colourbar overlay.").allow_multiple() From d357c0db7ea3af87d861b436625da9d689c3a931 Mon Sep 17 00:00:00 2001 From: J-Donald Tournier Date: Fri, 17 Feb 2023 11:16:53 +0000 Subject: [PATCH 05/22] Update GitHub Actions workflow for merge queue The plan is to make use of GitHub's new merge queue feature, to reduce the faff when trying to merge multiple pull requests at the same time. More detail on the GitHub docs: https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue Note that this has been activated on master. --- .github/workflows/checks.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 89f556ed21..17fdd705a7 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -3,9 +3,11 @@ name: checks on: pull_request: types: [opened, synchronize] - branches: - - master - - dev + branches: [ master, dev ] + merge_group: + types: [checks_requested] + branches: [ master ] + From f4097203052ded3cd65561abce324a7240f78a9c Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Wed, 1 Mar 2023 12:43:26 +1100 Subject: [PATCH 06/22] Add file ".git-blame-ignore-revs" Closes #2590. --- .git-blame-ignore-revs | 81 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 .git-blame-ignore-revs diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs new file mode 100644 index 0000000000..c42f444727 --- /dev/null +++ b/.git-blame-ignore-revs @@ -0,0 +1,81 @@ +# .git-blame-ignore-revs + +9268f0874fc8b2c3af0a090b8084bd91707a115e +#Author: Daan Christiaens +#Date: Fri Dec 11 14:41:03 2015 +0100 +# Update copyright header in all source files, as discussed in #9 and #31. + +3024a116078eabd3b1353fc8329a73144632c86c +#Author: Thijs Dhollander +#Date: Thu Nov 22 17:01:59 2018 +1100 +# Copyright message update for next release (probably 2019-ish) +# Also updated the warranty statement: it appears this was still a formulation left over from the previous GPL license. The new formulation comes directly from the MPL 2.0, but is also more comprehensive; I'm more comfortable with having this more comprehensive. Furthermore, apart from it apparently being quite important to have a warranty statement at the top of source code files, it's apparently also of little value if it's not either very comprehensive, or it also directly refers to the license where the full text can be found. Hence, I added that as a sentence. +# Finally, also of note (and TODO) would be to add the copyright statement and a copy of the license somewhere on our (mrtrix.org) website. The final link is otherwise relatively useless in the context of the copyright and license statement. + +6c6c553ed9e9004ca8495ece436fec4db465d82b +#Author: Thijs Dhollander +#Date: Wed Dec 13 15:09:37 2017 +1100 +# Copyright update for upcoming RC which will probably end up around New Year's +# Also includes a change of mention of MRtrix to MRtrix3, in line with the phrase MRtrix3 developers, which was already in there/ +# Also includes removal of 2 .s, because they hindered clickability of the links in certain environments. + +654b7281c953f1068142d1f8a152f786862d8876 +#Author: Thijs Dhollander +#Date: Wed Jan 25 17:57:11 2017 +1100 +# header files + +b623e418e36de6884511d9c233b04392ecce89bb +#Author: Thijs Dhollander +#Date: Wed Jan 25 17:06:35 2017 +1100 +# Copyright for 2017 in headers + +95b144df3d9eb837fae08129cc292c3fb8490eac +#Author: Robert Smith +#Date: Tue Oct 8 14:33:10 2019 +1100 +# Add copyright notice to non-CPP files +# Also includes some line ending conversions and indentation changes. + +be9a46286a9053fd1c1951fe5394206a95b61bfa +#Author: MRtrixBot +#Date: Thu May 14 11:30:13 2020 +1000 +# Initial commit of "update_copyright" changes + +5e3112eae6ba4027aba854b3385f032c67dbfedf +#Author: MRtrixBot +#Date: Mon Feb 7 16:48:14 2022 +0000 +# update copyright notice and corresponding docs + +e8edf6d946822c8ec23d077c2b1e9eef471b2539 +#Author: MRtrixBot +#Date: Tue Jan 3 13:41:40 2023 +0100 +# Update copyright notice + +74ff7cf0b76e2c9595b1341a7e4a49fa5491ce2f +#Author: MRtrixBot +#Date: Wed Jan 6 12:50:54 2021 +0000 +# Update copyright notice + +65b3ea5e549f3f66a9e3c5d9ebdabb2a9bb462b4 +#Author: Thijs Dhollander +#Date: Thu Nov 22 17:30:36 2018 +1100 +# Fixed typo in copyright/warranty statement. + +729dd6cfc1a773f0f16592b8533c6e9d89d03ac3 +#Author: Thijs Dhollander +#Date: Mon May 15 10:33:05 2017 +1000 +# copyright update and cleanup + +76ad4fbb3ea60ea56ec94e3debfce1b7a35a9535 +#Author: rtabbara +#Date: Thu Feb 18 11:55:53 2016 +1100 +# User docs: Update commands list with new copyright + +6552f6ebe4f9fda441255063230ef9f8d9912591 +#Author: J-Donald Tournier +#Date: Wed Feb 5 12:50:53 2020 +0000 +# remove carriage returns + +811361d3af3ecfec03e20c2ee0f342634810a9c2 +#Author: Thijs Dhollander +#Date: Tue May 9 09:09:51 2017 +1000 +# docs update From 68a663494c14acb118015c578fc1e80ca70ff549 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Wed, 1 Mar 2023 12:53:34 +1100 Subject: [PATCH 07/22] .git-blame-ignore-revs: Final addition and instructions --- .git-blame-ignore-revs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index c42f444727..ec7f0e9408 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -1,4 +1,7 @@ # .git-blame-ignore-revs +# If you want this file to be utilised whener you utilise "git blame" on your local instance of the repository, +# run the following command from its root directory: +# git config blame.ignoreRevsFile .git-blame-ignore-revs 9268f0874fc8b2c3af0a090b8084bd91707a115e #Author: Daan Christiaens @@ -79,3 +82,8 @@ e8edf6d946822c8ec23d077c2b1e9eef471b2539 #Author: Thijs Dhollander #Date: Tue May 9 09:09:51 2017 +1000 # docs update + +1eb36099870a0fffbc307ac40523dd8b6e35436f +#Author: Thijs Dhollander +#Date: Thu Feb 2 12:26:20 2017 +1100 +# standardise number of blank lines between copyright header and the rest of a file (was getting a bit out of hand for some files; now it's 2 blank lines for all) From 7daaa53bda9107b2c92b63cf55b3f35569ed4e68 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Wed, 1 Mar 2023 13:02:42 +1100 Subject: [PATCH 08/22] .git-blame-ignore-revs: Add 2013 copyright update commits --- .git-blame-ignore-revs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index ec7f0e9408..9c547f300a 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -87,3 +87,13 @@ e8edf6d946822c8ec23d077c2b1e9eef471b2539 #Author: Thijs Dhollander #Date: Thu Feb 2 12:26:20 2017 +1100 # standardise number of blank lines between copyright header and the rest of a file (was getting a bit out of hand for some files; now it's 2 blank lines for all) + +e8edf6d946822c8ec23d077c2b1e9eef471b2539 +#Author: MRtrixBot +#Date: Tue Jan 3 13:41:40 2023 +0100 +# Update copyright notice + +aad44d847ac48d02bb7f8badf801dbfaa0ccdac0 +#Author: MRtrixBot +#Date: Tue Jan 3 13:42:58 2023 +0100 +# Update Copyright notice in command docs From e4b353017157890bcaf8600f4c794c0abeb5f335 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Fri, 3 Mar 2023 12:46:07 +1100 Subject: [PATCH 09/22] VTK surface format changes - Change binary export to single-precision floating-point. - Change binary export to big-endian. - Support import of both little-endian and big-endian data. Closes #2593. --- src/surface/mesh.cpp | 157 ++++++++++++++++++++++++++----------------- 1 file changed, 95 insertions(+), 62 deletions(-) diff --git a/src/surface/mesh.cpp b/src/surface/mesh.cpp index 51fc4008ba..a319d900d2 100644 --- a/src/surface/mesh.cpp +++ b/src/surface/mesh.cpp @@ -92,6 +92,20 @@ namespace MR + namespace { + template + void load_vtk_points_binary (std::ifstream& in, const size_t num_vertices, vector>& out) + { + out.reserve (num_vertices); + Eigen::Matrix v; + for (int i = 0; i != num_vertices; ++i) { + in.read (reinterpret_cast(v.data()), 3 * sizeof (T)); + out.push_back (v); + } + } + } + + void Mesh::load_vtk (const std::string& path) { @@ -136,6 +150,13 @@ namespace MR in.seekg (offset); } + // Won't know endianness of file when the vertex positions are read, + // only when the polygon information is encountered; + bool change_endianness = false; + + // If both float and big-endian, need to store natively as floats and swap endianness later + vector> vertices_float; + // From here, don't necessarily know which parts of the data will come first while (!in.eof()) { @@ -166,25 +187,22 @@ namespace MR throw Exception ("Error in reading binary .vtk file: Unsupported datatype (\"" + line + "\""); vertices.reserve (num_vertices); - for (int i = 0; i != num_vertices; ++i) { + if (!is_double) + vertices_float.reserve (num_vertices); - Vertex v; - if (is_ascii) { + Vertex v; + Eigen::Matrix v_float; + + if (is_ascii) { + for (int i = 0; i != num_vertices; ++i) { std::getline (in, line); sscanf (line.c_str(), "%lf %lf %lf", &v[0], &v[1], &v[2]); - } else { - if (is_double) { - double data[3]; - in.read (reinterpret_cast(&data[0]), 3 * sizeof (double)); - v = { data[0], data[1], data[2] }; - } else { - float data[3]; - in.read (reinterpret_cast(&data[0]), 3 * sizeof (float)); - v = { data[0], data[1], data[2] }; - } + vertices.push_back (v); } - vertices.push_back (v); - + } else if (is_double) { + load_vtk_points_binary (in, num_vertices, vertices); + } else { + load_vtk_points_binary (in, num_vertices, vertices_float); } } else if (line.substr (0, 8) == "POLYGONS") { @@ -206,8 +224,16 @@ namespace MR in.read (reinterpret_cast(&vertex_count), sizeof (int)); } + if (!is_ascii) { + if (change_endianness) { + vertex_count = ByteOrder::swap (vertex_count); + } else if (vertex_count != 3 && vertex_count != 4) { + vertex_count = ByteOrder::swap (vertex_count); + change_endianness = true; + } + } if (vertex_count != 3 && vertex_count != 4) - throw Exception ("Could not parse file \"" + path + "\"; only suppport 3- and 4-vertex polygons"); + throw Exception ("Could not parse file \"" + path + "\": only suppport 3- and 4-vertex polygons"); vector t (vertex_count, 0); @@ -237,17 +263,41 @@ namespace MR } } - // TODO If reading a binary file, may want to test endianness of data - // There's no explicit flag for this, but just calculating the standard - // deviations of all vertex positions may be adequate - // (likely to be huge if the endianness is wrong) - // Alternatively, just test the polygon indices: if there's at least one that exceeds the - // number of vertices, it may be saved in big-endian format, so try flipping everything - // Actually, should pop up at the first polygon read: number of points in polygon won't be 3 or 4 + if (change_endianness) { + INFO ("File \"" + path + "\" is stored as " + +#if MRTRIX_IS_BIG_ENDIAN + "little" +#else + "big" +#endif + + " endian; swapping memory contents to match system"); + for (auto& v : vertices) { + for (size_t i = 0; i != 3; ++i) + v[i] = ByteOrder::swap (v[i]); + } + for (auto& v : vertices_float) { + for (size_t i = 0; i != 3; ++i) + v[i] = ByteOrder::swap (v[i]); + } + for (auto& t : triangles) { + for (size_t i = 0; i != 3; ++i) + t[i] = ByteOrder::swap (t[i]); + } + for (auto& q : quads) { + for (size_t i = 0; i != 4; ++i) + q[i] = ByteOrder::swap (q[i]); + } + } + + if (vertices_float.size()) { + assert (!vertices.size()); + for (const auto& v : vertices_float) + vertices.emplace_back (Vertex (v.cast())); + } try { verify_data(); - } catch(Exception& e) { + } catch (Exception& e) { throw Exception (e, "Error verifying surface data from VTK file \"" + path + "\""); } } @@ -634,66 +684,49 @@ namespace MR ProgressBar progress ("writing mesh to file", vertices.size() + triangles.size() + quads.size()); if (binary) { - // FIXME Binary VTK output _still_ not working (crashes ParaView) - // Can however export as binary then -reconvert to ASCII and al is well...? - // Changed to big-endian output, doesn't seem to have fixed... - out.close(); out.open (path, std::ios_base::out | std::ios_base::app | std::ios_base::binary); - const bool is_double = (sizeof(default_type) == 8); - const std::string str_datatype = is_double ? "double" : "float"; - const std::string points_header ("POINTS " + str(vertices.size()) + " " + str_datatype + "\n"); + const std::string points_header ("POINTS " + str(vertices.size()) + " double\n"); out.write (points_header.c_str(), points_header.size()); - for (VertexList::const_iterator i = vertices.begin(); i != vertices.end(); ++i) { - //float temp[3]; - //for (size_t id = 0; id != 3; ++id) - // MR::putBE ((*i)[id], &temp[id]); - if (is_double) { - const double temp[3] { double((*i)[0]), double((*i)[1]), double((*i)[2]) }; - out.write (reinterpret_cast(temp), 3 * sizeof(double)); - } else { - const float temp[3] { float((*i)[0]), float((*i)[1]), float((*i)[2]) }; - out.write (reinterpret_cast(temp), 3 * sizeof(float)); - } + std::array temp_vertex; + for (const auto& v : vertices) { + temp_vertex = { ByteOrder::BE (double(v[0])), ByteOrder::BE (double(v[1])), ByteOrder::BE (double(v[2])) }; + out.write (reinterpret_cast(&temp_vertex), 3 * sizeof(double)); ++progress; } const std::string polygons_header ("POLYGONS " + str(triangles.size() + quads.size()) + " " + str(4*triangles.size() + 5*quads.size()) + "\n"); out.write (polygons_header.c_str(), polygons_header.size()); - const uint32_t num_points_triangle = 3; - for (TriangleList::const_iterator i = triangles.begin(); i != triangles.end(); ++i) { + const uint32_t num_points_triangle = ByteOrder::BE (uint32_t(3)); + std::array temp_triangle; + for (const auto& t : triangles) { out.write (reinterpret_cast(&num_points_triangle), sizeof(uint32_t)); - //uint32_t temp[3]; - //for (size_t id = 0; id != 3; ++id) - // MR::putBE ((*i)[id], &temp[id]); - const uint32_t temp[3] { (*i)[0], (*i)[1], (*i)[2] }; - out.write (reinterpret_cast(temp), 3 * sizeof(uint32_t)); + temp_triangle = { ByteOrder::BE (t[0]), ByteOrder::BE (t[1]), ByteOrder::BE (t[2]) }; + out.write (reinterpret_cast(&temp_triangle), 3 * sizeof(uint32_t)); ++progress; } - const uint32_t num_points_quad = 4; - for (QuadList::const_iterator i = quads.begin(); i != quads.end(); ++i) { + const uint32_t num_points_quad = ByteOrder::BE (uint32_t(4)); + std::array temp_quad; + for (const auto& q : quads) { out.write (reinterpret_cast(&num_points_quad), sizeof(uint32_t)); - //uint32_t temp[4]; - //for (size_t id = 0; id != 4; ++id) - // MR::putBE ((*i)[id], &temp[id]); - const uint32_t temp[4] { (*i)[0], (*i)[1], (*i)[2], (*i)[3] }; - out.write (reinterpret_cast(temp), 4 * sizeof(uint32_t)); + temp_quad = { ByteOrder::BE (q[0]), ByteOrder::BE (q[1]), ByteOrder::BE (q[2]), ByteOrder::BE (q[3]) }; + out.write (reinterpret_cast(&temp_quad), 4 * sizeof(uint32_t)); ++progress; } } else { out << "POINTS " << str(vertices.size()) << " float\n"; - for (VertexList::const_iterator i = vertices.begin(); i != vertices.end(); ++i) { - out << str((*i)[0]) << " " << str((*i)[1]) << " " << str((*i)[2]) << "\n"; + for (const auto& v : vertices) { + out << str(v[0]) << " " << str(v[1]) << " " << str(v[2]) << "\n"; ++progress; } out << "POLYGONS " + str(triangles.size() + quads.size()) + " " + str(4*triangles.size() + 5*quads.size()) + "\n"; - for (TriangleList::const_iterator i = triangles.begin(); i != triangles.end(); ++i) { - out << "3 " << str((*i)[0]) << " " << str((*i)[1]) << " " << str((*i)[2]) << "\n"; + for (const auto& t : triangles) { + out << "3 " << str(t[0]) << " " << str(t[1]) << " " << str(t[2]) << "\n"; ++progress; } - for (QuadList::const_iterator i = quads.begin(); i != quads.end(); ++i) { - out << "4 " << str((*i)[0]) << " " << str((*i)[1]) << " " << str((*i)[2]) << " " << str((*i)[3]) << "\n"; + for (const auto& q : quads) { + out << "4 " << str(q[0]) << " " << str(q[1]) << " " << str(q[2]) << " " << str(q[3]) << "\n"; ++progress; } From 11d3bf2b2eac6cfb946c5c6763827c21f62796b4 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Mon, 6 Mar 2023 10:49:07 +1100 Subject: [PATCH 10/22] VTK load: Change terminal message RE endianness --- src/surface/mesh.cpp | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/surface/mesh.cpp b/src/surface/mesh.cpp index a319d900d2..193db5d521 100644 --- a/src/surface/mesh.cpp +++ b/src/surface/mesh.cpp @@ -233,7 +233,7 @@ namespace MR } } if (vertex_count != 3 && vertex_count != 4) - throw Exception ("Could not parse file \"" + path + "\": only suppport 3- and 4-vertex polygons"); + throw Exception ("Could not parse file \"" + path + "\": only support 3- and 4-vertex polygons"); vector t (vertex_count, 0); @@ -263,14 +263,23 @@ namespace MR } } - if (change_endianness) { - INFO ("File \"" + path + "\" is stored as " + #if MRTRIX_IS_BIG_ENDIAN - "little" + if (change_endianness) { + WARN("File \"" + path + "\" is little-endian, so is not format-compliant (may have been generated using an older MRtrix3 version); " + "imported contents will be converted to system big-endian"); + } else { + INFO("File \"" + path + "\" is big-endian; no format conversion required as executing on big-endian system"); + } #else - "big" + if (change_endianness) { + INFO("Converting imported contents of file \"" + path + "\" to native little-endian"); + } else { + WARN("File \"" + path + "\" already in native little-endian format, so no endianness conversion required; " + "but file is therefore not format-compliant (may have been generated using an older MRtrix3 version)"); + } #endif - + " endian; swapping memory contents to match system"); + + if (change_endianness) { for (auto& v : vertices) { for (size_t i = 0; i != 3; ++i) v[i] = ByteOrder::swap (v[i]); From 2fa95def879047f5219a02a8dda004dd0a45c778 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Mon, 6 Mar 2023 11:35:28 +1100 Subject: [PATCH 11/22] VTK: Change binary output to single-precision Commit e4b353017157890bcaf8600f4c794c0abeb5f335 was supposed to include this change, but while write was made independent of the width of default_type, it was erroneously hard-wired to double-precision rather than single. --- src/surface/mesh.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/surface/mesh.cpp b/src/surface/mesh.cpp index 193db5d521..ee9ff74b9e 100644 --- a/src/surface/mesh.cpp +++ b/src/surface/mesh.cpp @@ -695,12 +695,12 @@ namespace MR out.close(); out.open (path, std::ios_base::out | std::ios_base::app | std::ios_base::binary); - const std::string points_header ("POINTS " + str(vertices.size()) + " double\n"); + const std::string points_header ("POINTS " + str(vertices.size()) + " float\n"); out.write (points_header.c_str(), points_header.size()); - std::array temp_vertex; + std::array temp_vertex; for (const auto& v : vertices) { - temp_vertex = { ByteOrder::BE (double(v[0])), ByteOrder::BE (double(v[1])), ByteOrder::BE (double(v[2])) }; - out.write (reinterpret_cast(&temp_vertex), 3 * sizeof(double)); + temp_vertex = { ByteOrder::BE (float(v[0])), ByteOrder::BE (float(v[1])), ByteOrder::BE (float(v[2])) }; + out.write (reinterpret_cast(&temp_vertex), 3 * sizeof(float)); ++progress; } const std::string polygons_header ("POLYGONS " + str(triangles.size() + quads.size()) + " " + str(4*triangles.size() + 5*quads.size()) + "\n"); From 95696c5a6f2ec828dd5aa8290fb2322a374ab81b Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Mon, 6 Mar 2023 12:04:47 +1100 Subject: [PATCH 12/22] meshconvert: Updated tests for endianness support --- testing/binaries/data | 2 +- testing/binaries/tests/meshconvert | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/testing/binaries/data b/testing/binaries/data index 26b8e57335..e5646b6b5a 160000 --- a/testing/binaries/data +++ b/testing/binaries/data @@ -1 +1 @@ -Subproject commit 26b8e573353e30e1283d5446495391eaacdd1cf1 +Subproject commit e5646b6b5a5311df287610c1b0ea4e13388d6740 diff --git a/testing/binaries/tests/meshconvert b/testing/binaries/tests/meshconvert index 2a762d25bf..38683a4686 100644 --- a/testing/binaries/tests/meshconvert +++ b/testing/binaries/tests/meshconvert @@ -1,10 +1,12 @@ -meshconvert meshconvert/in.vtk tmp.vtk -force && testing_diff_mesh tmp.vtk meshconvert/in.vtk 0.001 -meshconvert meshconvert/in.vtk tmp.vtk -binary -force && testing_diff_mesh tmp.vtk meshconvert/in.vtk 0.001 -meshconvert meshconvert/in.vtk tmp.obj -force && testing_diff_mesh tmp.obj meshconvert/in.vtk 0.001 -meshconvert meshconvert/in.vtk tmp.obj -binary -force && testing_diff_mesh tmp.obj meshconvert/in.vtk 0.001 -meshconvert meshconvert/in.vtk tmp.stl -force && testing_diff_mesh tmp.stl meshconvert/in.vtk 0.001 -meshconvert meshconvert/in.vtk tmp.stl -binary -force && testing_diff_mesh tmp.stl meshconvert/in.vtk 0.001 -meshconvert meshconvert/in.vtk tmp.vtk -transform real2first meshconvert/image.mif.gz -force && testing_diff_mesh tmp.vtk meshconvert/first.vtk 0.001 -meshconvert meshconvert/first.vtk tmp.vtk -transform first2real meshconvert/image.mif.gz -force && testing_diff_mesh tmp.vtk meshconvert/in.vtk 0.001 -meshconvert meshconvert/in.vtk tmp.vtk -transform real2voxel meshconvert/image.mif.gz -force && testing_diff_mesh tmp.vtk meshconvert/voxel.vtk 0.001 -meshconvert meshconvert/voxel.vtk tmp.vtk -transform voxel2real meshconvert/image.mif.gz -force && testing_diff_mesh tmp.vtk meshconvert/in.vtk 0.001 +meshconvert meshconvert/in_ascii.vtk tmp.vtk -force && testing_diff_mesh tmp.vtk meshconvert/in_ascii.vtk 0.001 +meshconvert meshconvert/in_ascii.vtk tmp.vtk -binary -force && testing_diff_mesh tmp.vtk meshconvert/in_ascii.vtk 0.001 +meshconvert meshconvert/in_ascii.vtk tmp.obj -force && testing_diff_mesh tmp.obj meshconvert/in_ascii.vtk 0.001 +meshconvert meshconvert/in_ascii.vtk tmp.obj -binary -force && testing_diff_mesh tmp.obj meshconvert/in_ascii.vtk 0.001 +meshconvert meshconvert/in_ascii.vtk tmp.stl -force && testing_diff_mesh tmp.stl meshconvert/in_ascii.vtk 0.001 +meshconvert meshconvert/in_ascii.vtk tmp.stl -binary -force && testing_diff_mesh tmp.stl meshconvert/in_ascii.vtk 0.001 +meshconvert meshconvert/in_ascii.vtk tmp.vtk -transform real2first meshconvert/image.mif.gz -force && testing_diff_mesh tmp.vtk meshconvert/first.vtk 0.001 +meshconvert meshconvert/first.vtk tmp.vtk -transform first2real meshconvert/image.mif.gz -force && testing_diff_mesh tmp.vtk meshconvert/in_ascii.vtk 0.001 +meshconvert meshconvert/in_ascii.vtk tmp.vtk -transform real2voxel meshconvert/image.mif.gz -force && testing_diff_mesh tmp.vtk meshconvert/voxel.vtk 0.001 +meshconvert meshconvert/voxel.vtk tmp.vtk -transform voxel2real meshconvert/image.mif.gz -force && testing_diff_mesh tmp.vtk meshconvert/in_ascii.vtk 0.001 +meshconvert meshconvert/in_le.vtk tmp.vtk -force && testing_diff_mesh tmp.vtk meshconvert/in_ascii.vtk 0.001 +meshconvert meshconvert/in_be.vtk tmp.vtk -force && testing_diff_mesh tmp.vtk meshconvert/in_ascii.vtk 0.001 From c49eb0d3f608348c66743f611c48b24cba24d4ae Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Mon, 6 Mar 2023 12:44:09 +1100 Subject: [PATCH 13/22] Fixes to pass CI tests for VTK changes (#2594) --- src/surface/mesh.cpp | 2 +- testing/binaries/tests/mesh2voxel | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/surface/mesh.cpp b/src/surface/mesh.cpp index ee9ff74b9e..0935feea35 100644 --- a/src/surface/mesh.cpp +++ b/src/surface/mesh.cpp @@ -98,7 +98,7 @@ namespace MR { out.reserve (num_vertices); Eigen::Matrix v; - for (int i = 0; i != num_vertices; ++i) { + for (size_t i = 0; i != num_vertices; ++i) { in.read (reinterpret_cast(v.data()), 3 * sizeof (T)); out.push_back (v); } diff --git a/testing/binaries/tests/mesh2voxel b/testing/binaries/tests/mesh2voxel index fe419fbb18..c893b8c318 100644 --- a/testing/binaries/tests/mesh2voxel +++ b/testing/binaries/tests/mesh2voxel @@ -1 +1 @@ -mesh2voxel meshconvert/in.vtk meshconvert/image.mif.gz - | testing_diff_image - mesh2voxel/out.mif.gz -abs 1.5e-3 +mesh2voxel meshconvert/in_ascii.vtk meshconvert/image.mif.gz - | testing_diff_image - mesh2voxel/out.mif.gz -abs 1.5e-3 From 380105f97620eceb3b9670b7384751bdb1da6f92 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Wed, 10 May 2023 18:49:31 +1000 Subject: [PATCH 14/22] App::get_matches(): Fix buffer overrun Replaces both d5767a7 and ebb35a5. The latter broke functionality of use of command-line option substrings, as discvered by failing CI in #2629. --- core/app.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/app.cpp b/core/app.cpp index 0561070797..d6c526597f 100644 --- a/core/app.cpp +++ b/core/app.cpp @@ -128,7 +128,7 @@ namespace MR inline void get_matches (vector& candidates, const OptionGroup& group, const std::string& stub) { for (size_t i = 0; i < group.size(); ++i) { - if (stub.compare (0, stub.size(), group[i].id, stub.size()) == 0) + if (stub.compare (0, stub.size(), std::string(group[i].id), 0, stub.size()) == 0) candidates.push_back (&group[i]); } } From 7fe277ca5254f62e3e0080a8c9d161a2c53a5137 Mon Sep 17 00:00:00 2001 From: J-Donald Tournier Date: Tue, 13 Jun 2023 16:37:02 +0100 Subject: [PATCH 15/22] Mesh::load_vtk() always open files in binary mode --- src/surface/mesh.cpp | 49 ++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/src/surface/mesh.cpp b/src/surface/mesh.cpp index 0935feea35..74bab19015 100644 --- a/src/surface/mesh.cpp +++ b/src/surface/mesh.cpp @@ -110,14 +110,14 @@ namespace MR void Mesh::load_vtk (const std::string& path) { - std::ifstream in (path.c_str(), std::ios_base::in); + std::ifstream in (path.c_str(), std::ios_base::binary); if (!in) throw Exception ("Error opening input file!"); std::string line; // First line: VTK version ID - std::getline (in, line); + MR::getline (in, line); // Strip the version numbers line[23] = line[25] = 'x'; // Verify that the line is correct @@ -125,10 +125,10 @@ namespace MR throw Exception ("Incorrect first line of .vtk file"); // Second line: identifier - std::getline (in, line); + MR::getline (in, line); // Third line: format of data - std::getline (in, line); + MR::getline (in, line); bool is_ascii = false; if (line == "ASCII") is_ascii = true; @@ -136,20 +136,13 @@ namespace MR throw Exception ("unknown data format in .vtk data"); // Fourth line: Data set type - std::getline (in, line); + MR::getline (in, line); if (line.substr(0, 7) != "DATASET") throw Exception ("Error in definition of .vtk dataset"); line = line.substr (8); if (line == "STRUCTURED_POINTS" || line == "STRUCTURED_GRID" || line == "UNSTRUCTURED_GRID" || line == "RECTILINEAR_GRID" || line == "FIELD") throw Exception ("Unsupported dataset type (" + line + ") in .vtk file"); - if (!is_ascii) { - const std::streampos offset = in.tellg(); - in.close(); - in.open (path.c_str(), std::ios_base::in | std::ios_base::binary); - in.seekg (offset); - } - // Won't know endianness of file when the vertex positions are read, // only when the polygon information is encountered; bool change_endianness = false; @@ -162,7 +155,7 @@ namespace MR // Need to read line in either ASCII mode or in raw binary if (is_ascii) { - std::getline (in, line); + MR::getline (in, line); } else { line.clear(); char c = 0; @@ -195,7 +188,7 @@ namespace MR if (is_ascii) { for (int i = 0; i != num_vertices; ++i) { - std::getline (in, line); + MR::getline (in, line); sscanf (line.c_str(), "%lf %lf %lf", &v[0], &v[1], &v[2]); vertices.push_back (v); } @@ -218,7 +211,7 @@ namespace MR int vertex_count; if (is_ascii) { - std::getline (in, line); + MR::getline (in, line); sscanf (line.c_str(), "%u", &vertex_count); } else { in.read (reinterpret_cast(&vertex_count), sizeof (int)); @@ -263,21 +256,23 @@ namespace MR } } + if (!is_ascii) { #if MRTRIX_IS_BIG_ENDIAN - if (change_endianness) { - WARN("File \"" + path + "\" is little-endian, so is not format-compliant (may have been generated using an older MRtrix3 version); " - "imported contents will be converted to system big-endian"); - } else { - INFO("File \"" + path + "\" is big-endian; no format conversion required as executing on big-endian system"); - } + if (change_endianness) { + WARN("File \"" + path + "\" is little-endian, so is not format-compliant (may have been generated using an older MRtrix3 version); " + "imported contents will be converted to system big-endian"); + } else { + INFO("File \"" + path + "\" is big-endian; no format conversion required as executing on big-endian system"); + } #else - if (change_endianness) { - INFO("Converting imported contents of file \"" + path + "\" to native little-endian"); - } else { - WARN("File \"" + path + "\" already in native little-endian format, so no endianness conversion required; " - "but file is therefore not format-compliant (may have been generated using an older MRtrix3 version)"); - } + if (change_endianness) { + INFO("Converting imported contents of file \"" + path + "\" to native little-endian"); + } else { + WARN("File \"" + path + "\" already in native little-endian format, so no endianness conversion required; " + "but file is therefore not format-compliant (may have been generated using an older MRtrix3 version)"); + } #endif + } if (change_endianness) { for (auto& v : vertices) { From 924fc680b21a6ef9c46a9b495fd0ed875dedcc8b Mon Sep 17 00:00:00 2001 From: J-Donald Tournier Date: Thu, 29 Jun 2023 14:46:47 +0100 Subject: [PATCH 16/22] mrview: fix handling of colour mapping for track scalar files Fixes #2662 --- src/gui/mrview/displayable.h | 8 ++++- .../tool/tractography/track_scalar_file.cpp | 5 ++-- .../mrview/tool/tractography/tractogram.cpp | 8 +++-- src/gui/mrview/tool/tractography/tractogram.h | 3 -- .../mrview/tool/tractography/tractography.cpp | 29 +++++++------------ 5 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/gui/mrview/displayable.h b/src/gui/mrview/displayable.h index 2c65d1dff9..309f626442 100644 --- a/src/gui/mrview/displayable.h +++ b/src/gui/mrview/displayable.h @@ -138,10 +138,16 @@ namespace MR flags_ = cmap; } - void set_colour (std::array &c) { + void set_colour (const std::array &c) { colour = c; } + void set_colour (const QColor &c) { + colour[0] = c.red(); + colour[1] = c.green(); + colour[2] = c.blue(); + } + void set_use_discard_lower (bool yesno) { if (!discard_lower_enabled()) return; set_bit (DiscardLower, yesno); diff --git a/src/gui/mrview/tool/tractography/track_scalar_file.cpp b/src/gui/mrview/tool/tractography/track_scalar_file.cpp index dad1f5a948..7ceb216d62 100644 --- a/src/gui/mrview/tool/tractography/track_scalar_file.cpp +++ b/src/gui/mrview/tool/tractography/track_scalar_file.cpp @@ -159,7 +159,8 @@ namespace MR window().colourbar_renderer.render (tractogram.colourmap, tractogram.scale_inverted(), min_value, max_value, - tractogram.scaling_min(), tractogram.display_range, tractogram.colour); + tractogram.scaling_min(), tractogram.display_range, + { tractogram.colour[0]/255.0f, tractogram.colour[1]/255.0f, tractogram.colour[2]/255.0f }); } @@ -446,7 +447,7 @@ namespace MR window().updateGL(); } } - + void TrackScalarFileOptions::set_colourmap (int colourmap_index) { if (tractogram) { diff --git a/src/gui/mrview/tool/tractography/tractogram.cpp b/src/gui/mrview/tool/tractography/tractogram.cpp index 5749e542a7..54fe44c22e 100644 --- a/src/gui/mrview/tool/tractography/tractogram.cpp +++ b/src/gui/mrview/tool/tractography/tractogram.cpp @@ -67,6 +67,7 @@ namespace MR "uniform float slab_width;\n" "uniform float offset, scale;\n" "uniform float scale_x, scale_y;\n" + "uniform vec3 colourmap_colour;\n" "out vec3 v_tangent;\n" "out vec2 v_end;\n"; @@ -220,7 +221,7 @@ namespace MR std::string source = "uniform float lower, upper;\n" - "uniform vec3 const_colour;\n" + "uniform vec3 colourmap_colour;\n" "uniform mat4 MV;\n" "out vec3 colour;\n"; @@ -282,7 +283,7 @@ namespace MR : " colour = v_colour;\n"; break; case TrackColourType::Manual: - source += " colour = const_colour;\n"; + source += " colour = colourmap_colour;\n"; } if (use_lighting && (using_geom || using_points)) { @@ -429,7 +430,8 @@ namespace MR } if (color_type == TrackColourType::Manual) - gl::Uniform3fv (gl::GetUniformLocation (track_shader, "const_colour"), 1, colour.data()); + gl::Uniform3f (gl::GetUniformLocation (track_shader, "colourmap_colour"), + colour[0]/255.0, colour[1]/255.0, colour[2]/255.0); if (color_type == TrackColourType::ScalarFile) { gl::Uniform1f (gl::GetUniformLocation (track_shader, "offset"), display_midpoint - 0.5f * display_range); diff --git a/src/gui/mrview/tool/tractography/tractogram.h b/src/gui/mrview/tool/tractography/tractogram.h index 40e9610ca0..ba83dddabb 100644 --- a/src/gui/mrview/tool/tractography/tractogram.h +++ b/src/gui/mrview/tool/tractography/tractogram.h @@ -69,8 +69,6 @@ namespace MR TrackThresholdType get_threshold_type() const { return threshold_type; } TrackGeometryType get_geometry_type() const { return geometry_type; } - void set_colour (float c[3]) { colour = { c[0], c[1], c[2] }; } - float get_threshold_rate() const { switch (threshold_type) { case TrackThresholdType::None: return NaN; @@ -90,7 +88,6 @@ namespace MR bool scalarfile_by_direction; bool show_colour_bar; bool should_update_stride; - Eigen::Array3f colour; float original_fov; float line_thickness; std::string intensity_scalar_filename; diff --git a/src/gui/mrview/tool/tractography/tractography.cpp b/src/gui/mrview/tool/tractography/tractography.cpp index e5d1e2bdde..64170b9be3 100644 --- a/src/gui/mrview/tool/tractography/tractography.cpp +++ b/src/gui/mrview/tool/tractography/tractography.cpp @@ -564,11 +564,12 @@ namespace MR colour[2] = rng(); } while (colour[0] < 0.5 && colour[1] < 0.5 && colour[2] < 0.5); tractogram->set_color_type (TrackColourType::Manual); - tractogram->set_colour (colour); + QColor c (colour[0]*255.0f, colour[1]*255.0f, colour[2]*255.0f); + tractogram->set_colour (c); if (tractogram->get_threshold_type() == TrackThresholdType::UseColourFile) tractogram->set_threshold_type (TrackThresholdType::None); if (!i) - colour_button->setColor (QColor (colour[0]*255.0f, colour[1]*255.0f, colour[2]*255.0f)); + colour_button->setColor (c); } colour_combobox->blockSignals (true); colour_combobox->setCurrentIndex (2); @@ -584,13 +585,12 @@ namespace MR { QColor color; color = QColorDialog::getColor(Qt::red, this, "Select Color", QColorDialog::DontUseNativeDialog); - float colour[] = {float(color.redF()), float(color.greenF()), float(color.blueF())}; if (color.isValid()) { QModelIndexList indices = tractogram_list_view->selectionModel()->selectedIndexes(); for (int i = 0; i < indices.size(); ++i) { Tractogram* tractogram = tractogram_list_model->get_tractogram (indices[i]); tractogram->set_color_type (TrackColourType::Manual); - tractogram->set_colour (colour); + tractogram->set_colour (color); if (tractogram->get_threshold_type() == TrackThresholdType::UseColourFile) tractogram->set_threshold_type (TrackThresholdType::None); } @@ -599,7 +599,7 @@ namespace MR colour_combobox->clearError(); colour_combobox->blockSignals (false); colour_button->setEnabled (true); - colour_button->setColor (QColor (colour[0]*255.0f, colour[1]*255.0f, colour[2]*255.0f)); + colour_button->setColor (color); update_scalar_options(); } window().updateGL(); @@ -668,9 +668,8 @@ namespace MR const QColor color = colour_button->color(); if (color.isValid()) { QModelIndexList indices = tractogram_list_view->selectionModel()->selectedIndexes(); - float c[3] = { color.red()/255.0f, color.green()/255.0f, color.blue()/255.0f }; for (int i = 0; i < indices.size(); ++i) - tractogram_list_model->get_tractogram (indices[i])->set_colour (c); + tractogram_list_model->get_tractogram (indices[i])->set_colour (color); colour_combobox->blockSignals (true); colour_combobox->setCurrentIndex (3); // In case it was on random colour_combobox->clearError(); @@ -717,7 +716,7 @@ namespace MR const Tractogram* first_tractogram = tractogram_list_model->get_tractogram (indices[0]); TrackColourType color_type = first_tractogram->get_color_type(); - Eigen::Array3f color = first_tractogram->colour; + QColor color (first_tractogram->colour[0], first_tractogram->colour[1], first_tractogram->colour[2]); TrackGeometryType geom_type = first_tractogram->get_geometry_type(); bool color_type_consistent = true, geometry_type_consistent = true; float mean_thickness = first_tractogram->line_thickness; @@ -743,7 +742,7 @@ namespace MR case TrackColourType::Manual: colour_combobox->setCurrentIndex (3); colour_button->setEnabled (true); - colour_button->setColor (QColor (color[0]*255.0f, color[1]*255.0f, color[2]*255.0f)); + colour_button->setColor (color); break; case TrackColourType::ScalarFile: colour_combobox->setCurrentIndex (4); @@ -988,16 +987,10 @@ namespace MR const float max_value = std::max ({ values[0], values[1], values[2] }); if (std::min ({ values[0], values[1], values[2] }) < 0.0 || max_value > 255) throw Exception ("values provided to -tractogram.colour must be either between 0.0 and 1.0, or between 0 and 255"); - const float multiplier = max_value <= 1.0 ? 1.0 : 1.0/255.0; + const float multiplier = max_value <= 1.0 ? 255.0 : 1.0; //input need to be a float * - float colour_input[3] = { - multiplier * float (values[0]), - multiplier * float (values[1]), - multiplier * float (values[2]) - }; - - QColor colour (int(values[0]*255.0), int(values[1]*255.0), int(values[2]*255.0)); + QColor colour (multiplier*values[0], multiplier*values[1], multiplier*values[2]); QModelIndexList indices = tractogram_list_view->selectionModel()->selectedIndexes(); @@ -1008,7 +1001,7 @@ namespace MR // set the color tractogram->set_color_type (TrackColourType::Manual); - tractogram->set_colour (colour_input); + tractogram->set_colour (colour); // update_color_type_gui colour_combobox->setCurrentIndex (3); From ad39dc8f0c3ecfaede4bc92488d22d0a7827c173 Mon Sep 17 00:00:00 2001 From: J-Donald Tournier Date: Thu, 29 Jun 2023 15:38:03 +0100 Subject: [PATCH 17/22] mrview: remove unnecessary functionality in colourmap_menu.h/cpp --- src/gui/mrview/colourmap_button.cpp | 16 ++-- src/gui/mrview/colourmap_button.h | 18 +++-- src/gui/mrview/colourmap_menu.cpp | 67 ----------------- src/gui/mrview/colourmap_menu.h | 49 ------------- src/gui/mrview/tool/overlay.cpp | 2 +- .../tool/tractography/track_scalar_file.cpp | 73 +++++-------------- .../tool/tractography/track_scalar_file.h | 22 +++--- 7 files changed, 52 insertions(+), 195 deletions(-) delete mode 100644 src/gui/mrview/colourmap_menu.cpp delete mode 100644 src/gui/mrview/colourmap_menu.h diff --git a/src/gui/mrview/colourmap_button.cpp b/src/gui/mrview/colourmap_button.cpp index 14988cf309..55f76ede94 100644 --- a/src/gui/mrview/colourmap_button.cpp +++ b/src/gui/mrview/colourmap_button.cpp @@ -118,10 +118,10 @@ void ColourMapButton::init_special_colour_menu_items(bool create_shortcuts) void ColourMapButton::init_customise_state_menu_items() { - auto show_colour_bar = colourmap_menu->addAction(tr("Show colour bar"), this, SLOT(show_colour_bar_slot(bool))); - show_colour_bar->setCheckable(true); - show_colour_bar->setChecked(true); - addAction(show_colour_bar); + show_colour_bar_action = colourmap_menu->addAction(tr("Show colour bar"), this, SLOT(show_colour_bar_slot(bool))); + show_colour_bar_action->setCheckable(true); + show_colour_bar_action->setChecked(true); + addAction(show_colour_bar_action); invert_scale_action = colourmap_menu->addAction(tr("Invert"), this, SLOT(invert_colourmap_slot(bool))); invert_scale_action->setCheckable(true); @@ -163,12 +163,18 @@ void ColourMapButton::set_colourmap_index(size_t index) } } -void ColourMapButton::set_scale_inverted(bool yesno) +void ColourMapButton::set_scale_inverted (bool yesno) { assert (invert_scale_action != nullptr); invert_scale_action->setChecked (yesno); } +void ColourMapButton::set_show_colourbar (bool yesno) +{ + assert (invert_scale_action != nullptr); + show_colour_bar_action->setChecked (yesno); +} + void ColourMapButton::set_fixed_colour() { diff --git a/src/gui/mrview/colourmap_button.h b/src/gui/mrview/colourmap_button.h index 871654e72d..057fe82273 100644 --- a/src/gui/mrview/colourmap_button.h +++ b/src/gui/mrview/colourmap_button.h @@ -34,11 +34,11 @@ class ColourMapButton; class ColourMapButtonObserver { NOMEMALIGN public: - virtual void selected_colourmap(size_t, const ColourMapButton&) {} - virtual void selected_custom_colour(const QColor&, const ColourMapButton&) {} - virtual void toggle_show_colour_bar(bool, const ColourMapButton&) {} - virtual void toggle_invert_colourmap(bool, const ColourMapButton&) {} - virtual void reset_colourmap(const ColourMapButton&) {} + virtual void selected_colourmap (size_t, const ColourMapButton&) {} + virtual void selected_custom_colour (const QColor&, const ColourMapButton&) {} + virtual void toggle_show_colour_bar (bool, const ColourMapButton&) {} + virtual void toggle_invert_colourmap (bool, const ColourMapButton&) {} + virtual void reset_colourmap (const ColourMapButton&) {} }; @@ -46,12 +46,13 @@ class ColourMapButton : public QToolButton { MEMALIGN(ColourMapButton) Q_OBJECT public: - ColourMapButton(QWidget* parent, ColourMapButtonObserver& obs, + ColourMapButton (QWidget* parent, ColourMapButtonObserver& obs, bool use_shortcuts = false, bool use_special_colourmaps = true, bool use_customise_state_items = true); - void set_colourmap_index(size_t index); - void set_scale_inverted(bool yesno); + void set_colourmap_index (size_t index); + void set_scale_inverted (bool yesno); + void set_show_colourbar (bool yesno); void set_fixed_colour(); vector colourmap_actions; void open_menu (const QPoint& p) { colourmap_menu->exec (p); } @@ -71,6 +72,7 @@ class ColourMapButton : public QToolButton QMenu* colourmap_menu; QAction* custom_colour_action; QAction* invert_scale_action; + QAction* show_colour_bar_action; size_t fixed_colour_index; diff --git a/src/gui/mrview/colourmap_menu.cpp b/src/gui/mrview/colourmap_menu.cpp deleted file mode 100644 index 814d4b8ce0..0000000000 --- a/src/gui/mrview/colourmap_menu.cpp +++ /dev/null @@ -1,67 +0,0 @@ -/* Copyright (c) 2008-2023 the MRtrix3 contributors. - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. - * - * Covered Software is provided under this License on an "as is" - * basis, without warranty of any kind, either expressed, implied, or - * statutory, including, without limitation, warranties that the - * Covered Software is free of defects, merchantable, fit for a - * particular purpose or non-infringing. - * See the Mozilla Public License v. 2.0 for more details. - * - * For more details, see http://www.mrtrix.org/. - */ - -#include "gui/gui.h" -#include "gui/mrview/colourmap_menu.h" - -namespace MR -{ - namespace GUI - { - namespace MRView - { - - - - void create_colourmap_menu (QWidget* parent, QActionGroup*& group, - QMenu* menu, QAction** & actions, - bool create_shortcuts, bool use_special) - { - group = new QActionGroup (parent); - group->setExclusive (true); - actions = new QAction* [ColourMap::num()]; - bool in_scalar_section = true; - - for (size_t n = 0; ColourMap::maps[n].name; ++n) { - if (ColourMap::maps[n].special && !use_special) - continue; - QAction* action = new QAction (ColourMap::maps[n].name, parent); - action->setCheckable (true); - group->addAction (action); - - if (ColourMap::maps[n].special && in_scalar_section) { - menu->addSeparator(); - in_scalar_section = false; - } - - menu->addAction (action); - parent->addAction (action); - - if (create_shortcuts) - action->setShortcut (qstr ("Ctrl+" + str (n+1))); - - actions[n] = action; - } - - actions[0]->setChecked (true); - } - - - - } - } -} - diff --git a/src/gui/mrview/colourmap_menu.h b/src/gui/mrview/colourmap_menu.h deleted file mode 100644 index 3fe4d16b34..0000000000 --- a/src/gui/mrview/colourmap_menu.h +++ /dev/null @@ -1,49 +0,0 @@ -/* Copyright (c) 2008-2023 the MRtrix3 contributors. - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. - * - * Covered Software is provided under this License on an "as is" - * basis, without warranty of any kind, either expressed, implied, or - * statutory, including, without limitation, warranties that the - * Covered Software is free of defects, merchantable, fit for a - * particular purpose or non-infringing. - * See the Mozilla Public License v. 2.0 for more details. - * - * For more details, see http://www.mrtrix.org/. - */ - -#ifndef __gui_mrview_colourmap_menu_h__ -#define __gui_mrview_colourmap_menu_h__ - -#include "colourmap.h" -#include "gui/opengl/gl.h" - -namespace MR -{ - namespace GUI - { - namespace MRView - { - - - - void create_colourmap_menu (QWidget* parent, - QActionGroup*& group, - QMenu* menu, - QAction** & actions, - bool create_shortcuts = false, - bool use_special = true); - - - - } - } -} - -#endif - - - - diff --git a/src/gui/mrview/tool/overlay.cpp b/src/gui/mrview/tool/overlay.cpp index dfcfe137bd..dc07de40d9 100644 --- a/src/gui/mrview/tool/overlay.cpp +++ b/src/gui/mrview/tool/overlay.cpp @@ -631,7 +631,7 @@ namespace MR if (colourmap_index == -2) colourmap_index = overlay->colourmap; else - colourmap_index = -1; + colourmap_index = -1; } num_inverted += overlay->scale_inverted(); rate += overlay->scaling_rate(); diff --git a/src/gui/mrview/tool/tractography/track_scalar_file.cpp b/src/gui/mrview/tool/tractography/track_scalar_file.cpp index 7ceb216d62..701ca0c108 100644 --- a/src/gui/mrview/tool/tractography/track_scalar_file.cpp +++ b/src/gui/mrview/tool/tractography/track_scalar_file.cpp @@ -17,7 +17,6 @@ #include "gui/mrview/tool/tractography/track_scalar_file.h" #include "gui/dialog/file.h" -#include "gui/mrview/colourmap_menu.h" #include "gui/mrview/tool/tractography/tractogram.h" @@ -52,34 +51,7 @@ namespace MR connect (intensity_file_button, SIGNAL (clicked()), this, SLOT (open_intensity_track_scalar_file_slot ())); hlayout->addWidget (intensity_file_button); - // Colourmap menu: - colourmap_menu = new QMenu (tr ("Colourmap menu"), this); - - MRView::create_colourmap_menu (this, colourmap_group, colourmap_menu, colourmap_actions, false, false); - connect (colourmap_group, SIGNAL (triggered (QAction*)), this, SLOT (select_colourmap_slot())); - colourmap_actions[1]->setChecked (true); - - colourmap_menu->addSeparator(); - - show_colour_bar = colourmap_menu->addAction (tr ("Show colour bar"), this, SLOT (show_colour_bar_slot())); - show_colour_bar->setCheckable (true); - show_colour_bar->setChecked (true); - addAction (show_colour_bar); - - invert_scale = colourmap_menu->addAction (tr ("Invert"), this, SLOT (invert_colourmap_slot())); - invert_scale->setCheckable (true); - addAction (invert_scale); - - colourmap_menu->addSeparator(); - - QAction* reset_intensity = colourmap_menu->addAction (tr ("Reset intensity"), this, SLOT (reset_intensity_slot())); - addAction (reset_intensity); - - colourmap_button = new QToolButton (this); - colourmap_button->setToolTip (tr ("Colourmap menu")); - colourmap_button->setIcon (QIcon (":/colourmap.svg")); - colourmap_button->setPopupMode (QToolButton::InstantPopup); - colourmap_button->setMenu (colourmap_menu); + colourmap_button = new ColourMapButton(this, *this, false, false, true); hlayout->addWidget (colourmap_button); vlayout->addLayout (hlayout); @@ -181,10 +153,10 @@ namespace MR min_entry->setValue (tractogram->scaling_min()); max_entry->setValue (tractogram->scaling_max()); - colourmap_menu->setEnabled (true); - colourmap_actions[tractogram->colourmap]->setChecked (true); - show_colour_bar->setChecked (tractogram->show_colour_bar); - invert_scale->setChecked (tractogram->scale_inverted()); + colourmap_button->setEnabled (true); + colourmap_button->set_colourmap_index(tractogram->colourmap); + colourmap_button->set_scale_inverted (tractogram->scale_inverted()); + colourmap_button->set_show_colourbar (tractogram->show_colour_bar); assert (tractogram->intensity_scalar_filename.length()); intensity_file_button->setText (qstr (shorten (Path::basename (tractogram->intensity_scalar_filename), 35, 0))); @@ -260,28 +232,30 @@ namespace MR return scalar_file.size(); } - void TrackScalarFileOptions::show_colour_bar_slot () + void TrackScalarFileOptions::toggle_show_colour_bar(bool show_colour_bar, const ColourMapButton&) { if (tractogram) { - tractogram->show_colour_bar = show_colour_bar->isChecked(); + tractogram->show_colour_bar = show_colour_bar; window().updateGL(); } } - void TrackScalarFileOptions::select_colourmap_slot () + void TrackScalarFileOptions::selected_colourmap (size_t cmap, const ColourMapButton&) { if (tractogram) { - QAction* action = colourmap_group->checkedAction(); - size_t n = 0; - while (action != colourmap_actions[n]) - ++n; - tractogram->colourmap = n; + tractogram->colourmap = cmap; window().updateGL(); } } - + void TrackScalarFileOptions::selected_custom_colour (const QColor& c, const ColourMapButton&) + { + if (tractogram) { + tractogram->set_colour (c); + window().updateGL(); + } + } void TrackScalarFileOptions::set_threshold(GUI::MRView::Tool::TrackThresholdType dataSource, default_type min, default_type max)//TrackThresholdType dataSource { @@ -430,7 +404,7 @@ namespace MR } - void TrackScalarFileOptions::reset_intensity_slot () + void TrackScalarFileOptions::reset_colourmap (const ColourMapButton&) { if (tractogram) { tractogram->reset_windowing(); @@ -440,19 +414,10 @@ namespace MR } - void TrackScalarFileOptions::invert_colourmap_slot () - { - if (tractogram) { - tractogram->set_invert_scale (invert_scale->isChecked()); - window().updateGL(); - } - } - - void TrackScalarFileOptions::set_colourmap (int colourmap_index) + void TrackScalarFileOptions::toggle_invert_colourmap (bool invert, const ColourMapButton&) { if (tractogram) { - tractogram->colourmap = colourmap_index; - update_UI(); + tractogram->set_invert_scale (invert); window().updateGL(); } } diff --git a/src/gui/mrview/tool/tractography/track_scalar_file.h b/src/gui/mrview/tool/tractography/track_scalar_file.h index 1dae8dba89..698716213f 100644 --- a/src/gui/mrview/tool/tractography/track_scalar_file.h +++ b/src/gui/mrview/tool/tractography/track_scalar_file.h @@ -18,6 +18,7 @@ #define __gui_mrtrix_tools_tractography_scalar_file_options_h__ #include "gui/mrview/adjust_button.h" +#include "gui/mrview/colourmap_button.h" #include "gui/mrview/displayable.h" #include "gui/mrview/tool/base.h" #include "gui/mrview/tool/tractography/tractogram_enums.h" @@ -36,7 +37,7 @@ namespace MR class Tractogram; class Tractography; - class TrackScalarFileOptions : public QGroupBox, public DisplayableVisitor + class TrackScalarFileOptions : public QGroupBox, public ColourMapButtonObserver, public DisplayableVisitor { MEMALIGN(TrackScalarFileOptions) Q_OBJECT @@ -51,7 +52,13 @@ namespace MR void update_UI(); void set_scaling(default_type min, default_type max); void set_threshold(GUI::MRView::Tool::TrackThresholdType dataSource, default_type min, default_type max); - void set_colourmap (int colourmap_index); + void set_colourmap (int colourmap_index) { colourmap_button->set_colourmap_index (colourmap_index); } + + void selected_colourmap (size_t, const ColourMapButton&) override; + void selected_custom_colour (const QColor&, const ColourMapButton&) override; + void toggle_show_colour_bar (bool, const ColourMapButton&) override; + void toggle_invert_colourmap (bool, const ColourMapButton&) override; + void reset_colourmap (const ColourMapButton&) override; public slots: bool open_intensity_track_scalar_file_slot (); @@ -59,17 +66,13 @@ namespace MR private slots: - void show_colour_bar_slot(); - void select_colourmap_slot (); void on_set_scaling_slot (); bool threshold_scalar_file_slot (int); void threshold_lower_changed (int unused); void threshold_upper_changed (int unused); void threshold_lower_value_changed (); void threshold_upper_value_changed (); - void invert_colourmap_slot (); - void reset_intensity_slot (); - + protected: Tractography* tool; @@ -78,10 +81,7 @@ namespace MR QGroupBox *colour_groupbox; QAction *show_colour_bar; QAction *invert_scale; - QMenu *colourmap_menu; - QAction **colourmap_actions; - QActionGroup *colourmap_group; - QToolButton *colourmap_button; + ColourMapButton* colourmap_button; QPushButton *intensity_file_button; AdjustButton *max_entry, *min_entry; QComboBox *threshold_file_combobox; From 2ac2e7bf73678533bf139ad87e77d4274f876049 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Tue, 4 Jul 2023 11:29:52 +1000 Subject: [PATCH 18/22] tcksift2: Fix check for allocation of memory for weights vector If path to output weights file is erroneous, do not issue a misleading warning message claiming that the issue relates to an inability to allocate memory. Fixes bug introduced in #626. Solves #2668. Replaces #2671. --- src/dwi/tractography/SIFT2/tckfactor.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/dwi/tractography/SIFT2/tckfactor.cpp b/src/dwi/tractography/SIFT2/tckfactor.cpp index 0375cb5cf2..d28bf78309 100644 --- a/src/dwi/tractography/SIFT2/tckfactor.cpp +++ b/src/dwi/tractography/SIFT2/tckfactor.cpp @@ -344,19 +344,22 @@ namespace MR { { if (size_t(coefficients.size()) != contributions.size()) throw Exception ("Cannot output weighting factors if they have not first been estimated!"); + decltype(coefficients) weights; try { - decltype(coefficients) weights (coefficients.size()); - for (SIFT::track_t i = 0; i != num_tracks(); ++i) - weights[i] = (coefficients[i] == min_coeff || !std::isfinite(coefficients[i])) ? - 0.0 : - std::exp (coefficients[i]); - save_vector (weights, path); + weights.resize (coefficients.size()); } catch (...) { WARN ("Unable to assign memory for output factor file: \"" + Path::basename(path) + "\" not created"); + return; } + for (SIFT::track_t i = 0; i != num_tracks(); ++i) + weights[i] = (coefficients[i] == min_coeff || !std::isfinite(coefficients[i])) ? + 0.0 : + std::exp (coefficients[i]); + save_vector (weights, path); } + void TckFactor::output_coefficients (const std::string& path) const { save_vector (coefficients, path); From 0e0224f96aa55945bb77e78095727dbbb503bfb7 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Thu, 13 Jul 2023 11:29:47 +1000 Subject: [PATCH 19/22] Population_template: Terminal tweaks - Tweak help page to better reflect multi-contrast operation. - Make better use of run.command() with list input to compress terminal feedback. --- bin/population_template | 34 +++++++++++-------- .../commands/population_template.rst | 13 +++++-- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/bin/population_template b/bin/population_template index a0dc7aa918..0095ef4c44 100755 --- a/bin/population_template +++ b/bin/population_template @@ -30,17 +30,23 @@ DEFAULT_NL_LMAX = [ 2, 2, 2, 2, 2, 2, 2, 2, 4, 4, 4, 4, 4, 4, 4 REGISTRATION_MODES = ['rigid', 'affine', 'nonlinear', 'rigid_affine', 'rigid_nonlinear', 'affine_nonlinear', 'rigid_affine_nonlinear'] -AGGREGATION_MODES = ["mean", "median"] +AGGREGATION_MODES = ['mean', 'median'] -IMAGEEXT = 'mif nii mih mgh mgz img hdr'.split() +IMAGEEXT = ['mif', 'nii', 'mih', 'mgh', 'mgz', 'img', 'hdr'] def usage(cmdline): #pylint: disable=unused-variable cmdline.set_author('David Raffelt (david.raffelt@florey.edu.au) & Max Pietsch (maximilian.pietsch@kcl.ac.uk) & Thijs Dhollander (thijs.dhollander@gmail.com)') cmdline.set_synopsis('Generates an unbiased group-average template from a series of images') cmdline.add_description('First a template is optimised with linear registration (rigid and/or affine, both by default), then non-linear registration is used to optimise the template further.') - cmdline.add_argument("input_dir", nargs='+', help='Input directory containing all images used to build the template') - cmdline.add_argument("template", help='Corresponding output template image. For multi-contrast registration, provide multiple paired input_dir and template arguments. Example: WM_dir WM_template.mif GM_dir GM_template.mif') + cmdline.add_argument('input_dir', nargs='+', help='Directory containing all input images of a given contrast') + cmdline.add_argument('template', help='Output template image') + + cmdline.add_example_usage('Multi-contrast registration', + 'population_template input_WM_ODFs/ output_WM_template.mif input_GM_ODFs/ output_GM_template.mif', + 'When performing multi-contrast registration, the input directory and corresponding output template ' + 'image for a given contrast are to be provided as a pair, ' + 'with the pairs corresponding to different contrasts provided sequentially.') options = cmdline.add_argument_group('Multi-contrast options') options.add_argument('-mc_weight_initial_alignment', help='Weight contribution of each contrast to the initial alignment. Comma separated, default: 1.0') @@ -979,7 +985,7 @@ def execute(): #pylint: disable=unused-variable elif cns.n_volumes[cid] == 1: run.function(copy, avh4d, 'average_header' + cns.suff[cid] + '.mif') else: - run.command('mrcat ' + ' '.join([avh3d] * cns.n_volumes[cid]) + ' -axis 3 average_header' + cns.suff[cid] + '.mif') + run.command(['mrcat', [avh3d] * cns.n_volumes[cid], '-axis', '3', 'average_header' + cns.suff[cid] + '.mif']) run.function(os.remove, avh3d) run.function(os.remove, avh4d) else: @@ -1075,7 +1081,7 @@ def execute(): #pylint: disable=unused-variable datatype_option) progress.increment() # update average space of first contrast to new extent, delete other average space images - run.command('mraverageheader ' + ' '.join([inp.ims_transformed[cid] + '_translated.mif' for inp in ins]) + ' average_header_tight.mif') + run.command(['mraverageheader', [inp.ims_transformed[cid] + '_translated.mif' for inp in ins], 'average_header_tight.mif']) progress.done() if voxel_size is None: @@ -1235,12 +1241,12 @@ def execute(): #pylint: disable=unused-variable # - If one subject's registration fails, this will affect the average and therefore the template which could result in instable behaviour. # - The template appearance changes slightly over levels, but the template and trafos are affected in the same way so should not affect template convergence. if not app.ARGS.linear_no_drift_correction: - run.command('transformcalc ' + ' '.join([os.path.join('linear_transforms_initial', inp.uid + '.txt') for _inp in ins]) + - ' average linear_transform_average_init.txt -quiet', force=True) - run.command('transformcalc ' + ' '.join([os.path.join('linear_transforms_%02i' % level, inp.uid + '.txt') for _inp in ins]) + - ' average linear_transform_average_%02i_uncorrected.txt -quiet' % level, force=True) - run.command('transformcalc linear_transform_average_%02i_uncorrected.txt invert ' % level + - 'linear_transform_average_%02i_uncorrected_inv.txt -quiet' % level, force=True) + run.command(['transformcalc', [os.path.join('linear_transforms_initial', inp.uid + '.txt') for _inp in ins], + 'average', 'linear_transform_average_init.txt', '-quiet'], force=True) + run.command(['transformcalc', [os.path.join('linear_transforms_%02i' % level, inp.uid + '.txt') for _inp in ins], + 'average', 'linear_transform_average_%02i_uncorrected.txt' % level, '-quiet'], force=True) + run.command(['transformcalc', 'linear_transform_average_%02i_uncorrected.txt' % level, + 'invert', 'linear_transform_average_%02i_uncorrected_inv.txt' % level, '-quiet'], force=True) transform_average_init = matrix.load_transform('linear_transform_average_init.txt') transform_average_current_inv = matrix.load_transform('linear_transform_average_%02i_uncorrected_inv.txt' % level) @@ -1259,8 +1265,8 @@ def execute(): #pylint: disable=unused-variable matrix.save_transform(os.path.join('linear_transforms_%02i' % level, inp.uid + '.txt'), transform_updated, force=True) # compute average trafos and its properties for easier debugging - run.command('transformcalc ' + ' '.join([os.path.join('linear_transforms_%02i' % level, _inp.uid + '.txt') for _inp in ins]) + - ' average linear_transform_average_%02i.txt -quiet' % level, force=True) + run.command(['transformcalc', [os.path.join('linear_transforms_%02i' % level, _inp.uid + '.txt') for _inp in ins], + 'average', 'linear_transform_average_%02i.txt' % level, '-quiet'], force=True) run.command('transformcalc linear_transform_average_%02i.txt decompose linear_transform_average_%02i.dec' % (level, level), force=True) diff --git a/docs/reference/commands/population_template.rst b/docs/reference/commands/population_template.rst index 01a9cd10e3..1194447b20 100644 --- a/docs/reference/commands/population_template.rst +++ b/docs/reference/commands/population_template.rst @@ -15,14 +15,23 @@ Usage population_template input_dir template [ options ] -- *input_dir*: Input directory containing all images used to build the template -- *template*: Corresponding output template image. For multi-contrast registration, provide multiple paired input_dir and template arguments. Example: WM_dir WM_template.mif GM_dir GM_template.mif +- *input_dir*: Directory containing all input images of a given contrast +- *template*: Output template image Description ----------- First a template is optimised with linear registration (rigid and/or affine, both by default), then non-linear registration is used to optimise the template further. +Example usages +-------------- + +- *Multi-contrast registration*:: + + $ population_template input_WM_ODFs/ output_WM_template.mif input_GM_ODFs/ output_GM_template.mif + + When performing multi-contrast registration, the input directory and corresponding output template image for a given contrast are to be provided as a pair, with the pairs corresponding to different contrasts provided sequentially. + Options ------- From be145790d96b18d97e7d85a6a42df582d97889c5 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Tue, 18 Jul 2023 15:25:44 +1000 Subject: [PATCH 20/22] Resolving compilation after merging dev into fixelconnectivity --- core/fixel/helpers.h | 6 ------ src/fixel/filter/smooth.h | 5 ++--- src/fixel/matrix.cpp | 17 ++++++++++------- src/fixel/matrix.h | 4 ++-- 4 files changed, 14 insertions(+), 18 deletions(-) diff --git a/core/fixel/helpers.h b/core/fixel/helpers.h index 39b483bfea..e932dec065 100644 --- a/core/fixel/helpers.h +++ b/core/fixel/helpers.h @@ -335,12 +335,6 @@ namespace MR return header; } - //! Generate a header for a sparse data file (Nx1x1) using an index image as a template - template - FORCE_INLINE Header data_header_from_index (IndexHeaderType& index) { - return make_data_header (get_number_of_fixels (index)); - } - //! Generate a header for a fixel directions data file (Nx3x1) using an index image as a template template FORCE_INLINE Header directions_header_from_index (IndexHeaderType& index) { diff --git a/src/fixel/filter/smooth.h b/src/fixel/filter/smooth.h index 2256a20957..7bb5a6c999 100644 --- a/src/fixel/filter/smooth.h +++ b/src/fixel/filter/smooth.h @@ -18,8 +18,7 @@ #ifndef __fixel_filter_smooth_h__ #define __fixel_filter_smooth_h__ -#include "fixel/types.h" - +#include "fixel/fixel.h" #include "fixel/matrix.h" #include "fixel/filter/base.h" @@ -53,7 +52,7 @@ namespace MR */ class Smooth : public Base - { + { public: Smooth (Image index_image, diff --git a/src/fixel/matrix.cpp b/src/fixel/matrix.cpp index 3332cdb47c..d6b04fe88e 100644 --- a/src/fixel/matrix.cpp +++ b/src/fixel/matrix.cpp @@ -190,7 +190,7 @@ namespace MR template class Receiver - { NOMEMALIGN + { public: Receiver (MatrixType& data) : data (data) { } @@ -270,14 +270,14 @@ namespace MR void Writer::set_count_path (const std::string& path) { assert (!count_image.valid()); - count_image = Image::create (path, MR::Fixel::make_data_header (matrix.size())); + count_image = Image::create (path, MR::Fixel::data_header_from_nfixels (matrix.size())); } template void Writer::set_extent_path (const std::string& path) { assert (!extent_image.valid()); - extent_image = Image::create (path, MR::Fixel::make_data_header (matrix.size())); + extent_image = Image::create (path, MR::Fixel::data_header_from_nfixels (matrix.size())); } @@ -322,10 +322,10 @@ namespace MR index_type num_connections = 0; { - ProgressBar progress ("Computing number of fixels in output", matrix.size()); + ProgressBar progress ("Computing number of fixel-fixel connections in matrix", matrix.size()); for (size_t fixel_index = 0; fixel_index != matrix.size(); ++fixel_index) { - const connectivity_value_type normalisation_factor = connectivity_value_type(1) / connectivity_value_type (matrix[fixel_index].count()); + const connectivity_value_type normalisation_factor = matrix[fixel_index].norm_factor(); for (auto& it : matrix[fixel_index]) { const connectivity_value_type connectivity = normalisation_factor * it.value(); if (connectivity >= threshold) @@ -349,6 +349,7 @@ namespace MR fixel_header.keyval()["nfixels"] = str(matrix.size()); fixel_header.datatype() = DataType::from(); fixel_header.datatype().set_byte_order_native(); + Header value_header (fixel_header); value_header.datatype() = DataType::from(); value_header.datatype().set_byte_order_native(); @@ -370,11 +371,13 @@ namespace MR const ssize_t connection_offset = fixel_image.index(0); index_type connection_count = 0; - const connectivity_value_type normalisation_factor = connectivity_value_type(1) / connectivity_value_type (matrix[fixel_index].count()); + connectivity_value_type sum_connectivity = connectivity_value_type (0.0); + const connectivity_value_type normalisation_factor = matrix[fixel_index].norm_factor(); for (auto& it : matrix[fixel_index]) { const connectivity_value_type connectivity = normalisation_factor * it.value(); if (connectivity >= threshold) { ++connection_count; + sum_connectivity += connectivity; fixel_image.value() = it.index(); ++fixel_image.index(0); value_image.value() = connectivity; @@ -388,7 +391,7 @@ namespace MR if (count_image.valid()) { count_image.index(0) = fixel_index; - count_image.value() = fixel_buffer.size(); + count_image.value() = connection_count; } if (extent_image.valid()) { extent_image.index(0) = fixel_index; diff --git a/src/fixel/matrix.h b/src/fixel/matrix.h index c109a08ece..65a234411b 100644 --- a/src/fixel/matrix.h +++ b/src/fixel/matrix.h @@ -21,7 +21,7 @@ #include "image.h" #include "types.h" #include "file/ofstream.h" -#include "fixel/types.h" +#include "fixel/fixel.h" namespace MR { @@ -254,7 +254,7 @@ namespace MR template class Writer - { MEMALIGN(Writer) + { public: Writer (MatrixType& matrix, const connectivity_value_type threshold) : From ab8bf844c7e8eff089b12b0f287aef7070dc8cfc Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Wed, 19 Jul 2023 00:36:37 +1000 Subject: [PATCH 21/22] fixelconnectivity": New tests and test data to reflect enhancements --- testing/binaries/data | 2 +- testing/binaries/tests/fixelconnectivity | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/testing/binaries/data b/testing/binaries/data index 747223c963..309728c7c1 160000 --- a/testing/binaries/data +++ b/testing/binaries/data @@ -1 +1 @@ -Subproject commit 747223c9635094939d55dd6c639a6e07c1c84427 +Subproject commit 309728c7c12ac0e81eb9e572eda290b45f952f4c diff --git a/testing/binaries/tests/fixelconnectivity b/testing/binaries/tests/fixelconnectivity index 9a45f4d719..6ba11c37fc 100644 --- a/testing/binaries/tests/fixelconnectivity +++ b/testing/binaries/tests/fixelconnectivity @@ -1,3 +1,4 @@ -fixelconnectivity SIFT_phantom/fixels/ SIFT_phantom/tracks.tck tmp/ -force && testing_diff_image tmp/index.mif SIFT_phantom/matrix/index.mif && testing_diff_image tmp/fixels.mif SIFT_phantom/matrix/fixels.mif && testing_diff_image tmp/values.mif SIFT_phantom/matrix/values.mif -fixelconnectivity SIFT_phantom/fixels/ SIFT_phantom/tracks.tck tmp/ -mask SIFT_phantom/fixels/upper.mif -force && testing_diff_image tmp/index.mif fixelconnectivity/masked/index.mif && testing_diff_image tmp/fixels.mif fixelconnectivity/masked/fixels.mif && testing_diff_image tmp/values.mif fixelconnectivity/masked/values.mif +fixelconnectivity SIFT_phantom/fixels/ SIFT_phantom/tracks.tck tmp/ -count tmp-count.mif -extent tmp-extent.mif -force && testing_diff_image tmp/index.mif fixelconnectivity/default/index.mif && testing_diff_image tmp/fixels.mif fixelconnectivity/default/fixels.mif && testing_diff_image tmp/values.mif fixelconnectivity/default/values.mif && testing_diff_image tmp-count.mif fixelconnectivity/default_count.mif && testing_diff_image tmp-extent.mif fixelconnectivity/default_extent.mif +fixelconnectivity SIFT_phantom/fixels/ SIFT_phantom/tracks.tck tmp/ -count tmp-count.mif -extent tmp-extent.mif -mask SIFT_phantom/fixels/upper.mif -force && testing_diff_image tmp/index.mif fixelconnectivity/masked/index.mif && testing_diff_image tmp/fixels.mif fixelconnectivity/masked/fixels.mif && testing_diff_image tmp/values.mif fixelconnectivity/masked/values.mif && testing_diff_image tmp-count.mif fixelconnectivity/masked_count.mif && testing_diff_image tmp-extent.mif fixelconnectivity/masked_extent.mif +fixelconnectivity SIFT_phantom/fixels/ SIFT_phantom/tracks.tck tmp/ -count tmp-count.mif -extent tmp-extent.mif -tck_weights_in SIFT_phantom/weights.csv -force && testing_diff_image tmp/index.mif fixelconnectivity/weighted/index.mif && testing_diff_image tmp/fixels.mif fixelconnectivity/weighted/fixels.mif && testing_diff_image tmp/values.mif fixelconnectivity/weighted/values.mif && testing_diff_image tmp-count.mif fixelconnectivity/weighted_count.mif && testing_diff_image tmp-extent.mif fixelconnectivity/weighted_extent.mif From cdc1c3b84b55a7d48aa2205b965e287467f24745 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Wed, 19 Jul 2023 00:37:21 +1000 Subject: [PATCH 22/22] Fixel::Matrix: Neaten by using typedefs --- src/fixel/matrix.cpp | 4 ++-- src/fixel/matrix.h | 25 +++++++++++++------------ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/fixel/matrix.cpp b/src/fixel/matrix.cpp index d6b04fe88e..d9dd2624d3 100644 --- a/src/fixel/matrix.cpp +++ b/src/fixel/matrix.cpp @@ -296,7 +296,7 @@ namespace MR if (App::overwrite_files) { File::remove (path); } else { - throw Exception ("Cannot create fixel-fixel connectivity matrix \"" + path + "\": Already exists as file"); + throw Exception ("Cannot create fixel-fixel connectivity matrix directory \"" + path + "\": Already exists as file"); } } } else { @@ -322,7 +322,7 @@ namespace MR index_type num_connections = 0; { - ProgressBar progress ("Computing number of fixel-fixel connections in matrix", matrix.size()); + ProgressBar progress ("Computing number of supra-threshold fixel-fixel connections", matrix.size()); for (size_t fixel_index = 0; fixel_index != matrix.size(); ++fixel_index) { const connectivity_value_type normalisation_factor = matrix[fixel_index].norm_factor(); diff --git a/src/fixel/matrix.h b/src/fixel/matrix.h index 65a234411b..3f8e1524fa 100644 --- a/src/fixel/matrix.h +++ b/src/fixel/matrix.h @@ -100,18 +100,18 @@ namespace MR using BaseType::operator<; using ValueType = connectivity_value_type; InitElementWeighted() : - sum_weights (connectivity_value_type(0)) { } + sum_weights (ValueType(0)) { } InitElementWeighted (const fixel_index_type fixel_index) = delete; InitElementWeighted (const fixel_index_type fixel_index, const MappedTrack& all_data) : BaseType (fixel_index), sum_weights (all_data.get_weight()) { } InitElementWeighted (const InitElementWeighted&) = default; FORCE_INLINE fixel_index_type index() const { return BaseType::index(); } - FORCE_INLINE InitElementWeighted& operator+= (const connectivity_value_type increment) { sum_weights += increment; return *this; } + FORCE_INLINE InitElementWeighted& operator+= (const ValueType increment) { sum_weights += increment; return *this; } FORCE_INLINE InitElementWeighted& operator= (const InitElementWeighted& that) { BaseType::operator= (that); sum_weights = that.sum_weights; return *this; } FORCE_INLINE ValueType value() const { return sum_weights; } private: - connectivity_value_type sum_weights; + ValueType sum_weights; }; @@ -192,7 +192,7 @@ namespace MR FORCE_INLINE void normalise (const ValueType norm_factor) { connectivity_value *= norm_factor; } private: fixel_index_type fixel_index; - connectivity_value_type connectivity_value; + ValueType connectivity_value; }; @@ -205,24 +205,25 @@ namespace MR public: using ElementType = NormElement; using BaseType = vector; + using ValueType = connectivity_value_type; NormFixel() : - norm_multiplier (connectivity_value_type (1)) { } + norm_multiplier (ValueType (1)) { } NormFixel (const BaseType& i) : BaseType (i), - norm_multiplier (connectivity_value_type (1)) { } + norm_multiplier (ValueType (1)) { } NormFixel (BaseType&& i) : BaseType (std::move (i)), - norm_multiplier (connectivity_value_type (1)) { } + norm_multiplier (ValueType (1)) { } void normalise() { - norm_multiplier = connectivity_value_type (0); + norm_multiplier = ValueType (0); for (const auto& c : *this) norm_multiplier += c.value(); - norm_multiplier = norm_multiplier ? (connectivity_value_type (1) / norm_multiplier) : connectivity_value_type(0); + norm_multiplier = norm_multiplier ? (ValueType (1) / norm_multiplier) : ValueType(0); } - void normalise (const connectivity_value_type sum) { - norm_multiplier = sum ? (connectivity_value_type(1) / sum) : connectivity_value_type(0); + void normalise (const ValueType sum) { + norm_multiplier = sum ? (ValueType(1) / sum) : ValueType(0); } - connectivity_value_type norm_multiplier; + ValueType norm_multiplier; };