This commit fixes a issue where Cycles adaptive kernel compilation
would always undefine adaptive kernel features, resulting in various
issues like incorrect renders.
Pull Request: https://projects.blender.org/blender/blender/pulls/137804
The crash was caused by an overflow in the opgl_path_segment_storage
array. It happened because shadow catcher paths would write to the
segments but not clear them. This made it so the next render loop
iteration for the main path starts with non-empty segments in the
guiding data.
Disable training when megakernel is called for the shadow catcher
state.
To ensure this issue is not forgotten when the guiding is ported to
GPU add asserts in the `guiding.h`. While it is a no-op for default
GPU kernels sometimes we do compile debug kernels. But also it acts
as a plain-text reminder to the future-us in working on the code.
There is now also an assert before the main path megakernel to help
catching such cases in the future.
Pull Request: https://projects.blender.org/blender/blender/pulls/137291
The goal is to reduce the affect of the fmod() used in the noise code,
which was initially reported in the comment:
https://projects.blender.org/blender/blender/pulls/119884#issuecomment-1258902
Basic idea is to benefit from SIMD vectorization on CPU.
Tested on Linux i9-11900K and macOS on M2 Ultra, in both cases performance
after this change is very close to what it could be with the fmod() commented
out (the call itself, `p = p + precision_correction`).
On macOS the penalty of fmod() was about 10%, on Linux it was closer to 30%
when built with GCC-13. With Linux builds from the buildbot it is more like 18%.
The optimization is only done for 3d and 4d noise. It might be possible to
gain some performance improvement for 1d and 2d cases, but the approach would
need to be different: we'd need to optimize scalar version fmodf(). Maybe
tricks with integer cast will be faster (since we are a bit optimistic in the
kernel and do not guarantee exact behavior in extreme cases such as NaN inputs).
Pull Request: https://projects.blender.org/blender/blender/pulls/137109
Commit be63ebd961
This is causing issues with CUDA kernel compilation in some setups, even
though the builbot is ok. Since this isn't yet working for oneAPI anyway,
revert all the changes to Cycles kernel compilation for now.
Use VERBATIM to ensure spaces inside command line arguments don't get
escaped automatically.
On Linux and Windows the oneAPI kernel compilation still has problems.
There is an apparent bug with single quote escaping in add_custom_command
which means it's not easy to use VERBATIM.
The current logic disables WITH_CYCLES_ONEAPI_BINARIES when ocloc is not
found, which is fine, but prevented building for other non-Intel SYCL
targets without (unnecessary) ocloc.
The fix here is to remove spir64_gen target when
WITH_CYCLES_ONEAPI_BINARIES is disabled, instead of forcing only spir64.
Reduce the register pressure and branching in the switch() by using
subclass and cast from void* to the base class.
This ensures intersection functions are not inlined multiple times,
bringing performance back.
Alternative could be to avoid functions (they are quite large) but
that only partially resolves the performance regression.
Pull Request: https://projects.blender.org/blender/blender/pulls/136823
The initial issues that led to the choice of forcing the use of
linker.exe seem gone and there is currently no strong reason to use
linker.exe explicitly, so let's simplify and use the default setting.
The initial limitation preventing from using -ffast-math, worked around
in 09df1f4caf, got fixed upstream in LLVM
and the fix is part of current DPC++ compiler:
63ecd2a725
We're now able to go back to using -ffast-math, which helps simplifying
the set of compiler flags.
No performance nor conformance change is expected from this change (most
of the gain is achieved already with the use of -cl-fast-relaxed-math
since 284b89a0a3) and this has been
verified on Arc B580 under Windows.
HIP-RT functions do have access to kg, and it was used inconsistently:
some functions were passed actual kg, other were passed nullptr.
This change makes it consistent and passes kg everywhere.
Pull Request: https://projects.blender.org/blender/blender/pulls/136503
The code before this change was relying on the ShadowPayload have
the same "header" as RayPayload for some of the primitive types
(curve, motion triangle, point): intersection functions were shared
between "regular" and shadow rays (shadow in this case is shadow_all),
but extra filter function was used for shadow rays.
This is fragile if someone changes one of these structures. What is
worse is that compiler might actually decide to shuffle things in
some structs, or remove unused fields.
This change also solves confusion about ShadowPayload::prim_type
seemingly only being assigned to PRIMITIVE_NONE. With time it is
not impossible that compiler will also see this, and constant-fold
some checks, or even remove the field. If that happens then the
render result will be wrong. Maybe it is already happening as there
are some GPU and driver and optimization flag specific bugs in the
area.
It is unclear whether it was causing any actual problem: W7800
seems to render all hair correctly on Linux.
Also make some style decisions more consistent: for example,
the way how stop/continue search return value is commented.
Prefer lower vertical space for those.
Mainly readability purposes:
- Having variables called local_payload is ambiguous: does it refer to
LocalPayload type or to a variable be local in a function?
- Some of the functions are used for different ray types, so having the
type case in intersectFunc and filterFunc makes it easier to scan.
For the latter: now it is more obvious that Curve_Intersect_Shadow
expects RayPayload, but Curve_Filter_Shadow expects ShadowPayload.
It might not be a problem currently as ShadowPayload has the same
"header" RayPayload, but it might change in the future. Also, compiler
might optimize fields out from one but not from the other.
There is a known precision bug in the current HIP compiler version (RDNA2 family/Windows) that has already been fixed and will be available in
a future HIP SDK release. Enabling more precise math prevents the artifacts.
This may cause a 5-10% performance drop in some scenes.
Fix#136138: Microfacet BSDF
Fix#136449: Hair BSDF
Pull Request: https://projects.blender.org/blender/blender/pulls/136341
The new correction avoids washed out areas near the shadow terminator,
preserving more detail from normal and bump maps.
It implements the method from the paper "A Microfacet-Based Shadowing
Function to Solve the Bump Terminator Problem" by Alejandro Conty Estevez,
Pascal Lecocq, and Clifford Stein.
Pull Request: https://projects.blender.org/blender/blender/pulls/135380
This makes it possible to restore previous Blender 4.3 behavior of bump
mapping, where the large filter width was sometimes (ab)used to get a bevel
like effect on stepwise textures.
For bump from the displacement socket, filter width remains fixed at 0.1.
Ref #133991, #135841
Pull Request: https://projects.blender.org/blender/blender/pulls/136465
On the user level spatial splits on hair BVH leads to very long build times,
without giving too much advantage in the render times.
There is also some issues and possibly bugs in the builder which lead to all
sort of numerical issues (like divisions by zero). There are also performance
issues that comes from the fact that the alignment space is applied every time
primitive's aligned bounds are requested. It also seems that the splitting
might not be considering aligned space consistently when calculating SAH and
performing splits.
It does sound like issues we'd get fixed ideally, but the importance of the
BVH2 is fading out with the HW-RT becoming more and more popular.
This change contains fix needed for the split algorithm to avoid numerical
issue reported by UBSAN when rendering the `BVH2 particle simple.blend` from
the #126508.
Ref #126508
Ref #136245
Pull Request: https://projects.blender.org/blender/blender/pulls/136430
The transparent bounce test was too optimistic in regards to the intersection
being considered. The check needs to happen after it has been validated that
it is not duplicate.
It was already the case for Metal and HIP-RT, but not for Embree and BVH2.
Tests updated by: Alaska <Alaskayou01@gmail.com>
Pull Request: https://projects.blender.org/blender/blender/pulls/136325
The reason for this to happen is because when spatial split is used
the same intersection could be recorded twice (via different BVH nodes).
This change introduces check for the intersection being already recoded,
similar to the check in the local BVH. The check is done during BVH
intersection which allows to properly ignore intersections even for the
maximum bounce number check. A faster approach would be to do such
filtering after sorting, but then we can not keep bounce check in the
BVH code consistent with and without spatial splits.
Intuitively it seems that it should be possible to merge the new loop
with the one that checks for which intersection to keep. But it is not
so trivial in practice: it doesn't run for all intersections, and also
it is formulated in a way that updates isect_index for the next record.
Pull Request: https://projects.blender.org/blender/blender/pulls/136251
The code which was checking whether local intersection is to be
recorded, and under which index was duplicated for triangles,
motion triangles, and HIP-RT triangle filter function.
This change moves the common logic to an utility function which
is reused from all the places mentioned above.
Pull Request: https://projects.blender.org/blender/blender/pulls/136244
There is code that properly handles panoramic cameras in
`camera_world_to_ndc`, the transform matrices (e.g.
`OSLRenderServices::get_inverse_matrix`) in the `transform("NDC", P)`
call dont do the "full work" here (maybe they should though?).
But we can get to `camera_world_to_ndc` by just getting the "NDC"
attribute, so use that for now.
Pull Request: https://projects.blender.org/blender/blender/pulls/136097
This change fixes the remaining failing tests with SSS when using HIP-RT.
This includes crash when SSS is used on curves, and objects with motion
blur and SSS rendering black.
The root cause for both cases was the fact that traversal was always
assuming regular BVH (built for triangles), while curves and motion
triangles are using custom primitives, which requires specialized BVH
traversal.
This change includes:
- Early output from `scene_intersect_local()` for non-triangle and
non-motion-triangle primitives. This fixes `sss_hair.blend` test,
and also avoids unnecessary BVH traversal when the local intersection
is requested from curve object. The same early-output could be added
to other BVH traversal implementation.
- Use `hiprtGeomCustomTraversalAnyHitCustomStack` for motion triangles
primitives. This fixes motion blur on objects with SSS render black.
Fixes#135856
Co-authored-by: Sahar A. Kashi <sahar.alipourkashi@amd.com>
Co-authored-by: Sergey Sharybin <sergey@blender.org>
Pull Request: https://projects.blender.org/blender/blender/pulls/135943
Profiling on Arc B580 shown that sd->num_closure queries were often
stalling.
Packing it closer to other often accessed elements within ShaderData
(type, flag..) does speedup rendering by ~5% in most scenes.
Pull Request: https://projects.blender.org/blender/blender/pulls/135980
While auto lets the compiler make the right choice for shade_surface
kernel when compiling for Battlemage and Lunar Lake, that's not the case
for Alchemist and Meteor Lake, so now we force this mode.
It was always hard-coded to be 0.
It does not seem to result in any extra tests passing, but they are
probably not sophisticated enough.
Noticed while looking into details for the #135856.
Pull Request: https://projects.blender.org/blender/blender/pulls/135878
* Add SubdAttributeInterpolation class for linear attribute interpolation.
* Dicing computes ptex UV and face ID for interpolation.
* Simplify mesh storage of subd primitive counts
* Remove kernel code for subd attribute interpolation
* Remove patch table packing and upload
The old optimization adds a fair amount of complexity to the kernel, affecting
performance even when not using the feature. It's also not that useful as it
does not work for UVs that needs special interpolation. With this simpler code
it should be easier to make it feature complete.
Pull Request: https://projects.blender.org/blender/blender/pulls/135681
Make the ray self primitives store and restore reliable for cases when
the intersect_shadow kernel is called multiple times:
- Light object and primitive are stored in dedicated fields in the
state. This adds 2 integers per state.
- The self object and primitive are used from the previous intersection
when the intersect_shadow is called multiple times.
There is more detailed explanation added in the code.
The issue was introduced by the light refactor to be objects in #134846.
Pull Request: https://projects.blender.org/blender/blender/pulls/135573
