 |
|
|
|
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Looking through the code further, I see the following piece of fun in
media.cpp around line 580:
>>>
if (sampleCount < 2)
sampleCount = 2;
/* set static sample count */
sampCount_s = sampleCount - 1;
if (sampleCount < 2)
{
<<<
Note that the second sampleCount<2 test can never succeed. I assume this
second test and the associated block of code are optimised away by the
compiler, but does this not give a warning? Clearly one of the tests
should not be there: either the first because it stops the
three-samples-only special case code running, or the second because it
is superfluous. Can some explain a bit further please?
A bit lower down, line 635, I see:
>>>
/* do some attenuation, too, since we are doing samples in order */
Media_Interval[i].te[0]+=Result[0] *
exp(-Media_Interval[i].od[0]/sampleCount);
/* should this be sampleCount+1? */
Media_Interval[i].te[1]+=Result[1] *
exp(-Media_Interval[i].od[1]/sampleCount);
Media_Interval[i].te[2]+=Result[2] *
exp(-Media_Interval[i].od[2]/sampleCount);
<<<
Note particularly the comment. Though I haven't finished studying the
media code yet, I strongly suspect that it should not be sampleCount+1,
because although we are doing sampleCount+1 samples in the interval,
there are only sampleCount gaps. What I don't yet get, though, is that
Media_Interval[i].od[] holds the cumulative optical density - it is
added to on each sample by the ODResult variable coming from the
sample_media_rec call. If Media_Interval[i].od[] holds the entire
optical density so far along this interval, why are we dividing it by
the number of gaps? Surely that would just attenuate by the average
optical density? Would it not be better to not divide it and attenuate
by the whole optical density? If it needs to be a per-sample thing, why
not use the optical density from this sample, held in ODResult?
Perhaps my further exploration will help, but if anyone who knows about
this is out there perhaps they would consider tossing a hint my way.
Thanks,
Daniel
--
A most peculiar man With the windows closed And Mrs Reardon says
He died last Saturday So he'd never wake up He has a brother somewhere
He turned on the gas To his silent world Who should be notified soon
And he went to sleep And his tiny room .oO( http://sad.istic.org/ )
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Well, I guess I would be the author of that ugly code. ;-)
Looking at the code, it looks like I first wrote a special case for anything
with the sample count less than two (the second "if"). Then later I decided
to consolidate the code, so I wrote the first "if", which in effect converts
the special case into a normal case. This makes code maintenance easier,
because it generalizes all cases into one and removes a whole block of code.
However, then I never removed that block of code. At least that's my guess
as to what I was thinking... that was a long time ago and I think I did most
of that coding at 2:00am.
Regarding the attenuation... I, unfortunately, didn't know much about the
attenuation algorithm when I wrote that code. And, to be honest, I still
don't know much about it. My goal was to try to attenuate the light on a
per-sample basis, with each sample using the optical depth accumulated from
earlier samples (which we should be able to do because we're gathering the
samples in sequential order). I think your last comment, about attenuating
by ODResult may be correct. Another possibility is that we should be
dividing by "j", which is the number of samples gathered so far in the loop.
Looking at it all these years later, the only think I know for sure is that
this is ugly code in high need of refactoring and better commenting!
Maybe it would be worthwhile to create some POV scenes that would test this.
You can try different algorithms to see which produces the correct result.
If/when you manage to find the correct algorithm, please post your
recommendations here so that we can incorporate the fix into the official
version of POV. Thanks!
-Nathan Kopp
"Daniel Hulme" <pho### [at] istic org> wrote...
> Looking through the code further, I see the following piece of fun in
> media.cpp around line 580:
> >>>
> if (sampleCount < 2)
> sampleCount = 2;
>
> /* set static sample count */
> sampCount_s = sampleCount - 1;
>
> if (sampleCount < 2)
> {
> <<<
>
> Note that the second sampleCount<2 test can never succeed. I assume this
> second test and the associated block of code are optimised away by the
> compiler, but does this not give a warning? Clearly one of the tests
> should not be there: either the first because it stops the
> three-samples-only special case code running, or the second because it
> is superfluous. Can some explain a bit further please?
>
> A bit lower down, line 635, I see:
> >>>
> /* do some attenuation, too, since we are doing samples in order */
> Media_Interval[i].te[0]+=Result[0] *
> exp(-Media_Interval[i].od[0]/sampleCount);
> /* should this be sampleCount+1? */
> Media_Interval[i].te[1]+=Result[1] *
> exp(-Media_Interval[i].od[1]/sampleCount);
> Media_Interval[i].te[2]+=Result[2] *
> exp(-Media_Interval[i].od[2]/sampleCount);
> <<<
> Note particularly the comment. Though I haven't finished studying the
> media code yet, I strongly suspect that it should not be sampleCount+1,
> because although we are doing sampleCount+1 samples in the interval,
> there are only sampleCount gaps. What I don't yet get, though, is that
> Media_Interval[i].od[] holds the cumulative optical density - it is
> added to on each sample by the ODResult variable coming from the
> sample_media_rec call. If Media_Interval[i].od[] holds the entire
> optical density so far along this interval, why are we dividing it by
> the number of gaps? Surely that would just attenuate by the average
> optical density? Would it not be better to not divide it and attenuate
> by the whole optical density? If it needs to be a per-sample thing, why
> not use the optical density from this sample, held in ODResult?
>
> Perhaps my further exploration will help, but if anyone who knows about
> this is out there perhaps they would consider tossing a hint my way.
>
> Thanks,
> Daniel
>
> --
> A most peculiar man With the windows closed And Mrs Reardon says
> He died last Saturday So he'd never wake up He has a brother somewhere
> He turned on the gas To his silent world Who should be notified soon
> And he went to sleep And his tiny room .oO( http://sad.istic.org/ )
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
Nathan Kopp wrote:
> Looking at it all these years later, the only think I know for sure is that
> this is ugly code in high need of refactoring and better commenting!
>
Actually, I had a look at the media code some time back when experimenting
with media-based clouds and got the same impression. After having patched it
so that half of these "dozens of function arguments" were stuffed into one data
structure, I begun to understand what it was doing...
> Maybe it would be worthwhile to create some POV scenes that would test this.
> You can try different algorithms to see which produces the correct result.
> If/when you manage to find the correct algorithm, please post your
> recommendations here so that we can incorporate the fix into the official
> version of POV. Thanks!
>
...beause I wanted to implement a different integration algorithm.
What is the mathematical reason to use MCI (Monte-Carlo integration) for
sampling along the ray? For smooth density functions, some normal
adaptive numerical integration algorithm (e.g. adaptive simpson rule) should
give results comparable to MCI with much less evaluation points, shouldn't it?
Wolfgang Wieser
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
On Sun, 17 Oct 2004 11:25:02 +0200, Wolfgang Wieser wrote:
> Nathan Kopp wrote:
>
>> Looking at it all these years later, the only think I know for sure is
>> that this is ugly code in high need of refactoring and better
>> commenting!
>>
> Actually, I had a look at the media code some time back when
> experimenting with media-based clouds and got the same impression. After
> having patched it so that half of these "dozens of function arguments"
> were stuffed into one data structure, I begun to understand what it was
> doing...
>
While we are complaining about media... I was also trying to make some
modifications to the media code to permit varying the lists of media that
are considered at run-time (for example, to use an bounding hierarchy to
limit the media that need to be sampled). However, to do this would
require changing tons of the code that has explicit dependence on the
"array of linked list" of media objects, making it nearly impossible to
make the change without in depth knowledge of how all the media code is
working. If this code is rewritten, it would be SO much easier if this
were better abstracted (use of an object to represent media lists). From
what I can tell, same goes for a lot of the other code in media (or
POV-Ray for that matter, at least the bounding and object related stuff
I've looked at)
Andrew
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
In article <pan### [at] uwaterlooca> , Andrew Clinton
<ajc### [at] uwaterlooca> wrote:
> While we are complaining about media... I was also trying to make some
> modifications to the media code to permit varying the lists of media that
> are considered at run-time (for example, to use an bounding hierarchy to
> limit the media that need to be sampled). However, to do this would
> require changing tons of the code that has explicit dependence on the
> "array of linked list" of media objects, making it nearly impossible to
> make the change without in depth knowledge of how all the media code is
> working. If this code is rewritten, it would be SO much easier if this
> were better abstracted (use of an object to represent media lists). From
> what I can tell, same goes for a lot of the other code in media (or
> POV-Ray for that matter, at least the bounding and object related stuff
> I've looked at)
Well, all this is no news at all! - Obviously there is a reason why POV-Ray
4.0 is going to be a rewrite... :-)
Thorsten
____________________________________________________
Thorsten Froehlich, Duisburg, Germany
e-mail: tho### [at] trfde
Visit POV-Ray on the web: http://mac.povray.org
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
> Well, all this is no news at all! - Obviously there is a reason why POV-Ray
> 4.0 is going to be a rewrite... :-)
...because it will take longer to do it that way? :-)
Andrew.
*runs away, very fast*
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
> Well, all this is no news at all! - Obviously there is a reason why
> POV-Ray 4.0 is going to be a rewrite... :-)
>
> Thorsten
>
>
True :) But it has been no less than 2 or 3 years since the POV-Team has
announced the plan for a full rewrite. During that time, I have only seen
2 really good discussions of potential design considerations for the new
version of POV-Ray, each around 2 years ago, with some strong resistance
to new ideas from many members of the POV-Team (Yes, I have been
intermittently following the groups for that long!).
I am hoping that when the time comes for the rewrite, the POV-Team will
open up discussions to the greater community so that we (perhaps me as
well) could also provide some input. This might give me a greater
confidence that this mythical rewrite actually is going to come to pass
:-)
Andrew
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
In article <pan### [at] uwaterlooca> , Andrew Clinton
<ajc### [at] uwaterlooca> wrote:
> True :) But it has been no less than 2 or 3 years since the POV-Team has
> announced the plan for a full rewrite. During that time, I have only seen
> 2 really good discussions of potential design considerations for the new
> version of POV-Ray, each around 2 years ago, with some strong resistance
> to new ideas from many members of the POV-Team (Yes, I have been
> intermittently following the groups for that long!).
Then you fundamentally misunderstood the point we always made: We will not
discuss the design in public, and we do not endorse design discussions in
public either. Every discussion about design ideas is just ignored,
regardless of what is being discussed. As such, there is simply no judgment
at all made on any idea discussed in public.
The list of reasons to not endorse public design discussions is almost
endless, so whatever reasons you can think of why we would not do it are
probably on that list. Oh, and if somebody cannot think of any reason on
that list, well, that would also be on the list! ;-)
Thorsten
____________________________________________________
Thorsten Froehlich
e-mail: mac### [at] povrayorg
I am a member of the POV-Ray Team.
Visit POV-Ray on the web: http://mac.povray.org
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
In article <pan### [at] uwaterlooca> , Andrew Clinton
<ajc### [at] uwaterlooca> wrote:
> Date: Sun, 17 Oct 2004 15:40:52 -0500
BTW, could you please fix your time or timezone setting? Thanks!
Thorsten
____________________________________________________
Thorsten Froehlich, Duisburg, Germany
e-mail: tho### [at] trfde
Visit POV-Ray on the web: http://mac.povray.org
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
| |
|
|
On Sun, 17 Oct 2004 23:11:15 +0200, Thorsten Froehlich wrote:
>
> Then you fundamentally misunderstood the point we always made: We will
> not discuss the design in public, and we do not endorse design
> discussions in public either. Every discussion about design ideas is
> just ignored, regardless of what is being discussed. As such, there is
> simply no judgment at all made on any idea discussed in public.
>
I'm really sorry to hear that. This opinion seems so unusual for an open
source project that I'm wondering if all the team members feel the same
way. Anyway, good luck with the redesign for the next version of POV-Ray
(in a vacuum), and I hope that you have the skills, resources, and
enthusiasm to pull it off. I'll look forward to see what you have created
at the end, whenever that may be.
Andrew
Post a reply to this message
|
|
| |
| |
|
|
|
|
| |
|
|
