fo-dicom / fo-dicom

Fellow Oak DICOM for .NET, .NET Core, Universal Windows, Android, iOS, Mono and Unity
https://fo-dicom.github.io/stable/v5/index.html
Other
1.06k stars 640 forks source link

Private tag is listed out or order. #535

Closed BobSter3000 closed 7 years ago

BobSter3000 commented 7 years ago

Actual behavior

The private tag (0019,1153) is listed in the DicomDataSet before tag (0019,1006).

Steps to reproduce the behavior

Use the DicomDump utility and open the attached image. Tag (0019,1153) is listed out of order. Version 2.x the DicomDump utility displays the correct order. Wireshark shows the tag sent out of order when sending the image with DicomClient.

fo-dicom version and OS/platform

3.1 alpha also tested in 3.0.2 with the same result. Windows 10/Desktop

00000001.zip

pjlammertyn commented 7 years ago

Adding orderby to the foreach statement in the BuildWalkQueue function of the DicomDatasetWalker class seems to do the trick: dataset.OrderBy(di => di.Tag.Group).ThenBy(di => di.Tag.Element)

private static void BuildWalkQueue(IEnumerable<DicomItem> dataset, Queue<DicomItem> items)
{
    foreach (var item in dataset.OrderBy(di => di.Tag.Group).ThenBy(di => di.Tag.Element))
    {
        if (item is DicomElement)
        {
            items.Enqueue(item);
        }
        else if (item is DicomFragmentSequence)
        {
            var sq = item as DicomFragmentSequence;
            items.Enqueue(item);
            foreach (var fragment in sq)
            {
                items.Enqueue(new DicomFragmentItem(fragment));
            }
            items.Enqueue(new EndDicomFragment());
        }
        else if (item is DicomSequence)
        {
            var sq = item as DicomSequence;
            items.Enqueue(item);
            foreach (var sqi in sq)
            {
                items.Enqueue(new BeginDicomSequenceItem(sqi));
                BuildWalkQueue(sqi, items);
                items.Enqueue(new EndDicomSequenceItem());
            }
            items.Enqueue(new EndDicomSequence());
        }
    }
}
anders9ustafsson commented 7 years ago

The issue here is due to the bug fix #351, solving issue #319. In the bug fix, the DicomTag.CompareTo was modified to avoid comparing the high byte of the tag element for private tags.

The file that highlights the error in this issue contains an (incorrectly numbered?) tag (0019,1153). There is no private creator associated with 11xx, only 10xx (HOLOLOGIC INC.).

The bug fix here should solve the tag sorting problem so that tags are sorted in strict group, element, creator order, and maintain the ability to identify private tags on the low byte only in the tag element (#319 / #351).