U correction not correctly applied to viewport #18875
Conversation
This code is badly indented. While we're herewe should fix it
samueljackson92
added
Component: GUI
Patch Candidate
| // Position, relative to origin | ||
| // Mantid::Kernel::V3D pos = det->getPos() - | ||
| // m_pos; | ||
| Mantid::Kernel::V3D pos = m_instrActor->getDetPos(i) - m_pos; |
There was a problem hiding this comment.
@samueljackson92 you should talk to @SimonHeybrock and @mantid-roman about these changes.
There was a problem hiding this comment.
This part of the code is actually unchanged. Are the detector changes already available in this part of the code? Also, as I understand it, there is a chance that we could have a patch release (with these changes). Wouldn't that mean that for the patch we would have to pull in all changes required to make the new detectorInfo work in the instrument view?
There was a problem hiding this comment.
Yes, those bits have been available for quite some time now.
There was a problem hiding this comment.
As @AntonPiccardoSelg points out the code is really unchanged. Those changes are only to whitespace. Whitespace changes are also a separate commit so could be dropped easily.
There was a problem hiding this comment.
@mantid-roman has an open PR that modifies this section of code. Maybe you can undo the whitespace changes to avoid merge conflicts.
| std::pair<double, double> RotationSurface::calculateViewRectOffsets() { | ||
| const auto dU = fabs(m_u_max - m_u_min); | ||
| const auto dV = fabs(m_v_max - m_v_min); | ||
| auto du = dU * 0.05; |
There was a problem hiding this comment.
What are these hard-coded values?
There was a problem hiding this comment.
As above, this is how the code was already, so I'm not sure.
Code Review:
Functional Testing:Before the fix: After the fix:
The fix seems to have worked. I applied the U correction and both detector banks were visible, therefore please |
|
This needs release notes. |
|
Final change was rst only, cancelled build to save resources |
Refs #18869 Fix U correction view rect (cherry picked from commit 25d21c2) Refs #18869 Fix code formatting This code is badly indented. While we're herewe should fix it (cherry picked from commit 9fff85c) Revert "Refs #18869 Fix code formatting" This reverts commit 9fff85c. (cherry picked from commit 0deffd9) Refs #18869 Fix blank view with no U correction (cherry picked from commit 9667cea) Add patch release note
This correctly applies the U correction to the viewport when viewing a rotation surface in the instrument view.
To test:
Fixes #18869.
Reviewer
Please comment on the following (full description):
Code Review
Functional Tests
Do changes function as described? Add comments below that describe the tests performed?
How do the changes handle unexpected situations, e.g. bad input?
Has the relevant documentation been added/updated?
Is user-facing documentation written in a user-friendly manner?
Has developer documentation been updated if required?
Does everything look good? Comment with the ship it emoji but don't merge. A member of
@mantidproject/gatekeeperswill take care of it.