blender/cycles
8
15
Fork 10
Code Issues 2 Pull Requests 5 Releases Activity

Fixed bug with un-normalised ellipse area lights not having proper area calculation and erroneously reverting to quad lights. #4

Closed
Alex Fuller wants to merge 1 commits from boberfly/cycles:fixes/ellipse_unnormalize_bug into main
pull from: boberfly/cycles:fixes/ellipse_unnormalize_bug
merge into: blender:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 7 Commits 1 Files Changed 1 +2 -2
Alex Fuller commented 2023-05-27 09:11:49 +02:00
Contributor
Copy Link

Hi all, not too sure how this one got away but we found in Gaffer that un-normalised ellipse area lights (we have disk lights similar to USD Lux) came out as quad lights again. This calculation should re-establish the pi calculation as well as the negative sign for the kernel-side code to detect that it is an ellipse again.

Hi all, not too sure how this one got away but we found in Gaffer that un-normalised ellipse area lights (we have disk lights similar to USD Lux) came out as quad lights again. This calculation should re-establish the pi calculation as well as the negative sign for the kernel-side code to detect that it is an ellipse again.
Alex Fuller added 1 commit 2023-05-27 09:11:51 +02:00
Alex Fuller requested review from Brecht Van Lommel 2023-05-27 09:12:10 +02:00
Alex Fuller commented 2023-05-27 09:15:37 +02:00
Author
Contributor
Copy Link

I am not sure if area calculation should be -M_PI_4_F or just -1.0f for un-normalised ellipsed area lights though...

I am not sure if area calculation should be `-M_PI_4_F` or just `-1.0f` for un-normalised ellipsed area lights though...
Alex Fuller commented 2023-05-30 07:43:21 +02:00
Author
Contributor
Copy Link

Hmm something different but it's worth making a note of it, it seems like un-normalising of lights only works for specular reflection and not diffuse for some reason, for point and spot lights only.

https://github.com/GafferHQ/gaffer/pull/5239#issuecomment-1567538345

Looking into it, not sure exactly what's up here but it is probably in Cycles and probably an issue in the Hydra delegate also...

edit: some more hints, it looks like energy loss when increasing roughness on the spec lobe, so it isn't just diffuse.

Hmm something different but it's worth making a note of it, it seems like un-normalising of lights only works for specular reflection and not diffuse for some reason, for point and spot lights only. https://github.com/GafferHQ/gaffer/pull/5239#issuecomment-1567538345 Looking into it, not sure exactly what's up here but it is probably in Cycles and probably an issue in the Hydra delegate also... edit: some more hints, it looks like energy loss when increasing roughness on the spec lobe, so it isn't just diffuse.
Brecht Van Lommel commented 2023-05-30 14:24:13 +02:00
Owner
Copy Link

If it works different for low and high roughness, I would expect there to be a different in the two multiple importance sampling methods, light_sample and light_intersect.

In Blender, the Debug panel has an option to disable MIS and switch between the two, to verify if they give the same result.

@weizhen is doing work in this area currently, to get more correct and physically based light units.

If it works different for low and high roughness, I would expect there to be a different in the two multiple importance sampling methods, `light_sample` and `light_intersect`. In Blender, the Debug panel has an option to disable MIS and switch between the two, to verify if they give the same result. @weizhen is doing work in this area currently, to get more correct and physically based light units.
👍 1
Alex Fuller commented 2023-05-31 07:40:14 +02:00
Author
Contributor
Copy Link

I remember that point/spot lights are an approximate of an oriented disc, so I am not sure if that has something to do with things, so I'll re-link an old patch from @Stefan_Werner here:
https://archive.blender.org/developer/differential/0004/0004676/index.html

Not too sure if it might be "the time" to integrate this over the approximate in another PR for 4.0 (if at all). I will do a test of this soon.

I remember that point/spot lights are an approximate of an oriented disc, so I am not sure if that has something to do with things, so I'll re-link an old patch from @Stefan_Werner here: https://archive.blender.org/developer/differential/0004/0004676/index.html Not too sure if it might be "the time" to integrate this over the approximate in another PR for 4.0 (if at all). I will do a test of this soon.
Weizhen Huang commented 2023-06-01 11:52:18 +02:00
Member
Copy Link

Not too sure if it might be "the time" to integrate this over the approximate in another PR for 4.0 (if at all). I will do a test of this soon.

I am working on changing point light to sphere light #108506. How to deal with spot light I'm not sure yet.

> Not too sure if it might be "the time" to integrate this over the approximate in another PR for 4.0 (if at all). I will do a test of this soon. I am working on changing point light to sphere light [#108506](https://projects.blender.org/blender/blender/pulls/108506). How to deal with spot light I'm not sure yet.
👍 1
Alex Fuller commented 2023-06-04 08:52:38 +02:00
Author
Contributor
Copy Link

Hi @weizhen

That's great news! For spotlights, USDLux/Prman treats these exactly like sphere lights except it has "shaping" applied to it to be in the shape of a cone or an IES profile applied:
https://openusd.org/dev/api/class_usd_lux_shaping_a_p_i.html

If that helps you in figuring out how to deal with spot lights.

Hi @weizhen That's great news! For spotlights, USDLux/Prman treats these exactly like sphere lights except it has "shaping" applied to it to be in the shape of a cone or an IES profile applied: https://openusd.org/dev/api/class_usd_lux_shaping_a_p_i.html If that helps you in figuring out how to deal with spot lights.
Brecht Van Lommel commented 2024-03-25 14:57:04 +01:00
Owner
Copy Link

This should be addressed now.

This should be addressed now.

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Brecht Van Lommel
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Reference: blender/cycles#4
No description provided.
Delete Branch "boberfly/cycles:fixes/ellipse_unnormalize_bug"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?