blender
/
blender
136
407
Fork 589
Code Issues 6.3k Pull Requests 768 Projects 19 Wiki Activity

Fix #103977: Cycles handling ray visibility inconsistent for forward and nee #107747

Open
Sebastian Herholz wants to merge 6 commits from sherholz/blender:light-visibility-fix into main
pull from: sherholz/blender:light-visibility-fix
merge into: blender:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 15 Commits 6 Files Changed 9 +15 -15
2 changed files with 15 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 7a4f223c47 - Show all commits

24
intern/cycles/kernel/light/visibility.h
View File

@ -11,20 +11,19 @@ CCL_NAMESPACE_BEGIN
ccl_device_inline SpectrumRGBE SpectrumToSpectrumRGBE(const Spectrum &spec)
{
SpectrumRGBE specRGBE;
char *specRGBEPtr = (char *)&specRGBE;
Sergey Sharybin commented 2023-05-09 09:14:37 +02:00
Review
Copy Link

Can we define SpectrumRGBE as

struct SpectrumRGBE {
  uint8_t r, g, b, e;
};

and avoid such memory casts, and casts between signed/unsigned char?

This also seems to a bit arbitrary place to have such utility functions. Within the current file structure it seems types_spectrum.h is the most fitted place for it.

Can we define `SpectrumRGBE ` as ``` struct SpectrumRGBE { uint8_t r, g, b, e; }; ``` and avoid such memory casts, and casts between signed/unsigned `char`? This also seems to a bit arbitrary place to have such utility functions. Within the current file structure it seems `types_spectrum.h` is the most fitted place for it.
Sebastian Herholz commented 2023-05-09 09:49:14 +02:00
Review
Copy Link

sure no problem.

sure no problem.
Sebastian Herholz commented 2023-05-09 10:05:31 +02:00
Review
Copy Link

@Sergey done.
I am not sure if you can compress the data even more.
The positive side on this approach is that we can realize handling scalar ray types light interactions straight forward instead of only binary ones.

@Sergey done. I am not sure if you can compress the data even more. The positive side on this approach is that we can realize handling scalar ray types light interactions straight forward instead of only binary ones.
Sebastian Herholz commented 2023-05-09 10:13:28 +02:00
Review
Copy Link

Another thing worth considering is moving the conversion methods to a different place (not into visibility.h).
Do you have a good suggestion for it?

Another thing worth considering is moving the conversion methods to a different place (not into `visibility.h`). Do you have a good suggestion for it?
Sergey Sharybin commented 2023-09-11 16:11:57 +02:00
Review
Copy Link

The immediate thought is kernel/util/color.h. There is already spectrum_to_rgb so intuitively feels like spectrum_to_spectrum_rgbe could fit there as well.

The immediate thought is `kernel/util/color.h`. There is already `spectrum_to_rgb` so intuitively feels like `spectrum_to_spectrum_rgbe` could fit there as well.
float v;
int e;
v = max(spec.x, max(spec.y, spec.z));
if (v < 1e-32f) {
specRGBEPtr[0] = specRGBEPtr[1] = specRGBEPtr[2] = specRGBEPtr[3] = 0;
specRGBE.r = specRGBE.g = specRGBE.b = specRGBE.e = 0;
}
else {
v = frexp(v, &e) * 256.0f / v;
specRGBEPtr[0] = (unsigned char)(spec.x * v);
specRGBEPtr[1] = (unsigned char)(spec.y * v);
specRGBEPtr[2] = (unsigned char)(spec.z * v);
specRGBEPtr[3] = (unsigned char)(e + 128);
specRGBE.r = (uint8_t)(spec.x * v);
specRGBE.g = (uint8_t)(spec.y * v);
specRGBE.b = (uint8_t)(spec.z * v);
specRGBE.e = (uint8_t)(e + 128);
}
return specRGBE;
}
@ -32,13 +31,12 @@ ccl_device_inline SpectrumRGBE SpectrumToSpectrumRGBE(const Spectrum &spec)
ccl_device_inline Spectrum SpectrumRGBEToSpectrum(const SpectrumRGBE &specRGBE)
{
Spectrum spec;
SpectrumRGBE specRGBE_ = specRGBE;
char *specRGBEPtr = (char *)&specRGBE_;
if (specRGBEPtr[3]) {
const float f = ldexp(1.0f, specRGBEPtr[3] - (int)(128 + 8));
spec.x = specRGBEPtr[0] * f;
spec.y = specRGBEPtr[1] * f;
spec.z = specRGBEPtr[2] * f;
if (specRGBE.e) {
const float f = ldexp(1.0f, specRGBE.e - (int)(128 + 8));
spec.x = specRGBE.r * f;
spec.y = specRGBE.g * f;
spec.z = specRGBE.b * f;
}
else {
spec.x = spec.y = spec.z = 0.0f;

6
intern/cycles/util/types_spectrum.h
View File

@ -13,11 +13,13 @@ CCL_NAMESPACE_BEGIN
#define SPECTRUM_CHANNELS 3
#define SPECTRUM_DATA_TYPE float3
#define PACKED_SPECTRUM_DATA_TYPE packed_float3
#define SPECTRUM_RGBE_DATATYPE int32_t
using Spectrum = SPECTRUM_DATA_TYPE;
using PackedSpectrum = PACKED_SPECTRUM_DATA_TYPE;
using SpectrumRGBE = SPECTRUM_RGBE_DATATYPE;
struct SpectrumRGBE {
uint8_t r, g, b, e;
};
#define make_spectrum(f) CONCAT(make_, SPECTRUM_DATA_TYPE(f))
#define load_spectrum(f) CONCAT(load_, SPECTRUM_DATA_TYPE(f))