-
Notifications
You must be signed in to change notification settings - Fork 73
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.
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
CompareImage: Feature a consistent autoscale #3823
Conversation
d7a725e
to
77af645
Compare
77af645
to
9a3a2f4
Compare
9a3a2f4
to
4e565e9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a miss-spelling of pinned
and probably a missing change regarding colormap2
.
Otherwise, it's OK for me. We can live with ColormapWithPinnedData
as long as it is not exposed publicly.
4e565e9
to
638094a
Compare
This have to be rebaed on top of #3845 |
I have committed an alternative implementation to drop the custom colormap class.
That maybe a bit less efficient than the previous implementation, but this could be improved, and for now who cares. The good thing is that the displayed histogram is properly computed from the concatenation (previously the histogram was computed from the crop of one of the images) |
3553550
to
2ca415d
Compare
6e4e9cd
to
5d650bb
Compare
Can you rebase to include commit ccdc13b from |
5d650bb
to
656ca9d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Colormap range handling is as expected, thanks!
I'd keep backward compatibility for the updateColormap
arguments, it's doesn't hurt.
Co-authored-by: Thomas VINCENT <thomas.vincent@esrf.fr>
3f9a002
to
981a60b
Compare
While i was here, i have used the auto scale to improve the contrast on the composite image comparison. The result is probably not exactly the same for the B&W composition. @payno would you have some times to check if it's fine for you? Else i anyway have to fix something because the B&W was not working anyway. The good thing with this change is the colormap is used in a consistent way anyway the visualisation mode. |
@payno thinking about that a bit more, i think it is better to revert my change on the compare image for the B&W composition, it would be better to talk about that together before merging. |
I'd be +1 to merge this PR as it was and work on additional feature/fix in another PR. |
@t20100 i am fine with it, let's merge it as it is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Feature a consistent autoscale for the CompareImage.
Now
Changelog:
updateColormap=
was dropped, this can be set withgetColormap()