Cycles: Implement better ellipse sampling for area lights #122935
No reviewers
Labels
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 project
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#122935
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "LukasStockner/blender:ellipse-sampling"
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?
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.
Update: The code implements the numerical (non-lookup-based) solid angle sampling now.
TODOs:
ellint_3
)ellint_3
implementation (probably just base on the GSL version - there are better approaches, but they're more complex from what I can see).Quick comparison is attached, note the artifacts in the solid-angle version.
Update:
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
andbeta/alpha
looks fairly smooth, it seems like a numerical fit of the forma * (x*x*y) * (c+b*cos(x*pi/2))
gets within 2.5% and tabulating the residual by remapping the parameters asx**2
andy**2
with e.g. 32^2 resolution should be within 0.2% or so.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.
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:
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))
andy **= 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):
efa897d048
to55e3fb2b35
Update:
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.
WIP: Cycles: Implement better ellipse sampling for area lightsto Cycles: Implement better ellipse sampling for area lights@ -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
orscript
folder?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?
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.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 :)
@blender-bot build
The failing tests from the buildbot:
Seems all within expected sample changes, not a real regression?
Yeah, the test failures are expected, I'll update them when landing.
Thanks for the updates and clarifications. Looks good to me now!
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.
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.
@ -0,0 +1,223 @@
# SPDX-FileCopyrightText: 2011-2024 Blender Foundation
What is this
doc
folder for? It makes sense to put it in a folder namedscripts
, but thisdoc
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 inaddon
. 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).
*/
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)
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.
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);
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.
Checkout
From your project repository, check out a new branch and test the changes.