Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HW5 Solution #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

otmanesabir
Copy link

No description provided.

// --- PUT YOUR CODE HERE ---
return Vec2f::all(0.5f);
{
return Vec2f::all(Random::U<float>());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here both components will be initialized with the same random number. U() method will be called only once.

{
size_t n = sqrt(this->getNumSamples());
// get i, j from row major format
float i = (s % getNumSamples()) + 0.5;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In such case I will be equal to 0 all the time, you need to use n here

return Vec2f::all(0.5f);
auto rd = Random::U<float>();
size_t n = sqrt(this->getNumSamples());
float i = s % getNumSamples();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same error is here

size_t n = sqrt(this->getNumSamples());
float i = s % getNumSamples();
float j = (float)s / n;
return Vec2f((j + rd) / n, (i + rd) / n);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rd should be different

if (res2.has_value())
{
k++;
agg += res2.value() * m_glossiness;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is a mis-use go the glossiness effect. This implementation just makes glossiness to vanish, but the m_glossiness parameter should modulate how much the normal is deviated - thus, for m_glossiness close to 1, the samples should be concentrated on top of the hemisphere, whereas for small m_glossiness close to 0, the samples should be equally distributed over the hemisphere

float y = r * sinf(theta);
s = Vec2f(x, y);
float z = sqrtf(max(0.0f, 1.0f - s[0] * s[0] - s[1] * s[1]));
auto nS = Vec3f(s[0], s[1], z);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You derive nS, but never use it. It should be used instead of the normal. m_glossiness should also participate in the derivation of the nS. For example, for glossiness close to 1, you can divide x and y parameters by a large number, etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the code above is not in use

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants