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.

Sign up for GitHub

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

Jump to bottom

Tweak ctkVTKConnection - Fixes Slicer issue #3361 and also optimize connection creattion/removal #476

Merged
merged 13 commits into from
May 28, 2014
Merged

Tweak ctkVTKConnection - Fixes Slicer issue #3361 and also optimize connection creattion/removal #476

merged 13 commits into from
May 28, 2014

Commits on May 27, 2014

  1. Fix qDebug operator to handle case when VTKObject is null 

    In case the debug statement is used in the destructor, this commit
will prevent the application from crashing.
    @jcfr
    jcfr committed May 27, 2014
    Configuration menu
    Copy the full SHA
    ef27caf View commit details
    Browse the repository at this point in the history

  2. Improve documentation of setStrictTypeCheck() method

    @jcfr
    jcfr committed May 27, 2014
    Configuration menu
    Copy the full SHA
    0107f3b View commit details
    Browse the repository at this point in the history

  3. Remove unused signal "isBroke()"

    @jcfr
    jcfr committed May 27, 2014
    Configuration menu
    Copy the full SHA
    2ec6f79 View commit details
    Browse the repository at this point in the history

  4. Refactor ctkVTKConnectionTest1 for easier maintenance and readability

    @jcfr
    jcfr committed May 27, 2014
    Configuration menu
    Copy the full SHA
    5eff65e View commit details
    Browse the repository at this point in the history

  5. Update ctkVTKConnectionTest1 to use ctkCallback 

    By using ctkCallback instead of QTimer, we can associate
a function that increment a counter each it is invoked. This
allows to check that the Qt slot or VTK callback have been invoked.

This commit also compute the stats associated with the connection5.
    @jcfr
    jcfr committed May 27, 2014
    Configuration menu
    Copy the full SHA
    a35eedb View commit details
    Browse the repository at this point in the history

  6. Update ctkVTKConnectionTest1 to allow each test case to be run indepe… 

    …ndently

This is particularly useful when instrumenting the code and running
callgrind on each test case independently.
    @jcfr
    jcfr committed May 27, 2014
    Configuration menu
    Copy the full SHA
    a30b176 View commit details
    Browse the repository at this point in the history

  7. Split disconnect function

    @jcfr
    jcfr committed May 27, 2014
    Configuration menu
    Copy the full SHA
    947e53b View commit details
    Browse the repository at this point in the history

  8. Allow ctkVTKConnection pimpl to be subclassed 

    This can be useful for both providing a custom implementation and for testing.
    @jcfr
    jcfr committed May 27, 2014
    Configuration menu
    Copy the full SHA
    cd40307 View commit details
    Browse the repository at this point in the history

  9. Add method to return VTK object associated with ctkVTKConnection

    @jcfr
    jcfr committed May 27, 2014
    Configuration menu
    Copy the full SHA
    bcd6292 View commit details
    Browse the repository at this point in the history

  10. Add ctkVTKConnectionTestObjectDelete

    @jcfr
    jcfr committed May 27, 2014
    Configuration menu
    Copy the full SHA
    a3cab8f View commit details
    Browse the repository at this point in the history

  11. Fix ctkVTKConnectionTestObjectDelete 

    * Use vtkWeakPointer to track the VTK object. The observation of the
DeleteEvent is now required only to disconnect the Qt slots. Using a weak
pointer also ensure that the pointer to VTK object will be set to zero
with/without observing deletion.

* Simplify ctkVTKConnection: There is not need to explicitly disconnect
slots when object is deleted. Indeed, according to Qt documentation, "all
signals to and from the object are automatically disconnected" there is no
need to explicitly disconnect them."
[1] http://qt-project.org/doc/qt-4.8/qobject.html#dtor.QObject

By also removing the call to 'disconnectSlots' in the 'disconnect()' function,
it resolves Slicer issue #3361 [2] where it was reported "No such slot" error.
Since the disconnect happened in the connection destructor and the connection
were destroyed in the base class but the slot belonged to the derived class,
this explained the error message.

[2] http://www.na-mic.org/Bug/view.php?id=3361

* Simplify ctkVTKConnection: There is no need to explicit remove VTK
observers where connection is deleted. Indeed, observers of a VTK object
are automatically removed when an object is deleted.
    @jcfr
    jcfr committed May 27, 2014
    Configuration menu
    Copy the full SHA
    9e86635 View commit details
    Browse the repository at this point in the history

  12. Improve performance disabling "deletion" observation 

    Now the VTK pointer is set to zero independently of the DeleteEvent
observation, the observation is only needed to explicitly
disconnect the Qt slots. Since keeping the connected slots around doesn't
impact performance, this commit disabled the "deletion" observation.

If needed, a future additional improvement would be the pruning of
connection there are associated with deleted VTK object. This could be
done in the same loop taking care of shadow connection.

Results of ctkVTKObjectTest1:

[ObserveDeletion = true]
148: Create  10000  connections...
148:   elapsed time:  3.06955 seconds
148: Remove  10000  connections...
148:   elapsed time:  3.44588 seconds

[ObserveDeletion = false]
148: Create  10000  connections...
148:   elapsed time:  0.775543 seconds
148: Remove  10000  connections...
148:   elapsed time:  0.760212 seconds
    @jcfr
    jcfr committed May 27, 2014
    Configuration menu
    Copy the full SHA
    1d9648c View commit details
    Browse the repository at this point in the history

  13. Fix ctkVTKConnectionTest1 removing connection time ratio test 

    As discussed in [1], the test should check that the code works
as expected and not set a bar that we cannot yet attain.

[1] commontk#378 (comment)
    @jcfr
    jcfr committed May 27, 2014
    Configuration menu
    Copy the full SHA
    949d26c View commit details
    Browse the repository at this point in the history