[Insight-users] PNGwriter writes wrong scaling information

[Bill Lorensen]

That will be great.

Happy New Tear,

Bill

On Thu, Dec 20, 2012 at 5:26 PM, Dženan Zukić <dzenanz at gmail.com> wrote:
> I do, or can at least make one. I am on vacation until 2nd Jan, but then I
> can supply one.
>
> On Dec 20, 2012 5:06 PM, "Bill Lorensen" <bill.lorensen at gmail.com> wrote:
>>
>> I think your gerrit patch goes too far. pHYs measure pixel size/aspect
>> ration. sCAL is the proper parameter, but as you pointed out, ITK is
>> not writing out a correct sCAL unit type. Since ITK does not have a
>> concept of unit, I think we just need to repair the output image to
>> specify a valid unit type, which probably should be meters rather than
>> radians.
>>
>> I'll modify the patch if you don't mind.
>>
>> Also, would you have a small png file that has a valid sCAL component?
>>
>> Bill
>> On Tue, Dec 11, 2012 at 10:53 AM, Dženan Zukić <dzenanz at gmail.com> wrote:
>> > It looks like Jim Miller introduced invalid unit usage in fix
>> > 15b9450b61249261d098506d9ff878744768025a from 2003-02-19.
>> >
>> > PNG spec defines meters and radians as allowed units in sCAL chunk, vs
>> > unknown and meters in pHYs chunk. Hence the warning I have been seeing
>> > with
>> > OpenCV and NConvert.
>> >
>> >
>> > On Tue, Dec 11, 2012 at 4:03 PM, Dženan Zukić <dzenanz at gmail.com> wrote:
>> >>
>> >> Current behavior is to ignore units, but it does not handle unit
>> >> completely correctly (only handles the extension sCAL for metadata, but
>> >> not
>> >> the standard pHYs metadata).
>> >>
>> >> Extending current way would transform 2000 pixels/meter (pixel size 0.5
>> >> mm) into spacing of 0.0005.
>> >>
>> >>
>> >> On Tue, Dec 11, 2012 at 3:50 PM, Bradley Lowekamp
>> >> <blowekamp at mail.nih.gov>
>> >> wrote:
>> >>>
>> >>>
>> >>> Having very small or very large spacing can have big impact on the
>> >>> numerical stability of derivatives and evolving differential equations
>> >>> such
>> >>> as level-sets or diffusion.
>> >>>
>> >>> I think the least surprising thing to the users would be to just keep
>> >>> it
>> >>> the same as to avoid adverse numerical effects. Also this would not
>> >>> change
>> >>> the current behavior, correct?
>> >>>
>> >>> Brad
>> >>>
>> >>> On Dec 11, 2012, at 9:42 AM, Dženan Zukić <dzenanz at gmail.com> wrote:
>> >>>
>> >>> PNG has three following units: meters, radians, and unknown. Currently
>> >>> PNG reader ignores unit, and writes unknown units.
>> >>>
>> >>> I guess I Should keep writing unknown units. But when reading PNG with
>> >>> meter units (eg. 2000 pixels/meter), should I transform it into pixel
>> >>> spacing 0.5 (assuming customary millimeter) or into 0.0005?
>> >>>
>> >>>
>> >>> On Tue, Dec 11, 2012 at 3:36 PM, Bill Lorensen
>> >>> <bill.lorensen at gmail.com>
>> >>> wrote:
>> >>>>
>> >>>> No units. ITK considers units an application thing. You can add a
>> >>>> unit
>> >>>> type if the format supports it. It could help applications. For
>> >>>> example, most medical images use mm's.
>> >>>>
>> >>>> On Tue, Dec 11, 2012 at 8:25 AM, Dženan Zukić <dzenanz at gmail.com>
>> >>>> wrote:
>> >>>> > Is ITK still assuming no units, or millimeters are units for pixel
>> >>>> > spacing?
>> >>>> >
>> >>>> >
>> >>>> > On Mon, Dec 10, 2012 at 5:25 PM, Bill Lorensen
>> >>>> > <bill.lorensen at gmail.com>
>> >>>> > wrote:
>> >>>> >>
>> >>>> >> If you submit a gerrit patch, I'll write or modify a test.
>> >>>> >>
>> >>>> >> How will we know if the fix works unless we test it? How will we
>> >>>> >> know
>> >>>> >> if someone reintroduces the bug if we don't test it?
>> >>>> >>
>> >>>> >>
>> >>>> >> On Mon, Dec 10, 2012 at 10:41 AM, Dženan Zukić <dzenanz at gmail.com>
>> >>>> >> wrote:
>> >>>> >> > But this would be a bug fix, not a patch introducing new
>> >>>> >> > features.
>> >>>> >> > This
>> >>>> >> > should already be working, and ideally having a test.
>> >>>> >> >
>> >>>> >> > I know that many bugs don't get fixed, because they are not
>> >>>> >> > important
>> >>>> >> > enough
>> >>>> >> > and there is not enough people working on ITK. However a
>> >>>> >> > requirement to
>> >>>> >> > write a 20-30 line test for a few lines of fixed code is a great
>> >>>> >> > deterrence
>> >>>> >> > for me to contribute bug fixes (such as this one).
>> >>>> >> >
>> >>>> >> >
>> >>>> >> > On Mon, Dec 10, 2012 at 4:26 PM, Bill Lorensen
>> >>>> >> > <bill.lorensen at gmail.com>
>> >>>> >> > wrote:
>> >>>> >> >>
>> >>>> >> >> Looks like ti should be fixed. But if you submit a gerrit
>> >>>> >> >> patch,
>> >>>> >> >> make
>> >>>> >> >> sure oyu also supply a test or modify an existing test.
>> >>>> >> >>
>> >>>> >> >> The lack of a test is why your previous gerrit patch has not
>> >>>> >> >> been
>> >>>> >> >> approved.
>> >>>> >> >>
>> >>>> >> >> Bill
>> >>>> >> >>
>> >>>> >> >> On Mon, Dec 10, 2012 at 8:35 AM, Dženan Zukić
>> >>>> >> >> <dzenanz at gmail.com>
>> >>>> >> >> wrote:
>> >>>> >> >> > Hi everyone,
>> >>>> >> >> >
>> >>>> >> >> > I have noticed that ImageFileWriter, when writing png format,
>> >>>> >> >> > writes
>> >>>> >> >> > wrong
>> >>>> >> >> > pixel size information. It is not completely wrong, but it is
>> >>>> >> >> > not in
>> >>>> >> >> > accordance with png specification.
>> >>>> >> >> >
>> >>>> >> >> > When reading such files with OpenCV, I get following warning:
>> >>>> >> >> > libpng warning: Invalid sCAL ignored: invalid unit
>> >>>> >> >> > This is not just annoying and clutter-causing, but displaying
>> >>>> >> >> > it
>> >>>> >> >> > for
>> >>>> >> >> > every
>> >>>> >> >> > single image slows down my OpenCV processing.
>> >>>> >> >> >
>> >>>> >> >> > I used XnView to batch-edit the extracted slices and set some
>> >>>> >> >> > scaling
>> >>>> >> >> > which
>> >>>> >> >> > adheres to the specification to get rid of this warning.
>> >>>> >> >> >
>> >>>> >> >> > Now the question: should I correct this bug, or submit a bug
>> >>>> >> >> > report?
>> >>>> >> >> > My
>> >>>> >> >> > last
>> >>>> >> >> > bug-fixing ended non-gloriously :(
>> >>>> >> >> >
>> >>>> >> >> > _____________________________________
>> >>>> >> >> > Powered by www.kitware.com
>> >>>> >> >> >
>> >>>> >> >> > Visit other Kitware open-source projects at
>> >>>> >> >> > http://www.kitware.com/opensource/opensource.html
>> >>>> >> >> >
>> >>>> >> >> > Kitware offers ITK Training Courses, for more information
>> >>>> >> >> > visit:
>> >>>> >> >> > http://www.kitware.com/products/protraining.php
>> >>>> >> >> >
>> >>>> >> >> > Please keep messages on-topic and check the ITK FAQ at:
>> >>>> >> >> > http://www.itk.org/Wiki/ITK_FAQ
>> >>>> >> >> >
>> >>>> >> >> > Follow this link to subscribe/unsubscribe:
>> >>>> >> >> > http://www.itk.org/mailman/listinfo/insight-users
>> >>>> >> >> >
>> >>>> >> >>
>> >>>> >> >>
>> >>>> >> >>
>> >>>> >> >> --
>> >>>> >> >> Unpaid intern in BillsBasement at noware dot com
>> >>>> >> >
>> >>>> >> >
>> >>>> >>
>> >>>> >>
>> >>>> >>
>> >>>> >> --
>> >>>> >> Unpaid intern in BillsBasement at noware dot com
>> >>>> >
>> >>>> >
>> >>>>
>> >>>>
>> >>>>
>> >>>> --
>> >>>> Unpaid intern in BillsBasement at noware dot com
>> >>>
>> >>>
>> >>> _____________________________________
>> >>> Powered by www.kitware.com
>> >>>
>> >>> Visit other Kitware open-source projects at
>> >>> http://www.kitware.com/opensource/opensource.html
>> >>>
>> >>> Kitware offers ITK Training Courses, for more information visit:
>> >>> http://www.kitware.com/products/protraining.php
>> >>>
>> >>> Please keep messages on-topic and check the ITK FAQ at:
>> >>> http://www.itk.org/Wiki/ITK_FAQ
>> >>>
>> >>> Follow this link to subscribe/unsubscribe:
>> >>> http://www.itk.org/mailman/listinfo/insight-users
>> >>>
>> >>>
>> >>
>> >
>>
>>
>>
>> --
>> Unpaid intern in BillsBasement at noware dot com



-- 
Unpaid intern in BillsBasement at noware dot com