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

Vtk master bump #1348

Merged
merged 3 commits into from Jun 2, 2015

Conversation

Projects
None yet
3 participants
@allisonvacanti
Contributor

allisonvacanti commented May 27, 2015

Update code to work with new VTK master. See commit messages for details of changes.

This PR requires the following changes to the following repos:

I'm missing one baseline from the testdata patch. I'm on arch, so the dv3d vector test fails for me (https://open.cdash.org/testDetails.php?test=339707560&build=3831990). Can someone run that test with this uvcdat/vtk and send me a baseline with the new text, but not the line. IIRC the bug doesn't affect ubuntu or mac.

This PR should also be manually tested, since the buildbots won't pick up the VTK change.

@doutriaux1 @chaosphere2112

allisonvacanti added some commits May 27, 2015

Remove vcs2vtk.stripGrid.
This requires a change in VTK that skips blanked cells in vtkGlyph2D. It
is needed since the blanking mechanism has changed.
Update vcs2vtk.putMaskOnVTKGrid to reflect blanking changes in VTK.
All blanking information is now stored by setting bits on the
corresponding element in a field data array.
@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented May 27, 2015

@aashish24 The changes to vector plots (removing vcs2vtk.stripGrid) required a patch to VTK. The corresponding VTK merge request is here:

https://gitlab.kitware.com/vtk/vtk/merge_requests/242

@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented May 27, 2015

@dlonie I've just finished a build with this update, so I'm going to run the tests + give you the new baseline.

@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented May 27, 2015

@dlonie Here are my results... your updated baselines for test_vtk_ui_label_(top|x|y|left) are all incorrect. Don't know why the lines rendered that way on your system, but they should look like they used to (with a single pixel line).

@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented May 27, 2015

@dlonie Looks like the dv3d tests are only different in the label, so they should be fine.

@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented May 27, 2015

@dlonie Rest of the issues were all with this:

  File "/Users/samfries/bin/uvcdat/new_vtk/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/vcs/vcs2vtk.py", line 62, in putMaskOnVTKGrid
    vtk.vtkDataSetAttributes.GhostArrayName())
AttributeError: GhostArrayName
@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented May 27, 2015

@dlonie And this:

  File "/Users/samfries/bin/uvcdat/new_vtk/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/vcs/vcs2vtk.py", line 79, in putMaskOnVTKGrid
    vtk.vtkDataSetAttributes.HIDDENPOINT
AttributeError: HIDDENPOINT

@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented May 27, 2015

@dlonie Also has an issue with GhostArrayName on line 51 of vcs2vtk

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented May 28, 2015

@chaosphere2112

Re: Text alignment: I noticed the line was thicker on my system, but the text looks better aligned to my eye. Might be another platform issue? Feel free to send me some better ones if they look better on your system.

Re: vtkDataSetAttributes.* issues: Did you checkout the new VTK branch, and run make install in build/VTK-build? Those symbols are defined in the new VTK.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 1, 2015

@dlonie what's the status on this? Should I try and generate a new baseline? From Ubunut? From Mac?

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Jun 1, 2015

I think Mac would be ideal, since I've run the tests on linux, and should check for platform specific stuff on Mac. Just be sure to build and install the VTK branch mentioned in the first post. Let me know if you run into any issues with it.

Thanks!

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 2, 2015

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 2, 2015

merging

doutriaux1 added a commit that referenced this pull request Jun 2, 2015

@doutriaux1 doutriaux1 merged commit c6392bc into master Jun 2, 2015

2 of 4 checks passed

continuous-integration/kitware-buildbot/uvcdat-garant-linux-release/ Build done.
Details
continuous-integration/kitware-buildbot/uvcdat-test-laptop-linux-release/ Build done.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@doutriaux1 doutriaux1 deleted the vtk-master-bump branch Jun 2, 2015

@doutriaux1 doutriaux1 restored the vtk-master-bump branch Jun 2, 2015

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Jun 3, 2015

What is the state of this at the moment? I see that this was merged, and then there's a closed, unmerged PR to revert it, and neither this branch nor the revert branch appear in master.

The VTK branch will have to be manually pushed, unless we have a new procedure for it. We've been just rebasing and pushing manually in the past to keep the modifications on top to more easily identify the difference with upstream.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Jun 3, 2015

@dlonie I first merged this one then realized that would break everything since I couldn't update VTK. So I created a revert PR. But then I realize that @aashish24 would complain that the history is not clean. So I pushed back (-f) to the state before this PR goes in and closed the revert PR. Once you merged VTK we can use the re-issued PR to merge this ( #1357 ) which you merged so we should be good.

@allisonvacanti allisonvacanti deleted the vtk-master-bump branch Jun 4, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment