Page MenuHomePhabricator

IO for 4D PointSets
Closed, ResolvedPublic

Description

4D PointSet is not saved in the scene viewer. Just the first timestep is in the file. See: g:\home\kast\Testscene4D\GravityCenter 4D fiducials.mps there should be 48 timesteps.

Related Objects

Event Timeline

This is a bug or feature of the PointSetWriter. For an overhaul of this class we should:

  • write out a file format version
  • respect the file format version read from a file, i.e. read old files and new ones alike
  • write and read each time step of a point set

Could be a party bug.

neuhaus added a subscriber: neuhaus.Sep 3 2008, 1:12 PM

party bug!

wegner added a subscriber: wegner.Sep 3 2008, 2:22 PM

assigning anne

wie ist denn der aktuelle Status?

(In reply to comment #4)

wie ist denn der aktuelle Status?

Hab den Writer schon implementiert, aber noch nicht ausreichend getestet. Mit dem Reader hab ich angefangen. Habs deswegen noch nicht comitted.

Party tag entfernt.

  • reader and writer are now expanded to 4D
  • test still fails, but only sometimes
  • when reading the same xml file (where the test failed) the same point-coordinate is read wrong everytime (same timestep, same point, same position) which means that the one or two numbers are cut off
  • we did not find a pattern why this specific point is read wrong (whether it is because of the size, position etc. )
  • we debugged the related classes and found out, that something doesn't work correctly in mitk::vtkPointSetXMLParser::CharacterDataHandler

expanded classes for saving a 4D pointset

  • expanded writer class works well. the resulting xml-sheet looks good.
  • tested the PointSetIOTest under Linux and Windows. As Result some numbers of the coordinates of the points are overwritten
  • find out that the bug has to be found in the reader class

Ingmar, you should know the most about this bug in the moment.
Please take it or reassign.

this must be fixed for 0.12 release.

Talked to Mathias who has a similar bug.
We will check how to proceed

The problem occurs on a rather low level during VTK XML parsing. For unknown
reasons, the parse string during reading 4D point sets is locally corrupted in
some rare cases, which results in a wrong coordinate component (e.g. the XML
file contains "<z>271</z>", which is for example interpreted as "<z>27</z>", or
"<z>0</z>".

Strangely, the parsing recovers after this and the remaining data is read
correctly.

There are several options how this could be handled:

1.) Try to find and fix the bug (might be difficult, could even be in VTK)

2.) Re-write the class, e.g. by taking the mitk::SubdivisionCurveReader class
as reference (which has a similar data structure), or by using an external XML
library such as tinyxml

3.) Use other existing data formats. I think VTK should support this (.vtp?),
not sure about time series handling

In my opinion this is not relevant for 0.12 release since there have been so
far (to my knowledge) no external users who have requested 4D pointset
reading/writing.

wegner added a comment.May 5 2009, 5:40 PM

removing it from target milestone 0.12

wegner added a comment.May 6 2009, 5:20 PM

removing keyword mandatory-0.12

what is the status of this bug? We still can't write 4D pointsets!

I think a new implementation of PointSet writer/reader, supporting 4D point sets, would be the best option, because the current code uses outdated VTK-based XML writing/parsing.

I'm not sure if this is relevant for 3M3, otherwise it would be a nice bug/feature-request for bug-squashing (for someone with some experience in writers/readers)

I agree. Party bug then?
3M3 relevant?

Bug originally processed by Anne Sauer. Still relevant (no Scene Viewer anymore, Status or Write Project?)

Well, what happens if you save a 4D pointset now from DataManager?
If only the 0th timestep is saved, this is quite critical.
Possible solution for 1-0 release:

  1. Fix. (Would be best)
  2. Or Issue an error when trying to save.

Fixing this bug can involve quite some work, especially because we need to remain compatible with the old point set format. I'd only do it if there is someone who needs this feature now and has the time to implement it in the next weeks.

nolden added a subscriber: nolden.Jan 20 2010, 4:58 PM

Ok, to clarify the issue. Release relevant (minimal requirement) is a correct error handling of this. Saving 4D data and finding out later that is was not saved properly would be a frustrating experience for users.

assigning to default

(In reply to comment #24)

assigning to default

Why?

Marco, please reassign to someone with open capabilities for the release.

@Mathias: Is this maybe related to T2439 or could both problems get fixed at once?

(In reply to comment #27)

@Mathias: Is this maybe related to T2439 or could both problems get fixed at
once?

My favorite solution to this bug would still be to write entirely new IO classes for point sets with time support. T2439 would then probably be no longer relevant. Anyway, we probably would used tinyxml instead of the VTK-XML-IO classes.

Is there someone with free capacities who could do this for 1.0?

We have reproduced the previously posted patch under the current version MITK. The error with mitkPointSetReader still occurs (when reading 4D pointsets, occassionally, one digit of the read coordinates gets lost). The mitkPointSetWriter seems to work fine.

The attached patch contains the original changes (with slight modifications/style changes) compatible with current MITK directory structure (r21462).

Remove hard-coded directory from PointSetFileIOTest patch

kast added a subscriber: kast.Feb 26 2010, 10:40 AM

I have time to reimplement the reader writer mechanism for 4D pointsets next week during the bug squashing...

(In reply to comment #31)

I have time to reimplement the reader writer mechanism for 4D pointsets next
week during the bug squashing...

Reimplementing the reader is one option, but we should first try once more to get the current implementation going. The bug occurs reproducibly. Also, only the reader seems to have a bug, the writer works fine.

If we reimplement Pointset IO, I would recommend to adapt an established file format. I know that 3D Slicer has one. I suppose that ITK/VTK have file formats for pointsets too (VTK stores points in a .vtp file that is also used for polydatas)

kast added a comment.Mar 2 2010, 11:07 AM

So we have this options:

  1. reimplement reader & writer with an common file format as supposed by Jochen
  2. reimplement only the reader
  3. find the bug in the reader

I think the fastest option is the second one.

If we use an existing file format, the most important requirement is that it supports time-resolved point sets :)

4D point set as produced by the mitk::PointSetWriter

kast added a comment.Mar 3 2010, 5:33 PM

Reimplementation with tinyXML, see attached Patch.
The Patch is not working yet.

Trying to solve this Bug

When you reimplement reader/writers, please do not forget that mitk::PointSet also has a PointData member for each point. It's content should also be read/written. (Another shortcoming of the old reader/writer!)

Please make sure, that the new reader is still able to read the old point set file format!

The bug should be resolved

The final versoin of the reworked Class file

  • is the new file format and the io classes documented somewhere? Please provide some documentation in Doxygen
  • what about Jochen's comment (#39)? Is this topic covered?
  • does the test case cover
    1. old file versions?
    2. the new 3D+t data?

Please comment and update the change request.

The Reader is sill able to read old Point Set files correct. That was tested by me manually. The Test does not cover this case, but i will implement that during the next Bug- Squashing Party. I will also write a documentation which contains all changes.

All changes are documented in Doxygen now. The Reader is sill able to read old Point Set files correct.

The Documentation

[SVN revision 22003]
FIX (#1571): Fixed PointSetReader as described in Bug

There are still some problems with the locale tests

[SVN revision 22086]
FIX (#1571): Fixed the Bug for reading old PointSets. Also described in T3487. Also fixed the locale settings, to make it possible to read decimal numbers by the american way

[SVN revision 22423]
ENH (#1571): extended the test for reading and writing floating point number.

Having the old format in the reference Pointset XML file was the reason that the Test failed. We changed the reference Pointset from the old format to the new Pointset format to check if the writer writes the correct Data.

[SVN revision 22557]
FIX (#1571): added pointSet.mps in the new format

[SVN revision 22711]
ENH (#1571): activated pointSetLocalTest again.

kislinsk merged a task: Restricted Maniphest Task.Aug 1 2016, 10:47 PM