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

VTK and Python 3 support in fvtk #6

Closed
skoudoro opened this issue Sep 20, 2018 · 8 comments
Closed

VTK and Python 3 support in fvtk #6

skoudoro opened this issue Sep 20, 2018 · 8 comments
Labels
state: needs check We need to check if this issue is relevant type:Bug Fix Something isn't working
Milestone

Comments

@skoudoro
Copy link
Contributor

From @grlee77 on January 29, 2016 23:15

It looks like the soon to be released VTK 7 finally supports Python 3! However, vtkVolumeTextureMapper2D is now deprecated in VTK7 so it can no longer be used in fvtk.volume (see here: http://www.vtk.org/Wiki/VTK/API_Changes_6_3_0_to_7_0_0 ). I am not sure what is an equivalent replacement if any. The good news is that it seems minimal changes to fvtk will be required to support VTK 7.

I ran many examples and the tests for VTK7 from the current release branch and things are looking pretty good. I propose a few minor fixes (mostly related to Python 3, not VTK 7) in #853

I only got two test failures with VTK 7 / Python 3.4:

ERROR: dipy.viz.tests.test_fvtk.test_fvtk_functions
----------------------------------------------------------------------

File "/media/Data1/src_repositories/my_git/dipy/dipy/viz/fvtk.py", line 494, in volume
    mapper = vtk.vtkVolumeTextureMapper2D()
AttributeError: 'module' object has no attribute 'vtkVolumeTextureMapper2D'

That one is avoided via b99d107227058b7e8bc58c8f1f64810f69d74235

However I am not sure what is causing the issue in this one:

FAIL: dipy.viz.tests.test_fvtk_window.test_order_transparent
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/lee8rx/anaconda/envs/py34/lib/python3.4/site-packages/nose-1.3.7-py3.4.egg/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/lee8rx/anaconda/envs/py34/lib/python3.4/site-packages/nose-1.3.7-py3.4.egg/nose/util.py", line 620, in newfunc
    return func(*arg, **kw)
  File "/home/lee8rx/anaconda/envs/py34/lib/python3.4/site-packages/numpy/testing/decorators.py", line 146, in skipper_func
    return f(*args, **kwargs)
  File "/home/lee8rx/anaconda/envs/py34/lib/python3.4/site-packages/dipy/testing/decorators.py", line 68, in test_with_xvfb
    my_test()
  File "/home/lee8rx/anaconda/envs/py34/lib/python3.4/site-packages/dipy/viz/tests/test_fvtk_window.py", line 224, in test_order_transparent
    npt.assert_equal(arr[150, 150][0] > arr[150, 150][1], True)
  File "/home/lee8rx/anaconda/envs/py34/lib/python3.4/site-packages/numpy/testing/utils.py", line 355, in assert_equal
    raise AssertionError(msg)
AssertionError:
Items are not equal:
 ACTUAL: False
 DESIRED: True

The images produced look like this:

green_front.png
green_front

red_front.png
red_front

Copied from original issue: dipy/dipy#854

@skoudoro
Copy link
Contributor Author

From @grlee77 on January 29, 2016 23:25

Actually, I guess vtk.vtkVolumeRayCastMapper and vtk.vtkVolumeRayCastCompositeFunction are also now deprecated, but they still seem to exist in the version I compiled. vtkVolumeTextureMapper2D was not available at all in the Python wrappers (at least for the compilation flags I used).

These are the CMake flags I had set at compile time. I can provide a conda build recipe if interested.

         CC=gcc
         CXX=g++
         PY_VER=3.4
         PY_VERM="${PY_VER}m"
         PY_LIB="libpython${PY_VERM}.so" 

        -DCMAKE_C_COMPILER=$CC \
        -DCMAKE_CXX_COMPILER=$CXX \
        -DCMAKE_BUILD_TYPE=Release \
        -DCMAKE_INSTALL_PREFIX="${PREFIX}" \
        -DCMAKE_INSTALL_RPATH:STRING="${PREFIX}/lib" \
        -DBUILD_DOCUMENTATION=OFF \
        -DVTK_HAS_FEENABLEEXCEPT=OFF \
        -DBUILD_TESTING=OFF \
        -DBUILD_EXAMPLES=OFF \
        -DBUILD_SHARED_LIBS=ON \
        -DVTK_WRAP_PYTHON=ON \
        -DPYTHON_EXECUTABLE=${PYTHON} \
        -DPYTHON_INCLUDE_PATH=${PREFIX}/include/python${PY_VERM} \
        -DPYTHON_LIBRARY=${PREFIX}/lib/${PY_LIB} \
        -DVTK_INSTALL_PYTHON_MODULE_DIR=${SP_DIR} \
        -DModule_vtkRenderingMatplotlib=ON \
        -DVTK_USE_X=ON \
        -DVTK_PYTHON_VERSION=3

@skoudoro skoudoro added type:Bug Fix Something isn't working state: needs PR labels Sep 20, 2018
@skoudoro
Copy link
Contributor Author

From @Garyfallidis on February 1, 2016 11:40

Hi @grlee77 thank you again for your excellent feedback. First I would like to say that I am very excited about having Python 3 support in VTK that was the only dependency we had in Dipy that was not Python 3 compatible. We need to update our docs.

Second, the formely known fvtk module is being slowly replaced with the new dipy.viz system. Have a look at the visualization section of http://nipy.org/dipy/examples_index.html#visualization
That means that fvtk.volume will probably change the next months and be updated to the VTK 6 and 7. Most likely the old version will be soon deprecated as the function's signature will change a lot. This was a function written very fast and many years ago. Let me know if you are using it and for what purpose. When I will start playing with vtk.vtkVolumeTextureMapper2D() and try to see if it works I will ask you to test in your system if that is okay with you. All you will need to do is update a branch and run the tests.

Now about the order transparency issue. This has been a nightmare for debugging. Order transparency in VTK is very unstable and it depends mostly on the type of the card that you have and what opengl features are supported in your card. There is a large number of extensions that allow for fast order transparency which is what we are trying to support here. There is an alternative but it will make the code much more complicated. So, for now we are planning to support only that and hope that the VTK guys will make it more stable.

Now, if that ordered transparency stopped working only when you switched to Python 3/VTK 7 but it was working on Python 2.7 on the same machine we definitely need to report it to the VTK developers. I am happy to help if this is the case.

@skoudoro
Copy link
Contributor Author

From @grlee77 on February 1, 2016 17:17

Hi @Garyfallidis, I am not sure if the same transparency problem is also present in VTK6/Python 2.7. I may be able to check later this week.

It seems that VTK7 uses a newer "OpenGL2" backend by default which has some differences in the volume rendering methods that are implemented. The following file indicates which classes will be present depending on various build flags:
https://gitlab.kitware.com/vtk/vtk/blob/master/Rendering/Volume/CMakeLists.txt

I am not currently using the volume rendering code, but only some of the point and tube plotting functionality (I was actually making some visualizations of 3D non-Cartesian k-space trajectories unrelated to diffusion imaging!). Matplotlib 3D plotting leaves much to be desired and mayavi has a PR where they are working on Python 3 support (also dependent on the upcoming VTK7), but nothing released yet, so I thought I would see if the functions in dipy.viz would work.

@skoudoro
Copy link
Contributor Author

From @Garyfallidis on February 1, 2016 18:23

Cool, thank you for the tips. Most likely you will see the same effect in the same machine. If not I will definitely need to know that and report it to VTK.

@skoudoro
Copy link
Contributor Author

From @grlee77 on February 1, 2016 19:4

Anaconda already has a VTK 6.3 package for Python 2.7, so it was easy to test against that. The result is that green_front.png looks the same as above, but red_front.png looks correct now:

red_front

I don't know enough to say whether this is a VTK 7 bug or if it could be due to some differences in compilation flags.

@MarcCote
Copy link
Contributor

MarcCote commented Oct 6, 2018

Not sure this issue is still relevant since fury requires vtk8.1 now?

@skoudoro
Copy link
Contributor Author

skoudoro commented Oct 6, 2018

The Title is not relevant but I think we still have this error. We have to check this error before.

@dmreagan dmreagan added state: needs check We need to check if this issue is relevant and removed state: needs PR labels Dec 6, 2018
@skoudoro skoudoro added this to the v0.2.0 milestone Dec 13, 2018
@skoudoro
Copy link
Contributor Author

fix by #40

skoudoro referenced this issue in skoudoro/fury Nov 27, 2019
update snapshot to use load_image
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: needs check We need to check if this issue is relevant type:Bug Fix Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants