Page MenuHome

Fix cycles standalone build
ClosedPublic

Authored by Charles Flèche (charlesf) on Dec 30 2020, 5:08 PM.

Details

Summary

Cycles standalone doesn't build properly from master because of changes in
the cycles API. This diff intends to allow building cycles standalone again.

Build system:

  • States cycles intern dependencies to build
  • Set default CYCLES_INSTALL_PATH to CMAKE_INSTALL_PREFIX

By default with a make cycles this will build to
${CMAKE_BINARY_DIR}/bin

Fixes after API changes:

  • Add accessors for Camera screen_width, screen_height and screen_resolution
  • Use the new NODE_SOCKET_API based API

Diff Detail

Repository
rB Blender

Event Timeline

Charles Flèche (charlesf) requested review of this revision.Dec 30 2020, 5:08 PM
Charles Flèche (charlesf) created this revision.

Tested this patch locally, everything works fine, cycles standalone is buildable again.

(For reference, the commit that made cycles standalone stop working is (I think) 31a620b9420cab6292b0aa1ea21c9dd1cf70b8bc )

Resigning as a reviewer, as I'm not involved with Cycles directly.

I haven't tested this yet but this looks OK. However, I don't think Camera.width and Camera.height have to be exposed (through Camera.screen_width() and Camera.screen_height() here). Essentially, those will be updated before rendering anyway from options.width and options.height, which are passed to Session.params. For the same reason, neither do I think set_screen_size_and_resolution should be used here, if the parameters are setup correctly, rendering should be fine. I can verify this and cleanup this logic if needed before committing though.

One thing I am not sure about is the changes in the CMake files, I don't think they should be part of the same patch as this does not come from the API refactor, @Brecht Van Lommel (brecht) is it OK in one patch to fix it all, or should they be committed separately?

(On a side note, I should also make sure that the standalone app also compiles with D10082.)

I prefer the CMake changes to be committed separately, but it's no big deal either way.

That part of tbe patch looks good to me, will leave reviewing the rest to @Kévin Dietrich (kevindietrich).

intern/cycles/app/cycles_xml.cpp
427 ↗(On Diff #32342)

Hey I had a look into the code of array and this one copies the vector. The unnecessary copy can be removed with little effort of changing xml_read* functions

It is possible to not expose Camera.width, Camera.height, and Camera.resolution, but for that to work properly we need to use Camera.full_width and Camera.full_height in Camera.compute_auto_viewplane() so the aspect ratio is not computed with outdated values when resizing the window. Width, height, and resolution can then stay private and for internal use only.

intern/cycles/app/cycles_xml.cpp
427 ↗(On Diff #32342)

It could be good to avoid this copy, but such a change would be a bit involved for this compilation fix I think. Then, if performance was a problem, I would blame using XML over this copy.