Cycles: Remove separate OSL attribute map and instead always use SVM attribute map
ClosedPublic
Actions

Authored by Patrick Mours (pmoursnv) on Sep 8 2022, 7:34 PM.

Details

Reviewers

Brecht Van Lommel (brecht)

Maniphest Tasks

T101222: Cycles OSL with OptiX

Commits

rBef7c9e793ec5: Cycles: Remove separate OSL attribute map and instead always use SVM attribute…

Summary

The SVM attribute map is always generated and uses a simple
linear search to lookup by an opaque ID, so can reuse that for OSL
as well and simply use the attribute name hash as ID instead of
generating a unique value separately. This works for both object
and geometry attributes since the SVM attribute map already
stores both. Simplifies code somewhat and reduces memory
usage slightly.

This patch was split off from D15902.

Diff Detail

Repository: rB Blender

Event Timeline

Patrick Mours (pmoursnv) requested review of this revision.Sep 8 2022, 7:34 PM

Patrick Mours (pmoursnv) created this revision.

Harbormaster completed remote builds in B23674: Diff 55453.Sep 8 2022, 7:34 PM

https://builder.blender.org/admin/#/builders/136/builds/24

Patrick Mours (pmoursnv) mentioned this in D15902: Cycles: Add support for using OSL with OptiX.Sep 8 2022, 7:58 PM

Brecht Van Lommel (brecht) mentioned this in D15917: Cycles: Generate OSL closures using macros and a template file.Sep 8 2022, 8:04 PM

Brecht Van Lommel (brecht) requested changes to this revision.Sep 8 2022, 8:15 PM

Brecht Van Lommel (brecht) added inline comments.

intern/cycles/kernel/osl/services.cpp
1108	I would rather see `find_attribute` changed to take `sd->object, sd->prim` arguments instead of `sd`, than temporarily modifying `ShaderData` for this.
intern/cycles/kernel/types.h
658–662	Replace `size_t` with `uint64_t` here and in other places. Not sure it makes a difference in practice, but seems safer for kernel code shared between CPU and GPU.

This revision now requires changes to proceed.Sep 8 2022, 8:15 PM

Added find_attribute overload taking an object and changed attribute ID data type to uint64_t.

I also noticed a flaw: In a few places in the kernel, attributes are referenced/loaded using their standard ID (e.g. ATTR_STD_MOTION_VERTEX_POSITION to get motion vertex data in motion_triangle_vertices()). This broke since the entries in the attribute map were now all using a hash as ID in case of OSL and not that standard ID anymore. To fix this I adjusted attribute map generation to add duplicate entries for standard attributes if it detects that the attribute ID mismatches the standard ID. That way SVM is unaffected and for OSL it just adds a few small entries to the attribute map (they are just 16 bytes, so it's not the end of the world).

Harbormaster completed remote builds in B23679: Diff 55487.Sep 9 2022, 12:40 PM

Brecht Van Lommel (brecht) accepted this revision.Sep 9 2022, 2:53 PM

This revision is now accepted and ready to land.Sep 9 2022, 2:53 PM

Closed by commit rBef7c9e793ec5: Cycles: Remove separate OSL attribute map and instead always use SVM attribute… (authored by Patrick Mours (pmoursnv)). · Explain WhySep 9 2022, 3:36 PM

This revision was automatically updated to reflect the committed changes.

Patrick Mours (pmoursnv) added a commit: rBef7c9e793ec5: Cycles: Remove separate OSL attribute map and instead always use SVM attribute….

Brecht Van Lommel (brecht) added a task: T101222: Cycles OSL with OptiX.Oct 4 2022, 6:10 PM