Cycles: Implement better ellipse sampling for area lights #122935

Open
Lukas Stockner wants to merge 14 commits from LukasStockner/blender:ellipse-sampling into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Member

First step in implementing sampling of spherical ellipses.

Currently, the code only projects the ellipse to be tangent to the unit sphere around the shading point, then samples that as usual.
However, that is arguably the hard part - the remainder is described and implemented in "Area-Preserving Parameterizations for Spherical Ellipses", but unlike their projection step, this one also works for planar ellipses, not just disks.

The eigendecomposition code can probably be improved, I just used a function that was left over from the old denoiser.

Attached is a playground file with a Python implementation of the various methods and a basic visualization for debugging.

First step in implementing sampling of spherical ellipses. Currently, the code only projects the ellipse to be tangent to the unit sphere around the shading point, then samples that as usual. However, that is arguably the hard part - the remainder is described and implemented in "Area-Preserving Parameterizations for Spherical Ellipses", but unlike their projection step, this one also works for planar ellipses, not just disks. The eigendecomposition code can probably be improved, I just used a function that was left over from the old denoiser. Attached is a playground file with a Python implementation of the various methods and a basic visualization for debugging.
Author
Member

Update: The code implements the numerical (non-lookup-based) solid angle sampling now.

TODOs:

  • There are numerical issues are grazing angles (and sometimes domain exceptions in ellint_3)
  • We need a standalone ellint_3 implementation (probably just base on the GSL version - there are better approaches, but they're more complex from what I can see).
    • Also look into whether a simpler approximation is good enough.
  • The eigendecomposition can probably be improved quite a bit (we have 3x3 symmetric matrices with a known zero element. This approach looks promising, for example.)
  • The numerical inversion is way too slow to be practical, it should be replaced with the LUT method. Seems easy, but I'd also like code to reproduce the table.
Update: The code implements the numerical (non-lookup-based) solid angle sampling now. TODOs: - There are numerical issues are grazing angles (and sometimes domain exceptions in `ellint_3`) - We need a standalone `ellint_3` implementation (probably just base on [the GSL version](https://github.com/ampl/gsl/blob/master/specfunc/ellint.c) - there are [better approaches](https://www.sciencedirect.com/science/article/pii/S037704271300201X), but they're more complex from what I can see). - Also look into whether a simpler approximation is good enough. - The eigendecomposition can probably be improved quite a bit (we have 3x3 symmetric matrices with a known zero element. [This approach](https://www.geometrictools.com/Documentation/RobustEigenSymmetric3x3.pdf) looks promising, for example.) - The numerical inversion is way too slow to be practical, it should be replaced with the LUT method. Seems easy, but I'd also like code to reproduce the table.
Author
Member

Quick comparison is attached, note the artifacts in the solid-angle version.

Quick comparison is attached, note the artifacts in the solid-angle version.
Author
Member

Update:

  • Axes are now consistent across samples to solve the noise boundary artifacts
  • Eigensolver is replaced with an efficient 3x3 symmetric implementation

I've also looked into implementing the linked comp_ellint_3 implementations. The GSL variant works, but is even slower than the stdlib version. The more efficient version works, but is very complex (it depends on piecewise fits with dozens of parameters each) and also iterative.

So, different idea: Just approximate the solid angle. We only need it for the pdf, so if we're off by e.g. 1% that's not that big of a deal. Tabulating the solid angle w.r.t alpha and beta/alpha looks fairly smooth, it seems like a numerical fit of the form a * (x*x*y) * (c+b*cos(x*pi/2)) gets within 2.5% and tabulating the residual by remapping the parameters as x**2 and y**2 with e.g. 32^2 resolution should be within 0.2% or so.

Update: - Axes are now consistent across samples to solve the noise boundary artifacts - Eigensolver is replaced with an efficient 3x3 symmetric implementation I've also looked into implementing the linked `comp_ellint_3` implementations. The GSL variant works, but is even slower than the stdlib version. The more efficient version works, but is very complex (it depends on piecewise fits with dozens of parameters each) and also iterative. So, different idea: Just approximate the solid angle. We only need it for the pdf, so if we're off by e.g. 1% that's not that big of a deal. Tabulating the solid angle w.r.t `alpha` and `beta/alpha` looks fairly smooth, it seems like a numerical fit of the form `a * (x*x*y) * (c+b*cos(x*pi/2))` gets within 2.5% and tabulating the residual by remapping the parameters as `x**2` and `y**2` with e.g. 32^2 resolution should be within 0.2% or so.
Author
Member

After some tweaking, I got the approximation error down to 0.004% on average and 0.08% maximum (out of 10000 random inputs) using a 32x32 table.

This also behaves well in the limits from what I can see - for example, the bright pixel artifacts in the attached comparison are gone.

Analytical Approximation
ellipse-numeric.png ellipse-lookup.png
After some tweaking, I got the approximation error down to 0.004% on average and 0.08% maximum (out of 10000 random inputs) using a 32x32 table. This also behaves well in the limits from what I can see - for example, the bright pixel artifacts in the attached comparison are gone. | Analytical | Approximation | | - | - | | ![ellipse-numeric.png](/attachments/a6e76ac5-6e1c-45b3-bcd1-ab84e7ac5277) | ![ellipse-lookup.png](/attachments/012db723-d451-4a94-b739-d2e0b1ce2b7c) |
Author
Member

Update: Here's a version with the CDF lookup table approach, as described in the paper. The LUT is generated by the attached script: precompute_sample_table.py

However, when plotting the LUT in 2D (see attachment), it becomes obvious that the axis encoding is hugely wasteful - the actual non-trivial part of the table is maybe a 4x4 or so region of the 256x256 table, everything else is just linear:
lut.png

This explains why the paper uses a 1024x1024 table - you need that kind of size to get any proper detail there. However, that also means that with some coordinate remapping, we should be able to use a much smaller table (ideally 32x32 or so). I've got promising attempts (the currently best one is x **= 1/(0.4*(y + 5e-2)) and y **= 4), I'll look into it further.

Finally, note that there appears to be some remaining numerical issue. When making the lamp very small, you get very interesting patterns (this is supposed to be a smooth falloff from the center):
ellipseglitch_lr.png

Update: Here's a version with the CDF lookup table approach, as described in the paper. The LUT is generated by the attached script: [precompute_sample_table.py](/attachments/ede23608-058c-44f4-b875-323fc40e0885) However, when plotting the LUT in 2D (see attachment), it becomes obvious that the axis encoding is hugely wasteful - the actual non-trivial part of the table is maybe a 4x4 or so region of the 256x256 table, everything else is just linear: ![lut.png](/attachments/d62bb632-6970-498b-a4ba-0a390436c69f) This explains why the paper uses a 1024x1024 table - you need that kind of size to get any proper detail there. However, that also means that with some coordinate remapping, we should be able to use a much smaller table (ideally 32x32 or so). I've got promising attempts (the currently best one is `x **= 1/(0.4*(y + 5e-2))` and `y **= 4`), I'll look into it further. Finally, note that there appears to be some remaining numerical issue. When making the lamp very small, you get very interesting patterns (this is supposed to be a smooth falloff from the center): ![ellipseglitch_lr.png](/attachments/68a75848-c627-45d7-8ac5-a46caa2d71fe)
Lukas Stockner force-pushed ellipse-sampling from efa897d048 to 55e3fb2b35 2024-10-15 20:53:25 +02:00 Compare
Author
Member

Update:

  • Implemented CDF remapping and reduced size to 64x64
    • I've used a basic x^4 remapping for both axes for now, this appears to be the ideal balance between complexity, max error and average error
    • 256x256 without mapping has max error of 0.184 and avg error of 0.00027, the remapped 64x64 has max error 0.002 and avg error 0.000109
    • The more complex mapping from above reduces max error further, but increases avg error and is more expensive to compute (requiring a powf)
  • Cleaned up precomputation code and added it to doc/ (not sure where else to put it)
  • Formatting, cleanups etc.

The remaining issues with tiny scales are due to numerical precision in the eigenvalue solver, I don't think we can do significantly better in single precision.

Update: - Implemented CDF remapping and reduced size to 64x64 - I've used a basic x^4 remapping for both axes for now, this appears to be the ideal balance between complexity, max error and average error - 256x256 without mapping has max error of 0.184 and avg error of 0.00027, the remapped 64x64 has max error 0.002 and avg error 0.000109 - The more complex mapping from above reduces max error further, but increases avg error and is more expensive to compute (requiring a powf) - Cleaned up precomputation code and added it to doc/ (not sure where else to put it) - Formatting, cleanups etc. The remaining issues with tiny scales are due to numerical precision in the eigenvalue solver, I don't think we can do significantly better in single precision.
Lukas Stockner changed title from WIP: Cycles: Implement better ellipse sampling for area lights to Cycles: Implement better ellipse sampling for area lights 2024-10-15 20:57:23 +02:00
Lukas Stockner requested review from Sergey Sharybin 2024-10-15 20:57:38 +02:00
Lukas Stockner requested review from Weizhen Huang 2024-10-15 20:57:38 +02:00
Sergey Sharybin added 1 commit 2024-10-16 12:42:51 +02:00
Fix runtime kernel compilation
Some checks failed
buildbot/vexp-code-patch-darwin-x86_64 Build done.
buildbot/vexp-code-patch-darwin-arm64 Build done.
buildbot/vexp-code-patch-lint Build done.
buildbot/vexp-code-patch-linux-x86_64 Build done.
buildbot/vexp-code-patch-windows-amd64 Build done.
buildbot/vexp-code-patch-coordinator Build done.
01dc54a2e3
Allows to compile kernels for Metal backend, which happens at runtime.
Sergey Sharybin reviewed 2024-10-16 12:48:21 +02:00
@ -0,0 +1,223 @@
# SPDX-FileCopyrightText: 2011-2024 Blender Foundation

Feels a bit random to have table generator in the doc folder, next to licenses.
Maybe have it in a gen or script folder?

Feels a bit random to have table generator in the `doc` folder, next to licenses. Maybe have it in a `gen` or `script` folder?
Author
Member

Yeah, I'm not super happy with it either - I mostly wanted to make it clear that it's just for reference, not an active part of Cycles itself. scripts sounds good.

Yeah, I'm not super happy with it either - I mostly wanted to make it clear that it's just for reference, not an active part of Cycles itself. `scripts` sounds good.
@ -11,0 +28,4 @@
* to make the data more linear (which lets us reduce the table size).
*/
ccl_device_inline float booth_inversion_table(KernelGlobals kg,

What exactly is "booth"? Maybe can be some terse comment for those who only skimmed through the paper?

What exactly is "booth"? Maybe can be some terse comment for those who only skimmed through the paper?
Author
Member

True, I've renamed it to spherical_ellipse_sample_phi.

True, I've renamed it to `spherical_ellipse_sample_phi`.
@ -0,0 +22,4 @@
*
* Based on "A Robust Eigensolver for 3 x 3 Symmetric Matrices" by David Eberly
* (https://www.geometrictools.com/Documentation/RobustEigenSymmetric3x3.pdf). */
ccl_device float3 eigendecomposition_3x3_symmetric(float a00,

Can we have some rudimentary unit test for the function? Should be easy to add test/math_eigensolver_test.cc. I can also help with that.

Can we have some rudimentary unit test for the function? Should be easy to add `test/math_eigensolver_test.cc`. I can also help with that.
Author
Member

Good point, I added it (and found a bug while doing so!)

Good point, I added it (and found a bug while doing so!)

Did an initial pass. Overall looks fine, but I didn't verify actual formula or anything like that.

I was a bit confused about impact on the user level (there are some images here, but was hard to see). So, made my own :)

before after
before.png } after.png
Did an initial pass. Overall looks fine, but I didn't verify actual formula or anything like that. I was a bit confused about impact on the user level (there are some images here, but was hard to see). So, made my own :) | before | after | | -- | -- | | ![before.png](/attachments/8f072d6a-c328-4631-a196-c7b66d33ea73) } | ![after.png](/attachments/0a8f55be-0def-401f-8cfc-fb0fc32b29f6) |

@blender-bot build

@blender-bot build

The failing tests from the buildbot: Screenshot 2024-10-17 at 15.57.58.png

Seems all within expected sample changes, not a real regression?

The failing tests from the buildbot: ![Screenshot 2024-10-17 at 15.57.58.png](/attachments/2a13f488-8310-49d3-95a0-a966bb27ef1d) Seems all within expected sample changes, not a real regression?
Sergey Sharybin added this to the Module: Render & Cycles project 2024-10-17 16:01:32 +02:00
Lukas Stockner added 1 commit 2024-10-17 22:41:46 +02:00
Author
Member

Yeah, the test failures are expected, I'll update them when landing.

Yeah, the test failures are expected, I'll update them when landing.
Sergey Sharybin approved these changes 2024-10-18 11:42:34 +02:00
Sergey Sharybin left a comment
Owner

Thanks for the updates and clarifications. Looks good to me now!

Thanks for the updates and clarifications. Looks good to me now!
Member

There are some quite weird artifacts? The sample quality near grazing angles is improved, but there are other regions with worse sample quality, which was unexpected. Especially the lines in the middle looks weird.

Before After
before.png after.png

You had lots of thoughts and improvement in the comment, it would be nice if you summarize them in the main PR message, to make it more obvious what is the current method and what's still a limitation.

There are some quite weird artifacts? The sample quality near grazing angles is improved, but there are other regions with worse sample quality, which was unexpected. Especially the lines in the middle looks weird. |Before|After| |--|--| |![before.png](/attachments/8e89c7d5-0435-4054-9d4e-315667b4db12)|![after.png](/attachments/d1fa31f3-9004-4360-8cbc-1d11351a1bde)| You had lots of thoughts and improvement in the comment, it would be nice if you summarize them in the main PR message, to make it more obvious what is the current method and what's still a limitation.
Weizhen Huang reviewed 2024-10-21 11:56:13 +02:00
@ -0,0 +1,223 @@
# SPDX-FileCopyrightText: 2011-2024 Blender Foundation
Member

What is this doc folder for? It makes sense to put it in a folder named scripts, but this doc still feels weird.
BTW, we also have another file intern/cycles/blender/addon/camera.py, the code too is only for reference, and feels totally random to sit in addon. We should probably put them together.

What is this `doc` folder for? It makes sense to put it in a folder named `scripts`, but this `doc` still feels weird. BTW, we also have another file `intern/cycles/blender/addon/camera.py`, the code too is only for reference, and feels totally random to sit in `addon`. We should probably put them together.
@ -11,0 +27,4 @@
* for what would otherwise be the y coordinate, and we remap coordinates to their fourth power
* to make the data more linear (which lets us reduce the table size).
*/
Member

Seems that this large block of comment holds for all the following functions, which was not obvious at first sight. I would suggest to make it a code section https://developer.blender.org/docs/handbook/guidelines/c_cpp/#comment-sections

Seems that this large block of comment holds for all the following functions, which was not obvious at first sight. I would suggest to make it a code section https://developer.blender.org/docs/handbook/guidelines/c_cpp/#comment-sections
@ -11,0 +104,4 @@
return make_float2(u, sqr(r));
}
ccl_device_inline float spherical_ellipse_S(KernelGlobals kg, const float alpha, const float beta)
Member

Would be more clear to call the function spherical_ellipse_solid_angle.
Also, you mentioned this solid angle is approximated? Would be good to add a comment here.

Would be more clear to call the function `spherical_ellipse_solid_angle`. Also, you mentioned this solid angle is approximated? Would be good to add a comment here.
@ -11,0 +116,4 @@
}
/* Importance-sample a spherical ellipse w.r.t. solid angle.
* Inputs are at and bt, the semi-major/minor axis of the tangential(!) ellipse.
Member

What is tangential ellipse?

What is tangential ellipse?
@ -628,6 +628,8 @@ void ShaderManager::device_update_common(Device * /*device*/,
ktables->sheen_ltc = ensure_bsdf_table(dscene, scene, table_sheen_ltc);
ktables->ggx_gen_schlick_ior_s = ensure_bsdf_table(dscene, scene, table_ggx_gen_schlick_ior_s);
ktables->ggx_gen_schlick_s = ensure_bsdf_table(dscene, scene, table_ggx_gen_schlick_s);
ktables->ellipse_S = ensure_bsdf_table(dscene, scene, table_ellipse_S);
Member

This is minor and can be done outside of this PR, but the ellipse tables are not bsdf tables. Probably we should rename it to just lookup table or something.

This is minor and can be done outside of this PR, but the ellipse tables are not bsdf tables. Probably we should rename it to just lookup table or something.
Merge conflict checking is in progress. Try again in few moments.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u ellipse-sampling:LukasStockner-ellipse-sampling
git checkout LukasStockner-ellipse-sampling
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Code Documentation
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Asset System
Module
Core
Module
Development Management
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
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.

Dependencies

No dependencies set.

Reference: blender/blender#122935
No description provided.