Cycles: Switch to default Tabulated Sobol when loading older files #112553

Alaska · 2023-09-19T06:25:27+02:00

Alaska commented

2023-09-19 06:25:27 +02:00

When loading files older files, switch over to using
the Tabulated Sobol sampling pattern.

This is done because Tabulated Sobol is the default sampler for Cycles
from Blender 3.5+ and has various benefits
(it's faster and supports the Scrambling Distance settings).

When loading files older files, switch over to using the Tabulated Sobol sampling pattern. This is done because Tabulated Sobol is the default sampler for Cycles from Blender 3.5+ and has various benefits (it's faster and supports the Scrambling Distance settings).

Alaska added the

Module

Render & Cycles

label 2023-09-19 06:25:27 +02:00

Alaska added 1 commit 2023-09-19 06:25:39 +02:00

Switch to Tabulated Sobol in newer versions of Blender 7794549594

Alaska requested review from Brecht Van Lommel 2023-09-19 06:25:48 +02:00

Alaska requested review from Sergey Sharybin 2023-09-19 06:25:55 +02:00

Alaska commented

2023-09-19 06:32:48 +02:00

I had a few questions related to this change:

Should this be back ported to Blender 3.6?
Is if version <= (3, 5, 3): the right version to check against?

Back when Tabulated Sobol was added, if version <= (3, 5, 3) would of been the right place to apply this change [1]. But maybe that's not the case anymore? For example, a user could of saved a file in Blender 3.4 with Sobol Burley, then opened it in 3.5 or 3.6 and saved it. Now the version will be (3,5,10) or higher and this pull request will do nothing. Meaning the file will continue to use Sobol Burley, Scrambling Distance settings will be greyed out, and the user will have to enable debug settings if they want to fix it.

So maybe the check should be if version <= (4, 0, 23) to cover all the cases we didn't catch by not applying this change until now?

Note: Some test scenes may render differently after this change (as they switch from Sobol Burley to Tabulated Sobol)
Note 2: Some benchmark scenes may also render differently and possibly faster after this change.

[1] This is based on the fact the addition of Tabulated Sobol (b0cc8e8dde) came right after this change (ecfcf1b97b). And I'm just assuming that ecfcf1b97b was applied in a timely manner.

I had a few questions related to this change: 1. Should this be back ported to Blender 3.6? 2. Is `if version <= (3, 5, 3):` the right version to check against? Back when Tabulated Sobol was added, `if version <= (3, 5, 3)` would of been the right place to apply this change [1]. But maybe that's not the case anymore? For example, a user could of saved a file in Blender 3.4 with Sobol Burley, then opened it in 3.5 or 3.6 and saved it. Now the version will be `(3,5,10)` or higher and this pull request will do nothing. Meaning the file will continue to use Sobol Burley, Scrambling Distance settings will be greyed out, and the user will have to enable debug settings if they want to fix it. So maybe the check should be `if version <= (4, 0, 23)` to cover all the cases we didn't catch by not applying this change until now? --- Note: Some test scenes may render differently after this change (as they switch from Sobol Burley to Tabulated Sobol) Note 2: Some benchmark scenes may also render differently and possibly faster after this change. --- [1] This is based on the fact the addition of Tabulated Sobol (b0cc8e8dde) came right after this change (ecfcf1b97b70). And I'm just assuming that ecfcf1b97b70 was applied in a timely manner.

Alaska changed title from ~~Fix #103284: Cycles - Switch to using Tabulated Sobol when loading older files~~ to Fix #103284: Cycles - Use Tabulated Sobol when loading older files

2023-09-19 06:37:56 +02:00

Sergey Sharybin commented

2023-09-19 12:05:30 +02:00

The cscene.sampling_pattern is supposed to be ignored and TABULATED_SOBOL to be assumed when the Cycles Debug is enabled. If there is some code which accesses the sampling pattern without the debug option enabled check it needs to be corrected. Otherwise even after the fix here it is still possible that someone opens file in Blender when having Debug enabled, changes sampling pattern, save file and share it, making the next person looking into the file to be confused.

For the development purposes we should not override the sampler for every file we save or load. Also, it is not easy to tell if the sampling pattern was manually modified after the addition of the new sampling pattern. So from this perspective we can reset the sampling to the new default using the version at which the option was introduced.

For moving forward:

The proper fix for the issue would be to make it so code ignores the sampling pattern when Debug is OFF
[Optional] Reset the sampling pattern for the files saved prior to when the default has changed
The fix would need to be ported to 3.6 indeed.

The `cscene.sampling_pattern` is supposed to be ignored and `TABULATED_SOBOL` to be assumed when the Cycles Debug is enabled. If there is some code which accesses the sampling pattern without the debug option enabled check it needs to be corrected. Otherwise even after the fix here it is still possible that someone opens file in Blender when having Debug enabled, changes sampling pattern, save file and share it, making the next person looking into the file to be confused. For the development purposes we should not override the sampler for every file we save or load. Also, it is not easy to tell if the sampling pattern was manually modified after the addition of the new sampling pattern. So from this perspective we can reset the sampling to the new default using the version at which the option was introduced. For moving forward: - The proper fix for the issue would be to make it so code ignores the sampling pattern when Debug is OFF - [Optional] Reset the sampling pattern for the files saved prior to when the default has changed - The fix would need to be ported to 3.6 indeed.

Alaska commented

2023-09-20 01:02:13 +02:00

For moving forward:

The proper fix for the issue would be to make it so code ignores the sampling pattern when Debug is OFF

I've submitted a pull request for that here: #112606

[Optional] Reset the sampling pattern for the files saved prior to when the default has changed

This change probably isn't needed anymore. But I'll leave this pull request open just in case you do want to commit it.

> For moving forward: > - The proper fix for the issue would be to make it so code ignores the sampling pattern when Debug is OFF I've submitted a pull request for that here: #112606 > - [Optional] Reset the sampling pattern for the files saved prior to when the default has changed This change probably isn't needed anymore. But I'll leave this pull request open just in case you do want to commit it.

Alaska changed title from ~~Fix #103284: Cycles - Use Tabulated Sobol when loading older files~~ to Cycles: Switch to default Tabulated Sobol when loading older files

2023-09-20 01:02:34 +02:00

Alaska added 1 commit 2023-09-20 04:13:14 +02:00

Merge branch 'main' into fix-103284 ef7ecae639

Sergey Sharybin commented

2023-09-21 10:05:37 +02:00

I think it will be nice to commit this change as well. So that the noise pattern does not change when enabling developer options.

Brecht Van Lommel referenced this issue from a commit

2023-10-06 19:38:39 +02:00

Cycles: Use tabulated Sobol for old files, for better compatibility

Brecht Van Lommel manually merged commit 61a8d1f7d0 into main

2023-10-06 20:12:13 +02:00

Alaska deleted branch fix-103284