Skip to content

Blank instrument view with U correction#18959

Merged
martyngigg merged 3 commits into
masterfrom
18869_u_correction_instrument_view
Feb 23, 2017
Merged

Blank instrument view with U correction#18959
martyngigg merged 3 commits into
masterfrom
18869_u_correction_instrument_view

Conversation

@samueljackson92
Copy link
Copy Markdown
Contributor

It appears the start up the instrument view doesn't draw the correct viewport unless a U correction is applied. This appears to have been missed in the original PR, probably because the U correction gets cached in the QSettings.

To test:
Follow the same instructions as in PR #18875.

Fixes #18869 .


Reviewer

Please comment on the following (full description):

Code Review
  • Is the code of an acceptable quality?
  • Does the code conform to the coding standards? Is it well structured with small focussed classes/methods/functions?
  • Are there unit/system tests in place? Are the unit tests small and test the a class in isolation?
  • If there are changes in the release notes then do they describe the changes appropriately?
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/gatekeepers will take care of it.

@samueljackson92 samueljackson92 added Component: GUI Patch Candidate Urgent issues that must be included in a patch following a release labels Feb 21, 2017
@samueljackson92 samueljackson92 added this to the Release 3.10 milestone Feb 21, 2017
udet.u = applyUCorrection(udet.u);
}
}
updateViewRectForUCorrection();
Copy link
Copy Markdown
Member

@AndreiSavici AndreiSavici Feb 21, 2017

Choose a reason for hiding this comment

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

Does the text need to be indented so much from line 89 to 164? Otherwise it fixes the problem

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, I tried to fix this in the previous PR but the code has detector info changes which I don't think we want in the patch release so I took that commit out.

@peterfpeterson
Copy link
Copy Markdown
Member

Works for me.

@samueljackson92 samueljackson92 removed the request for review from AntonPiccardoSelg February 22, 2017 08:05
@AndreiSavici
Copy link
Copy Markdown
Member

:shipit:

@martyngigg martyngigg merged commit ffa01bf into master Feb 23, 2017
@martyngigg martyngigg deleted the 18869_u_correction_instrument_view branch February 23, 2017 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Patch Candidate Urgent issues that must be included in a patch following a release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants