[Bf-committers] [Bf-blender-cvs] [2f79d1c0584] master: Cycles: Replace use_qbvh boolean flag with an enum-based property

Jaros Milan milan.jaros at vsb.cz
Tue Jan 23 10:19:59 CET 2018


Hi Sergey,

It is great, I am working on BVH16 (Skylake). It could be to use for other types: BVH8, BVH_EMBREE.

There is some bugs in BVH8 (https://developer.blender.org/D2875) and it could be to change from AVX2 to AVX.

I am working on new tests. Older tests you can find here: http://blender.it4i.cz/research/bounding-volume-hierarchy-bvh.

Best regards

Milan

-----Original Message-----
From: Bf-committers [mailto:bf-committers-bounces at blender.org] On Behalf Of Sergey Sharybin
Sent: Monday, January 22, 2018 5:49 PM
To: Blender Developers <bf-committers at blender.org>
Subject: Re: [Bf-committers] [Bf-blender-cvs] [2f79d1c0584] master: Cycles: Replace use_qbvh boolean flag with an enum-based property

Hi,

The commit message is a bit of of date actually (didn't notice this before `git push` was done..).

Basically, all the TODOs from the message are solved :)

On Mon, Jan 22, 2018 at 5:22 PM, Sergey Sharybin <noreply at git.blender.org>
wrote:

> Commit: 2f79d1c0584f4d72984e56db1f5878be3360e044
> Author: Sergey Sharybin
> Date:   Fri Jan 19 10:59:58 2018 +0100
> Branches: master
> https://developer.blender.org/rB2f79d1c0584f4d72984e56db1f5878be3360e0
> 44
>
> Cycles: Replace use_qbvh boolean flag with an enum-based property
>
> This was we can introduce other types of BVH, for example, wider ones, 
> without causing too much mess around boolean flags.
>
> Thoughs:
>
> - Ideally device info should probably return bitflag of what BVH types it
>   supports.
>
>   It is possible to implement based on simple logic in device/ and 
> mesh.cpp,
>   rest of the changes will stay the same.
>
> - Not happy with workarounds in util_debug and duplicated enum in kernel.
>   Maybe enbum should be stores in kernel, but then it's kind of weird 
> to include
>   kernel types from utils. Soudns some cyclkic dependency.
>
> Reviewers: brecht, maxim_d33
>
> Reviewed By: brecht
>
> Differential Revision: https://developer.blender.org/D3011
>
> ===================================================================
>
> M       intern/cycles/blender/addon/properties.py
> M       intern/cycles/blender/addon/ui.py
> M       intern/cycles/blender/blender_python.cpp
> M       intern/cycles/blender/blender_sync.cpp
> M       intern/cycles/bvh/bvh.cpp
> M       intern/cycles/bvh/bvh4.cpp
> M       intern/cycles/bvh/bvh_params.h
> M       intern/cycles/device/device.cpp
> M       intern/cycles/device/device.h
> M       intern/cycles/device/device_cpu.cpp
> M       intern/cycles/device/device_cuda.cpp
> M       intern/cycles/device/device_network.cpp
> M       intern/cycles/device/device_opencl.cpp
> M       intern/cycles/kernel/bvh/bvh_local.h
> M       intern/cycles/kernel/bvh/bvh_shadow_all.h
> M       intern/cycles/kernel/bvh/bvh_traversal.h
> M       intern/cycles/kernel/bvh/bvh_volume.h
> M       intern/cycles/kernel/bvh/bvh_volume_all.h
> M       intern/cycles/kernel/kernel_types.h
> M       intern/cycles/render/mesh.cpp
> M       intern/cycles/render/scene.h
> M       intern/cycles/util/util_debug.cpp
> M       intern/cycles/util/util_debug.h
>
> ===================================================================
>
> diff --git a/intern/cycles/blender/addon/properties.py
> b/intern/cycles/blender/addon/properties.py
> index 7410cc32342..a63d84f2da6 100644
> --- a/intern/cycles/blender/addon/properties.py
> +++ b/intern/cycles/blender/addon/properties.py
> @@ -47,6 +47,11 @@ enum_displacement_methods = (
>      ('BOTH', "Both", "Combination of displacement and bump mapping"),
>      )
>
> +enum_bvh_layouts = (
> +    ('BVH2', "BVH2", "", 1),
> +    ('BVH4', "BVH4", "", 2),
> +    )
> +
>  enum_bvh_types = (
>      ('DYNAMIC_BVH', "Dynamic BVH", "Objects can be individually 
> updated, at the cost of slower render time"),
>      ('STATIC_BVH', "Static BVH", "Any object modification requires a 
> complete BVH rebuild, but renders faster"), @@ -670,7 +675,11 @@ class 
> CyclesRenderSettings(bpy.types.PropertyGroup):
>          cls.debug_use_cpu_sse41 = BoolProperty(name="SSE41", default=True)
>          cls.debug_use_cpu_sse3 = BoolProperty(name="SSE3", default=True)
>          cls.debug_use_cpu_sse2 = BoolProperty(name="SSE2", default=True)
> -        cls.debug_use_qbvh = BoolProperty(name="QBVH", default=True)
> +        cls.debug_bvh_layout = EnumProperty(
> +                name="BVH Layout",
> +                items=enum_bvh_layouts,
> +                default='BVH4',
> +                )
>          cls.debug_use_cpu_split_kernel = BoolProperty(name="Split 
> Kernel", default=False)
>
>          cls.debug_use_cuda_adaptive_compile = 
> BoolProperty(name="Adaptive Compile", default=False) diff --git 
> a/intern/cycles/blender/addon/ui.py
> b/intern/cycles/blender/addon/ui.py
> index 5d58ecc99eb..543ba7c6d4d 100644
> --- a/intern/cycles/blender/addon/ui.py
> +++ b/intern/cycles/blender/addon/ui.py
> @@ -1609,7 +1609,7 @@ class CYCLES_RENDER_PT_debug(CyclesButtonsPanel,
> Panel):
>          row.prop(cscene, "debug_use_cpu_sse41", toggle=True)
>          row.prop(cscene, "debug_use_cpu_avx", toggle=True)
>          row.prop(cscene, "debug_use_cpu_avx2", toggle=True)
> -        col.prop(cscene, "debug_use_qbvh")
> +        col.prop(cscene, "debug_bvh_layout")
>          col.prop(cscene, "debug_use_cpu_split_kernel")
>
>          col.separator()
> diff --git a/intern/cycles/blender/blender_python.cpp
> b/intern/cycles/blender/blender_python.cpp
> index 687ddd9e7c3..792597cbad5 100644
> --- a/intern/cycles/blender/blender_python.cpp
> +++ b/intern/cycles/blender/blender_python.cpp
> @@ -69,7 +69,7 @@ bool debug_flags_sync_from_scene(BL::Scene b_scene)
>         flags.cpu.sse41 = get_boolean(cscene, "debug_use_cpu_sse41");
>         flags.cpu.sse3 = get_boolean(cscene, "debug_use_cpu_sse3");
>         flags.cpu.sse2 = get_boolean(cscene, "debug_use_cpu_sse2");
> -       flags.cpu.qbvh = get_boolean(cscene, "debug_use_qbvh");
> +       flags.cpu.bvh_layout = (BVHLayout)get_enum(cscene,
> "debug_bvh_layout");
>         flags.cpu.split_kernel = get_boolean(cscene, 
> "debug_use_cpu_split_kernel");
>         /* Synchronize CUDA flags. */
>         flags.cuda.adaptive_compile = get_boolean(cscene, 
> "debug_use_cuda_adaptive_compile");
> diff --git a/intern/cycles/blender/blender_sync.cpp
> b/intern/cycles/blender/blender_sync.cpp
> index b7c6fbc9d29..283aa5600fd 100644
> --- a/intern/cycles/blender/blender_sync.cpp
> +++ b/intern/cycles/blender/blender_sync.cpp
> @@ -662,7 +662,7 @@ SceneParams 
> BlenderSync::get_scene_params(BL::Scene&
> b_scene,
>                 params.texture_limit = 0;
>         }
>
> -       params.use_qbvh = DebugFlags().cpu.qbvh;
> +       params.bvh_layout = DebugFlags().cpu.bvh_layout;
>
>         return params;
>  }
> diff --git a/intern/cycles/bvh/bvh.cpp b/intern/cycles/bvh/bvh.cpp 
> index 6531ca50d9a..5bfe9d4ed1a 100644
> --- a/intern/cycles/bvh/bvh.cpp
> +++ b/intern/cycles/bvh/bvh.cpp
> @@ -26,10 +26,46 @@
>  #include "bvh/bvh_node.h"
>
>  #include "util/util_foreach.h"
> +#include "util/util_logging.h"
>  #include "util/util_progress.h"
>
>  CCL_NAMESPACE_BEGIN
>
> +/* BVH Parameters. */
> +
> +const char *bvh_layout_name(BVHLayout layout) {
> +       switch(layout) {
> +               case BVH_LAYOUT_BVH2: return "BVH2";
> +               case BVH_LAYOUT_BVH4: return "BVH4";
> +               case BVH_LAYOUT_NONE: return "NONE";
> +               case BVH_LAYOUT_ALL:  return "ALL";
> +       }
> +       LOG(DFATAL) << "Unsupported BVH layout was passed.";
> +       return "";
> +}
> +
> +BVHLayout BVHParams::best_bvh_layout(BVHLayout requested_layout,
> +                                     BVHLayoutMask supported_layouts) 
> +{
> +       const BVHLayoutMask requested_layout_mask =
> (BVHLayoutMask)requested_layout;
> +       /* Check whether requested layout is supported, if so -- no 
> + need
> to do
> +        * any extra computation.
> +        */
> +       if(supported_layouts & requested_layout_mask) {
> +               return requested_layout;
> +       }
> +       /* Some bit magic to get widest supported BVH layout. */
> +       /* This is a mask of supported BVH layouts which are narrower 
> + than
> the
> +        * requested one.
> +        */
> +       const BVHLayoutMask allowed_layouts_mask =
> +               (supported_layouts & (requested_layout_mask - 1));
> +       /* We get widest from allowed ones and convert mask to actual
> layout. */
> +       const BVHLayoutMask widest_allowed_layout_mask =
> __bsr(allowed_layouts_mask);
> +       return (BVHLayout)widest_allowed_layout_mask;
> +}
> +
>  /* Pack Utility */
>
>  BVHStackEntry::BVHStackEntry(const BVHNode *n, int i) @@ -51,10 
> +87,17 @@ BVH::BVH(const BVHParams& params_, const vector<Object*>& 
> objects_)
>
>  BVH *BVH::create(const BVHParams& params, const vector<Object*>& 
> objects)  {
> -       if(params.use_qbvh)
> -               return new BVH4(params, objects);
> -       else
> -               return new BVH2(params, objects);
> +       switch(params.bvh_layout) {
> +               case BVH_LAYOUT_BVH2:
> +                       return new BVH2(params, objects);
> +               case BVH_LAYOUT_BVH4:
> +                       return new BVH4(params, objects);
> +               case BVH_LAYOUT_NONE:
> +               case BVH_LAYOUT_ALL:
> +                       break;
> +       }
> +       LOG(DFATAL) << "Requested unsupported BVH layout.";
> +       return NULL;
>  }
>
>  /* Building */
> @@ -248,7 +291,8 @@ void BVH::pack_instances(size_t nodes_size, size_t
> leaf_nodes_size)
>          * BVH's are stored in global arrays. This function merges 
> them into the
>          * top level BVH, adjusting indexes and offsets where appropriate.
>          */
> -       const bool use_qbvh = params.use_qbvh;
> +       /* TODO(sergey): This code needs adjustment for wider BVH than 4.
> */
> +       const bool use_qbvh = (params.bvh_layout == BVH_LAYOUT_BVH4);
>
>         /* Adjust primitive index to point to the triangle in the 
> global array, for
>          * meshes with transform applied and already in the top level BVH.
> diff --git a/intern/cycles/bvh/bvh4.cpp b/intern/cycles/bvh/bvh4.cpp 
> index b910feccec8..4faf47af7bb 100644
> --- a/intern/cycles/bvh/bvh4.cpp
> +++ b/intern/cycles/bvh/bvh4.cpp
> @@ -55,7 +55,7 @@ static bool node_qbvh_is_unaligned(const BVHNode 
> *node)  BVH4::BVH4(const BVHParams& params_, const vector<Object*>& 
> objects_)
>  : BVH(params_, objects_)
>  {
> -       params.use_qbvh = true;
> +       params.bvh_layout = BVH_LAYOUT_BVH4;
>  }
>
>  void BVH4::pack_leaf(const BVHStackEntry& e, const LeafNode *leaf) 
> diff --git a/intern/cycles/bvh/bvh_params.h b/intern/cycles/bvh/bvh_ 
> params.h index 7dd699b33a4..89a379cf356 100644
> --- a/intern/cycles/bvh/bvh_params.h
> +++ b/intern/cycles/bvh/bvh_params.h
> @@ -24,11 +24,29 @@
>
>  CCL_NAMESPACE_BEGIN
>
> +/* Layout of BVH tree.
> + *
> + * For example, how wide BVH tree is, in terms of number of children
> + * per node.
> + */
> +typedef KernelBVHLayout BVHLayout;
> +
> +/* Names bitflag type to denote which BVH layouts are supported by
> + * particular area.
> + *
> + * Bitflags are the BVH_LAYOUT_* values.
> + */
> +typedef int BVHLayoutMask;
> +
> +/* Get human readable name of BVH layout. */ const char 
> +*bvh_layout_name(BVHLayout layout);
> +
>  /* BVH Parameters */
>
>  class BVHParams
>  {
>  public:
> +
>         /* spatial split area threshold */
>         bool use_spatial_split;
>         float spatial_split_alpha;
> @@ -50,8 +68,8 @@ public:
>         /* object or mesh level bvh */
>         bool top_level;
>
> -       /* QBVH */
> -       bool use_qbvh;
> +       /* BVH layout to be built. */
> +       BVHLayout bvh_layout;
>
>         /* Mask of primitives to be included into the BVH. */
>         int primitive_mask;
> @@ -98,7 +116,7 @@ public:
>                 max_motion_curve_leaf_size = 4;
>
>                 top_level = false;
> -               use_qbvh = false;
> +               bvh_layout = BVH_LAYOUT_BVH2;
>                 use_unaligned_nodes = false;
>
>                 primitive_mask = PRIMITIVE_ALL; @@ -119,6 +137,14 @@ 
> public:
>
>         __forceinline bool small_enough_for_leaf(int size, int level)
>         { return (size <= min_leaf_size || level >= MAX_DEPTH); }
> +
> +       /* Gets best matching BVH.
> +        *
> +        * If the requested layout is supported by the device, it will 
> + be
> used.
> +        * Otherwise, widest supported layout below that will be used.
> +        */
> +       static BVHLayout best_bvh_layout(BVHLayout requested_layout,
> +                                        BVHLayoutMask 
> + supported_layouts);
>  };
>
>  /* BVH Reference
> diff --git a/intern/cycles/device/device.cpp 
> b/intern/cycles/device/device.cpp index d55a999c454..1ec0bc3e1c6 
> 100644
> --- a/intern/cycles/device/device.cpp
> +++ b/intern/cycles/device/device.cpp
> @@ -362,7 +362,7 @@ DeviceInfo Device::get_multi_device(const 
> vector<DeviceInfo>& subdevices, int th
>         info.has_fermi_limits = false;
>         info.has_half_images = true;
>         info.has_volume_decoupled = true;
> -       info.has_qbvh = true;
> +       info.bvh_layout_mask = BVH_LAYOUT_ALL;
>         info.has_osl = true;
>
>         foreach(const DeviceInfo &device, subdevices) { @@ -399,7 
> +399,7 @@ DeviceInfo Device::get_multi_device(const 
> vector<DeviceInfo>& subdevices, int th
>                                         device.has_fermi_limits;
>                 info.has_half_images &= device.has_half_images;
>                 info.has_volume_decoupled &= device.has_volume_decoupled;
> -               info.has_qbvh &= device.has_qbvh;
> +               info.bvh_layout_mask = device.bvh_layout_mask &
> info.bvh_layout_mask;
>                 info.has_osl &= device.has_osl;
>         }
>
> diff --git a/intern/cycles/device/device.h 
> b/intern/cycles/device/device.h index 528a6dc10f6..99e80d10424 100644
> --- a/intern/cycles/device/device.h
> +++ b/intern/cycles/device/device.h
> @@ -19,6 +19,8 @@
>
>  #include <stdlib.h>
>
> +#include "bvh/bvh_params.h"
> +
>  #include "device/device_memory.h"
>  #include "device/device_task.h"
>
> @@ -52,14 +54,14 @@ public:
>         string description;
>         string id; /* used for user preferences, should stay fixed 
> with changing hardware config */
>         int num;
> -       bool display_device;         /* GPU is used as a display device. */
> -       bool advanced_shading;       /* Supports full shading system. */
> -       bool has_fermi_limits;       /* Fixed number of textures limit. */
> -       bool has_half_images;        /* Support half-float textures. */
> -       bool has_volume_decoupled;   /* Decoupled volume shading. */
> -       bool has_qbvh;               /* Supports both BVH2 and BVH4
> raytracing. */
> -       bool has_osl;                /* Support Open Sh
>
> @@ Diff output truncated at 10240 characters. @@
>
> _______________________________________________
> Bf-blender-cvs mailing list
> Bf-blender-cvs at blender.org
> https://lists.blender.org/mailman/listinfo/bf-blender-cvs
>



--
With best regards, Sergey Sharybin
_______________________________________________
Bf-committers mailing list
Bf-committers at blender.org
https://lists.blender.org/mailman/listinfo/bf-committers


More information about the Bf-committers mailing list