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.

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

Disable projected click, fix time values #1800

Merged
merged 4 commits into from Feb 3, 2016
Merged

Disable projected click, fix time values #1800

merged 4 commits into from Feb 3, 2016

Conversation

@chaosphere2112
Copy link
Contributor

@chaosphere2112 chaosphere2112 commented Jan 21, 2016

When clicking on data that was projected, the wrong value was displayed. The issue is that we are not "de-projecting" the coordinates of the mouse. I wasn't able to come up with a quick fix for that (as my ability to reason through projection stuff is pretty limited), so I disabled the value checker for all nonlinear projections. I also removed some redundant logic for extracting the world coordinate (we had a call in vcs.utils for this). As an added bonus, we now get correct values when clicking on the plot after having animated it or advanced the current frame beyond the initial frame now. I'm pretty sure the time logic I used is solid, and it should fall back nicely to the "N/A" if it's not able to determine which axis is time.

@sankhesh @doutriaux1 @aashish24 @danlipsa Please review and merge so we can release.

@doutriaux1
Copy link
Contributor

@doutriaux1 doutriaux1 commented Jan 21, 2016

@chaosphere2112 we should really unproject we have all the info via the vtk arguments it's not too hard to revert projection in vtk. We should do this the right way.

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Jan 21, 2016

@chaosphere2112 okay if @danlipsa and @aashish take over this branch?

@chaosphere2112
Copy link
Contributor Author

@chaosphere2112 chaosphere2112 commented Jan 21, 2016

@aashish24 @doutriaux1 Sure thing, I was just getting this pushed out. I do need to make one tweak to the time axis logic still, but other than that anyone can take it over.

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Jan 21, 2016

@chaosphere2112 roger that.

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Jan 27, 2016

@danlipsa can you look in this for the projected plots?

@danlipsa
Copy link
Contributor

@danlipsa danlipsa commented Feb 2, 2016

@doutriaux1 @chaosphere2112 @aashish24 I think a proper fix for this may need plotting_dataset_bounds which I return from plot in my branch:
#1806
Probably I could do something now, but I will need to revisit it after that branch is merged.
Given that this never worked, I would say to merge it as is (with projected datasets not showing the coordinates at all) instead holding the release.

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Feb 2, 2016

@danlipsa this is not holding the release (at least not yet :))

@danlipsa
Copy link
Contributor

@danlipsa danlipsa commented Feb 2, 2016

That's good. :-)

doutriaux1 added a commit that referenced this issue Feb 3, 2016
Disable projected click, fix time values
@doutriaux1 doutriaux1 merged commit 18643ff into master Feb 3, 2016
3 of 8 checks passed
@doutriaux1 doutriaux1 deleted the fix_projected_click branch Feb 3, 2016
@aashish24 aashish24 mentioned this pull request Feb 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants