For future reference.

On Ubuntu 18.04 g++9

povray bogus.pov Radiosity_File_Name="RadSamples" +rfo +rfi

coredumps unless the RadSamples file exists. The file can be empty
and things are fine but it must exist. We certainly should not
crash, and in my view given how +rfo always appends to any existing 
file, the user should not have to change the rfi flag use based on 
whether the file exists or not.

Bill P.

Post a reply to this message

I agree, if I understand this correctly (extrapolating your comments to the v3.8
Windows version.)

The particular 'development version' I'm using that piggybacks on top of v3.7.0
does not accept the +rfo +rfi commands for some reason; but if I instead use
only these two .INI commands...

Radiosity_File_Name= *whatever*
Radiosity_From_File=on

.....it results in a failed render. Not an outright crash, but annoying. Of
course, it does not make much sense anyway to try reading data from a
non-existant(?) file. (I learned that the hard way, ha.) But if the LINUX builds
are actually crashing, that's not good!

Perhaps a fix would be for POV-ray to internally create a small 'blank' file on
start-up, with a 'dummy' name that is then overridden by Radiosity_File_Name?

[I hope I'm not misunderstanding your comments.]

Post a reply to this message

hi,

William F Pokorny <ano### [at] anonymousorg> wrote:
> For future reference.
>
> On Ubuntu 18.04 g++9
>
> povray bogus.pov Radiosity_File_Name="RadSamples" +rfo +rfi
>
> coredumps unless the RadSamples file exists. The file can be empty
> and things are fine but it must exist. ...

and not just under Ubuntu, same for Slackware (alpha.10064268).  :-(  of
interest perhaps, I found a scene which segfaults even when 'RadSamples' exists.
 also, the file, so far, remains empty for scenes tried.


regards, jr.

Post a reply to this message

On 12/7/20 3:42 PM, jr wrote:
> hi,
> 
> William F Pokorny <ano### [at] anonymousorg> wrote:
>> For future reference.
>>
>> On Ubuntu 18.04 g++9
>>
>> povray bogus.pov Radiosity_File_Name="RadSamples" +rfo +rfi
>>
>> coredumps unless the RadSamples file exists. The file can be empty
>> and things are fine but it must exist. ...
> 
> and not just under Ubuntu, same for Slackware (alpha.10064268).  :-(  of
> interest perhaps, I found a scene which segfaults even when 'RadSamples' exists.
>   also, the file, so far, remains empty for scenes tried.
> 
> regards, jr.
> 
Thanks for testing.

On remaining empty. My only guess is perhaps there is no radiosity set 
up in the scene? It "might" be with certain scenes default settings do 
nothing.

I'm no radiosity expert, but I think in playing yesterday I picked up 
there is some sensitivity to scene scale. Meaning any given collection 
of radiosity settings might work better with a scene covering a range of 
+-10000 than one within a range of +-2 (1). Maybe I'm getting fooled / 
fooling myself - more work necessary to be sure.

If you have time to post more on the segfault when RadSamples exists, it 
might be useful when I look at fixing this for povr. In any case, I'll 
keep what you posted in mind.

Bill P.

Post a reply to this message

On 12/7/20 1:33 PM, Kenneth wrote:
> I agree, if I understand this correctly (extrapolating your comments to the v3.8
> Windows version.)
...
> [I hope I'm not misunderstanding your comments.]
> 
Thanks for the information. You understand - as much as I myself so. 
Which is perhaps not saying all that much... ;-)

Bill P.

Post a reply to this message

hi,

William F Pokorny <ano### [at] anonymousorg> wrote:
> On 12/7/20 3:42 PM, jr wrote:
> > William F Pokorny <ano### [at] anonymousorg> wrote:
> >> For future reference. ...
> > and not just under Ubuntu, same for Slackware (alpha.10064268).  :-(  of
> > interest perhaps, I found a scene which segfaults even when 'RadSamples' exists.
> >   also, the file, so far, remains empty for scenes tried.
>
> On remaining empty. My only guess is perhaps there is no radiosity set
> up in the scene? It "might" be with certain scenes default settings do
> nothing.

>
> I'm no radiosity expert, but I think in playing yesterday I picked up
> there is some sensitivity to scene scale. Meaning any given collection
> of radiosity settings might work better with a scene covering a range of
> +-10000 than one within a range of +-2 (1). Maybe I'm getting fooled /
> fooling myself - more work necessary to be sure.

haven't much time at present to play/explore, on to the todo list.

> If you have time to post more on the segfault when RadSamples exists, it
> might be useful when I look at fixing this for povr. In any case, I'll
> keep what you posted in mind.

I've a 'tube.inc' and 'tube_000.pov' (a 'nixie tube' implementation), posted
apparently by user "Thomas", the datestamps say 'May 19th 2020' (though likely
to be date the copy was created); cannot remember whether downloaded from here
or found elsewhere on the net, and searches so far w/out result.  can post a zip
(on the understanding not my code/no (copy)right/etc)


regards, jr.

Post a reply to this message

On 12/7/20 12:16 PM, William F Pokorny wrote:
> For future reference.
> 
> On Ubuntu 18.04 g++9
> 
> povray bogus.pov Radiosity_File_Name="RadSamples" +rfo +rfi
> 
> coredumps unless the RadSamples file exists. The file can be empty
> and things are fine but it must exist.
...
> 

This is a problem in source/core/lighting/radiosity.cpp. In the function 
RadiosityCache::Load change:

     if (fd != nullptr)
     {

to

     if (*fd)  // Natural is (fd != nullptr), but IStream has overrides
     {         // of !fd and *fd for testing.

Bill P.

Post a reply to this message

Le 06/02/2021 à 21:11, William F Pokorny a écrit :
> On 12/7/20 12:16 PM, William F Pokorny wrote:
>> For future reference.
>>
>> On Ubuntu 18.04 g++9
>>
>> povray bogus.pov Radiosity_File_Name="RadSamples" +rfo +rfi
>>
>> coredumps unless the RadSamples file exists. The file can be empty
>> and things are fine but it must exist.
> ...
>>
> 
> This is a problem in source/core/lighting/radiosity.cpp. In the function
> RadiosityCache::Load change:
> 
>     if (fd != nullptr)
>     {
> 
> to
> 
>     if (*fd)  // Natural is (fd != nullptr), but IStream has overrides
>     {         // of !fd and *fd for testing.
> 
> Bill P.

Beware of leakage when opening fails.

I wonder if the change should not be a two-level check:
1. is the object != nullptr (as previous) ( if (fd != nullptr) )
2. is the read/open not in fail state ( if (*fd) )

The "delete fd;" being outside the second block.

The first block handle memory failure (or not, "new" would throw, and
that is not catched locally).

So actually, I think your change of the test is correct, BUT the "delete
fd;" must be moved outside the block too.

Post a reply to this message

On 2/7/21 7:34 AM, Le_Forgeron wrote:
> Le 06/02/2021 à 21:11, William F Pokorny a écrit :
...
> 
> Beware of leakage when opening fails.
> 
> I wonder if the change should not be a two-level check:
> 1. is the object != nullptr (as previous) ( if (fd != nullptr) )
> 2. is the read/open not in fail state ( if (*fd) )
> 
> The "delete fd;" being outside the second block.
> 
> The first block handle memory failure (or not, "new" would throw, and
> that is not catched locally).
> 
> So actually, I think your change of the test is correct, BUT the "delete
> fd;" must be moved outside the block too.
> 

Thank you! :-)

Just checked a delete of a null pointer is safe and it appears it is. To 
confirm, what I now have is:

IStream* fd = NewIStream(inputFile, POV_File_Data_RCA);
if (*fd)
{
...
}
delete fd;

Bill P.

Post a reply to this message