Remove point size restriction - #234
Merged
Merged
Conversation
When the source picker index points to a dataset, verify that the selected scalar source dataset has the same number of points as the ScatterplotPlugin's position dataset. If the counts differ, suppress emitting sourceSelectionChanged and add a user notification explaining the mismatch to prevent invalid selection. Also fix a parameter name in ScalarSourceAction.h's doc comment (variantMap).
Cache the number of points for the scalar source and position datasets and use those values in the mismatch notification. This avoids repeated getNumPoints() calls and provides a clearer notification message (includes numPositions and numScalars) when the two datasets have different sizes.
ChangLi27
approved these changes
Feb 25, 2026
ThomasKroes
added a commit
that referenced
this pull request
Aug 3, 2026
) * Add missing actions to serialization * Fix HUD connection race (#223) * Remove duplicate connection * Prevent possible race * Group all positionDataset connections * Refactor: Group connections * Feature/Fix gui name rename crash (#230) * Refactor scalar source model and update ScalarAction Convert ScalarSourceModel from QAbstractListModel to QStandardItemModel and introduce typed row items (NameItem, IdItem, Item) and a two-column layout (Name/ID). Datasets are now stored as rows (with helper Row class) and looked up by dataset ID via matching; getDatasets(), getDataset(), add/remove dataset and removeAllDatasets were adapted accordingly. Added getRowIndex() and removed the old per-dataset vector/updateData machinery. Update GUI data handling into Item/NameItem/IdItem (decoration, display and tooltip roles) and connect GUI name changes to emit updates. In ScalarAction: use getRowIndex() instead of rowIndex(), changed setCurrentSourceIndex parameter to std::int32_t, simplified getCurrentDataset() return, and left a TODO-commented dataset connection block. These changes centralize model data in QStandardItem rows, enable multi-column metadata, and simplify dataset lookup/removal by ID. * Pass model reference to Item/Row Give Item/NameItem/Row access to their parent ScalarSourceModel by adding a const ScalarSourceModel& parameter and storing it in Item::_scalarSourceModel. Update construction sites (appendRow/Row) to pass *this. Use the model reference in Item::data() to respect getShowFullPathName() and use getGuiName() for display. Add getScalarSourceModel() accessor. Also change getDatasets() to return by value and remove some dead/commented code. These changes allow items to query model state reliably and simplify row construction. * Do not update the HUD during de-serialization, do it post-de-serialization. Put guard in updateHeadsUpDisplay * Properly sync HUD with loaded color, point size and point opacity dataset(s) * Do not update the HUD during de-serialization, do it post-de-serialization. Put guard in updateHeadsUpDisplay * Properly sync HUD with loaded color, point size and point opacity dataset(s) * Addd point size and opacity dataset picker actions and implement the synchronization logic * Add connection logic * Fix point size and opacity dataset selection reset logic * Avoid setting color dataset during project open Only call setCurrentColorDataset when the picked dataset is valid and a project is not currently opening. Previously the code unconditionally updated the current color dataset in the DatasetPickerAction handler; this change prevents dataset changes from being applied while a project load is in progress, avoiding potential race conditions or invalid state during project open. * Move dataset tracking into DatasetsAction Refactor dataset handling by moving color/size/opacity dataset setup and tracking out of ScatterplotPlugin into DatasetsAction. Add setup helper methods, getters, serialization (from/to QVariantMap), and connect/disconnect logic for public actions. Update ScatterplotPlugin to use the SettingsAction's coloring state when building the HUD and remove redundant dataset members and signal handlers from the plugin. This improves separation of concerns and centralizes dataset-related UI wiring in DatasetsAction. * Connect dataset pickers to HUD update Hook dataset picker changes to the heads-up display so the HUD updates immediately when datasets are picked. Adds connections from position, color, point size, and point opacity DatasetPickerAction::datasetPicked signals to ScatterplotPlugin::updateHeadsUpDisplay in init() (src/ScatterplotPlugin.cpp). * Refactor dataset pickers and HUD updates Add and wire up dataset picker actions for color, point size and point opacity; rename setup helpers to *PickerAction variants and re-run setup when a project is opened. Replace older datasetPicked connections with currentIndexChanged/sourceSelectionChanged signals for more reliable heads-up-display (HUD) updates, and connect dataset guiNameChanged directly to ScatterplotPlugin::updateHeadsUpDisplay. Add filter checks to ensure point-size/opacity datasets match position dataset sizes. Clean up commented/old code and move updateHeadsUpDisplayTextColor visibility to align with the refactor. * Refactor dataset picker setup and opacity sync Extracted repeated dataset picker initializations into setupDatasetPickerActions and call it on construction and when a project opens to reduce duplication. Switched the point opacity picker connection to currentIndexChanged and added logic to synchronize the picker with the ScalarAction (set default Constant source when no dataset, update picker when sourceSelection changes, clear index when not using a dataset). Added descriptive comments for the new helper and existing setup methods in the header. * Move dataset picker setup into ScatterplotPlugin Move control of dataset picker initialization from DatasetsAction to ScatterplotPlugin so the plugin can defer or re-run setup during project open. Make setupDatasetPickerActions protected and add ScatterplotPlugin as a friend so the plugin can call it. Refactor point-size dataset picker: tighten the filter to require PointType, a valid position dataset, and matching point counts; rework signal wiring to keep the DatasetPickerAction and ScalarAction synchronized and to set a default constant source when no dataset is selected. Minor debug logging added to aid troubleshooting. * Invalidate dataset picker filters Add DatasetsAction::invalidateDatasetPickerActionFilters() (declared in the header and implemented) which calls invalidateFilter() on all dataset picker actions (_position, _color, _pointSize, _pointOpacity). Update ScatterplotPlugin::init() to call this new method instead of setupDatasetPickerActions() when initializing so picker filters are refreshed to reflect the current datasets without reinitializing the actions. * Clear color dataset and refresh dependent pickers When the position dataset changes, clear the current color dataset and refresh dependent dataset pickers (color, point size, point opacity). Mark the position picker as Clearable, connect its datasetPicked signal to invalidate the other pickers, and invalidate them initially. Update filter callbacks to require a valid position dataset (and replace getPositionSourceDataset() calls with getPositionDataset()). This ensures pickers reflect the current position dataset and prevents stale/invalid color selections after position changes. * Remove debug qDebug logs in dataset check Remove leftover qDebug() calls from DatasetsAction::setupPointSizeDatasetPickerAction to clean up noisy debug output during dataset validation. The changes eliminate three intermediate debug prints while preserving the validation logic that ensures the dataset is of PointType and matches the position dataset point count. * Refactor dataset picker invalidation and reset (#232) Introduce helper lambdas to centralize dataset picker state updates: invalidateFilters() to invalidate dependent filters and resetAuxiliaryDatasets() to clear aux picker selections (setCurrentIndex(-1)). Connect the position picker signal to run the invalidation and connect position dataset change signals to reset auxiliary pickers so color/size/opacity selections are cleared when position datasets change. This removes duplicated calls and ensures auxiliary pickers don't hold stale selections. * Remove point size restriction (#234) * Remove point size restriction * Check scalar source dataset point count When the source picker index points to a dataset, verify that the selected scalar source dataset has the same number of points as the ScatterplotPlugin's position dataset. If the counts differ, suppress emitting sourceSelectionChanged and add a user notification explaining the mismatch to prevent invalid selection. Also fix a parameter name in ScalarSourceAction.h's doc comment (variantMap). * Show point counts in dataset mismatch warning Cache the number of points for the scalar source and position datasets and use those values in the mismatch notification. This avoids repeated getNumPoints() calls and provides a clearer notification message (includes numPositions and numScalars) when the two datasets have different sizes. * Fix opacity updates (#237) * Cache current dataset and handle dataChanged Add a _currentDataset member to ScalarAction and wire up its dataChanged signal when a new source is selected. Previously connections could accumulate; the code now disconnects any existing dataset signal before connecting the new one and forwards updates via a lambda that calls sourceDataChanged(getCurrentDataset()). Also update several parameter names in comments for clarity (variantMap, sourceSelectionIndex, offset). * Embed source dataset picker in ScalarAction Centralize dataset picking for scalar-driven properties by adding a DatasetPickerAction to ScalarAction and removing the separate point-size/opacity pickers from DatasetsAction. DatasetsAction no longer owns point size/opacity picker members; it instead reuses the ScalarAction pickers from the point plot settings and adjusts reset/connect logic accordingly (old filter code was commented out). ScalarAction now manages current dataset selection, connects dataset changes to emit sourceDataChanged and GUI name updates, and includes serialization for the new picker. ScatterplotPlugin was updated to listen to ScalarAction signals (sourceSelectionChanged/sourceDataChanged) for HUD updates and to refresh the scatter plot widget when scalar source data changes. Headers and getters were updated to reflect the new picker location. * Use LabelProxyAction for Size/Opacity pickers Replace direct DatasetPickerAction additions for point size and opacity with LabelProxyAction wrappers (labels "Size" and "Opacity") and include actions/LabelProxyAction.h instead of QMenu. Remove the now-unused setupPointSizeDatasetPickerAction and setupPointOpacityDatasetPickerAction declarations and implementations, and clean up related filter/invalidation code. This simplifies the dataset picker UI wiring and removes dead/commented code in DatasetsAction.cpp/.h. * Make SettingsAction and toolbar QPointers (#238) Convert SettingsAction and HorizontalToolbarAction members to QPointer types and create them with new, updating construction and all call sites accordingly. Update ScatterplotPlugin::getSettingsAction() to return a reference to the dereferenced QPointer. Replace direct member accesses with pointer-style (->) calls and adjust several call sites to obtain SettingsAction via dynamic_cast from parent()/parent()->parent() where appropriate (DatasetsAction, DensityPlotAction). Miscellaneous fixes: update toolbar widget creation, parent assignment, and serialization calls to use the new pointer members; small formatting and reference adjustments throughout impacted files. * Fix warnings and update points (#240) * Update number of points to uint64 * Use reference dataset * Add some const * Rename lambda capture variable to not shadow function paramters * More uint64 * Set MSVC warning level to W3 * We only want one dataset * Update core requirement due to previous commit * Adhere to new serialization API (#243) * Use new getter for clarity (avoid negation) (#242) * Use `mv_project_defaults()` for setting CMake defaults (#241) * Use mv project defaults * Simplify unity build setup * Prefer target based properties * Set cache variable instead of normal variable for CMake option * Adhere to revamped core --------- Co-authored-by: Alexander Vieth <a.vieth@tudelft.nl> * Set current point dataset when opacity dataset changed * Add extra null guard * Extends coloring options for scatterplot (addressing issue #24) (#247) Adds 2D and 3D coloring options. 2D allows arbitrary 2 channels using the build in 2D colormaps 3D allows arbitrary 3 channels mapping directly to RGB (normalized in shader) Modes are automatically picked when datasets with exactly 2 or 3 channels are set as color or can be manually set using the extended color action Renames 2D colormaps according to their authors * Fixes Qt 6.10 build Replaced deprecated 'mirrored' method with 'flipped' for color maps. * CI: Remove Release build and install steps (#248) * Upgrade build workflow actions and Python version Updated build workflow to use newer versions of actions and Python. * Revert principal dimension action name change (#250) * Revert principle dimension action name change * Ignore loading errors for newly introduced actions Do this for backwards compatibility --------- Co-authored-by: Julian Thijssen <julianthijssen@gmail.com> Co-authored-by: Alexander Vieth <a.vieth@tudelft.nl> Co-authored-by: Thomas Kroes <t.kroes@lumc.nl> Co-authored-by: Thomas Kroes <tkroes81@gmail.com> Co-authored-by: Soumyadeep Basu <44787782+sbvis@users.noreply.github.com> Co-authored-by: Thomas Höllt <thoellt@me.com> Co-authored-by: Alexander Vieth <a.vieth@lumc.nl>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This video illustrates the improved behavior
This pull request makes improvements to dataset validation and error handling in the scatterplot and scalar actions. The main focus is on ensuring that datasets used for point size, opacity, and scalar sources match the expected criteria, particularly the number of points, and providing user feedback when mismatches occur.
Validation and error handling improvements:
setupPointSizeDatasetPickerActionandsetupPointOpacityDatasetPickerActionto ensure that only datasets matching the expected data type and number of points are accepted. The checks now first validate the position dataset and then the data type, simplifying the logic and preventing mismatches. (src/DatasetsAction.cpp, [1] [2]ScalarActionconstructor to check that the selected scalar source dataset has the same number of points as the position dataset. If there is a mismatch, the selection change is not emitted and a user notification is shown with the details of the mismatch. (src/ScalarAction.cpp, src/ScalarAction.cppR21-R38)Documentation improvement:
fromVariantMapmethod inScalarSourceActionfor better readability and accuracy. (src/ScalarSourceAction.h, src/ScalarSourceAction.hL57-R57)