Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

U correction not correctly applied to viewport #18875

Merged
merged 5 commits into from Feb 16, 2017
Merged

U correction not correctly applied to viewport #18875

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
25d21c2
Refs #18869 Fix U correction view rect
samueljackson92 Feb 15, 2017
9fff85c
Refs #18869 Fix code formatting
samueljackson92 Feb 15, 2017
005202c
Refs #18869 Add release notes.
samueljackson92 Feb 16, 2017
0deffd9
Revert "Refs #18869 Fix code formatting"
samueljackson92 Feb 16, 2017
146414e
made release note more user friendly
NickDraper Feb 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions MantidQt/MantidWidgets/inc/MantidQtMantidWidgets/InstrumentView/RotationSurface.h
View file
Open in desktop
Expand Up @@ -46,6 +46,11 @@ class RotationSurface : public UnwrappedSurface {
/// UnwrappedSurface::project().
double applyUCorrection(double u) const;

/// Update the view rect to offset for the U correction
void updateViewRectForUCorrection();
/// Calculate UV offsets from the view rect
std::pair<double, double> calculateViewRectOffsets();

const Mantid::Kernel::V3D m_pos; ///< Origin (sample position)
const Mantid::Kernel::V3D
m_zaxis; ///< The z axis of the surface specific coord system
Expand Down
203 changes: 107 additions & 96 deletions MantidQt/MantidWidgets/src/InstrumentView/RotationSurface.cpp
View file
Open in desktop
Expand Up @@ -86,102 +86,111 @@ void RotationSurface::init() {

// For each detector in the order of actors
// cppcheck-suppress syntaxError
PRAGMA_OMP(parallel for)
for (int ii = 0; ii < int(ndet); ++ii) {
if (!exceptionThrown)
try {
size_t i = size_t(ii);

unsigned char color[3];
Mantid::detid_t id = m_instrActor->getDetID(i);

boost::shared_ptr<
const Mantid::Geometry::IDetector> det;
try {
det = inst->getDetector(id);
} catch (
Mantid::Kernel::Exception::NotFoundError &) {
}

if (!det ||
detectorInfo.isMonitor(
detectorInfo.indexOf(id)) ||
(id < 0)) {
// Not a detector or a monitor
// Make some blank, empty thing that won't draw
m_unwrappedDetectors[i] = UnwrappedDetector();
} else {
// A real detector.
m_instrActor->getColor(id).getUB3(&color[0]);

// Position, relative to origin
// Mantid::Kernel::V3D pos = det->getPos() -
// m_pos;
Mantid::Kernel::V3D pos =
m_instrActor->getDetPos(i) - m_pos;

// Create the unwrapped shape
UnwrappedDetector udet(&color[0], det);
// Calculate its position/size in UV coordinates
this->calcUV(udet, pos);

m_unwrappedDetectors[i] = udet;
} // is a real detectord
} catch (std::exception &e) {
// stop executing the body of the loop
exceptionThrown = true;
g_log.error() << e.what() << '\n';
} catch (...) {
// stop executing the body of the loop
exceptionThrown = true;
g_log.error("Unknown exception thrown.");
}
} // for each detector in pick order

// if the loop above has thrown stop execution
if (exceptionThrown) {
throw std::runtime_error(
"An exception was thrown. See log for detail.");
}

// find the overall edges in u and v coords
findUVBounds();

// apply a shift in u-coord either found automatically
// or set manually
if (!m_manual_u_correction) {
// automatic gap correction
findAndCorrectUGap();
} else {
// apply manually set shift
m_u_min = manual_u_min;
m_u_max = manual_u_max;
for (size_t i = 0; i < m_unwrappedDetectors.size();
++i) {
auto &udet = m_unwrappedDetectors[i];
udet.u = applyUCorrection(udet.u);
}
}

double dU = fabs(m_u_max - m_u_min);
double dV = fabs(m_v_max - m_v_min);
double du = dU * 0.05;
double dv = dV * 0.05;
if (m_width_max > du && std::isfinite(m_width_max)) {
if (du > 0 && !(dU >= m_width_max)) {
m_width_max = dU;
}
du = m_width_max;
}
if (m_height_max > dv && std::isfinite(m_height_max)) {
if (dv > 0 && !(dV >= m_height_max)) {
m_height_max = dV;
}
dv = m_height_max;
}

m_viewRect = RectF(QPointF(m_u_min - du, m_v_min - dv),
QPointF(m_u_max + du, m_v_max + dv));
PRAGMA_OMP(parallel for)
for (int ii = 0; ii < int(ndet); ++ii) {
if (!exceptionThrown)
try {
size_t i = size_t(ii);

unsigned char color[3];
Mantid::detid_t id = m_instrActor->getDetID(i);

boost::shared_ptr<const Mantid::Geometry::IDetector> det;
try {
det = inst->getDetector(id);
} catch (Mantid::Kernel::Exception::NotFoundError &) {
}

if (!det || detectorInfo.isMonitor(detectorInfo.indexOf(id)) ||
(id < 0)) {
// Not a detector or a monitor
// Make some blank, empty thing that won't draw
m_unwrappedDetectors[i] = UnwrappedDetector();
} else {
// A real detector.
m_instrActor->getColor(id).getUB3(&color[0]);

// Position, relative to origin
// Mantid::Kernel::V3D pos = det->getPos() -
// m_pos;
Mantid::Kernel::V3D pos = m_instrActor->getDetPos(i) - m_pos;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samueljackson92 you should talk to @SimonHeybrock and @mantid-roman about these changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, those bits have been available for quite some time now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mantid-roman has an open PR that modifies this section of code. Maybe you can undo the whitespace changes to avoid merge conflicts.


// Create the unwrapped shape
UnwrappedDetector udet(&color[0], det);
// Calculate its position/size in UV coordinates
this->calcUV(udet, pos);

m_unwrappedDetectors[i] = udet;
} // is a real detectord
} catch (std::exception &e) {
// stop executing the body of the loop
exceptionThrown = true;
g_log.error() << e.what() << '\n';
} catch (...) {
// stop executing the body of the loop
exceptionThrown = true;
g_log.error("Unknown exception thrown.");
}
} // for each detector in pick order

// if the loop above has thrown stop execution
if (exceptionThrown) {
throw std::runtime_error("An exception was thrown. See log for detail.");
}

// find the overall edges in u and v coords
findUVBounds();

// apply a shift in u-coord either found automatically
// or set manually
if (!m_manual_u_correction) {
// automatic gap correction
findAndCorrectUGap();
} else {
// apply manually set shift
m_u_min = manual_u_min;
m_u_max = manual_u_max;
for (size_t i = 0; i < m_unwrappedDetectors.size(); ++i) {
auto &udet = m_unwrappedDetectors[i];
udet.u = applyUCorrection(udet.u);
}
}
}

/** Update the view rect to account for the U correction
*/
void RotationSurface::updateViewRectForUCorrection() {
const auto offsets = calculateViewRectOffsets();
const auto min = QPointF(m_u_min - offsets.first, m_v_min - offsets.second);
const auto max = QPointF(m_u_max + offsets.first, m_v_max + offsets.second);
m_viewRect = RectF(min, max);
}

/** Calculate UV offsets to the view rect
*
* @return a std::pair containing the u & v offsets for the view rect
*/
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these hard-coded values?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, this is how the code was already, so I'm not sure.

auto dv = dV * 0.05;

if (m_width_max > du && std::isfinite(m_width_max)) {
if (du > 0 && !(dU >= m_width_max)) {
m_width_max = dU;
}
du = m_width_max;
}

if (m_height_max > dv && std::isfinite(m_height_max)) {
if (dv > 0 && !(dV >= m_height_max)) {
m_height_max = dV;
}
dv = m_height_max;
}

return std::make_pair(du, dv);
}

void RotationSurface::findUVBounds() {
Expand Down Expand Up @@ -308,6 +317,7 @@ void RotationSurface::setUCorrection(double umin, double umax) {
}
m_manual_u_correction = true;
updateDetectors();
updateViewRectForUCorrection();
}

/**
Expand All @@ -316,6 +326,7 @@ void RotationSurface::setUCorrection(double umin, double umax) {
void RotationSurface::setAutomaticUCorrection() {
m_manual_u_correction = false;
updateDetectors();
updateViewRectForUCorrection();
}

} // MantidWidgets
Expand Down