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

Do not wrap again in update_input #1657

Merged
merged 6 commits into from Nov 16, 2015
Merged

Do not wrap again in update_input #1657

merged 6 commits into from Nov 16, 2015

Conversation

@sankhesh
Copy link
Contributor

@sankhesh sankhesh commented Nov 2, 2015

This change fixes the issue wherein no output was being produced
when animating projected data. Each draw_frame call invokes
VTKPlots::update_input which in turn calls doWrap on the
actor. Trying to wrap projected data fails.

Fixes #1086

@sankhesh
Copy link
Contributor Author

@sankhesh sankhesh commented Nov 2, 2015

@doutriaux1 @aashish24 Please review

This change fixes the issue wherein no output was being produced
when animating projected data. Each draw_frame call invokes
VTKPlots::update_input which in turn calls doWrap on the
actor. Trying to wrap projected data fails.

Fixes #1086
@sankhesh sankhesh force-pushed the 1086_animation_bug branch from 33d725f to de8d649 Nov 2, 2015
@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Nov 3, 2015

@sankhesh this is great! Makes sense and LGTM 👍 Should we add a test for projected dataset animation as well?

@sankhesh
Copy link
Contributor Author

@sankhesh sankhesh commented Nov 3, 2015

Yup. I can add one.

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Nov 6, 2015

@sankhesh can add one soon so that we can make it done?

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Nov 9, 2015

@sankhesh we should really get this one in master asap. Can you please add test soon?

Thanks,

@sankhesh
Copy link
Contributor Author

@sankhesh sankhesh commented Nov 10, 2015

Baselines added in CDAT/uvcdat-testdata#81

@sankhesh
Copy link
Contributor Author

@sankhesh sankhesh commented Nov 10, 2015

@doutriaux1 While adding the test for this issue, I found that meshfill plots with mercator projection fails. Documented here: #1671

@sankhesh
Copy link
Contributor Author

@sankhesh sankhesh commented Nov 11, 2015

Not sure why we have those test failures. They don't fail on my machine.
@aashish24 @doutriaux1 Can you reproduce those failures?

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Nov 11, 2015

@sankhesh which machine you are seeing these failures?

@sankhesh
Copy link
Contributor Author

@sankhesh sankhesh commented Nov 11, 2015

annie, crunchy

@doutriaux1
Copy link
Contributor

@doutriaux1 doutriaux1 commented Nov 12, 2015

@sankhesh both annie and crunchy are running behind Xvfb. Can you try with Xvfb?

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Nov 13, 2015

@sankhesh any luck with testing?

@sankhesh
Copy link
Contributor Author

@sankhesh sankhesh commented Nov 13, 2015

@aashish24 I've pushed a new change to test only the last image of the animation as you suggested. Waiting on the dashboards to complete.

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Nov 13, 2015

thanks @sankhesh much appreciated!

@sankhesh
Copy link
Contributor Author

@sankhesh sankhesh commented Nov 16, 2015

@doutriaux1 @chaosphere2112 This branch is ready to be merged. The build errors are not from this branch. The only true error is meshfill_mercator animation which will be fixed by #1672

doutriaux1 added a commit that referenced this issue Nov 16, 2015
Do not wrap again in update_input
@doutriaux1 doutriaux1 merged commit 91198b1 into master Nov 16, 2015
1 of 9 checks passed
@doutriaux1 doutriaux1 deleted the 1086_animation_bug branch Nov 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants