Page MenuHomePhabricator

Deformation of the clipping planes does not work
Closed, ResolvedPublic

Description

The surface will be colorized if the mouse pointer is over the plane. But if you change the range of the deformation field with the mouse wheel, there is no reaction.

Moreover the deformation by itself doesn't work.

Event Timeline

New remote branch pushed: bug-15386-ClippingPlaneFixes

WheelEvent Problem

CoreChangeRequest:
The problem is that the mouseWheel event isn't passed to the renderer because of a wrong check in the mitkRenderWindowBase class. This checks if the renderer should handle a wheel event or not. At the moment the check does the opposite. If ProcessWheelEvent is true, make nothing, otherwise send the wheelEvent.

Code:
if (! m_ProcessWheelEvents )

{
  if(m_Renderer.IsNotNull())
    m_Renderer->WheelEvent(we);
  return;
}

Solution:
if ( m_ProcessWheelEvents )

{
  if(m_Renderer.IsNotNull())
    m_Renderer->WheelEvent(we);
  return;
}

The deformation doesn't work because of the "back-transformation" of the vector representing the mouse move changes (for the deformation) from world to index coordinates. After the transformation the vector coordinates are null. Saving the transformed vector in a new vector is the solution.

Jasmin, I'm not sure about this one. The flag is not documented and RenderWindowBase::wheelMitkEvent() is not clear to me. I guess you talked with Christian about this method because he pushed something? Would be a good chance to add some comments about the internal logic/intent of this method.

Ok, after a little research we found out that in T4165 Markus E. describes the intended use of the variable. In his commit he makes the right check of the variable in the QmitkRenderWindow class. The check in this bug scope is comitted by Matthias B. in T4735.

The whole method is a litte confusing:

void mitk::RenderWindowBase::wheelMitkEvent(mitk::WheelEvent *we)
{

if ( !m_ProcessWheelEvents )
{
  if(m_Renderer.IsNotNull())
     m_Renderer->WheelEvent(we);
  return;
}

}

I think this was a copy/paste bug. And it should be, somthing like that:

void mitk::RenderWindowBase::wheelMitkEvent(mitk::WheelEvent *we)
{

if ( !m_ProcessWheelEvents )
{
  return;
}

if(m_Renderer.IsNotNull())

{
  m_Renderer->WheelEvent(we);
}

}

But Christian has noted that the variable would be obsolent by the new concept.
Christian?

After looking more into the idea behind this variable my proposed solution is
to remove the variable altogether (and while we're at it, also remove the void SetInvertScrollingDirection) as they both became obsolete with the new interactions.

m_ProcessWheelEvents:
intends to provide the possibility to switch off scrolling through slices while
working e.g. on a fixed plane - this should/and can now be done by changing the behaviour/configuration of the DisplayInteractor, so it is more sensitive than completely ignoring all wheel events (which affects also all other interactors (of the old interaction))

SetInvertScrollingDirection:
is obsolete since, this should be done by configuring the DisplayInteractor.

New remote branch pushed: bug-15386-RemoveObsoleteInteractionLeftOvers

Looks good to me. The two variables are no longer needed and can be replaced by properties in the new config-objects.

Is the wheel-scrolling done by distinct interactor or by the move'n'Zoom Interactor or the Scroll-Interactor?

The interactor should be adapted accordingly in case the logic is not implemented yet...

The wheel scrolling is implemented now in the DisplayInteractor and can be adapted by providing a different configuration file/object.

To change the behavior during runtime refer to:

http://docs.mitk.org/nightly-qt4/InteractionMigration.html

->How to modify the display interactor behavior

which allows to modify all properties of the display interaction,
example ( /Core/Code/Resources/Interactions/DisplayConfig.xml )

this should cover all use cases that were also possible before.

Since Markus has no objections it's fine for me. Please check if the Qt free rendering/interaction example is still working after your changes!

[ccb00a]: Merge branch 'bug-15386-ClippingPlaneFixes'

Merged commits:

2013-06-24 11:52:35 Jasmin Metzger [f221f6]
Add new variable for the "back"transformation from world to index geometry.

[44ec17]: Merge branch 'bug-15386-RemoveObsoleteInteractionLeftOvers'

Merged commits:

2013-06-21 14:23:17 Christian Weber [dfdb8d]
Removed m_InvertScrollingDirection and m_ProcessWheelEvents as well as their setters,
as they have no use anymore in MITK - and their functionality was replaced by the interaction
redesign. The decision of wheel events are processed should not be part of the renderwindow.