Standalone Cycles and modern CMake

Cycles Development
#1

Hi,

This is my first post about Cycles (which I am just starting to learn about), so be nice to me, please :slight_smile:

I want to get familiar with the project and have spent some time browsing the code. I noticed that cmake is a bit outdated. So, I have put some effort into turning them into more modern, target-based cmake. My priorities were:

• compile code into shared/static libraries
• executable, libraries and headers installation
• for each library export a cmake target
• make a config to be able to import it by cmake command find_package(Cycles REQUIRED)
• using it in my project as follows: target_link_libraries(foobar PUBLIC Cycles::Cycles)

If the community is interested in this approach, I would be more than happy to keep sharing and pushing it as a work in progress branch so we could have a discussion.

Note that I am currently unaware of project constraints.
I haven’t solved problem of cyclic dependencies. I ended up heaving shared libraries with undefined symbols. Also note that some features are missing, such as OCL, Cuda kernels, but it is just a matter of time that I get more familiar with the project!

Cheers,

1 Like
#2

This also came up in a rendering meeting:

Basically we’d be happy to get a contribution to improve the build system like this. Work will be needed to keep building Cycles as part of Blender as well.

1 Like
#3

Thanks for the answer @brecht.

Can you elaborate a bit how currently Cycles is build with Blender? It seems like variables for a standalone build are hard coded and they aren’t options. Does Blender use CMake that comes with Cycles? Or does it rely on the result static lib and header files?

#4

Think it’s best to detangle a few things, using modern cmake and being able to produce shared libraries are imho two different goals.

Modern cmake is pretty much someone needing to sit down and do the work, it’s not gonna be fun but on the other hand i’m also not seeing a whole lot of issues that would prevent it.

As for shared vs static, we have build static and only static for a long long time, shared is not something that is even occasionally tested and suffers from the following problems:

  1. Since we always build static, it was real easy to make circular dependencies and we either deliberately or accidentally made quite a few of these loops in blender as a whole. These circular dependencies just cannot exist for shared libs and need to be resolved. Luckily Cycles has a much smaller surface so we may be able to resolve it there, but detangling blender as a whole is pure utter punishment. To make things worse, I’m not convinced all library dependencies are properly modelled in cmake and we’re just being lucky the linker is not throwing a fit. I would really limit my self to cycles here.

  2. Exports, different platforms have different defaults regarding exports, GCC for the longest time had the stance “Export all the symbols wither you need them or not” only more recently has -fvisibility=hidden become an option, but as for now afaik this is still opt in. The MSVC compiler took a more conservative stance from the start and requires you to mark every symbol you want to export with an attribute and they have prevented the “export all symbols” option all together.

imho MS did the right thing here, any symbols exported by a library has to be a conscious choice by the library authors to prevent people from relying on internal API’s that may change at any time. Fixing this will require putting proper export/import attributes on all headers that need to be exported. This may tie in to @KevinDietrich 's project of modernizing the API. There still may be some gray zone here what is public api and what is internal api that may need to be resolved.

CMake offers the WINDOWS_EXPORT_ALL_SYMBOLS option to simulate the GCC behavior, however I’d really prefer if we did not go this route.

The route I’d personally take would probably be

  1. Detangle loops, while relying on all symbols being exported (by either building on linux or enabling the cmake option on windows)
  2. Export only the public api
  3. Modern cmake.
1 Like
#5

Best to look at the actual code in the Blender repo to see how it works specifically. Most of the CMake code is shared, with the difference being mainly in the top level CMake file and external library detection.

Right, Cycles does not have a clear public/private API yet, that is still being worked on. The CMake side of shared library build can be implemented already, in preparation for code changes to mark symbols as public or hidden.

1 Like
#6

Thanks @brecht

I pulled Blender’s code and I found Cycles in intern/cycles directory, but it looks to me as an exact copy, not a sub-module (yet?). How code duplication is being managed?

After few minutes of grepping, I discovered bf_intern_cycles library. And if I am not mistaken, to preserve backward compatibility, we have to produce a library with that name, because blender links with it.
Also, it looks like internally, within Cycles repo, we have a freedom to produce libraries as we wish. Please, correct me if I am wrong.

#7

Spend some time mapping out the dependency graph between the various cycles modules, although i cannot guarantee i didn’t miss anything I’m pretty confident it is pretty close to reality. The graphs are extracted from the symbols imported/exported from the individual static libs. (nm -C *.a > symbols.txt)

cycles

Arrow pointing means uses functionality from, ie libcycles_graph imports ccl::util_aligned_free(void*) from libcycles_util, a red arrow means there is a direct circular dependency between the libraries.

I’d probably try to convert the libraries to shared one at a time, the circular dependency between libcycles_util and libcycles_bvh is particularly weak, it’s a single call.

libcycles_util imports ccl::bvh_layout_name(ccl::KernelBVHLayout) from libcycles_bvh

I have not looked if that import is absolutely needed in libcycles_util, but if one were to break that circular dependency it straightens out the graph nicely and opens up libcycles_graph as low hanging fruit for a shared lib.

cycles_util

from there on it’ll be much much less straight forward.

#8

@LazyDodo

Cool visualization :slight_smile: I made same observation about libcycles_util.

Also, I checked what is required by util from bvh. It turns out that BVHLayout is an enum that is stored as a field in DebugFlags class. BVHLayout is initialized by default to BVH_LAYOUT_AUTO.
In the end, BVHLayout is only needed by std::ostream& operator<<(std::ostream& os, DebugFlagsConstRef debug_flags) when it’s being converted to a string representation.

I have a suggestion and a question at the same time. To remove a dependency between util and bvh, can’t we store a string representation of the layout, instead of BVHLayout?

It seems like DebugFlags is a singleton with a private constructor:

// util_debug.h
static DebugFlags &get()
{
  static DebugFlags instance;
  return instance;
}

inline DebugFlags &DebugFlags()
{
  return DebugFlags::get();
}

If not a string representation saved in DebugFlags. Another approach that I see could be to rename(or not :slight_smile: ) DebugFlags to DeviceDebugFlags and move to device/device_debug.h. What do you think? That sounds like more logical place for that class. I checked in Blender code repo and DebugFlags are not used anywhere. It seems like it’s a private header and it’s safe to move/modify it.

Let me know what are your thoughts.

#9

Think you are making it much more difficult than it needs to be, given it’s a debug helper function anyhow, you could probably justify moving bvh_layout_name it into libcycles_utiland be done with it.

that resolves the circular dependency issue with this library, however remember when i said

yeahhhh… at link time the linker throws a fit, cycles_util can’t link shared since it doesn’t know it has a dep on glog/guardedmalloc/numa so there’s missing symbols to deal with, on top of that I’m not entirely sure how if multiple copies of guardedmalloc/glog will play nicely together, so that may need to move shared first.

Even when limiting your self to cycles this will be neither a fun nor easy project.

#10

@LazyDodo Think you are making it much more difficult than it needs to be, given it’s a debug helper function anyhow, you could probably justify moving bvh_layout_name it into libcycles_utiland be done with it.

I have to say I disagree :slight_smile: Header file util_debug.h will still contain #include "bvh/bvh_params.h" and this is a dependency unfortunately.

I made a patch for you to review, but I am not sure where I should upload it. I found Blender’s differential where previously I uploaded one of my patches. My patch has been accepted, but has it been applied to Standalone Cycles’ master? I can’t find that information anywhere. I am used to github/gitlab git front end and all of it, especially separation of Blender and Cycles is quite confusing.

I can’t find differential for Standalone Cycles. Is there any? Can someone guide me please how can I apply a patch to Standalone Cycles?

#11

I don’t do cycles reviews, the patch you submitted was applied to blender master, usually this get synced to the cycles standalone repo after major blender releases.

As brecht suggested previously this work will have to be done in the blender repo not the standalone one

as for the header dependency, you are right it is a dep, and I agree a neater solution would be nicer, but for now it won’t cause any issues, baby steps, for now i’m just looking to link cycles_util shared, not looking for final quality code here.

1 Like