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

Adding custom interactor for vizualisation #1098

Merged
merged 23 commits into from Nov 2, 2016

Conversation

Projects
None yet
5 participants
@MarcCote
Contributor

MarcCote commented Jul 23, 2016

This PR adds a new vtkInteractorStyle that allows us to propagate events (mouse, keyboard, etc.) to objects/actors that have been selected.

This is needed for the coming PRs about UI framework that @ranveeraggarwal is working on.

*This PR is still missing some tests.

@codecov-io

This comment has been minimized.

codecov-io commented Jul 23, 2016

Current coverage is 85.30% (diff: 86.60%)

Merging #1098 into master will increase coverage by 4.45%

@@             master      #1098   diff @@
==========================================
  Files           200        219     +19   
  Lines         23023      24878   +1855   
  Methods           0          0           
  Messages          0          0           
  Branches       2309       2519    +210   
==========================================
+ Hits          18614      21222   +2608   
+ Misses         3933       3048    -885   
- Partials        476        608    +132   

Powered by Codecov. Last update c09a194...5eb74b3

@@ -486,6 +469,52 @@ def start(self):
del self.iren
del self.window
def record(self, filename="record.log"):

This comment has been minimized.

@Garyfallidis

Garyfallidis Sep 8, 2016

Member

Let's change the name to record_events (because we are already using this name in another ) and the next one play_recorded_events. Or we could add parameters to start(mode=None or 'record' or 'play')

This comment has been minimized.

@MarcCote

MarcCote Sep 16, 2016

Contributor

After thinking about it, I will simply rename the methods record to record_events and play to play_events. However, I think that adding a parameter mode to the start method won't be that useful and it would also require adding a filename parameter (which is mandatory for both record_events and play_events).

@MarcCote

This comment has been minimized.

@arokem

This comment has been minimized.

Member

arokem commented Oct 14, 2016

No idea. Looks like @yarikoptic added it (
https://github.com/nipy/dipy/blame/d0bee8c811daf00c5f9c153168ccbc82fa3b5557/.travis.yml#L85),
so he might know.

On Fri, Oct 14, 2016 at 11:26 AM, Marc-Alexandre Côté <
notifications@github.com> wrote:

@arokem https://github.com/arokem which Travis build is used for
codecov? It seems it doesn't have VTK. See https://codecov.io/gh/nipy/
dipy/compare/c09a194e48431325e071bf9d3607805bab75c318...
2f297fd#646970792F76697A2F696E74657261
63746F722E7079-8


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1098 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAHPNpocyc3yXe2oNMh_QI6DkZcrI9nCks5qz8lFgaJpZM4JTZLo
.

MarcCote added some commits Oct 14, 2016

@MarcCote MarcCote changed the title from WIP: Adding custom interactor for vizualisation to Adding custom interactor for vizualisation Oct 14, 2016

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Oct 15, 2016

Hi @MarcCote the coverage jumped from 82 to 88% and then back to 82? We should probably calculate the coverage with VTK enabled. Do you agree?

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Oct 15, 2016

I get this with VTK 5.
tests (enh_viz_renderer)]$ python test_fvtk_widgets.py
X Error of failed request: BadValue (integer parameter out of range for operation)
Major opcode of failed request: 154 (GLX)
Minor opcode of failed request: 3 (X_GLXCreateContext)
Value in failed request: 0x0
Serial number of failed request: 24
Current serial number in output stream: 29
Will try also with VTK 6 and see what happens.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Oct 15, 2016

This error was in VTK 5.10.1 but when I upgraded to VTK 6.2 the errors disappeared completely.

@@ -37,6 +37,7 @@ matrix:
- python: 2.7
sudo: true # This is set to true for apt-get
env:
- COVERAGE=1

This comment has been minimized.

@Garyfallidis

Garyfallidis Oct 15, 2016

Member

We should make sure that the coverage is calculate with vtk testing enabled.

This comment has been minimized.

@MarcCote

MarcCote Oct 15, 2016

Contributor

Should I delete the other ones: lines 26 and 30? That way, we will stop receiving multiple coveralls posts.

This comment has been minimized.

@MarcCote

MarcCote Oct 17, 2016

Contributor

See PR #1132

# major_version = vtk.vtkVersion.GetVTKMajorVersion()
else:
vtkInteractorStyleUser = object

This comment has been minimized.

@Garyfallidis

Garyfallidis Oct 15, 2016

Member

one more line

@npt.dec.skipif(not actor.have_vtk or not actor.have_vtk_colors or skip_it)
@xvfb_it
def test_custom_interactor_style_events(recording=False):
recording_filename = pjoin(DATA_DIR, "test_custom_interactor_style_events.log.gz")

This comment has been minimized.

@Garyfallidis

Garyfallidis Oct 15, 2016

Member

line too long (pep8)

# the show manager allows to break the rendering process
# in steps so that the widgets can be added properly
interactor_style = interactor.CustomInteractorStyle()
show_manager = window.ShowManager(renderer, size=(800, 800), interactor_style=interactor_style)

This comment has been minimized.

@Garyfallidis

Garyfallidis Oct 15, 2016

Member

same here

renderer.add(tube2)
# Define some counter callback.
states = defaultdict(lambda: 0)

This comment has been minimized.

@Garyfallidis

Garyfallidis Oct 15, 2016

Member

empty line

This comment has been minimized.

@MarcCote

MarcCote Oct 15, 2016

Contributor

Done

events = gzip.open(filename, 'r').read()
else:
events = open(filename).read()

This comment has been minimized.

@Garyfallidis

Garyfallidis Oct 15, 2016

Member

No need to close the file after reading it?

This comment has been minimized.

@MarcCote

MarcCote Oct 15, 2016

Contributor

Good point.

@coveralls

This comment has been minimized.

coveralls commented Oct 16, 2016

Coverage Status

Coverage increased (+4.8%) to 87.74% when pulling 5eb74b3 on MarcCote:enh_viz_renderer into c09a194 on nipy:master.

else:
self.style = interactor_style
self.iren = vtk.vtkRenderWindowInteractor()
self.iren.SetRenderWindow(self.window)

This comment has been minimized.

@Garyfallidis

Garyfallidis Oct 31, 2016

Member

Okay this removed section here will need to be replaced by one of our ui dialogues. But it should be called from a button not from a keyword. In that way you have more freedom to type and edit labels on the fly. This will be something to do when splitting PR #1111

This comment has been minimized.

@MarcCote

MarcCote Oct 31, 2016

Contributor

Agreed

Parameters
----------
events : str

This comment has been minimized.

@Garyfallidis

Garyfallidis Oct 31, 2016

Member

Is this a string or a list of strings?

This comment has been minimized.

@MarcCote

MarcCote Oct 31, 2016

Contributor

This is one string containing multiples events, one per line as stated in the description of that argument.

# Default interactor is responsible for moving the camera.
self.default_interactor = vtk.vtkInteractorStyleTrackballCamera()
# The picker allows us to know which object/actor is under the mouse.
self.picker = vtk.vtkPropPicker()

This comment has been minimized.

@Garyfallidis

Garyfallidis Oct 31, 2016

Member

Todo here is to add also a vtkcellpicker if the proppicker cannot pick individual voxels in 3D. To be checked.

This comment has been minimized.

@MarcCote

MarcCote Oct 31, 2016

Contributor

The interactor shouldn't be the one that picks individual voxels. The interactor is responsible for picking actor2D/3D. The way I see it, picking voxels should be handled in a callback function associated to the event LeftButtonPress. So, when the user clicks somewhere on a 3D slicer, the interactor sees that as picking the "3D volume" actor then calls the associated callback function in which we have a vtkcellpicker that uses the event position to retrieve the voxel that was picked..

Also, if I remember vtkcellpicker wasn't supported on some computers maybe because of wrong vtk version or not having a dedicated graphic.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Nov 1, 2016

Okay this PR looks good on my side. I will wait for a couple of days for others to comment and merge it. In the meantime @ranveeraggarwal and @MarcCote start breaking up #1111 into smaller PRs. GGT!!! :)

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Nov 2, 2016

Okay thanks @MarcCote :)

@Garyfallidis Garyfallidis merged commit 0548479 into nipy:master Nov 2, 2016

4 checks passed

codecov/patch 86.60% of diff hit (target 80.84%)
Details
codecov/project 85.30% (+4.45%) compared to c09a194
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+4.8%) to 87.74%
Details

@MarcCote MarcCote deleted the MarcCote:enh_viz_renderer branch Nov 2, 2016

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