last-svn-commit-540-gc4bb3f8 mln/io/dicom/load.hh: Fix invalid data while loading large image.
--- milena/ChangeLog | 5 +++++ milena/mln/io/dicom/load.hh | 32 +++++++++++++++++++------------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/milena/ChangeLog b/milena/ChangeLog index 77e5cc3..98f7f77 100644 --- a/milena/ChangeLog +++ b/milena/ChangeLog @@ -1,3 +1,8 @@ +2011-05-12 Guillaume Lazzara <z@lrde.epita.fr> + + * mln/io/dicom/load.hh: Fix invalid data while loading large + image. + 2010-12-10 Roland Levillain <roland@lrde.epita.fr> Ensure non implemented reconstructions on sets abort at run time. diff --git a/milena/mln/io/dicom/load.hh b/milena/mln/io/dicom/load.hh index f250a16..fb6996b 100644 --- a/milena/mln/io/dicom/load.hh +++ b/milena/mln/io/dicom/load.hh @@ -1,4 +1,5 @@ -// Copyright (C) 2009 EPITA Research and Development Laboratory (LRDE) +// Copyright (C) 2009, 2011 EPITA Research and Development Laboratory +// (LRDE) // // This file is part of Olena. // @@ -139,14 +140,21 @@ namespace mln algebra::vec<mln_site_(I)::dim, unsigned int> vmin; algebra::vec<mln_site_(I)::dim, unsigned int> vmax; algebra::vec<mln_site_(I)::dim, unsigned int> vdims; - for (int i = 0; i < ndims; ++i) + int j = ndims - 1; + for (int i = 0; i < ndims; ++i, --j) { vmin[i] = 0; vmax[i] = dims[i] - 1; if (i == 0) - vdims[i] = 1; + vdims[j] = 1; else - vdims[i] = dims[i - 1] * vdims[i - 1]; + vdims[j] = dims[i - 1] * vdims[j + 1]; + } + + if (ndims > 1) + { + std::swap(vmin[0], vmin[1]); + std::swap(vmax[0], vmax[1]); } mln_site(I) pmin(vmin); @@ -155,25 +163,23 @@ namespace mln initialize(ima, result); mln_piter(I) p(ima.domain()); unsigned int index = 0; + for_all(p) { index = 0; for (int i = 0; i < ndims; ++i) { - index += p.to_site().to_vec()[i] * vdims[i]; + index += p[i] * vdims[i]; } - ima(p) = (unsigned char) dataBuffer[(index * bytes_allocated) * samples_per_pixel]; + mln_value(I) v = (unsigned char) dataBuffer[(index * bytes_allocated) * samples_per_pixel]; // FIXME: RGB support, HighBit if HB == 1 - for (int j = 0; j < bytes_allocated; ++j) + for (unsigned int j = 0; j < bytes_allocated; ++j) { - ima(p) += ((unsigned char) dataBuffer[(index * bytes_allocated + j) * samples_per_pixel]) * 256 * j; + v += ((unsigned char) dataBuffer[(index * bytes_allocated + j) * samples_per_pixel]) * 256 * j; } - /*std::cout << "[ x = " << p.to_site().to_vec()[0] - << " | y = " << p.to_site().to_vec()[1] - << " | z = " << p.to_site().to_vec()[2] - << " ] => " << ima(p) - << std::endl;*/ + + ima(p) = v; } delete(dataBuffer); -- 1.5.6.5
Le 12 mai 2011 à 12:02, Guillaume Lazzara a écrit :
diff --git a/milena/ChangeLog b/milena/ChangeLog index 77e5cc3..98f7f77 100644 --- a/milena/ChangeLog +++ b/milena/ChangeLog @@ -1,3 +1,8 @@ +2011-05-12 Guillaume Lazzara <z@lrde.epita.fr> + + * mln/io/dicom/load.hh: Fix invalid data while loading large + image. + 2010-12-10 Roland Levillain <roland@lrde.epita.fr>
Ensure non implemented reconstructions on sets abort at run time. diff --git a/milena/mln/io/dicom/load.hh b/milena/mln/io/dicom/load.hh index f250a16..fb6996b 100644 --- a/milena/mln/io/dicom/load.hh +++ b/milena/mln/io/dicom/load.hh @@ -1,4 +1,5 @@ -// Copyright (C) 2009 EPITA Research and Development Laboratory (LRDE) +// Copyright (C) 2009, 2011 EPITA Research and Development Laboratory +// (LRDE) // // This file is part of Olena. // @@ -139,14 +140,21 @@ namespace mln algebra::vec<mln_site_(I)::dim, unsigned int> vmin; algebra::vec<mln_site_(I)::dim, unsigned int> vmax; algebra::vec<mln_site_(I)::dim, unsigned int> vdims; - for (int i = 0; i < ndims; ++i) + int j = ndims - 1; + for (int i = 0; i < ndims; ++i, --j) { vmin[i] = 0; vmax[i] = dims[i] - 1; if (i == 0) - vdims[i] = 1; + vdims[j] = 1; else - vdims[i] = dims[i - 1] * vdims[i - 1]; + vdims[j] = dims[i - 1] * vdims[j + 1]; + } + + if (ndims > 1) + { + std::swap(vmin[0], vmin[1]); + std::swap(vmax[0], vmax[1]); }
mln_site(I) pmin(vmin); @@ -155,25 +163,23 @@ namespace mln initialize(ima, result); mln_piter(I) p(ima.domain()); unsigned int index = 0; + for_all(p) { index = 0; for (int i = 0; i < ndims; ++i) { - index += p.to_site().to_vec()[i] * vdims[i]; + index += p[i] * vdims[i]; }
- ima(p) = (unsigned char) dataBuffer[(index * bytes_allocated) * samples_per_pixel]; + mln_value(I) v = (unsigned char) dataBuffer[(index * bytes_allocated) * samples_per_pixel]; // FIXME: RGB support, HighBit if HB == 1 - for (int j = 0; j < bytes_allocated; ++j) + for (unsigned int j = 0; j < bytes_allocated; ++j) { - ima(p) += ((unsigned char) dataBuffer[(index * bytes_allocated + j) * samples_per_pixel]) * 256 * j; + v += ((unsigned char) dataBuffer[(index * bytes_allocated + j) * samples_per_pixel]) * 256 * j; } - /*std::cout << "[ x = " << p.to_site().to_vec()[0] - << " | y = " << p.to_site().to_vec()[1] - << " | z = " << p.to_site().to_vec()[2] - << " ] => " << ima(p) - << std::endl;*/ + + ima(p) = v; }
delete(dataBuffer);
I didn't have the time to dive into the details of this patch, but I'm still interested in an explanation of its purpose (just out of curiosity). BTW corrections such as this one deserve a test case IMHO---all the more because it is I/O related, a place where bugs may be hard to spot. I do not recommend adding large input data to the distribution for test cases here; instead I propose such data be generated during tests.
participants (2)
-
Guillaume Lazzara -
Roland Levillain