Fix #111277: NaN in Vector Displacement leading to render errors #111294

Alaska · 2023-08-19T12:20:55+02:00

Alaska commented

2023-08-19 12:20:55 +02:00

Fixes NaN in Vector Displacement node caused by the normalization of
0, 0, 0 vectors.

This fixes both visual rendering issues and an "illegal address" error
on the GPU. The "illegal address" error came from the Light Tree
Sampling code not handling the NaN normals well, leading to weird code
paths being taken, eventually leading to a kernel_assert and a
user facing illegal address error.

Fixes NaN in Vector Displacement node caused by the normalization of 0, 0, 0 vectors. This fixes both visual rendering issues and an "illegal address" error on the GPU. The "illegal address" error came from the Light Tree Sampling code not handling the NaN normals well, leading to weird code paths being taken, eventually leading to a kernel_assert and a user facing illegal address error.

Alaska added the

Module

Render & Cycles

label 2023-08-19 12:20:55 +02:00

Alaska added 1 commit 2023-08-19 12:21:06 +02:00

9b4e01bef1 Fix 111277: NaN in Vector Displacement leading to render errors

Fixes NaN in Vector Displacement node caused by the normalization of
0, 0, 0 vectors.

Alaska referenced this pull request

2023-08-19 12:21:19 +02:00

Vector displacement suspected to cause geometry artefact and CUDA error #111277

Alaska commented

2023-08-19 12:33:58 +02:00

If this commit gets accepted, it needs to be backported to Blender 3.3 and 3.6.

Note 1: This fixes both the issues reported in 111277. It fixes the black faces issues (NaN normals), and the illegal address issue on CUDA.

Note 2: OSL does not need updating to use safe_normalize(). OSL always uses safe_normalize() when normalize() is called. EEVEE on the other hand already seemed to handle this case fine but I updated it to also use safe_normalize() just in case.

If this commit gets accepted, it needs to be backported to Blender 3.3 and 3.6. Note 1: This fixes both the issues reported in 111277. It fixes the black faces issues (NaN normals), and the illegal address issue on CUDA. Note 2: OSL does not need updating to use `safe_normalize()`. OSL always uses `safe_normalize()` when `normalize()` is called. EEVEE on the other hand already seemed to handle this case fine but I updated it to also use `safe_normalize()` just in case.

Alaska requested review from Brecht Van Lommel 2023-08-19 12:34:09 +02:00

Alaska requested review from Weizhen Huang 2023-08-19 12:34:16 +02:00

Alaska requested review from Sergey Sharybin 2023-08-19 12:34:24 +02:00

Alaska requested review from Lukas Stockner 2023-08-19 12:34:32 +02:00

Alaska changed title from ~~Fix 111277: NaN in Vector Displacement leading to render errors~~ to Fix #111277: NaN in Vector Displacement leading to render errors

2023-08-19 12:35:18 +02:00

Alaska added 1 commit 2023-08-19 13:08:28 +02:00

buildbot/vexp-code-patch-coordinator Build done. Details

31f72a9b1c Update EEVEE

Sergey Sharybin commented

2023-08-21 11:45:33 +02:00

@Alaska it should not be a problem to back-port to LTS releases, assuming code applies cleanly :)

Before landing to the main branch I am very curious about that Illegal CUDA Address you've mentioned. How is it NaN causing an invalid address issue?
It might be very good idea to mention this aspect of a fix, and perhaps elaborate on it in the PR description (which will become a commit message).

@Alaska it should not be a problem to back-port to LTS releases, assuming code applies cleanly :) Before landing to the main branch I am very curious about that Illegal CUDA Address you've mentioned. How is it NaN causing an invalid address issue? It might be very good idea to mention this aspect of a fix, and perhaps elaborate on it in the PR description (which will become a commit message).

Sergey Sharybin commented

2023-08-21 11:45:41 +02:00

@blender-bot package

Blender Bot commented

2023-08-21 11:45:46 +02:00

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR111294) when ready.

Alaska commented

2023-08-21 13:30:32 +02:00

Before landing to the main branch I am very curious about that Illegal CUDA Address you've mentioned. How is it NaN causing an invalid address issue?
It might be very good idea to mention this aspect of a fix, and perhaps elaborate on it in the PR description (which will become a commit message).

@Sergey, the NaN normals was leading to an issue where a light wasn't being selected in the light tree sampling code, leading to the default -1 index light being selected, which results in a kernel_assert because -1 was outside the range of the light storage being accessed, and this appears as an "illegal address" error to the user.

I added a catch case for when a light isn't selected (Which only appears to happen when a NaN ends up in the light sampling code) in this pull request: #111292

Basically, the NaN normals isn't causing the illegal address, the NaN normals was causing weird code paths to be taken in the light tree sampling code resulting in an illegal address error.

I'm not sure if I should include this in the commit message or not, but I've added an explanation anyway.

> Before landing to the main branch I am very curious about that Illegal CUDA Address you've mentioned. How is it NaN causing an invalid address issue? > It might be very good idea to mention this aspect of a fix, and perhaps elaborate on it in the PR description (which will become a commit message). @Sergey, the NaN normals was leading to an issue where a light wasn't being selected in the light tree sampling code, leading to the default -1 index light being selected, which results in a kernel_assert because -1 was outside the range of the light storage being accessed, and this appears as an "illegal address" error to the user. I added a catch case for when a light isn't selected (Which only appears to happen when a NaN ends up in the light sampling code) in this pull request: #111292 Basically, the NaN normals isn't causing the illegal address, the NaN normals was causing weird code paths to be taken in the light tree sampling code resulting in an illegal address error. --- I'm not sure if I should include this in the commit message or not, but I've added an explanation anyway.

Sergey Sharybin approved these changes 2023-08-21 15:20:36 +02:00

Sergey Sharybin left a comment

Thanks for the explanation!

And I do think it makes sense in the commit message.

Thanks for the explanation! And I do think it makes sense in the commit message.

Sergey Sharybin merged commit 7f65080ab4 into main

2023-08-21 15:22:12 +02:00

Sergey Sharybin deleted branch fix-111277

2023-08-21 15:22:14 +02:00

Sergey Sharybin referenced this issue from a commit

2023-08-21 15:22:16 +02:00

Fix #111277: NaN in Vector Displacement leading to render errors