POV-Ray : Newsgroups : povray.unofficial.patches : Possible bug in Radiance HDR output code Server Time
28 Apr 2024 17:02:27 EDT (-0400)
  Possible bug in Radiance HDR output code (Message 1 to 8 of 8)  
From: Ray Gardener
Subject: Possible bug in Radiance HDR output code
Date: 3 Feb 2006 18:10:41
Message: <43e3e2f1$1@news.povray.org>
Line 393 of module hdr.cpp rev 1.16, MegaPOV 1.2,
function void HDR_Image::Write_Line(COLOUR *line_data):

    if (width < MINELEN | width > MAXELEN)

The '|' symbol might need to be '||'.

Ray Gardener
Daylon Graphics Ltd.


Post a reply to this message

From: Ray Gardener
Subject: Re: Possible bug in Radiance HDR output code
Date: 3 Feb 2006 18:58:56
Message: <43e3ee40$1@news.povray.org>
Oh, ditto for line 717, same file, function
void Read_HDR_Image(IMAGE *Image, char *filename)

     if ((width < MINELEN) | (width > MAXELEN))

Ray


Post a reply to this message

From: Christoph Hormann
Subject: Re: Possible bug in Radiance HDR output code
Date: 4 Feb 2006 04:40:03
Message: <ds1sp6$jhp$1@chho.imagico.de>
Ray Gardener wrote:
> Line 393 of module hdr.cpp rev 1.16, MegaPOV 1.2,
> function void HDR_Image::Write_Line(COLOUR *line_data):
> 
>    if (width < MINELEN | width > MAXELEN)
> 
> The '|' symbol might need to be '||'.
> 

This making what difference for both arguments 0 or 1?

Note BTW that the original Radiance code does exactly the same.

Christoph

-- 
POV-Ray tutorials, include files, Landscape of the week:
http://www.imagico.de/ (Last updated 31 Oct. 2005)
MegaPOV with mechanics simulation: http://megapov.inetart.net/


Post a reply to this message

From: Warp
Subject: Re: Possible bug in Radiance HDR output code
Date: 4 Feb 2006 04:55:22
Message: <43e47a0a@news.povray.org>
Christoph Hormann <chr### [at] gmxde> wrote:
> Note BTW that the original Radiance code does exactly the same.

  That doesn't mean it's correct or good coding. ;)

-- 
                                                          - Warp


Post a reply to this message

From: Ray Gardener
Subject: Re: Possible bug in Radiance HDR output code
Date: 4 Feb 2006 17:33:21
Message: <43e52bb1$1@news.povray.org>
Christoph Hormann wrote:
> Ray Gardener wrote:
>> Line 393 of module hdr.cpp rev 1.16, MegaPOV 1.2,
>> function void HDR_Image::Write_Line(COLOUR *line_data):
>>
>>    if (width < MINELEN | width > MAXELEN)
>>
>> The '|' symbol might need to be '||'.
>>
> 
> This making what difference for both arguments 0 or 1?
> 
> Note BTW that the original Radiance code does exactly the same.

It's actually a good example of code that might work most or all of the 
time, but it reduces readability because it's not a standard way to code 
a logical OR expression. The second version has parantheses around the 
inequality tests, so I suspect the original author ran into trouble and 
'fixed' that line by forcing the operator precedence, but '||' would 
have been the better choice.

Ray


Post a reply to this message

From: Warp
Subject: Re: Possible bug in Radiance HDR output code
Date: 4 Feb 2006 18:36:15
Message: <43e53a6e@news.povray.org>
Ray Gardener <ray### [at] daylongraphicscom> wrote:
> but '||' would have been the better choice.

  Or, since we are using C++, just 'or'.

  (Stroustrup himself doesn't seem to consider 'or' (and the other
similar keywords) to be a more recommendable way of doing a comparison
for some reason. He mentions in his book that these keywords were
added "only" for those who can't write '|' in their system... go figure.
  However, personally I think 'or' is in every way much better than the
more cryptic '||'.)

-- 
                                                          - Warp


Post a reply to this message

From: Christoph Hormann
Subject: Re: Possible bug in Radiance HDR output code
Date: 4 Feb 2006 19:45:09
Message: <ds3hko$c75$1@chho.imagico.de>
Ray Gardener wrote:
>>
>> This making what difference for both arguments 0 or 1?
>>
>> Note BTW that the original Radiance code does exactly the same.
> 
> 
> It's actually a good example of code that might work most or all of the 
> time, but it reduces readability because it's not a standard way to code 
> a logical OR expression.

Well - all i wanted to say is that there isn't any urgency in making 
such a change since it does not in any way affect program operation.

Christoph

-- 
POV-Ray tutorials, include files, Landscape of the week:
http://www.imagico.de/ (Last updated 31 Oct. 2005)
MegaPOV with mechanics simulation: http://megapov.inetart.net/


Post a reply to this message

From: Ray Gardener
Subject: Re: Possible bug in Radiance HDR output code
Date: 4 Feb 2006 20:49:21
Message: <43e559a1$1@news.povray.org>
Christoph Hormann wrote:
> 
> Well - all i wanted to say is that there isn't any urgency in making 
> such a change since it does not in any way affect program operation.

Oh, certainly. I'm just reporting things as long as I'm spotting 
something out of the ordinary anyway.

Oh, it's on line 506 too. Just to be safe I did a testapp and yes, 
there's no danger with the original code; it's only a readability issue. 
To some extent open source projects have a higher emphasis on 
readability, since they're intended for public consumption, but of 
course it's the maintainer's call as to how to follow up.

MSVC issues this, interestingly enough:

warning C4554: '|' : check operator precedence for possible error; use 
parentheses to clarify precedence

So if someone had a treat-warnings-as-errors compiler setting, the build 
would fail, but that's not the same as a runtime issue for sure. And I 
imagine those doing a build would probably know enough coding or their 
compiler settings to resolve the issue locally. It depends on what build 
experience a provider is aiming for; totally turnkey or 
some-assembly-required. I imagine given MegaPOV's nature of 
experimentation it's more the latter; official POV-Ray might lean to the 
the former.

Ray


Post a reply to this message

Copyright 2003-2023 Persistence of Vision Raytracer Pty. Ltd.