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

WIP - FVTK refactor/cleanup #286

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@stefanv
Contributor

stefanv commented Dec 12, 2013

No description provided.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Dec 17, 2013

Hi Stefan, all this is interesting and it gave me some ideas on how to make 3D visualization better in Dipy. The problem is I can only start looking at this after the release and most likely after Christmas. But we will get there at the end. Thank you for looking at this!

@arokem

This comment has been minimized.

Member

arokem commented Dec 19, 2013

+1 on Eleftherios' comment here. These are good, but I don't think this
should go into 0.7.

BTW - what is the status of #279 ?

That one is definitely intended for this release, correct?

On Tue, Dec 17, 2013 at 11:03 AM, Eleftherios Garyfallidis <
notifications@github.com> wrote:

Hi Stefan, all this is interesting and it gave me some ideas on how to
make 3D visualization better in Dipy. The problem is I can only start
looking at this after the release and most likely after Christmas. But we
will get there at the end. Thank you for looking at this!


Reply to this email directly or view it on GitHubhttps://github.com//pull/286#issuecomment-30779921
.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Dec 19, 2013

Yes, of course it is intended for the release. Please start commenting on it. I have removed the WIP flag.

@stefanv

This comment has been minimized.

Contributor

stefanv commented Dec 19, 2013

There are several places marked with XXX where I did not have sufficient
knowledge to update the docs. Would you please comment on these?

The camera view is determined by three vectors:
- `cam_pos`: The location of the camera in three-dimensional space.
- `cam_focal`: The point at which the camera is aimed (XXX check)

This comment has been minimized.

@Garyfallidis

Garyfallidis Mar 11, 2014

Member

Correct. This is the focal point of the camera.

linewidth : float
Width of the streamtubes.
tube_sides : int
XXX Document me XXX

This comment has been minimized.

@Garyfallidis

Garyfallidis Mar 11, 2014

Member

Number of faces for each tube.

XXX Document me XXX
lod : bool
XXX Document me better XXX
use vtkLODActor rather than vtkActor

This comment has been minimized.

@Garyfallidis

Garyfallidis Mar 11, 2014

Member

vtkLODActor allows to show the vtkActor at a much lower resolution to increase rendering speed when for example the object is rotated interactively.

Parameters
----------
points : ndarray, shape (N, 3)
3-D positions of points.

This comment has been minimized.

@Garyfallidis

Garyfallidis Mar 11, 2014

Member

Points here are represented as small spheres.

point_radius : float
Point radii.
theta : int
XXX Document me XXX

This comment has been minimized.

@Garyfallidis

Garyfallidis Mar 11, 2014

Member

Theta resolution. Number of points used along the polar angle for drawing the spheres which represent the points.

theta : int
XXX Document me XXX
phi : int
XXX Document me XXX

This comment has been minimized.

@Garyfallidis

Garyfallidis Mar 11, 2014

Member

Phi resolution. Number of points used along the azimuthal angle for drawing the spheres which represent the points.

color=(1, 1, 1)):
''' Create a label actor.
XXX Todo: Re-examine this API -- why ren? XXX

This comment has been minimized.

@Garyfallidis

Garyfallidis Mar 11, 2014

Member

The label creates 3D text which is always facing the camera so that it always visible. By having renderer as a property you can take the active camera from the renderer. See line 597. It's convenient. We could set it to have a camera as an option rather than the renderer. Let's think of it.

The Visualization Toolkit [book] and VTK's online documentation &
online docs.
XXX FIX up this docstring XXX

This comment has been minimized.

@Garyfallidis

Garyfallidis Mar 11, 2014

Member

This function needs a complete rewrite. Will do it after your PR is done.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Apr 3, 2014

Hello, where are we with this? Is there something being expected on my side?

@arokem

This comment has been minimized.

Member

arokem commented Apr 3, 2014

Yes - you were supposed to make a PR against this one, with clarifications in the documentation. For example, a bit of explanation about the VTK camera parameters.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Apr 3, 2014

I have written the rest of clarifications as comments here. I prefer working on the camera on a different PR. Is that okay?

@stefanv

This comment has been minimized.

Contributor

stefanv commented Apr 5, 2014

Sorry, I've been traveling for the past two weeks, but can get started this
week.

@samuelstjean

This comment has been minimized.

Contributor

samuelstjean commented Dec 8, 2014

Does the refactoring works with vtk6? I tried an example for fun, and fvtk.show works while fvtk.record doesn't, as a heads-up. I think Elef did some work on vtk6 support, there still seems to be some functions missing the vtk5/vtk6 logic though.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Oct 29, 2015

@stefanv is it okay if I close this? We are going to merge #587 today.

@stefanv

This comment has been minimized.

Contributor

stefanv commented Oct 31, 2015

@arokem arokem closed this Oct 31, 2015

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