search

SaschaWillems / Vulkan-glTF-PBR

Physical based rendering with Vulkan using glTF 2.0 models
MIT License
915 stars 124 forks source link

Forget to add reference& on the parentBvh in void Model::calculateBoundingBox(Node *node, Node *parent) ? #64

Closed King19931229 closed 1 year ago

King19931229 commented 1 year ago

`void Model::calculateBoundingBox(Node node, Node parent) { BoundingBox parentBvh = parent ? parent->bvh : BoundingBox(dimensions.min, dimensions.max);

    if (node->mesh) {
        if (node->mesh->bb.valid) {
            node->aabb = node->mesh->bb.getAABB(node->getMatrix());
            if (node->children.size() == 0) {
                node->bvh.min = node->aabb.min;
                node->bvh.max = node->aabb.max;
                node->bvh.valid = true;
            }
        }
    }

    parentBvh.min = glm::min(parentBvh.min, node->bvh.min);
    parentBvh.max = glm::min(parentBvh.max, node->bvh.max);

    for (auto &child : node->children) {
        calculateBoundingBox(child, node);
    }
}`

Here is the code of Model::calculateBoundingBox(Node node, Node parent). On the first line we fetch the variable of parentBvh by BoundingBox parentBvh = parent ? parent->bvh : BoundingBox(dimensions.min, dimensions.max); The parentBvh is being modified by parentBvh.min = glm::min(parentBvh.min, node->bvh.min); parentBvh.max = glm::min(parentBvh.max, node->bvh.max); So the first shoud be BoundingBox& parentBvh = parent ? parent->bvh : BoundingBox(dimensions.min, dimensions.max);

dmarian commented 7 months ago

With this change it does not compile on Linux

FAILED: base/CMakeFiles/base.dir/VulkanglTFModel.cpp.o 
/usr/bin/g++-13 -DVK_EXAMPLE_DATA_DIR=\"/home/dario/proyectos/Vulkan/Vulkan-glTF-PBR/data/\" -D_CRT_SECURE_NO_WARNINGS -I/home/dario/proyectos/Vulkan/Vulkan-glTF-PBR/external -I/home/dario/proyectos/Vulkan/Vulkan-glTF-PBR/external/glm -I/home/dario/proyectos/Vulkan/Vulkan-glTF-PBR/external/gli -I/home/dario/proyectos/Vulkan/Vulkan-glTF-PBR/external/imgui -I/home/dario/proyectos/Vulkan/Vulkan-glTF-PBR/external/tinygltf -I/home/dario/proyectos/Vulkan/Vulkan-glTF-PBR/base -DVK_USE_PLATFORM_XCB_KHR -DNOMINMAX -D_USE_MATH_DEFINES -march=native -Ofast -DNDEBUG -w -I/home/dario/usr/include -DGLM_ENABLE_EXPERIMENTAL=1   -std=c++11 -MD -MT base/CMakeFiles/base.dir/VulkanglTFModel.cpp.o -MF base/CMakeFiles/base.dir/VulkanglTFModel.cpp.o.d -o base/CMakeFiles/base.dir/VulkanglTFModel.cpp.o -c /home/dario/proyectos/Vulkan/Vulkan-glTF-PBR/base/VulkanglTFModel.cpp
/home/dario/proyectos/Vulkan/Vulkan-glTF-PBR/base/VulkanglTFModel.cpp: In member function ‘void vkglTF::Model::calculateBoundingBox(vkglTF::Node*, vkglTF::Node*)’:
/home/dario/proyectos/Vulkan/Vulkan-glTF-PBR/base/VulkanglTFModel.cpp:1159:49: error: cannot bind non-const lvalue reference of type ‘vkglTF::BoundingBox&’ to an rvalue of type ‘vkglTF::BoundingBox’
 1159 |                 BoundingBox& parentBvh = parent ? parent->bvh : BoundingBox(dimensions.min, dimensions.max);
      |                                          ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[8/10] Building CXX object src/CMakeFiles/Vulkan-glTF-PBR.dir/main.cpp.o

Tested with GCC 7, GCC 13 and CLang 17