Ticket #10441 (closed: fixed)
Refactor the Spectrum Viewer & RefDetectorViewer
Reported by: | Dan Nixon | Owned by: | Dan Nixon |
---|---|---|---|
Priority: | major | Milestone: | Release 3.3 |
Component: | GUI | Keywords: | |
Cc: | Blocked By: | #10324 | |
Blocking: | Tester: | Harry Jeffery |
Description (last modified by Dan Nixon) (diff)
The spectrum viewer does not follow the code style of the rest of Mantid and as such is a pain to maintain.
It should be tidied up, the code style improved and a few small issues I noticed:
- The run title should be trimmed
- The Vertical Graph Info label should be in the same layout as the vertical graph info
- As much as possible the two curve plots should line up with the image plot
Change History
comment:1 Changed 6 years ago by Dan Nixon
- Status changed from new to assigned
- Blocked By 10324 added
comment:3 Changed 6 years ago by Dan Nixon
- Status changed from assigned to inprogress
Work in progress: moving to Mantid code style
Refs #10441
Changeset: 94269051586197ef671958c60fe2f7bf372176a7
comment:4 Changed 6 years ago by Dan Nixon
Spectum Viewer now follows code style
Refs #10441
Changeset: ebd9e7d100d21ddf52379d7f40eb5e39af76d68a
comment:5 Changed 6 years ago by Dan Nixon
Work in progress: refactoring RefDetectorViewer
Refs #10441
Changeset: a7c4b6d248c5fb1bd2265935915836de99bcf243
comment:6 Changed 6 years ago by Dan Nixon
- Summary changed from Refactor the Spectrum Viewer to Refactor the Spectrum Viewer & RefDetectorViewer
The RefDetectorViewer uses a lot of the code from Spectrum Viewer, some of it needs changed alongside the changes to Spectrum Viewer so may as well be refactored now as well.
comment:7 Changed 6 years ago by Dan Nixon
More refactoring in RefDetectorViewer
Refs #10441
Changeset: 803eddff89beda12e723d6b33e2fe21c1bf4bea7
comment:8 Changed 6 years ago by Dan Nixon
Make the UI keep it's shape a bit better
Refs #10441
Changeset: cb44e46987568d912fc5bf3b2deb800f8128e9ca
comment:10 Changed 6 years ago by Dan Nixon
comment:11 Changed 6 years ago by Dan Nixon
comment:12 Changed 6 years ago by Dan Nixon
comment:13 Changed 6 years ago by Dan Nixon
comment:14 Changed 6 years ago by Dan Nixon
Better documentation in SVConnections
Refs #10441
Changeset: d478622c212bfaa55074ca06ae06a743671de980
comment:15 Changed 6 years ago by Dan Nixon
Also this commit: 69ddf5afaef76ffbfd8dfec6df9983fedae7d404
comment:16 Changed 6 years ago by Dan Nixon
comment:17 Changed 6 years ago by Dan Nixon
To test:
- Load a workspace (any matrix workspace should be fine, >50 spectra recommended)
- Move the splitter between the image and spectra graph, the splitter between the vertical graph info and vertical graph should move to keep graphs in line.
- Do the same as above in reverse.
- Select points on each graph, ensure data is still shown correctly
- Code review to ensure Mantid code style is met
comment:18 Changed 6 years ago by Dan Nixon
- Status changed from inprogress to verify
- Resolution set to fixed
comment:19 Changed 6 years ago by Harry Jeffery
- Status changed from verify to verifying
- Tester set to Harry Jeffery
comment:20 Changed 6 years ago by Harry Jeffery
- Status changed from verifying to reopened
- Resolution fixed deleted
comment:21 Changed 6 years ago by Dan Nixon
- Status changed from reopened to inprogress
comment:22 Changed 6 years ago by Dan Nixon
comment:23 Changed 6 years ago by Dan Nixon
- Status changed from inprogress to verify
- Resolution set to fixed
comment:25 Changed 6 years ago by Harry Jeffery
- Status changed from verifying to reopened
- Resolution fixed deleted
- RefImageView.cpp:65 - image_display does not need to exist. You could just use m_imageDisplay.
- RefIVConnections.cpp:283-287 - These do not need to be destroyed by us. They all have parents, so Qt's QObject hierarchy will clean them up for us naturally.
- RefIVConnections.cpp:573 - If an exception is thrown before delete[] we leak memory. We should use a std::vector<unsigned int> here instead.
- Since we know the size in advance we can save the cost of resizing by telling the constructor to fill it with total_colors items each with a value of 0: std::vector<unsigned int> rgbData(total_colors, 0);
- Also, as vectors guarantee contiguous storage, the raw access on line 590-591 can be achieved with QImage image(&rgbData[0], static_cast<int>(total_colors), 1, QImage::Format_RGB32);
- DataArray.h:131 - We could a boost::shared_ptr<float> here to make the ownership more explicit. It's also worth wondering if this couldn't be changed into a boost::shared_ptr<std::vector<float>>
- ArrayDataSource.h:47 - Once again, why not use is a boost::shared_ptr<float> or even a shared pointer to a std::vector<float>.
Summary: Let's eliminate the use of pointers to raw dynamic arrays and replace them with pointers to std::vector<float>s and wrap them in appropriate smart pointers.
comment:26 Changed 6 years ago by Dan Nixon
- Status changed from reopened to inprogress
Removed use of arrays in colour images
Refs #10441
Changeset: cee589c06f6ac0ad4e3a2227e35dcf7b90161a5d
comment:27 Changed 6 years ago by Dan Nixon
comment:28 Changed 6 years ago by Dan Nixon
Use const shared pointers for DataArray
Refs #10441
Changeset: 9ff3dc339762b9f88040f1a0c4a330af81f26ccd
comment:29 Changed 6 years ago by Dan Nixon
Replace SpectrumDataSource with shared pointers
Refs #10441
Changeset: 4af1040b83989dd55885212faf6eca4f27d8853e
comment:30 Changed 6 years ago by Dan Nixon
comment:31 Changed 6 years ago by Dan Nixon
comment:32 Changed 6 years ago by Dan Nixon
Remove Spectrum View demo application
Refs #10441
Changeset: 27c3b8e2f34a5785261df4c4a135cfe67461f82f
comment:33 Changed 6 years ago by Dan Nixon
- Status changed from inprogress to verify
- Resolution set to fixed
comment:35 Changed 6 years ago by Harry Jeffery
- Status changed from verifying to closed
Merge remote-tracking branch 'origin/feature/10441_refactor_spectrum_viewer'
Full changeset: bf6d611be35b73645a5be717cdb77501c414dbb9
comment:36 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 11283