Closed npcottaris closed 9 years ago
Yes, that is the expected behavior.
There are three interacting actors: radiance, reflectance and illumination. The structures are organized so that radiance is set and illumination is set and reflectance is derived.
Thus, when you change the illuminant the radiance is unchanged but the reflectance is changed. When you change the radiance, the illuminant is unchanged but the reflectance changes.
I don't think we should alter this behavior at this time. The reason is that much of the data we get is the scene radiance. We know that. We often just make up the il luminance and reflectance. So, I prefer to keep the scene radiance as the quantity we address directly.
On the other hand, I would be fine with having little functions or set operations that did what you want implemented in the core operators. So, sceneSet(scene, 'illuminant preserve reflectance' param) is a good operator.
Brian
Stanford Sent from mobile device
On Oct 11, 2014, at 11:18 AM, Nicolas Cottaris notifications@github.com wrote:
Use code below to re-create and visualize this issue.
% Generate Macbeth scene with D65 illuminant fluorescentScene = sceneCreate('macbethfluorescent'); illuminantPhotons = sceneGet(fluorescentScene, 'illuminantPhotons'); peakRadiance = sceneGet(fluorescentScene, 'peakRadiance'); photonRadianceMap = sceneGet(fluorescentScene,'photons'); wavelengthSampling = sceneGet(fluorescentScene, 'wave');
figure(11); clf; subplot(3,3,1); hold on; plot(wavelengthSampling, illuminantPhotons, 'r-'); plot(wavelengthSampling, peakRadiance, 'k-'); legend('illuminant photons', 'peak radiance'); set(gca, 'FontSize', 12); title('fluorescent macbeth');
wavelengthSubSamplingInterval = 2; subplot(3,3, [2 3]); plotRadianceMap(photonRadianceMap, wavelengthSampling, wavelengthSubSamplingInterval, 'Radiance') set(gca, 'FontSize', 12);
% Generate Macbeth scene with D65 illuminant d65Scene = sceneCreate('macbethd65'); illuminantPhotons2 = sceneGet(d65Scene, 'illuminantPhotons'); peakRadiance2 = sceneGet(d65Scene, 'peakRadiance'); photonRadianceMap2 = sceneGet(d65Scene,'photons'); wavelengthSampling2 = sceneGet(d65Scene, 'wave');
subplot(3,3,4); hold on; plot(wavelengthSampling2, illuminantPhotons2, 'r-'); plot(wavelengthSampling2, peakRadiance2, 'k-'); legend('illuminant photons', 'peak radiance'); set(gca, 'FontSize', 12); title('D65 macbeth');
subplot(3,3, [5 6]); plotRadianceMap(photonRadianceMap2, wavelengthSampling2, wavelengthSubSamplingInterval, 'Radiance') set(gca, 'FontSize', 12);
% Change illuminant in macbeth d65 scene fluorescentIllum = sceneGet(fluorescentScene, 'illuminant'); d65Scene = sceneSet(d65Scene, 'illuminant', fluorescentIllum); illuminantPhotons3 = sceneGet(d65Scene, 'illuminantPhotons'); peakRadiance3 = sceneGet(d65Scene, 'peakRadiance'); photonRadianceMap3 = sceneGet(d65Scene,'photons'); wavelengthSampling3 = sceneGet(d65Scene, 'wave');
subplot(3,3,7); hold on; plot(wavelengthSampling3, illuminantPhotons3, 'r-'); plot(wavelengthSampling3, peakRadiance3, 'k-'); legend('illuminant photons', 'peak radiance'); title(sprintf('D65 macbeth re-illuminated \nwith fluorescent illuminant')); set(gca, 'FontSize', 12);
subplot(3,3, [8 9]); plotRadianceMap(photonRadianceMap3, wavelengthSampling3, wavelengthSubSamplingInterval, 'Radiance') set(gca, 'FontSize', 12); — Reply to this email directly or view it on GitHub.
I should have added that there is a function, sceneAdjustIlluminant(), that changes the illuminant and preserves the scene reflectance (and changes the scene radiance).
We could call that function from sceneSet(scene,'illuminant preserve reflectance',illuminant). It might be clearer to do all this via a sceneSet(). Not sure what you all might think.
Hello Brian,
Thank you for the quick response.
I think it is a little counterintuitive that the system's default behavior to an illuminant change is a change in scene reflectance vs a change in scene radiance. I can see this being problematic with new users of ISETBIO. We should definitely augment sceneSet() with the option of changing the scene radiance in response to an illuminant change.
Moreover, there is another problem with the system's current behavior. When the command
rgbImage = sceneGet(scene,'rgb image');
is issued after a change in the scene illuminant, the rendered image does not reflect this change. The returned rgbImage is identical to the one returned before the illuminant change, which seems to also originate in the fact that the radiance of the image is not changed.
Best, Nicolas
Yes, it is the same cause as you said. The rgb is derived from the radiance, which is the correct behavior IMHO.
I also agree that for many users this will not be intuitive. The issue is that we do not normally have reflectance. For example, a display monitor has no meaningful reflectance.
Brian
Sent from a mobile device
On Oct 11, 2014, at 1:38 PM, Nicolas Cottaris notifications@github.com wrote:
Hello Brian,
Thank you for the quick response.
I think it is a little counter intuitive that the system's default behavior to an illuminant change is a change in scene reflectance vs a change in scene radiance. I can see this being problematic with new users of ISETBIO. We should definitely augment sceneSet() with the option of changing the scene radiance in response to an illuminant change.
Moreover, there is another problem with the system's current behavior. When the command
rgbImage = sceneGet(scene,'rgb image');
is issued after a change in the scene illuminant, the rendered image does not reflect this change. The returned rgbImage is identical to the one returned before the illuminant change, which seems to also originate in the fact that the radiance of the image is not changed.
Best, Nicolas
— Reply to this email directly or view it on GitHub.
One way to solve this issue (for the time being) is to always recompute the radiance when the RGB image is requested. Same when the illuminant is set. If we had a full/fledged object oriented matlab implementation of the scene object, we would make the radiance and RGBimage dependent properties so that they would be computed when accessed. In that way all params would remain automatically in sync. Should we start thinking about a full/fledged OOP implementation?
Please let me know if you want me to go ahead with these temporary fixes, or if you would rather not change the system right now.
Best, Nicolas
I think the rgb is always computed when you request it. The rgb depends on the radiance. If the radiance changes, so will the rgb.
Changing the illuminant does not change the radiance. It changes the reflectance. That's what you don't lie and find counterintuitive. I understand your feeling about this. But I remain reluctant to change the architecture so that reflectance, not radiance, is central.
Brian
Stanford Sent from mobile device
On Oct 11, 2014, at 2:19 PM, Nicolas Cottaris notifications@github.com wrote:
One way to solve this issue (for the time being) is to always recompute the radiance when the RGB image is requested. Same when the illuminant is set. If we had a full/fledged object oriented matlab implementation of the scene object, we would make the radiance and RGBimage dependent properties so that they would be computed when accessed. In that way all params would remain automatically in sync. Should we start thinking about a full/fledged OOP implementation?
Please let me know if you want me to go ahead with these temporary fixes, or if you would rather not change the system right now.
Best, Nicolas
— Reply to this email directly or view it on GitHub.
I believe this issue is now settled and that we agree that the current behavior is fine. Closing the issue.
Use code below to re-create and visualize this issue.