Cycles PMJ adaptive sampling working poorly with many bounces #79190

Closed
opened 2020-07-23 14:32:51 +02:00 by CHET · 43 comments

System Information
Operating system: Windows-10-10.0.19041-SP0 64 Bits
Graphics card: GeForce GTX 950M/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 398.35

Blender Version
Broken: version: 2.90.0 Beta, branch: master, commit date: 2020-07-22 14:02, hash: 0e280b96ca

Short description of error
Adaptive sampling does not take into account the minimum number of samples, as well as the threshold, additionally leaving square artifacts
Without adaptive sampling
blender_1L4TLZBR8T.jpg
With adaptive sampling
blender_J25OMlXOlu.jpg
blender_BIKoW5fPIJ.jpg

Exact steps for others to reproduce the error

  • Open attached file
  • press {key F12} to render
    test.blend
**System Information** Operating system: Windows-10-10.0.19041-SP0 64 Bits Graphics card: GeForce GTX 950M/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 398.35 **Blender Version** Broken: version: 2.90.0 Beta, branch: master, commit date: 2020-07-22 14:02, hash: `0e280b96ca` **Short description of error** Adaptive sampling does not take into account the minimum number of samples, as well as the threshold, additionally leaving square artifacts Without adaptive sampling ![blender_1L4TLZBR8T.jpg](https://archive.blender.org/developer/F8715917/blender_1L4TLZBR8T.jpg) With adaptive sampling ![blender_J25OMlXOlu.jpg](https://archive.blender.org/developer/F8715919/blender_J25OMlXOlu.jpg) ![blender_BIKoW5fPIJ.jpg](https://archive.blender.org/developer/F8715921/blender_BIKoW5fPIJ.jpg) **Exact steps for others to reproduce the error** - Open attached file - press {key F12} to render [test.blend](https://archive.blender.org/developer/F8715927/test.blend)
Author

Added subscriber: @cheteron

Added subscriber: @cheteron

#89379 was marked as duplicate of this issue

#89379 was marked as duplicate of this issue
Germano Cavalcante changed title from Render bug to Render artefacts with adaptive sampling+Intel denoise 2020-07-23 20:47:38 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Changed status from 'Needs Triage' to: 'Confirmed'

Added subscriber: @mano-wii

Added subscriber: @mano-wii
Member

Added subscriber: @nacioss

Added subscriber: @nacioss
Member

Can't reproduce with RTX 2080ti.
Please try to update the Nvidia drivers of the GPU to the latest (your drivers seem outdated) and try again.

Can't reproduce with RTX 2080ti. Please try to update the Nvidia drivers of the GPU to the latest (your drivers seem outdated) and try again.
Author

It's not a driver problem, because in blender version: 2.90.0 Beta, branch: master, commit date: 2020-07-27 20:19, hash: 221604cdd6 - noise threshold works correctly
image.png

In #79190#990172, @nacioss wrote:
Can't reproduce with RTX 2080ti.
Please try to update the Nvidia drivers of the GPU to the latest (your drivers seem outdated) and try again.

It's not a driver problem, because in blender version: 2.90.0 Beta, branch: master, commit date: 2020-07-27 20:19, hash: 221604cdd663 - noise threshold works correctly ![image.png](https://archive.blender.org/developer/F8744117/image.png) > In #79190#990172, @nacioss wrote: > Can't reproduce with RTX 2080ti. > Please try to update the Nvidia drivers of the GPU to the latest (your drivers seem outdated) and try again.

Added subscriber: @SteffenD

Added subscriber: @SteffenD

Added subscriber: @brecht

Added subscriber: @brecht

The problem here is that adaptive sampling enables the PMJ sampling pattern, which seems to be doing poorly on this scene compared to Sobol and CMJ.

The problem here is that adaptive sampling enables the PMJ sampling pattern, which seems to be doing poorly on this scene compared to Sobol and CMJ.

Added subscriber: @Stefan_Werner

Added subscriber: @Stefan_Werner

@Stefan_Werner, maybe you have a guess for why PMJ would fail here.

@Stefan_Werner, maybe you have a guess for why PMJ would fail here.
Brecht Van Lommel changed title from Render artefacts with adaptive sampling+Intel denoise to Cycles PMJ adaptive sampling working poorly with many bounces 2020-11-09 15:35:21 +01:00

Added subscribers: @leesonw, @PratikPB2123

Added subscribers: @leesonw, @PratikPB2123
Member

Removed subscriber: @nacioss

Removed subscriber: @nacioss
Member

Have been investigating this and have a few ideas. So far I can confirm that it is not resorting to random sampling but I am beginning to think that maybe the sampler is setup slightly incorrectly which is causing the samples to be distributed oddly.

Have been investigating this and have a few ideas. So far I can confirm that it is not resorting to random sampling but I am beginning to think that maybe the sampler is setup slightly incorrectly which is causing the samples to be distributed oddly.
Member

It's the decorrelation for higher dimensions that I copied from our Sobol sampler that appears to be causing this.

It's the decorrelation for higher dimensions that I copied from our Sobol sampler that appears to be causing this.
Member

Thanks for the update. I'll look into that to see what I find.

Thanks for the update. I'll look into that to see what I find.
William Leeson self-assigned this 2021-06-28 15:53:52 +02:00
Member

I have investigated the CPU variant and the samples appear fine with random samples but on further analysis the cmj_hash_simple seems to bias the points which you can see below when performed on a regular grid see below samples.png. I am currently working on fixing this.

I have investigated the CPU variant and the samples appear fine with random samples but on further analysis the cmj_hash_simple seems to bias the points which you can see below when performed on a regular grid see below ![samples.png](https://archive.blender.org/developer/F10204788/samples.png). I am currently working on fixing this.
Member

OK, I have been working on how this can be improved using the table approach we already have. By just shuffling the sample index (so we get a randomish order for progressive samples) and also along the x row and y column to reduce the correlation between the sets nice samples can be obtained. This would also mean we only need 1 table of sample points instead of a number of them. This would be way better on the CPU. I have done some experimentation with this and managed to generate something like the correlated multi-jittered samples but that we can use in a progressive manner see the image below for the first 8 2D sets of points. shuffling_table_1.png Notice that they keep many of the properties of the original set but are all different. Also since the sample order is essentially random it can be used as a progressive set which converges towards the original set in a more representative manner. I still need to optimise the shuffle permutation to get more speed and get a nicer sample order. For comparison using our existing progressive method with the same fixed start table gives the following which can be relatively clumpy masking_table_1.png.

OK, I have been working on how this can be improved using the table approach we already have. By just shuffling the sample index (so we get a randomish order for progressive samples) and also along the x row and y column to reduce the correlation between the sets nice samples can be obtained. This would also mean we only need 1 table of sample points instead of a number of them. This would be way better on the CPU. I have done some experimentation with this and managed to generate something like the correlated multi-jittered samples but that we can use in a progressive manner see the image below for the first 8 2D sets of points. ![shuffling_table_1.png](https://archive.blender.org/developer/F10206717/shuffling_table_1.png) Notice that they keep many of the properties of the original set but are all different. Also since the sample order is essentially random it can be used as a progressive set which converges towards the original set in a more representative manner. I still need to optimise the shuffle permutation to get more speed and get a nicer sample order. For comparison using our existing progressive method with the same fixed start table gives the following which can be relatively clumpy ![masking_table_1.png](https://archive.blender.org/developer/F10206718/masking_table_1.png).

How can you shuffle the sample index of a progressive sampling pattern without losing stratification? A random subset of a stratified pattern is not stratified, unless there is something specific about the shuffling that preserves the stratification.

Maybe there is something specific about PMJ and a specific shuffling function that could work, but it's not obvious to me how that would be implemented.

I still don't understand why there is an issue here in the first place. Presumably the PMJ algorithm as described in the paper is correct. Is there an error in our implementation, either in how we generate the samples or how we use them? Are we running into a limitation of the algorithm with regards to number of dimensions or number of samples?

How can you shuffle the sample index of a progressive sampling pattern without losing stratification? A random subset of a stratified pattern is not stratified, unless there is something specific about the shuffling that preserves the stratification. Maybe there is something specific about PMJ and a specific shuffling function that could work, but it's not obvious to me how that would be implemented. I still don't understand why there is an issue here in the first place. Presumably the PMJ algorithm as described in the paper is correct. Is there an error in our implementation, either in how we generate the samples or how we use them? Are we running into a limitation of the algorithm with regards to number of dimensions or number of samples?
Member

I shuffle along the x and y of the original sample set which has a large number of samples (which is similar to what is done with the current hash) but I also shuffle using the index of the sample which reorders the samples. However, I still need to find a decent mapping for this, the current one is based off a left top right and top to bottom ordering which can work for some sets but not others also random selection seems to perform well also but I was thinking since it is just a 1D set some kind of shuffled quasi-random set would be good but I need to check this out.

The PMJ algorithm uses shuffling and scrambling in a similar manner to this. They use a Owen scrambling hash that was created by Brent Burley I can implement that.

There is an error in our implementation in that our hashing does not preserve the properties of the sample set very well. Also we don't really shuffle the sample order only select from different sets.

I shuffle along the x and y of the original sample set which has a large number of samples (which is similar to what is done with the current hash) but I also shuffle using the index of the sample which reorders the samples. However, I still need to find a decent mapping for this, the current one is based off a left top right and top to bottom ordering which can work for some sets but not others also random selection seems to perform well also but I was thinking since it is just a 1D set some kind of shuffled quasi-random set would be good but I need to check this out. The PMJ algorithm uses shuffling and scrambling in a similar manner to this. They use a Owen scrambling hash that was created by Brent Burley I can implement that. There is an error in our implementation in that our hashing does not preserve the properties of the sample set very well. Also we don't really shuffle the sample order only select from different sets.
Member

Instead of using the texture lookup I tried out using Kellers Owen Scrambled Sobol sequence and it gives very good results and even runs faster than the current implementation on my CPU (probably due to better caching as it does not use the texture fetch). However, there are a few issues that need resolving but it appears to be a good candidate. The multi-dimensional sample plots are as follows sobol_table.png
they not as good as the multi-jittered sets above but it is a infinite set that can be used without eventually resorting to random sampling. I think with some tweaking the sets could be made better.

Instead of using the texture lookup I tried out using Kellers Owen Scrambled Sobol sequence and it gives very good results and even runs faster than the current implementation on my CPU (probably due to better caching as it does not use the texture fetch). However, there are a few issues that need resolving but it appears to be a good candidate. The multi-dimensional sample plots are as follows ![sobol_table.png](https://archive.blender.org/developer/F10207887/sobol_table.png) they not as good as the multi-jittered sets above but it is a infinite set that can be used without eventually resorting to random sampling. I think with some tweaking the sets could be made better.
Member

It seems that even with the new sampling strategies the results are only slightly improved. It still appears that the non-progressive scheme is way better. However, it is not clear exactly why. I have found a few reasons for the tile edges being different due to correlated sampling issues at the edges of tiles due to the integrator seed matching the tile hash for when hash_uint2(seed, 0) matches hash_uint2(x, y) as these cancel each other out as they are combined using hash1^hash2.

It seems that even with the new sampling strategies the results are only slightly improved. It still appears that the non-progressive scheme is way better. However, it is not clear exactly why. I have found a few reasons for the tile edges being different due to correlated sampling issues at the edges of tiles due to the integrator seed matching the tile hash for when hash_uint2(seed, 0) matches hash_uint2(x, y) as these cancel each other out as they are combined using hash1^hash2.
Member

So I have found that the correlation is caused by the initialization of the pixel rng_hash with

*rng_hash = hash_uint2(x, y);

replacing this with rand() gives the correct results. I will work on a more suitable fix that gives better repeatability. Here is the CMJ image
cmj_hash.png
and the PMJ image with random hack mentioned above
pmj_rand.png
both with 256 samples per pixel. As you can see they work equally well.

So I have found that the correlation is caused by the initialization of the pixel rng_hash with ``` *rng_hash = hash_uint2(x, y); ``` replacing this with rand() gives the correct results. I will work on a more suitable fix that gives better repeatability. Here is the CMJ image ![cmj_hash.png](https://archive.blender.org/developer/F10216047/cmj_hash.png) and the PMJ image with random hack mentioned above ![pmj_rand.png](https://archive.blender.org/developer/F10216049/pmj_rand.png) both with 256 samples per pixel. As you can see they work equally well.

One of the renders seems to have denoising enabled so it's hard to compare?

But if the solution is just changing fixing hash functions or ensuring we don't use the same hash function and create correlations, that is hopefully quite straightforward.

One of the renders seems to have denoising enabled so it's hard to compare? But if the solution is just changing fixing hash functions or ensuring we don't use the same hash function and create correlations, that is hopefully quite straightforward.

Added subscriber: @juang3d

Added subscriber: @juang3d

The random hack seems to have quite more noise, am I right?

The random hack seems to have quite more noise, am I right?
Member

Yes there is more noise. I'll re-render the images but the main point is that there are more reflections than before. Previously very few samples reached the later bounces. What I think is causing it is that hash is generally a uint but in many cases it is mixed with an int. However, if the uint is too large then it becomes negative and this can be combined with the dimension the values are subtracted instead of a added causing correlation of some form. This appears validated by the fact that rand() returns an int and not a uint and so would have no negative values. However, upon replacing some of these uint and int combinations to only work in the uint space seems to make everything worse not better.

Yes there is more noise. I'll re-render the images but the main point is that there are more reflections than before. Previously very few samples reached the later bounces. What I think is causing it is that hash is generally a uint but in many cases it is mixed with an int. However, if the uint is too large then it becomes negative and this can be combined with the dimension the values are subtracted instead of a added causing correlation of some form. This appears validated by the fact that rand() returns an int and not a uint and so would have no negative values. However, upon replacing some of these uint and int combinations to only work in the uint space seems to make everything worse not better.
Member

The sampling issues are caused because the PMJ sampler uses the same samples for the same dimension. By shuffling the sample the correlation issue can be avoided as then the same sample pattern is not used at the same dimension for every pixel. There are 3 choices to shuffle the sample pattern used per dimension or the individual component values of the sample or both.

The sampling issues are caused because the PMJ sampler uses the same samples for the same dimension. By shuffling the sample the correlation issue can be avoided as then the same sample pattern is not used at the same dimension for every pixel. There are 3 choices to shuffle the sample pattern used per dimension or the individual component values of the sample or both.
Member

I have now fixed the PMJ sampler and added a patch D11682 with a whole lot more. I probably needs work to meet the correct standards. The only bits needed to actually fix just PMJ are the updates to

path_rng_1D
path_rng_2D
pmj_sample_1D
pmj_sample_2D

and the functions they drag in for shuffling. Here is an update image using the PMJ sampling with the fix
pmj_image_with_shuffling.png
the full patch also adds a different pixel hash and QMC sampling patterns with various shuffling and scrambling options. These appear of a similar or better quality especially if the number of samples is large I provided and image below
qmc_image_with_shuffling.png
Also added in the patch are some new options in the advanced section
advanced_ui.png

I have now fixed the PMJ sampler and added a patch [D11682](https://archive.blender.org/developer/D11682) with a whole lot more. I probably needs work to meet the correct standards. The only bits needed to actually fix just PMJ are the updates to ``` path_rng_1D path_rng_2D pmj_sample_1D pmj_sample_2D ``` and the functions they drag in for shuffling. Here is an update image using the PMJ sampling with the fix ![pmj_image_with_shuffling.png](https://archive.blender.org/developer/F10218759/pmj_image_with_shuffling.png) the full patch also adds a different pixel hash and QMC sampling patterns with various shuffling and scrambling options. These appear of a similar or better quality especially if the number of samples is large I provided and image below ![qmc_image_with_shuffling.png](https://archive.blender.org/developer/F10218764/qmc_image_with_shuffling.png) Also added in the patch are some new options in the advanced section ![advanced_ui.png](https://archive.blender.org/developer/F10218768/advanced_ui.png)

Added subscriber: @lsscpp

Added subscriber: @lsscpp

Are all these pattern/hash/shuffle options really needed by a user point of view? I mean, it's good to find solutions, but I think the best direction for Cycles is to remove sliders and checkboxes.
I wish all the sampling choices could be made under the hood. We're even close to see the "just set final quality" workflow...

Are all these pattern/hash/shuffle options really needed by a user point of view? I mean, it's good to find solutions, but I think the best direction for Cycles is to *remove* sliders and checkboxes. I wish all the sampling choices could be made under the hood. We're even close to see the "just set final quality" workflow...
Contributor

Added subscriber: @Raimund58

Added subscriber: @Raimund58

Added subscriber: @derekbarker

Added subscriber: @derekbarker

In #79190#1191115, @lsscpp wrote:
Are all these pattern/hash/shuffle options really needed by a user point of view? I mean, it's good to find solutions, but I think the best direction for Cycles is to remove sliders and checkboxes.
I wish all the sampling choices could be made under the hood. We're even close to see the "just set final quality" workflow...

Don't listen to this guy, the more options the merrier. Sure artists might never use it but what if the TD wants to save your production 1000s of dollars on a render? No reason not to have these under the hood somewhere. I've been using cycles for 10 years. optimizing renders is my thing and what I get paid to do. its near impossible for cycles to know what it should use in every scene, sure it can take a pretty good guess but it wont be perfect. We should at least have some TD options (that we can toggle like the debug options) for fine tuning renders that artists might not care about.

> In #79190#1191115, @lsscpp wrote: > Are all these pattern/hash/shuffle options really needed by a user point of view? I mean, it's good to find solutions, but I think the best direction for Cycles is to *remove* sliders and checkboxes. > I wish all the sampling choices could be made under the hood. We're even close to see the "just set final quality" workflow... Don't listen to this guy, the more options the merrier. Sure artists might never use it but what if the TD wants to save your production 1000s of dollars on a render? No reason not to have these under the hood somewhere. I've been using cycles for 10 years. optimizing renders is my thing and what I get paid to do. its near impossible for cycles to know what it should use in every scene, sure it can take a pretty good guess but it wont be perfect. We should at least have some TD options (that we can toggle like the debug options) for fine tuning renders that artists might not care about.

Added subscriber: @shotalot

Added subscriber: @shotalot

In #79190#1191115, @lsscpp wrote:
Are all these pattern/hash/shuffle options really needed by a user point of view? I mean, it's good to find solutions, but I think the best direction for Cycles is to remove sliders and checkboxes.
I wish all the sampling choices could be made under the hood. We're even close to see the "just set final quality" workflow...

This is a very horrible suggestion. You need that control on larger productions.

> In #79190#1191115, @lsscpp wrote: > Are all these pattern/hash/shuffle options really needed by a user point of view? I mean, it's good to find solutions, but I think the best direction for Cycles is to *remove* sliders and checkboxes. > I wish all the sampling choices could be made under the hood. We're even close to see the "just set final quality" workflow... This is a very horrible suggestion. You need that control on larger productions.
Member

Both cases can be addressed by using sensible default so you don't have to change the settings unless you are interested in doing so. I was wondering if the settings should be identical for all devices as certain settings would be better (read faster convergence) on a CPU than a GPU or is it better for the images to be more similar by using the exact same sampling algorithm. I need to perform some tests to figure this out and also the ideal default settings.

Both cases can be addressed by using sensible default so you don't have to change the settings unless you are interested in doing so. I was wondering if the settings should be identical for all devices as certain settings would be better (read faster convergence) on a CPU than a GPU or is it better for the images to be more similar by using the exact same sampling algorithm. I need to perform some tests to figure this out and also the ideal default settings.

All options need to have some justification for existing, we always look at it on a case-by-case basis. Note that more options here can negatively impact GPU rendering performance, since this random number sampling is inlined in many places and can lead to extra register pressure or less efficient instruction cache.

CPU and GPU should use the same sampling algorithm, especially since Cycles supports combined CPU and GPU rendering.

All options need to have some justification for existing, we always look at it on a case-by-case basis. Note that more options here can negatively impact GPU rendering performance, since this random number sampling is inlined in many places and can lead to extra register pressure or less efficient instruction cache. CPU and GPU should use the same sampling algorithm, especially since Cycles supports combined CPU and GPU rendering.

This issue was referenced by 70aa80b683812754b49b8b52820564074c1dd6a7

This issue was referenced by 70aa80b683812754b49b8b52820564074c1dd6a7
Member

Changed status from 'Confirmed' to: 'Resolved'

Changed status from 'Confirmed' to: 'Resolved'

This issue was referenced by blender/cycles@faf8f5f353

This issue was referenced by blender/cycles@faf8f5f353fcd7f7907c57c9fb90c3ee0f1162ec

This issue was referenced by 5eed7cdc8c

This issue was referenced by 5eed7cdc8c17b96ba9fd38647e86d412e682e137
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
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, Assets & 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
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
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
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
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
14 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#79190
No description provided.