Illum bug

Greg Ward wrote:

Aha! It's not a bug, it's a "feature!"

Let's call it a buggy feature, ok? :wink:

you saw it before in your IES sources because the source distribution
either didn't match the simulated source. It shows up well in this
example because your alternate material is a dark diffuse one rather
than something emitting.

There are light fixtures with the visual appearance of a dark
hole in the ceiling ("dark lights"). This is how I found the bug
in 1998 with distribution data applied, and the test case shows
the same effect without the data. I also had Rayfront users
complain because part of the impostor illums for their luminaires
became visible when they shouldn't (as in Robs example).

Both cases, a dark substitution material and void for transparency
are valid and necessary uses of the illum material, and there's no
other way to get the correct result in those situations. Well, as
it is now, there's no way to get the correct result at all.

I could eliminate illum's from the drawsources() calculations, but I
think the net result would be worse than what we currently have, as
illum sources then would be poorly sampled on the image plane when they
got to be small and/or thin.

I think that drawsources() (resp. the routine calling it) should
first check whether the target is not just an illum, but an illum
with a substitute material. If that is the case, then there's no
source to draw, hence no justification to invoke a shadow ray. The
possibility that the substitute material is again a luminous type
can probably be neglected, as it doesn't make any practical sense.

I didn't check in the sources how hard it will be to fix this,
but I consider the current behaviour to be fundamentally wrong.
Hey, this will give you a chance to brag at the workshop that you
just fixed "one of the oldest Radiance bugs ever found"... :wink:

-schorsch

路路路

--
Georg Mischler -- simulations developer -- schorsch at schorsch com
+schorsch.com+ -- lighting design tools -- http://www.schorsch.com/

Hi Schorsch,

I still disagree that this is really a bug. Unexpected behavior perhaps, but that's not the same thing. I'll try to explain my reasoning better, below.

From: Georg Mischler <[email protected]>
Date: Sun Sep 14, 2003 1:26:32 PM US/Pacific

Greg Ward wrote:

Aha! It's not a bug, it's a "feature!"

Let's call it a buggy feature, ok? :wink:

you saw it before in your IES sources because the source distribution
either didn't match the simulated source. It shows up well in this
example because your alternate material is a dark diffuse one rather
than something emitting.

There are light fixtures with the visual appearance of a dark
hole in the ceiling ("dark lights"). This is how I found the bug
in 1998 with distribution data applied, and the test case shows
the same effect without the data. I also had Rayfront users
complain because part of the impostor illums for their luminaires
became visible when they shouldn't (as in Robs example).

I don't believe this is triggering in any of Rob's renderings, as the lights are all large enough to be adequately sampled using standard sampling in rpict. He can check this by setting -ps 1 to see if anything changes.

Both cases, a dark substitution material and void for transparency
are valid and necessary uses of the illum material, and there's no
other way to get the correct result in those situations. Well, as
it is now, there's no way to get the correct result at all.

If the source distribution were correct, it would go to zero at low angles to match the "dark lights" you describe. If it doesn't, then the distribution data is in error. I have seen such an inconsistency with IES data, myself.

If you want to make sure you render the alternate material without invoking drawsources(), you can increase your sampling by increasing the resolution and/or setting -ps 1 so that all the sources in the image are adequately sampled. If you want to avoid drawsources() altogether, you can use vwrays with rtrace, instead. (So there is always a way to get around anything if you're determined to do it.)

I could eliminate illum's from the drawsources() calculations, but I
think the net result would be worse than what we currently have, as
illum sources then would be poorly sampled on the image plane when they
got to be small and/or thin.

I think that drawsources() (resp. the routine calling it) should
first check whether the target is not just an illum, but an illum
with a substitute material. If that is the case, then there's no
source to draw, hence no justification to invoke a shadow ray. The
possibility that the substitute material is again a luminous type
can probably be neglected, as it doesn't make any practical sense.

The reason for invoking the shadow ray is because the source is not adequately sampled on the image plane. This happens when the chances are 50-50 (or thereabouts) that an image sample will hit the source. If I remove the call to drawsources() for illum's, terrible aliasing will result, and this will be noteably worse than the current output in most cases. If the illum and its alternate are properly matched, then the current solution should be better than either doing nothing or sampling the alternate material instead of the illum, since we get the benefit of averaging.

The only time I really see this as being a problem is when someone is trying to "hide" certain light sources from view. This of course enters into the non-physical realm, where Radiance doesn't really compete with other renderers.

I didn't check in the sources how hard it will be to fix this,
but I consider the current behaviour to be fundamentally wrong.
Hey, this will give you a chance to brag at the workshop that you
just fixed "one of the oldest Radiance bugs ever found"... :wink:

We can discuss it further at the workshop if you like, but I'll hold off doing anything until then. I think we should agree first, and I don't feel like we're getting there with this exchange.

-Greg

Greg Ward wrote:

I don't believe this is triggering in any of Rob's renderings, as the
lights are all large enough to be adequately sampled using standard
sampling in rpict.

How do you explain the bright strips in this picture then?
聽聽http://www.rumblestrip.org/rad/illum2.png

Maybe there's an *additional* bug in the routines that determine
how big the sources will appear in the picture? The sources in my
test case are also big enough that I wouldn't expect any special
sampling to be necessary.

The reason for invoking the shadow ray is because the source is not
adequately sampled on the image plane. This happens when the chances
are 50-50 (or thereabouts) that an image sample will hit the source.

Allow me to repeat my central point:

聽聽If an illum has an alternate material, then there's no source to
聽聽sample for display in the picture, whether that sampling would
聽聽be theoretically adequate or not.

My proposed fix avoids exactly this, and prevents Radiance from
doing unnecessary work. The fix is straightforward, a patch is
appended for review. As far as I can tell, this patch has no
consequences whatsoever for the rest of the simulation. It's a
purely visual fix for a purely visual problem.

The only thing I'm not sure about is whether the test in
srcdraw.c is positioned correctly relative to the other tests
above and below it, you'll be able to decide about the optimal
sequence better than I can.

If I remove the call to drawsources() for illum's, terrible aliasing
will result, and this will be noteably worse than the current output in
most cases. If the illum and its alternate are properly matched,

I don't see what useful definition "properly matched" can have in
this context. Isn't the idea behind the alternate material that
the user may visually substitute *something else* for the light
source? Or in the case of impostor geometry, to have the "true"
light source visually disappear?

then
the current solution should be better than either doing nothing or
sampling the alternate material instead of the illum, since we get the
benefit of averaging.

The current implementation shows light sources where there are
supposed to be none visible. How can that be better than anything?

The only time I really see this as being a problem is when someone is
trying to "hide" certain light sources from view. This of course
enters into the non-physical realm, where Radiance doesn't really
compete with other renderers.

To the contrary. This is recommended and standard practise when
seperating light emission and geometry of a light source, either
by applying distribution data to an impostor geometry directly,
or by using mkillum. You personally recommended this approach to
Rob in the thread that started this discussion over in
radiance-general.

The following is an example that was sent to me by a Rayfront
user, and I was unable to offer him a solution at that time:
聽聽http://www.schorsch.com/stuff/rad/EdenInst_front_wall.jpg

This isn't anywhere in the non-physical realm. It's a massive
problem for people trying to get renderings that are both
physically *and* visually accurate, and who wish to do so within
reasonable time. Reducing -ps to 1 is at best a crude workaround
fighting the symptoms, and probably still not eliminating them in
all cases. The real cause of the problem can be easily fixed with
my patch.

We can discuss it further at the workshop if you like, but I'll hold
off doing anything until then. I think we should agree first, and I
don't feel like we're getting there with this exchange.

No need to rush anything, of course.

-schorsch

illumbug.diff (1.5 KB)

路路路

--
Georg Mischler -- simulations developer -- schorsch at schorsch com
+schorsch.com+ -- lighting design tools -- http://www.schorsch.com/

My response is inline, below.

From: Georg Mischler <[email protected]>
Date: Mon Sep 15, 2003 4:20:49 AM US/Pacific

Greg Ward wrote:

I don't believe this is triggering in any of Rob's renderings, as the
lights are all large enough to be adequately sampled using standard
sampling in rpict.

How do you explain the bright strips in this picture then?
聽聽http://www.rumblestrip.org/rad/illum2.png

Maybe there's an *additional* bug in the routines that determine
how big the sources will appear in the picture? The sources in my
test case are also big enough that I wouldn't expect any special
sampling to be necessary.

Well, indeed this is a bug, but not one I can easily fix. You see, it's pretty easy to figure out the projected area for polygons onto a perspective image, but gets quite tricky for fisheye views where straight lines don't come out straight, and the routines in srcdraw.c become somewhat crude in their approximations. I think the arrangement that Jack de Valpine suggested to Rob of a single polygon beneath each fixture is much better, anyway. (A bit of a copout, I admit.) Also, I think Rob was still been working on the coffer glow model, which wasn't nearly bright enough in this image.

The reason for invoking the shadow ray is because the source is not
adequately sampled on the image plane. This happens when the chances
are 50-50 (or thereabouts) that an image sample will hit the source.

Actually, I should correct this quote -- drawsources() kicks in when the sampling probability drops below 100% as it's trying to any problems with aliasing.

Allow me to repeat my central point:

聽聽If an illum has an alternate material, then there's no source to
聽聽sample for display in the picture, whether that sampling would
聽聽be theoretically adequate or not.

This is where we disagree. The surface, whatever material it's made of, be it glass, trans, or even plastic, has an illum attached to it because it is a concentrated source of illumination. That makes it a light source -- otherwise it should never have been assigned an illum in the first place. Sampling is therefore important because any errors in image sampling will result in very large variances in the filtered result.

My proposed fix avoids exactly this, and prevents Radiance from
doing unnecessary work. The fix is straightforward, a patch is
appended for review. As far as I can tell, this patch has no
consequences whatsoever for the rest of the simulation. It's a
purely visual fix for a purely visual problem.

The only thing I'm not sure about is whether the test in
srcdraw.c is positioned correctly relative to the other tests
above and below it, you'll be able to decide about the optimal
sequence better than I can.

If you turn off sampling of illum's in srcdraw.c, you will actually _create_ visual problems due to inadequate sampling of small illum's. How can I change something in good conscience when I believe it will make matters worse rather than better? Do I need to show you some example images to make my point?

If I remove the call to drawsources() for illum's, terrible aliasing
will result, and this will be noteably worse than the current output in
most cases. If the illum and its alternate are properly matched,

I don't see what useful definition "properly matched" can have in
this context. Isn't the idea behind the alternate material that
the user may visually substitute *something else* for the light
source? Or in the case of impostor geometry, to have the "true"
light source visually disappear?

No, that is not the purpose of illum's, to make the real light source disappear. The "something else" associated with the alternate material in an illum should have an output equivalent to the illum's output, on average. The point of the illum is to smooth this distribution and thus avoid high variance in the illumination calculation. If you are really using the illum to make invisible light sources, then you are in the non-physical realm that Radiance doesn't care about. (And by the way, the user would probably not specify an alternate material type in this case, as they would want nothing to replace the illum -- so your fix wouldn't do what you are saying is the right thing, either.)

then
the current solution should be better than either doing nothing or
sampling the alternate material instead of the illum, since we get the
benefit of averaging.

The current implementation shows light sources where there are
supposed to be none visible. How can that be better than anything?

Again, we disagree. The fact that there is an illum there means that something VERY bright should be visible. If it is not, then the illum is being used improperly. If there is, then sampling variance is a big issue, and the drawsources() routine can (and does) help.

The only time I really see this as being a problem is when someone is
trying to "hide" certain light sources from view. This of course
enters into the non-physical realm, where Radiance doesn't really
compete with other renderers.

To the contrary. This is recommended and standard practise when
seperating light emission and geometry of a light source, either
by applying distribution data to an impostor geometry directly,
or by using mkillum. You personally recommended this approach to
Rob in the thread that started this discussion over in
radiance-general.

You misunderstand me. The illum is not "hiding" the source -- it is substituting a smoother version of it. If the source then becomes too small to adequately sample in the view, the smoother version becomes a sensible substitute, as any small number of image samples is going to give a high variance result.

The following is an example that was sent to me by a Rayfront
user, and I was unable to offer him a solution at that time:
聽聽http://www.schorsch.com/stuff/rad/EdenInst_front_wall.jpg

This isn't anywhere in the non-physical realm. It's a massive
problem for people trying to get renderings that are both
physically *and* visually accurate, and who wish to do so within
reasonable time. Reducing -ps to 1 is at best a crude workaround
fighting the symptoms, and probably still not eliminating them in
all cases. The real cause of the problem can be easily fixed with
my patch.

Again, I don't think your patch would fix the problem if you restrict it to illum's with alternate materials. We'd have to avoid illum's in drawsources() altogether. (I can't figure out what's going on with your user's scene, or what it's supposed to look like -- there appear to be boxes and ovals overlapping, and light patches on the ceiling not associated with any sources.)

I agree that it takes a bit more effort to get a match between the illum and a detailed fixture it's replacing, especially going from an IES file. Mkillum usually does a reasonable job, so long as you set glow values sensibly and don't do anything else to the source model after you've run mkillum.

We can discuss it further at the workshop if you like, but I'll hold
off doing anything until then. I think we should agree first, and I
don't feel like we're getting there with this exchange.

No need to rush anything, of course.

These are complex issues. The best fix would be one that determined whether the illum was a good average of whatever it substitutes or not, which would be equivalent to running mkillum again. We could of course create a new option to turn drawsources on and off, and this might be the simplest compromise in the end.

-Greg