Renaming HtmlFormat to ToolTipFormat, and creating a new HtmlFormat. #343
+16,400
−6,834
Conversation
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
…tils, so that we can use those functions in our standalone TileListWidget.
…oftware#272) This requires ITK 5 Beta 2 or ITK 4.13.2, both are currently to-be-released.
…et for selecting a list of data containers
…property type from QVector<DataArrayPath> to QStringList. We only need to pass along a list of data container names, not full paths.
…iDataContainerSelection widget.
…dates from ITK to DREAM3D filters.
…ariable Signed-off-by: Tom Platt <tom.platt@bluequartz.net>
…e used by filters to register and stitch images with any data type and image type.
… dream3d file correctly.
resolves nathanblair/SIMPL/#2 Drag'n'Drop works for MultiDCSelection Widget Signed-off-by: Nathan Blair <nathan.blair@bluequartz.net>
Signed-off-by: Nathan Blair <nathan.blair@bluequartz.net>
Merge in @nathanblair work on MultiDataContainerFP
…tils, so that we can use those functions in our standalone TileListWidget.
…oftware#272) This requires ITK 5 Beta 2 or ITK 4.13.2, both are currently to-be-released.
…et for selecting a list of data containers
…property type from QVector<DataArrayPath> to QStringList. We only need to pass along a list of data container names, not full paths.
…iDataContainerSelection widget.
…dates from ITK to DREAM3D filters.
Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
…s through override Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
* Replacing PipelineMessage API with an AbstractMessage superclass and subclasses for every type of message. This eliminates the need for the MessageType enumeration. * Implementing a new, cleaner and more reusable API that observers can implement to be able to process specific types of messages. Take a look at the IssuesWidget and IssuesWidgetMessageHandler for a good example of this. Observers can decide which types of messages they are interested in by subclassing the AbstractMessageHandler class and reimplement the appropriate processMessage() methods. * Passing AbstractMessages from observable object to observer object using a shared pointer instead of by value. This allows observers to store the messages with all the relevant subclass data if they prefer. * Eliminating the use of humanLabel in Observable's and AbstractFilter's notify****Message method parameters. In AbstractFilter, these methods already set the human label into the message. * It is no longer necessary to set error and warning codes outside of the setErrorCondition and setWarningCondition methods. These methods will automatically set the code to the integer that is passed in. * Filters that emit progress messages while they are in an executing pipeline have their progress values automatically converted into overall pipeline progress values and re-emitted as pipeline progress messages. This allows filters to update the overall progress of the pipeline while they are running. Check out FilterPipelineMessageHandler at the top of the FilterPipeline implementation file.
+ Dimension initialization was 0,0,0 when it should be 1,1,1 + Ordered SIMPLArray to start at the generic case, then move from 2, 3, 4 element arrays + Removed unused variable from FilterPipeline Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
…r==() + the at() method used operator[] underneath which does NOT do bounds checking, breaking compatibility assumptions from STL + added operator==() method. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
…ueQuartzSoftware#338) When plugins are loaded, a FilterFactory<T> is created and stored for each filter that is in the plugin. If the developer only needs items such as human label, Html summary or other textual items then it is better and quicker to just use the FilterFactory itself instead of instantiating an entirely new Filter object. This leads to a speed up when launching currently. Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
This allows special cases such as ITKImageProcessing where different filters are enabled based on LeanAndMean, ITKv4 or ITKv5 Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
…lueQuartzSoftware#331) * Addressed arithmetic overflow and other code analysis warnings * Initialized PipelineMessage index * Marked a possible coding error in ImportAsciiDataArray where two identical error conditions are checked using || operator. * Fixed an incorrect cast in ReadASCIIData where a double is cast to a float for a method parameter that is expecting a double. Due to multiple calculations of the same value, it was saved to a constant before the first calculation was required. * Changed a for loop variable in geometry compute methods to avoid default castings when dereferencing a pointer multiple times per iteration. * Changed geometry requirements in addAttributeMatrix from using || for enum checks to &&. The previous versions could never be false and invalidated the entire method. * Fixed a possible overflow when calculating ImageGeom and RectGridGeom coords. The previous versions would have overflowed before casting to a larger size. * Updated variables used to store min and max float values to constexpr to take advantage of the fact that they are known at compile time and are never modified. * Removed unnecessary casts in SIMPLibRandom where doubles were cast to float and then saved as a double. * Fixed ColorTable::GetColorTable due to the current implementation going out of array bounds. The 2D array float color[8][3] was being accessed with indices [8][2]. * Updated VTKFileReader::skipVolume to use size_t dimensions * Changed xDim, yDim, and zDim parameters to size_t from int. If either one or three of them were negative, the total size variable used to allocate memory would save a negative number as an unsigned value, causing overflow. Moreover, any negative dimensions would silently prevent data from being read. * Updated ColorTable::GetColorTable with static_cast and rename * Updated preset color table only. * Removed duplicate error checks in ImportAsciDataArray * Other errors are already handled, so these seem to be redundant rather than typos. * Updated ColorTable::GetColorTable for json values * Applied fixes from preset color table method. * Updated incorrect currBinIndex maximum value in preset table method * Moved variables inside the loop where appropriate. * Updated numColors parameter to size_t both for cast issues and because it makes no sense to have a negative number of colors. * Made variables const where appropriate. * Ran clang-format on ColorTable.cpp for readability * Fixed more arithmetic overflow warnings with type casts * Performed casts on getNumberOfComponents() to size_t for calculations. This method should probably return size_t instead of a signed int if a negative component dimension is not possible. * Changed DynamicTableData::ExpandData parameters nRows and nCols from signed int to size_t. Negative values were not handled, and the values are more descriptive with their current type. size_t should be prefered over int when it comes to dimensions unless there is a good reason to include negative values as a valid option. * Added a hsize_t cast to H5DataArrayWriter when calculating rank size in H5DataArrayWriter.hpp By casting the first value, arithmetic operations progress it forward while preventing int32_t-related overflow. * Added casts to int64_t when faceId values to local variables in GeometryMath.cpp. These variables were used for referencing vertex pointer indices where int64_t values were expected * Fixed a few uninitialized variables * Fixed type casting in CubeOctohedronOps.cpp * Added missing `f` in plane4Comp and plane7Comp arithmetic operations. This was determined as a mistake by looking at other operations where the same calculations were consistently done with a floating point constant, but the 'f' was left off in two places. The static_cast<float> was determined to silence a warning regarding type casting down from double to float due to the propogation of the double type through the calculation. * Added default value to ErrorObject::ok boolean in ParseFunctors * Silenced unitialized variable warning * Added default construction to SIMPLib property macros * This is unlikely to affect the default values for integers, floating point numbers, and booleans (not created with SIMPL_BOOL_PROPERTY). * Hard sets boolean properties to false using SIMPL_BOOL_PROPERTY * Silences uninitialized variable warnings for other types. * These macros still have the potential to cause bugs due to uninitialized variables. * Added a default initialization for PipelineMessage code. * Initializes to 0 for the specified constructor. * Merge branch 'develop' into feature/FixCodeAnalysis # Conflicts: # Source/SIMPLib/CoreFilters/MassCreateData.cpp # Source/SIMPLib/CoreFilters/ReadASCIIData.cpp # Source/SIMPLib/Geometry/IGeometry.cpp # Source/SIMPLib/Geometry/ImageGeom.cpp * Revert "Merge branch 'develop' into feature/FixCodeAnalysis" This reverts commit 77a6fb1.
…ting from the old HtmlFormat is the same formatting used in the new ToolTipFormat. Everything that was using the old HtmlFormat is now using ToolTipFormat. Adding a new HtmlFormat that currently creates the same table as the ToolTipFormat, but without the yellow background color. The new HtmlFormat can be further refined later if it needs to be.
Contributor
Author
|
This was done in an effort to re-use the getInfoString method to be able to display information about data container geometries in IMFViewer. Currently before this PR, the HtmlFormat formatting looks super odd because it is mainly used for tooltips and has a pale yellow background. This PR addresses that and creates a basic HTML format with a blank background that applications can use. Mike's main concern is that the ToolTipFormat uses HTML also, so developers who want to use this API may get confused as to which one to use. I am open to hearing any better ways of designing this so that anything can ask a data container's geometry for its information string and get a usable string in return that looks nice. |
| case SIMPL::ToolTipFormat: | ||
| bgColor = "bgcolor=\"#FFFCEA\""; | ||
| nameBgColor = "bgcolor=\"#E9E7D6\""; | ||
| case SIMPL::HtmlFormat: |
Contributor
There was a problem hiding this comment.
Add a comment here (before the case SIMPL::HtmlFormat:) stating that the fall-through is intentional. Better yet, use a commented out [[fallthrough]] attribute to document the intention. Without any kind of documentation of intent, this looks like a mistake and could be easily "fixed" while trying to reduce compiler warnings.
| case SIMPL::ToolTipFormat: | ||
| bgColor = "bgcolor=\"#FFFCEA\""; | ||
| nameBgColor = "bgcolor=\"#E9E7D6\""; | ||
| case SIMPL::HtmlFormat: |
Contributor
There was a problem hiding this comment.
If fall-through is intentional, make sure to document it ever time. // [[falthrough]] should be good enough while remaining short and easy to read. We can't use the fallthrough attribute as it's C++17, but it's everything we would want to use while remaining far more easily readable than a longer comment stating that we want to use [[fallthrough]]
| ss << "<tr " << bgColorLabel << "><th align=\"right\">Extents: </th><td>" | ||
| << "<p>" << xExtentString << "</p>" | ||
| << "<p>" << yExtentString << "</p>" | ||
| << "<p>" << zExtentString << "</p>" |
Contributor
There was a problem hiding this comment.
This seems weird like some of these lines are supposed to be different than the others because half of them are treated the same with ss at the start and half do not. Try to keep them all consistent, or it looks like they are not supposed to be the same without any decipherable reason to their difference.
mmarineBlueQuartz
requested changes
Apr 21, 2019
Contributor
mmarineBlueQuartz
left a comment
mmarineBlueQuartz
left a comment
There was a problem hiding this comment.
Make sure you have some kind of documentation for every case intentionally involving fall-through between cases so that they do not look like coding errors and get "fixed" after compiler warnings label them as such. You did so rather consistently, so I only marked the first two.
Make sure you consistently use ss << ... and << ... because mixing and matching makes it look like there is supposed to be a difference. ss << ... is the most common, so use that. It looks far cleaner and is easier to read when it's formatted the same, especially when no one starts reading through them to find out why some of them are treated as special cases. At least one person already has.
imikejackson
force-pushed
the
develop
branch
from
4998629 to
1d8ba0b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Replacing the current HtmlFormat with a new ToolTipFormat. The formatting from the old HtmlFormat is the same formatting used in the new ToolTipFormat. Everything that was using the old HtmlFormat is now using ToolTipFormat.
Adding a new HtmlFormat that currently creates the same table as the ToolTipFormat, but without the yellow background color. The new HtmlFormat can be further refined later if it needs to be.