Fixed them, thank you for spotting the memory leaks.

Edit: On further investigation it seems like the issue may be caused by the way Blender is built, E.G. The program used to compile Blender into a executable. I have not fully figured out if this is the exact cause, and if it is, what setting causes it, but it's something useful to add.

Note: Most of this is a recap of part of the devtalk page plus some new stuff

I've been testing this new sky texture implementation for Blender during the early development stages and have found one issue that confuses both @Marco (nacioss) and myself and I hope including this information here will help the Cycles developers and/or other users identify the issue.

The issue is that at certain sun rotations and elevations in the new sky texture, Cycles will have a hard time sampling the sun.
Here's an example comparing the issue I'm experiencing and the expected result (Issue on the left, expected results on the right).

Initially we thought this was due to the multi-importance sampler being at too low of a resolution. However, increasing the multi-importance sampler resolution just makes the issue worse. As in, the issue no longer occurs at that elevation and rotation, but it occurs at more of the other elevations and rotations.

Also, increasing the sun size to large values like 20 degrees still shows the same issues at those same elevations and rotations, suggesting it probably isn't the multi-importance sampler having issues with the small sun.

Upon further investigation into what's causing the issue, I found two interesting things out.

The first is that this issue doesn't seem to occur while using GPU rendering. We've tested with CUDA on a GTX 1050Ti (@Alaska (Alaska)) and RTX 2080Ti (@Marco (nacioss)). We've also tested OptiX on a RTX 2070 Super (@Alaska (Alaska)).

The second thing is, it doesn't seem to affect Linux.
I have tested a few builds from graphicall by LazyDodo for Linux and Windows during the development of the sky texture and I can reliably get Windows to produce the issue, but have never been able to get Linux to reproduce the issue.

Edit: @Marco (nacioss) is able to reproduce the issue on Linux.

Here's an example of a bunch of frames rendered out using Windows and Linux. In this case, the exact same .blend file was used (Windows on the left, Linux on the right).

Thinking that this might be a build issue, I've setup clean build environments for Blender and built my own versions based on Blender Master. The only modification being made was the addition of the sky texture with diff 25310, the latest diff at the time of writing. Once again, Windows produces issues while Linux does not.

Here's the build and system information:

Windows:
Operating system: Windows-10-10.0.18362-SP0 64 Bits
CPU: Ryzen 9 3900X

Blender version: 2.90.0 Alpha, branch: master (modified), commit date: 2020-06-02 02:54, hash: rB18cda8be8768

Linux:
Operating system: Linux-5.4.0-7634-generic-x86_64-with-debian-bullseye-sid 64 Bits
CPU: Ryzen 9 3900X

Blender version: 2.90.0 Alpha, branch: master (modified), commit date: 2020-06-02 02:54, hash: rB18cda8be8768

I have tested over 100,000 different sun elevations and rotations on Linux and have not been able to reproduce the issue. This doesn't rule out the possibility of the issue being present on my Linux build, but the results suggest it's not there.

To eliminate Ryzen being the issue (both @Marco (nacioss) and I used Ryzen 3rd gen processors), I have tested on a computer with a Intel i5-8400 with Windows. And I can reproduce the same issue. I do not own the computer so can't install Linux and test it.

So to recap, this is what we know:

1. The issue only occurs when the "Sun disk" option in the new sky texture is enabled.
2. As the multi-importance sampler resolution increases, the issue occurs at more sun elevations and rotations.
3. This issue seems to only affect CPU rendering. Rendering with CUDA (Tested with a GTX 1050Ti and RTX 2080Ti) and OptiX (Tested with a RTX 2070 Super) does not seem to show these issues based on tests done so far. I have not tested with OpenCL.
4. This issue doesn't appear to affect Linux (in my case).

How to reproduce this issue:
At the moment, it seems the sun elevations and rotations that are affected depend on the build. So giving you a set of specific angles probably isn't going to help you pin point it. However, I have created a file which has an animation testing a range of different sun locations where the issue is fairly common. I have also increased the multi-importance sampling resolution to 8192 as it causes the issue to occur more frequently. 1024 still produces issues.

Note: Testing this file on my Windows build produces the first bad result at frame 5, then the next at frame 9, and so on. Testing with the Windows build from graphicall has issues at frames 1, 3, 5, 6, 7, 8, 9, etc (see below screenshot for results).

Graphical build tested:
Blender version: 2.90 (sub 4), branch: new-sky-texture, commit date: 2020-05-30 20:17, hash: rB681838e1c94d

Also, don't know if this is related, but it was suggested on the devtalk. T69745

Outside of this issue, the new sky texture seems pretty good.

I unknowingly tested the issue @Alaska (Alaska) is describing yesteday, on Linux, using the Graphicall Linux build "blender-2.90.0-git20200601.2c6c2006c4aa-x86_64"

Edit:

The same animation without denoising

The file used:

Three frames where the sun is consistently increasing latitude, but the change in illumination is non monotonic, going up then down, causing flickering

PS really looking forward to having this improved Sky!

Thanks for the patch, the model definitely looks very impressive!

On the code side, I didn't have the time to go through the entire thing yet, but some notes:

1. The precomputation looks like something that should be easily parallelizable.
2. Defining global constants with generic names such as u is not really great, maybe put a shared prefix on all the constants
3. Nitpick: I don't like how large some of the constants are - as mentioned, I haven't looked into it in detail yet, but that looks like it's just asking for numerical instability. If we change the units, it might be possible to reformulate some things to avoid this.
4. As for the sampling: Cycles will struggle to sample a relatively tiny sun with the usual 2D map approach. I've considered this in the past when I looked into adding sun support to the current model and as far as I can see, the best approach is to move the sun contribution into a separate light source that can be handled by the directional light sampling code. To keep the usage simple, Cycles would automatically generate this light, no need for the user to do anything other that adding the node, just like now. This is not trivial since the output of the model might go through other nodes, but it's definitely doable.

Would you mind if I have a look at some of these points (especially 1 and 4) today? As I said, I played around with the sky models before so I have some old code I can use as a basis.

In D7896#191123, @Nicola De Mitri (nicolasap) wrote:

I unknowingly tested the issue @Alaska (Alaska) is describing yesteday, on Linux, using the Graphicall Linux build "blender-2.90.0-git20200601.2c6c2006c4aa-x86_64"

That's not the same issue. The issue you're facing is due to the multi-importance sampler having issues dealing with the small sun. If you increase the resolution of the multi-importance sampler or increase the size of the sun disk, the flickering should be reduced.

The issue I'm talking about is that in some scenarios it seems like the sun is basically skipped during most of the sampling process resulting in the scenes lighting being almost entirely made up of the light from just the atmospheric scattering (see the attached image). The sun is still there, but an extremely high sample count like 200,000 is needed to get enough samples to resemble the lighting that's expected.
In the case of the issue I'm facing, changing the multi-importance sampler settings doesn't fix it. Neither does increasing the sun size. Looking at your animation, you don't seem to have any frames that experience this issue. Which is a good thing for helping pin pointing the issue.

@Marco (nacioss) Is your model based on this paper? http://www-ljk.imag.fr/Publications/Basilic/com.lmc.publi.PUBLI_Article@11e7cdda2f7_f64b69/article.pdf
As far as I know, this is the most recent sky model available. Some reading available here:
https://blendermarket.com/products/physical-starlight-and-atmosphere
Regarding sun, maybe it's possible to split sun and sky and use direction light for this? As this is some kind of sky refactoring maybe I mention my old bug report here:
https://developer.blender.org/T50458

@Alex P (eneen) This is a patch review page, you better ask these things here https://devtalk.blender.org/t/new-sky-texture/12708

@Marco (nacioss) sorry, of course, thank you!

@Brecht Van Lommel (brecht) waiting for your review