AmplifyCreations / AmplifyShaderEditor-Feedback

4 stars 0 forks source link

Normal Reconstruct Z: Could make normalization optional #342

Closed OwenTheProgrammer closed 3 weeks ago

OwenTheProgrammer commented 1 month ago

image image

Because of the way this function is structured, we can guarantee the length of the resulting vector will be 1 unit in length, thus normalized. This will not be recognized by the compiler, meaning the normalization will not be optimized away automatically.

Math Proof

\begin{aligned}
\sqrt{x^2+y^2+z^2} &= 1\quad \text{Solve for } z \\
\left(\sqrt{x^2+y^2+z^2}\right)^{\textcolor{cyan}{2}} &= 1^{\textcolor{cyan}{2}} \quad \left(\sqrt{v}\right)^2 = |v| \\
|x^2+y^2+z^2| &= 1  \quad \{ \forall v \in \mathbb{R} \;|\; 0 \le v^2 \} \implies |v^2| = v^2\\
x^2 + y^2 + z^2 &= 1 \\
x^2+y^2+z^2 \textcolor{cyan}{- \left(x^2+y^2\right)} &= 1 \textcolor{cyan}{- \left(x^2+y^2\right)} \\
z^2 &= 1 - x^2 + y^2 \\
\textcolor{cyan}{\sqrt{z^2}} &= \sqrt{1 - x^2 + y^2} \quad \sqrt{z^2} = z \\
z &= \sqrt{1 - x^2 + y^2} = \textcolor{green}{\sqrt{1 - \left(x, y\right) \cdot \left(x, y\right)}}
\end{aligned}

Shader Assembly Difference

Current Node Proposed New Node
image image

This removes 3 shader instructions for free

This will also put this node ahead of SG in terms of performance, as they also implement it the same way

OwenTheProgrammer commented 1 month ago

Those nodes by request

OwenTheProgrammer commented 1 month ago

Ah, for safety of the xy components not being within the ranges and out of bounds like NaN values, I'll change the topic to request normalization as an option

Dawie3565 commented 1 month ago

http://paste.amplify.pt/view/raw/ad521618 --- add function switch to make option --- set default as included for backward compatible --- added sticky notes image

diogovtx commented 3 weeks ago

I agree the normalization should be optional. However, we need to keep in ON by default to ensure backwards compatibility.

Just a note about the proof: we can't ensure the inputs are normalized, and often they aren't (e.g. sampling normal maps), so normalization is sometimes necessary.

This is a very quick change, so I moved it up to v1.9.7.