Page MenuHomePhabricator

PlanarSubdivisionPolygon is not well integrated
Closed, ResolvedPublic

Description

  1. It cannot be serialized, because the following line is missing from mitkPlanarFigureSubclassesSerializer.cpp:

MITK_REGISTER_PF_SUB_SERIALIZER(PlanarSubdivisionPolygon);

  1. The icon is missing (QtWidgetsExt)

I took liberty to create one (see attachment) - please feel free to use it.

Event Timeline

Ellipse icon in SVG and PNG

Do you really use PlanarFigureSubdivisionPolygon? I noticed a few days ago that we do not use it at all and thought about removing it.

I actually use it for the cases where manual segmentation of 2D contour of a vessel is needed. Probably I could use the recently added Bezier curve planar figure for the same purpose.

I will try that out and write a comment about the outcome here.

The PlanarBezierCurve doesn't support closed figures at the moment. I guess you need this feature? No worries, if it doesn't substitute PlanarSubdivisionPolygon, we do not remove it.

I am not able to pull your bug branch. Only "master" and "vs2013-compilation" are accessible. I am not a github nor a git expert but I guess you didn't set the upstream right for your bug branches?

However, if you agree, I will just use your icons you uploaded here as I already have a branch that integrates PlanarEllipse and PlanarSubdivisionPolygon completely (your branch is missing additional icons and code for measurement toolbox integration). My branch also implements measurement feature evaluation for PlanarEllipse. All what is missing are the two icons for MitkQtWidgetsExt. Of course I will explicitly name you as the creator of these two icons.

What do you think?

Yes, that is completely OK. You probably look at a different user (I have two forks under "khlebnikov" and "rkhlebnikov" - there was a reason for that, but I will soon delete the fork under "khlebnikov"). So if you look at the "rkhlebnikov" fork - the branch should be there. Plus, I'm pretty sure that you can merge the pull request (as opposed to a branch): https://help.github.com/articles/merging-a-pull-request

In any case, it's great the the changes will be there (even such small ones :))

Also, yes, I need the closed curve, so for now it would be nice to have the subdivision polygon functionality.

Thanks!

[18e0ed]: Merge branch 'bug-17623-IntegratePlanarEllipseAndSubdivisionPolygon'

Merged commits:

2014-04-22 14:11:04 Stefan Kislinskiy [cdbe78]
Merge branch 'bug-17623-PlanarSubdivisionPolygon-is-not-well-integrated' of https://github.com/rkhlebnikov/MITK into bug-17623-IntegratePlanarEllipseAndSubdivisionPolygon

Conflicts:
Modules/PlanarFigure/IO/mitkPlanarFigureSubclassesSerializer.cpp
Modules/QtWidgetsExt/QmitkExt.qrc


2014-04-22 12:16:31 Stefan Kislinskiy [5c832d]
Integrated PlanarEllipse and PlanarSubdivisionPolygon into measurement toolbox.


2014-04-17 16:47:04 Rostislav Khlebnikov [9a4e44]
Adding ellipse & subdivision polygon node descriptors


2014-04-17 14:55:51 Rostislav Khlebnikov [c0bb79]
Adding signed-off

Signed-off-by: Rostislav Khlebnikov <r.khlebnikov@gmail.com>


2014-04-17 14:54:13 Rostislav Khlebnikov [d886bd]
Added a serializer for PlanarSubdivisionPolygon and added icons for Ellipse and SubdivisionPolygon

You are right, I added the wrong remote. Now it works. Thank you for your contribution! :)

FYI, QmitkExt.qrc was removed a few weeks ago. All resources of MitkQtWidgetsExt are now inside the resources folder and their resource string start with ":/QtWidgetsExt/" instead of ":/QmitkExt/". You might have to adjust your local code if you are using resources of MitkQtWidgetsExt.