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

Fvtk sphere origin #287

Closed
wants to merge 4 commits into from
Closed

Fvtk sphere origin #287

wants to merge 4 commits into from

Conversation

arokem
Copy link
Contributor

@arokem arokem commented Dec 21, 2013

This is a real small one. But it can also wait until after the release - no hurry.

This allows moving the sphere_funcs from their current (0, 0, 0) position to
other positions in space, allowing using this together with a volume actor,
showing (for example) the b0 image.
@Garyfallidis
Copy link
Contributor

@arokem and @stefanv do you want me to merge this or we wait until we start redesigning the fvtk module?

@arokem
Copy link
Contributor Author

arokem commented Feb 24, 2014

Let's talk redesign a little bit:

I think that this one: #286 has
precedence, with the mild API change proposed. I think it's good - any
objections?

One that's in, as the first priority, we should work a bit on the
documentation, so that anyone can understand what is going on. An
explanation of the VTK camera model is sorely needed to make this a bit
easier to deal with.

After that, we can think about how to combine different visualization tools
together to make what we need, which was the goal of this PR. Primarily, I
think that a reasonable goal would be to make it really easy to make
figures like this one (taken from the mrtrix paper):

[image: Inline image 1]
To do this, we would need an API that knows about the brain coordinates and
knows how to translate that into the right camera parameters. The problem I
have with doing that right now is that I really don't understand the VTK
camera API, so we're back to the documentation of that. @Garyfallidis: do
you think you could take a crack at explaining that?

On Mon, Feb 24, 2014 at 6:24 AM, Eleftherios Garyfallidis <
notifications@github.com> wrote:

@arokem https://github.com/arokem and @stefanvhttps://github.com/stefanvdo you want me to merge this or we wait until we start redesigning the fvtk
module?


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

@stefanv
Copy link
Contributor

stefanv commented Feb 25, 2014

@arokem Your proposed plan sounds good to me. If everyone agrees, I can get that branch in a mergeable condition again.

(Unfortunately the image did not inline correctly)

@arokem
Copy link
Contributor Author

arokem commented Feb 25, 2014

Here's the image I tried attaching:

screen shot 2014-02-24 at 10 34 31 am

@Garyfallidis
Copy link
Contributor

Yes, I can try to explain this problem and others related to vtk visualization for brain imaging. It would be easier if we can attend a short meeting first.

@Garyfallidis
Copy link
Contributor

In the meantime @arokem you may want to watch one of these introductory webinars
http://www.vtk.org/VTK/resources/webinars.html

@arokem
Copy link
Contributor Author

arokem commented Mar 11, 2014

Roadmap for moving forward on that:

  1. Start with @stefanv's PR for making renderer more OO-like. Merge that first!
  2. Break things up: make fvtk a directory, with sub-modules from the current fvtk.py file.
  3. Actor base-class (or function) that handles affine transformations for all the actors. Then, all the actors inherit from this one, or call this one individually. Reduce boiler-plate spatial stuff in each actor
  4. Camera - a better API for that

@stefanv stefanv modified the milestone: 0.9 Mar 4, 2015
@arokem
Copy link
Contributor Author

arokem commented Jun 24, 2015

This one's obsolete. There are new things coming: #587. Closing this one

@arokem arokem closed this Jun 24, 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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants