0000266: v0.5 last fixes: bug in portal sampling in sky.cpp

(0000732)
radiance (reporter)
2008-06-22 07:45

best check in infinite.cpp and infinitesample.cpp lightsources if there's similar code.

good luck ! :)

Radiance

(0000733)
Dade (developer)
2008-06-22 10:50

Sky and infinite should be fine now however infinitesample has no support for portals at all (?).

I'm adding the missing support for portal in sky/infinte method:

// TODO - radiance - add portal implementation
SWCSpectrum SkyLight::Sample_L(const Scene *scene,
float u1, float u2, float u3, float u4,
Ray *ray, float *pdf) const

It is a basic requirement for photonmapping.

(0000734)
Dade (developer)
2008-06-22 11:03

Mmmm, still experiencing some random crash with portals. Investigating.

(0000735)
radiance (reporter)
2008-06-22 11:36

you might want to test infinitesample also, it's never been used since our changes since pbrt...

Greetz,
Radiance

(0000736)
Dade (developer)
2008-06-22 12:15

I'm unable to reproduce the problem at the moment.

(0000737)
Dade (developer)
2008-06-22 12:17

Radiance, infinitesample has no support for portals (I don't even know what kind of light sources it is (?)).

(0000738)
Dade (developer)
2008-06-22 13:40

I have found the damn problem:

        if(nrPortalShapes > 1)
            shapeidx = Floor2Int(u3 * nrPortalShapes);

this code can produce shapeidx=nrPortalShapes when u3 is exactly 1.0f and it leads to a crash (indeed it was quite rare).

Fixed with:
        if(nrPortalShapes > 1)
            shapeidx = min<float>(nrPortalShapes - 1,
                    Floor2Int(u3 * nrPortalShapes));

(0000741)
radiance (reporter)
2008-06-22 20:57

infinitesample is one of those undocumented (in the book) additional to pbrt, like exphotonmap.

i'm not %100 sure what's it needed for, but it does importance sampling based on luminance of the HDR map.

It's basically only for the light -> scene sampling part (for particle rays), such as bidir and photonmapping, so i guess it came with exphotonmap.

Radiance

PS: I'll see if i can test tonight after work, have you checked if portals still behave ok ?

(0000743)
jeanphi (administrator)
2008-06-23 10:14

infinite sample differs in that it sample according to map intensity, so if you have very bright spots in the map, it will sample those more (think the classic map of a cathedral interior for example). infinite light on the contrary samples uniformly and so has much more variance.

Jeanphi

(0000747)
Dade (developer)
2008-06-23 18:25

Closing this bug, It has been fixed. If we want to add portal support to infinitesample we can open a new task.

Notes
(0000732) radiance (reporter) 2008-06-22 07:45	best check in infinite.cpp and infinitesample.cpp lightsources if there's similar code. good luck ! :) Radiance

(0000733) Dade (developer) 2008-06-22 10:50	Sky and infinite should be fine now however infinitesample has no support for portals at all (?). I'm adding the missing support for portal in sky/infinte method: // TODO - radiance - add portal implementation SWCSpectrum SkyLight::Sample_L(const Scene scene, float u1, float u2, float u3, float u4, Ray ray, float *pdf) const It is a basic requirement for photonmapping.

(0000734) Dade (developer) 2008-06-22 11:03	Mmmm, still experiencing some random crash with portals. Investigating.

(0000735) radiance (reporter) 2008-06-22 11:36	you might want to test infinitesample also, it's never been used since our changes since pbrt... Greetz, Radiance

(0000736) Dade (developer) 2008-06-22 12:15	I'm unable to reproduce the problem at the moment.

(0000737) Dade (developer) 2008-06-22 12:17	Radiance, infinitesample has no support for portals (I don't even know what kind of light sources it is (?)).

(0000738) Dade (developer) 2008-06-22 13:40	I have found the damn problem: if(nrPortalShapes > 1) shapeidx = Floor2Int(u3 * nrPortalShapes); this code can produce shapeidx=nrPortalShapes when u3 is exactly 1.0f and it leads to a crash (indeed it was quite rare). Fixed with: if(nrPortalShapes > 1) shapeidx = min<float>(nrPortalShapes - 1, Floor2Int(u3 * nrPortalShapes));

(0000741) radiance (reporter) 2008-06-22 20:57	infinitesample is one of those undocumented (in the book) additional to pbrt, like exphotonmap. i'm not %100 sure what's it needed for, but it does importance sampling based on luminance of the HDR map. It's basically only for the light -> scene sampling part (for particle rays), such as bidir and photonmapping, so i guess it came with exphotonmap. Radiance PS: I'll see if i can test tonight after work, have you checked if portals still behave ok ?

(0000743) jeanphi (administrator) 2008-06-23 10:14	infinite sample differs in that it sample according to map intensity, so if you have very bright spots in the map, it will sample those more (think the classic map of a cathedral interior for example). infinite light on the contrary samples uniformly and so has much more variance. Jeanphi

(0000747) Dade (developer) 2008-06-23 18:25	Closing this bug, It has been fixed. If we want to add portal support to infinitesample we can open a new task.

Issue History
Date Modified	Username	Field	Change
2008-06-22 07:45	radiance	New Issue
2008-06-22 07:45	radiance	Status	new => assigned
2008-06-22 07:45	radiance	Assigned To	=> Dade
2008-06-22 07:45	radiance	Note Added: 0000732
2008-06-22 10:50	Dade	Note Added: 0000733
2008-06-22 11:03	Dade	Note Added: 0000734
2008-06-22 11:36	radiance	Note Added: 0000735
2008-06-22 12:15	Dade	Note Added: 0000736
2008-06-22 12:15	Dade	Status	assigned => feedback
2008-06-22 12:17	Dade	Note Added: 0000737
2008-06-22 13:40	Dade	Note Added: 0000738
2008-06-22 20:57	radiance	Note Added: 0000741
2008-06-22 20:59	radiance	Status	feedback => assigned
2008-06-23 10:14	jeanphi	Note Added: 0000743
2008-06-23 18:25	Dade	Status	assigned => resolved
2008-06-23 18:25	Dade	Resolution	open => fixed
2008-06-23 18:25	Dade	Note Added: 0000747
2009-11-25 00:29	jeanphi	Status	resolved => closed

Relationships