Optimizations and Settings #145

MuskettaMan · 2025-01-19T15:42:35Z

Reviewer steps

I can build the project locally
I have tested and approved the features from this pull request
I have checked and approved the test criteria
I have reviewed the files in the pull request
I have looked at and updated the related issues in Codecks

Description

Contains a number of optimizations and new settings

We can now "play" the game on the steam deck at stable 60fps

Optimized gbuffers, less storage space used for gbuffers, while still having the same output
Removed position gbuffer, and using depth reprojection instead now
Enabled mips again for textures from models, this resolves the performance issues inside the cathedral
Implemented a SettingsStore singleton that can be used to persistently store settings
Added Visit Struct for reflection purposes, used for serialization.
Added settings for existing rendering settings (fog, fxaa, ssao, etc.)
Added a number of different of tonemapping algorithms
Added tone adjustments, lens distortion, and vignetting
Resolved the game capturing input when using imgui

Issues

https://bubonic-brotherhood.codecks.io/card/1ij-imgui-input-is-propagated-into-game

Test criteria

Look whether PBR materials still look correct
Verify game runs faster
Try changing a setting and reloading the game

[ADDED] Defines for easier versioning

[UPDATED] Settings serialization with visit struct for reflection.

[REMOVED] GBuffer for emissive [UPDATED] Octwrap normals

github-actions · 2025-01-19T15:47:04Z

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

clang-format (v18.1.3) reports: 1 file(s) not formatted

engine/renderer/private/gpu_scene.cpp

clang-tidy (v18.1.3) reports: 216 concern(s)

engine/inspector/private/inspector_module.cpp:213:5: warning: [cppcoreguidelines-pro-type-vararg]

do not call c-style vararg functions
```
  213 |     ImGui::LabelText("Draw calls", "%i", stats.DrawCalls());
      |     ^
```
engine/inspector/private/inspector_module.cpp:214:5: warning: [cppcoreguidelines-pro-type-vararg]

do not call c-style vararg functions
```
  214 |     ImGui::LabelText("Triangles", "%i", stats.IndexCount() / 3);
      |     ^
```
engine/inspector/private/inspector_module.cpp:215:5: warning: [cppcoreguidelines-pro-type-vararg]

do not call c-style vararg functions
```
  215 |     ImGui::LabelText("Indirect draw commands", "%i", stats.IndirectDrawCommands());
      |     ^
```
engine/inspector/private/inspector_module.cpp:216:5: warning: [cppcoreguidelines-pro-type-vararg]

do not call c-style vararg functions
```
  216 |     ImGui::LabelText("Particles after simulation", "%i", stats.GetParticleCount());
      |     ^
```
engine/inspector/private/inspector_module.cpp:217:5: warning: [cppcoreguidelines-pro-type-vararg]

do not call c-style vararg functions
```
  217 |     ImGui::LabelText("Emitters", "%i", stats.GetEmitterCount());
      |     ^
```

engine/renderer/private/camera.cpp:94:86: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

   94 |         const Buffer* buffer = _context->Resources()->BufferResourceManager().Access(_buffers[i]);
      |                                                                                      ^

engine/renderer/private/camera.cpp:103:23: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  103 |             .dstSet = _descriptorSets[i],
      |                       ^
```

engine/renderer/private/camera.cpp:218:82: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  218 |     const Buffer* buffer = _context->Resources()->BufferResourceManager().Access(_buffers[currentFrame]);
      |                                                                                  ^

engine/renderer/private/camera_batch.cpp:49:156: warning: [performance-unnecessary-value-param]

the parameter 'buffer' is copied for each invocation but only used as a const reference; consider making it a const reference

   49 | vk::DescriptorSet CameraBatch::Draw::CreateDescriptor(const std::shared_ptr<GraphicsContext>& context, vk::DescriptorSetLayout dsl, ResourceHandle<Buffer> buffer)
      |                                                                                                                                                            ^
      |                                                                                                                                     const                 &

engine/renderer/private/camera_batch.cpp:81:19: warning: [performance-unnecessary-value-param]

parameter 'depthImage' is passed by value and only copied once; consider moving it to avoid unnecessary copies
```
    2 |     , _depthImage(depthImage)
      |                   ^         
      |                   std::move()
```
engine/renderer/private/gpu_resources.cpp:86:9: warning: [cppcoreguidelines-init-variables]

variable 'width' is not initialized
```
   86 |     int width;
      |         ^    
      |               = 0
```
engine/renderer/private/gpu_resources.cpp:87:9: warning: [cppcoreguidelines-init-variables]

variable 'height' is not initialized
```
   87 |     int height;
      |         ^     
      |                = 0
```
engine/renderer/private/gpu_resources.cpp:88:9: warning: [cppcoreguidelines-init-variables]

variable 'nrChannels' is not initialized
```
   88 |     int nrChannels;
      |         ^         
      |                    = 0
```

engine/renderer/private/gpu_resources.cpp:90:23: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

   90 |     std::byte* data = reinterpret_cast<std::byte*>(stbi_load(std::string(path).c_str(),
      |                       ^

engine/renderer/private/gpu_resources.cpp:104:16: warning: [cppcoreguidelines-init-variables]

variable 'format' is not initialized
```
  104 |     vk::Format format;
      |                ^
```

engine/renderer/private/gpu_resources.cpp:121:35: warning: [cppcoreguidelines-pro-bounds-pointer-arithmetic]

do not use pointer arithmetic

  121 |     initialData.assign(data, data + static_cast<ptrdiff_t>(width * height * nrChannels));
      |                                   ^

engine/renderer/private/gpu_resources.cpp:188:5: warning: [bugprone-branch-clone]

switch has 2 consecutive identical branches

  188 |     case ImageType::eShadowMap:
      |     ^
/home/runner/work/Y2024-25-PR-BB/Y2024-25-PR-BB/engine/renderer/private/gpu_resources.cpp:192:38: note: last of these clones ends here
  192 |         return vk::ImageViewType::e2D;
      |                                      ^

engine/renderer/private/gpu_resources.cpp:213:15: warning: [performance-unnecessary-value-param]

parameter 'textureSampler' is passed by value and only copied once; consider moving it to avoid unnecessary copies
```
    7 |     sampler = textureSampler;
      |               ^             
      |               std::move(    )
```

engine/renderer/private/gpu_resources.cpp:242:55: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

  242 |     util::vmaCreateImage(_context->MemoryAllocator(), reinterpret_cast<VkImageCreateInfo*>(&imageCreateInfo), &allocCreateInfo, reinterpret_cast<VkImage*>(&image), &allocation, nullptr);
      |                                                       ^

engine/renderer/private/gpu_resources.cpp:242:129: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

  242 |     util::vmaCreateImage(_context->MemoryAllocator(), reinterpret_cast<VkImageCreateInfo*>(&imageCreateInfo), &allocCreateInfo, reinterpret_cast<VkImage*>(&image), &allocation, nullptr);
      |                                                                                                                                 ^

engine/renderer/private/gpu_resources.cpp:290:36: warning: [bugprone-implicit-widening-of-multiplication-result]

performing an implicit widening conversion to type 'vk::DeviceSize' (aka 'unsigned long') of a multiplication performed in type 'int'

  290 |         vk::DeviceSize imageSize = width * height * depth * 4;
      |                                    ^
/home/runner/work/Y2024-25-PR-BB/Y2024-25-PR-BB/engine/renderer/private/gpu_resources.cpp:290:36: note: make conversion explicit to silence this warning
    7 |         vk::DeviceSize imageSize = width * height * depth * 4;
      |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                    static_cast<vk::DeviceSize>( )
/home/runner/work/Y2024-25-PR-BB/Y2024-25-PR-BB/engine/renderer/private/gpu_resources.cpp:290:36: note: perform multiplication in a wider type
  290 |         vk::DeviceSize imageSize = width * height * depth * 4;
      |                                    ^~~~~~~~~~~~~~~~~~~~~~
      |                                    static_cast<long>(    )

engine/renderer/private/gpu_resources.cpp:293:25: warning: [bugprone-implicit-widening-of-multiplication-result]

performing an implicit widening conversion to type 'vk::DeviceSize' (aka 'unsigned long') of a multiplication performed in type 'int'

  293 |             imageSize = width * height * depth;
      |                         ^
/home/runner/work/Y2024-25-PR-BB/Y2024-25-PR-BB/engine/renderer/private/gpu_resources.cpp:293:25: note: make conversion explicit to silence this warning
  293 |             imageSize = width * height * depth;
      |                         ^~~~~~~~~~~~~~~~~~~~~~
      |                         static_cast<vk::DeviceSize>( )
/home/runner/work/Y2024-25-PR-BB/Y2024-25-PR-BB/engine/renderer/private/gpu_resources.cpp:293:25: note: perform multiplication in a wider type
  293 |             imageSize = width * height * depth;
      |                         ^~~~~~~~~~~~~~
      |                         static_cast<long>( )

engine/renderer/private/gpu_resources.cpp:301:23: warning: [cppcoreguidelines-init-variables]

variable 'stagingBufferAllocation' is not initialized

  301 |         VmaAllocation stagingBufferAllocation;
      |                       ^                      
      |                                               = nullptr

engine/renderer/private/gpu_scene.cpp:123:82: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  123 |     const Buffer* buffer = _context->Resources()->BufferResourceManager().Access(_sceneFrameData[frameIndex].buffer);
      |                                                                                  ^

engine/renderer/private/gpu_scene.cpp:133:82: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  133 |     const Buffer* buffer = _context->Resources()->BufferResourceManager().Access(_pointLightFrameData[frameIndex].buffer);
      |                                                                                  ^

engine/renderer/private/gpu_scene.cpp:209:97: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  209 |     const Buffer* staticInstancesBuffer = _context->Resources()->BufferResourceManager().Access(_staticInstancesFrameData[frameIndex].buffer);
      |                                                                                                 ^

engine/renderer/private/gpu_scene.cpp:212:98: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  212 |     const Buffer* skinnedInstancesBuffer = _context->Resources()->BufferResourceManager().Access(_skinnedInstancesFrameData[frameIndex].buffer);
      |                                                                                                  ^

engine/renderer/private/gpu_scene.cpp:276:42: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  276 |         PointLightData& pointLightData = pointLightArray.lights[pointLightCount];
      |                                          ^
```
engine/renderer/private/gpu_scene.cpp:341:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  341 |         skinMatrices[offset + joint.jointIndex] = worldTransform * joint.inverseBindMatrix;
      |         ^
```

engine/renderer/private/gpu_scene.cpp:352:82: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  352 |     const Buffer* buffer = _context->Resources()->BufferResourceManager().Access(_skinBuffers[frameIndex]);
      |                                                                                  ^

engine/renderer/private/gpu_scene.cpp:487:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  487 |         _sceneFrameData[i].descriptorSet = descriptorSets[i];
      |         ^
```
engine/renderer/private/gpu_scene.cpp:487:44: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  487 |         _sceneFrameData[i].descriptorSet = descriptorSets[i];
      |                                            ^
```
engine/renderer/private/gpu_scene.cpp:507:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  507 |         _pointLightFrameData[i].descriptorSet = descriptorSets[i];
      |         ^
```
engine/renderer/private/gpu_scene.cpp:507:49: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  507 |         _pointLightFrameData[i].descriptorSet = descriptorSets[i];
      |                                                 ^
```

engine/renderer/private/gpu_scene.cpp:514:41: warning: [bugprone-implicit-widening-of-multiplication-result]

performing an implicit widening conversion to type 'std::size_t' (aka 'unsigned long') of a multiplication performed in type 'uint32_t' (aka 'unsigned int')

  514 |     std::array<vk::DescriptorSetLayout, MAX_FRAMES_IN_FLIGHT * 2> layouts {};
      |                                         ^
/home/runner/work/Y2024-25-PR-BB/Y2024-25-PR-BB/engine/renderer/private/gpu_scene.cpp:514:41: note: make conversion explicit to silence this warning
   30 |     std::array<vk::DescriptorSetLayout, MAX_FRAMES_IN_FLIGHT * 2> layouts {};
      |                                         ^~~~~~~~~~~~~~~~~~~~~~~~
      |                                         static_cast<std::size_t>( )
/home/runner/work/Y2024-25-PR-BB/Y2024-25-PR-BB/engine/renderer/private/gpu_scene.cpp:514:41: note: perform multiplication in a wider type
  514 |     std::array<vk::DescriptorSetLayout, MAX_FRAMES_IN_FLIGHT * 2> layouts {};
      |                                         ^~~~~~~~~~~~~~~~~~~~
      |                                         static_cast<std::size_t>( )

engine/renderer/private/gpu_scene.cpp:525:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  525 |         _staticInstancesFrameData[i].descriptorSet = descriptorSets[i * 2];
      |         ^
```
engine/renderer/private/gpu_scene.cpp:526:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  526 |         _skinnedInstancesFrameData[i].descriptorSet = descriptorSets[i * 2 + 1];
      |         ^
```

engine/renderer/private/gpu_scene.cpp:549:82: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  549 |     const Buffer* buffer = _context->Resources()->BufferResourceManager().Access(_sceneFrameData[frameIndex].buffer);
      |                                                                                  ^

engine/renderer/private/gpu_scene.cpp:559:26: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  559 |     bufferWrite.dstSet = _sceneFrameData[frameIndex].descriptorSet;
      |                          ^
```

engine/renderer/private/gpu_scene.cpp:571:82: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  571 |     const Buffer* buffer = _context->Resources()->BufferResourceManager().Access(_pointLightFrameData[frameIndex].buffer);
      |                                                                                  ^

engine/renderer/private/gpu_scene.cpp:581:26: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  581 |     bufferWrite.dstSet = _pointLightFrameData[frameIndex].descriptorSet;
      |                          ^
```

engine/renderer/private/gpu_scene.cpp:594:85: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  594 |     staticBufferInfo.buffer = _context->Resources()->BufferResourceManager().Access(_staticInstancesFrameData[frameIndex].buffer)->buffer;
      |                                                                                     ^

engine/renderer/private/gpu_scene.cpp:599:86: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  599 |     skinnedBufferInfo.buffer = _context->Resources()->BufferResourceManager().Access(_skinnedInstancesFrameData[frameIndex].buffer)->buffer;
      |                                                                                      ^

engine/renderer/private/gpu_scene.cpp:606:32: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  606 |     staticBufferWrite.dstSet = _staticInstancesFrameData[frameIndex].descriptorSet;
      |                                ^
```
engine/renderer/private/gpu_scene.cpp:614:33: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  614 |     skinnedBufferWrite.dstSet = _skinnedInstancesFrameData[frameIndex].descriptorSet;
      |                                 ^
```

engine/renderer/private/gpu_scene.cpp:626:82: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  626 |     const Buffer* buffer = _context->Resources()->BufferResourceManager().Access(_skinBuffers[frameIndex]);
      |                                                                                  ^

engine/renderer/private/gpu_scene.cpp:635:19: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  635 |         .dstSet = _skinDescriptorSets[frameIndex],
      |                   ^
```
engine/renderer/private/gpu_scene.cpp:660:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  660 |         _sceneFrameData[i].buffer = _context->Resources()->BufferResourceManager().Create(creation);
      |         ^
```
engine/renderer/private/gpu_scene.cpp:676:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  676 |         _pointLightFrameData[i].buffer = _context->Resources()->BufferResourceManager().Create(creation);
      |         ^
```
engine/renderer/private/gpu_scene.cpp:694:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  694 |         _staticInstancesFrameData[i].buffer = _context->Resources()->BufferResourceManager().Create(creation);
      |         ^
```
engine/renderer/private/gpu_scene.cpp:708:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  708 |         _skinnedInstancesFrameData[i].buffer = _context->Resources()->BufferResourceManager().Create(creation);
      |         ^
```
engine/renderer/private/gpu_scene.cpp:722:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  722 |         _skinBuffers[i] = _context->Resources()->BufferResourceManager().Create(creation);
      |         ^
```

engine/renderer/private/gpu_scene.cpp:724:90: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  724 |         const Buffer* skinBuffer = _context->Resources()->BufferResourceManager().Access(_skinBuffers[i]);
      |                                                                                          ^

engine/renderer/private/gpu_scene.cpp:728:72: warning: [cppcoreguidelines-pro-bounds-pointer-arithmetic]

do not use pointer arithmetic

  728 |             std::memcpy(static_cast<std::byte*>(skinBuffer->mappedPtr) + sizeof(glm::mat4) * j, &data, sizeof(glm::mat4));
      |                                                                        ^

engine/renderer/private/gpu_scene.cpp:744:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  744 |         _staticDraws[i].buffer = _context->Resources()->BufferResourceManager().Create(creation);
      |         ^
```
engine/renderer/private/gpu_scene.cpp:755:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  755 |         _skinnedDraws[i].buffer = _context->Resources()->BufferResourceManager().Create(creation);
      |         ^
```
engine/renderer/private/gpu_scene.cpp:788:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  788 |         _staticDraws[i].descriptorSet = descriptorSets[i];
      |         ^
```
engine/renderer/private/gpu_scene.cpp:790:54: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  790 |         vk::DescriptorBufferInfo& staticBufferInfo { bufferInfos[i] };
      |                                                      ^
```

engine/renderer/private/gpu_scene.cpp:791:89: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  791 |         staticBufferInfo.buffer = _context->Resources()->BufferResourceManager().Access(_staticDraws[i].buffer)->buffer;
      |                                                                                         ^

engine/renderer/private/gpu_scene.cpp:795:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  795 |         bufferWrites[i].dstSet = _staticDraws[i].descriptorSet;
      |         ^
```
engine/renderer/private/gpu_scene.cpp:795:34: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  795 |         bufferWrites[i].dstSet = _staticDraws[i].descriptorSet;
      |                                  ^
```
engine/renderer/private/gpu_scene.cpp:796:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  796 |         bufferWrites[i].dstBinding = 0;
      |         ^
```
engine/renderer/private/gpu_scene.cpp:797:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  797 |         bufferWrites[i].dstArrayElement = 0;
      |         ^
```
engine/renderer/private/gpu_scene.cpp:798:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  798 |         bufferWrites[i].descriptorType = vk::DescriptorType::eStorageBuffer;
      |         ^
```
engine/renderer/private/gpu_scene.cpp:799:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  799 |         bufferWrites[i].descriptorCount = 1;
      |         ^
```
engine/renderer/private/gpu_scene.cpp:800:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  800 |         bufferWrites[i].pBufferInfo = &staticBufferInfo;
      |         ^
```
engine/renderer/private/gpu_scene.cpp:808:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  808 |         _skinnedDraws[i].descriptorSet = descriptorSets[i];
      |         ^
```
engine/renderer/private/gpu_scene.cpp:810:55: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  810 |         vk::DescriptorBufferInfo& skinnedBufferInfo { bufferInfos[i] };
      |                                                       ^
```

engine/renderer/private/gpu_scene.cpp:811:90: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  811 |         skinnedBufferInfo.buffer = _context->Resources()->BufferResourceManager().Access(_skinnedDraws[i].buffer)->buffer;
      |                                                                                          ^

engine/renderer/private/gpu_scene.cpp:815:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  815 |         bufferWrites[i].dstSet = _skinnedDraws[i].descriptorSet;
      |         ^
```
engine/renderer/private/gpu_scene.cpp:815:34: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  815 |         bufferWrites[i].dstSet = _skinnedDraws[i].descriptorSet;
      |                                  ^
```
engine/renderer/private/gpu_scene.cpp:816:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  816 |         bufferWrites[i].dstBinding = 0;
      |         ^
```
engine/renderer/private/gpu_scene.cpp:817:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  817 |         bufferWrites[i].dstArrayElement = 0;
      |         ^
```
engine/renderer/private/gpu_scene.cpp:818:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  818 |         bufferWrites[i].descriptorType = vk::DescriptorType::eStorageBuffer;
      |         ^
```
engine/renderer/private/gpu_scene.cpp:819:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  819 |         bufferWrites[i].descriptorCount = 1;
      |         ^
```
engine/renderer/private/gpu_scene.cpp:820:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  820 |         bufferWrites[i].pBufferInfo = &skinnedBufferInfo;
      |         ^
```

engine/renderer/private/gpu_scene.cpp:830:88: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  830 |     const Buffer* staticBuffer = _context->Resources()->BufferResourceManager().Access(_staticDraws[frameIndex].buffer);
      |                                                                                        ^

engine/renderer/private/gpu_scene.cpp:831:89: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  831 |     const Buffer* skinnedBuffer = _context->Resources()->BufferResourceManager().Access(_skinnedDraws[frameIndex].buffer);
      |                                                                                         ^

engine/renderer/private/model_loader.cpp:223:28: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast
```
  223 |         vertexAttribute = *reinterpret_cast<T*>(&value);
      |                            ^
```

engine/renderer/private/model_loader.cpp:330:65: warning: [cppcoreguidelines-pro-bounds-pointer-arithmetic]

do not use pointer arithmetic

  330 |                 const std::byte* element = attributeBufferStart + i * gltfIndexTypeSize + (bufferView.byteStride.has_value() ? bufferView.byteStride.value() : 0);
      |                                                                 ^

engine/renderer/private/model_loader.cpp:330:89: warning: [cppcoreguidelines-pro-bounds-pointer-arithmetic]

do not use pointer arithmetic

  330 |                 const std::byte* element = attributeBufferStart + i * gltfIndexTypeSize + (bufferView.byteStride.has_value() ? bufferView.byteStride.value() : 0);
      |                                                                                         ^

engine/renderer/private/model_loader.cpp:331:58: warning: [cppcoreguidelines-pro-bounds-pointer-arithmetic]

do not use pointer arithmetic

  331 |                 uint32_t* indexPtr = mesh.indices.data() + i;
      |                                                          ^

engine/renderer/private/model_loader.cpp:373:32: warning: [cppcoreguidelines-init-variables]

variable 'width' is not initialized

  373 |                        int32_t width, height, nrChannels;
      |                                ^    
      |                                      = 0

engine/renderer/private/model_loader.cpp:373:39: warning: [cppcoreguidelines-init-variables]

variable 'height' is not initialized

  373 |                        int32_t width, height, nrChannels;
      |                                       ^     
      |                                              = 0

engine/renderer/private/model_loader.cpp:373:47: warning: [cppcoreguidelines-init-variables]

variable 'nrChannels' is not initialized

  373 |                        int32_t width, height, nrChannels;
      |                                               ^         
      |                                                          = 0

engine/renderer/private/model_loader.cpp:380:54: warning: [bugprone-implicit-widening-of-multiplication-result]

performing an implicit widening conversion to type 'size_type' (aka 'unsigned long') of a multiplication performed in type 'int'

  380 |                        data = std::vector<std::byte>(width * height * 4);
      |                                                      ^
/home/runner/work/Y2024-25-PR-BB/Y2024-25-PR-BB/engine/renderer/private/model_loader.cpp:380:54: note: make conversion explicit to silence this warning
   20 |                        data = std::vector<std::byte>(width * height * 4);
      |                                                      ^~~~~~~~~~~~~~~~~~
      |                                                      static_cast<size_type>( )
/home/runner/work/Y2024-25-PR-BB/Y2024-25-PR-BB/engine/renderer/private/model_loader.cpp:380:54: note: perform multiplication in a wider type
  380 |                        data = std::vector<std::byte>(width * height * 4);
      |                                                      ^~~~~~~~~~~~~~
      |                                                      static_cast<long>( )

engine/renderer/private/model_loader.cpp:381:49: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

  381 |                        std::memcpy(data.data(), reinterpret_cast<std::byte*>(stbiData), data.size());
      |                                                 ^

engine/renderer/private/model_loader.cpp:395:32: warning: [cppcoreguidelines-init-variables]

variable 'width' is not initialized

  395 |                        int32_t width, height, nrChannels;
      |                                ^    
      |                                      = 0

engine/renderer/private/model_loader.cpp:395:39: warning: [cppcoreguidelines-init-variables]

variable 'height' is not initialized

  395 |                        int32_t width, height, nrChannels;
      |                                       ^     
      |                                              = 0

engine/renderer/private/model_loader.cpp:395:47: warning: [cppcoreguidelines-init-variables]

variable 'nrChannels' is not initialized

  395 |                        int32_t width, height, nrChannels;
      |                                               ^         
      |                                                          = 0

engine/renderer/private/model_loader.cpp:396:66: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

  396 |                        stbi_uc* stbiData = stbi_load_from_memory(reinterpret_cast<const stbi_uc*>(vector.bytes.data()),
      |                                                                  ^

engine/renderer/private/model_loader.cpp:400:54: warning: [bugprone-implicit-widening-of-multiplication-result]

performing an implicit widening conversion to type 'size_type' (aka 'unsigned long') of a multiplication performed in type 'int'

  400 |                        data = std::vector<std::byte>(width * height * 4);
      |                                                      ^
/home/runner/work/Y2024-25-PR-BB/Y2024-25-PR-BB/engine/renderer/private/model_loader.cpp:400:54: note: make conversion explicit to silence this warning
  400 |                        data = std::vector<std::byte>(width * height * 4);
      |                                                      ^~~~~~~~~~~~~~~~~~
      |                                                      static_cast<size_type>( )
/home/runner/work/Y2024-25-PR-BB/Y2024-25-PR-BB/engine/renderer/private/model_loader.cpp:400:54: note: perform multiplication in a wider type
  400 |                        data = std::vector<std::byte>(width * height * 4);
      |                                                      ^~~~~~~~~~~~~~
      |                                                      static_cast<long>( )

engine/renderer/private/model_loader.cpp:401:49: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

  401 |                        std::memcpy(data.data(), reinterpret_cast<std::byte*>(stbiData), data.size());
      |                                                 ^

engine/renderer/private/model_loader.cpp:424:44: warning: [cppcoreguidelines-init-variables]

variable 'width' is not initialized

  424 |                                    int32_t width, height, nrChannels;
      |                                            ^    
      |                                                  = 0

engine/renderer/private/model_loader.cpp:424:51: warning: [cppcoreguidelines-init-variables]

variable 'height' is not initialized

  424 |                                    int32_t width, height, nrChannels;
      |                                                   ^     
      |                                                          = 0

engine/renderer/private/model_loader.cpp:424:59: warning: [cppcoreguidelines-init-variables]

variable 'nrChannels' is not initialized

  424 |                                    int32_t width, height, nrChannels;
      |                                                           ^         
      |                                                                      = 0

engine/renderer/private/model_loader.cpp:426:40: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

  426 |                                        reinterpret_cast<const stbi_uc*>(vector.bytes.data() + bufferView.byteOffset),
      |                                        ^

engine/renderer/private/model_loader.cpp:430:66: warning: [bugprone-implicit-widening-of-multiplication-result]

performing an implicit widening conversion to type 'size_type' (aka 'unsigned long') of a multiplication performed in type 'int'

  430 |                                    data = std::vector<std::byte>(width * height * 4);
      |                                                                  ^
/home/runner/work/Y2024-25-PR-BB/Y2024-25-PR-BB/engine/renderer/private/model_loader.cpp:430:66: note: make conversion explicit to silence this warning
  430 |                                    data = std::vector<std::byte>(width * height * 4);
      |                                                                  ^~~~~~~~~~~~~~~~~~
      |                                                                  static_cast<size_type>( )
/home/runner/work/Y2024-25-PR-BB/Y2024-25-PR-BB/engine/renderer/private/model_loader.cpp:430:66: note: perform multiplication in a wider type
  430 |                                    data = std::vector<std::byte>(width * height * 4);
      |                                                                  ^~~~~~~~~~~~~~
      |                                                                  static_cast<long>( )

engine/renderer/private/model_loader.cpp:431:61: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

  431 |                                    std::memcpy(data.data(), reinterpret_cast<std::byte*>(stbiData), data.size());
      |                                                             ^

engine/renderer/private/model_loader.cpp:498:55: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

  498 |                 timestamps = std::span<const float> { reinterpret_cast<const float*>(data), accessor.count };
      |                                                       ^

engine/renderer/private/model_loader.cpp:517:59: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

  517 |                     std::span<const Translation> output { reinterpret_cast<const Translation*>(data), accessor.count };
      |                                                           ^

engine/renderer/private/model_loader.cpp:526:56: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

  526 |                     std::span<const Rotation> output { reinterpret_cast<const Rotation*>(data), bufferView.byteLength / sizeof(Rotation) };
      |                                                        ^

engine/renderer/private/model_loader.cpp:530:44: warning: [bugprone-unchecked-optional-access]

unchecked access to optional value

  530 |                     for (size_t i = 0; i < spline->rotation.value().values.size(); ++i)
      |                                            ^

engine/renderer/private/model_loader.cpp:532:46: warning: [bugprone-unchecked-optional-access]

unchecked access to optional value

  532 |                         math::QuatXYZWtoWXYZ(spline->rotation.value().values[i]);
      |                                              ^

engine/renderer/private/model_loader.cpp:534:21: warning: [bugprone-unchecked-optional-access]

unchecked access to optional value

  534 |                     spline->rotation.value().timestamps = std::vector<float> { timestamps.begin(), timestamps.end() };
      |                     ^

engine/renderer/private/model_loader.cpp:540:53: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

  540 |                     std::span<const Scale> output { reinterpret_cast<const Scale*>(data), accessor.count };
      |                                                     ^

engine/renderer/private/model_loader.cpp:662:18: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast
```
  662 |                 *reinterpret_cast<glm::mat4x4*>(&inverseBindMatrix),
      |                  ^
```

engine/renderer/private/passes/gaussian_blur_pass.cpp:18:15: warning: [performance-unnecessary-value-param]

parameter 'source' is passed by value and only copied once; consider moving it to avoid unnecessary copies

   14 | #include <vector>
   15 | 
   16 | GaussianBlurPass::GaussianBlurPass(const std::shared_ptr<GraphicsContext>& context, ResourceHandle<GPUImage> source, ResourceHandle<GPUImage> target)
   17 |     : _context(context)
   18 |     , _source(source)
      |               ^     
      |               std::move( )

engine/renderer/private/passes/gaussian_blur_pass.cpp:21:19: warning: [performance-unnecessary-value-param]

parameter 'target' is passed by value and only copied once; consider moving it to avoid unnecessary copies
```
   21 |     _targets[1] = target;
      |                   ^     
      |                   std::move( )
```
engine/renderer/private/passes/gaussian_blur_pass.cpp:49:39: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
   49 |     vk::DescriptorSet descriptorSet = _sourceDescriptorSets[currentFrame];
      |                                       ^
```

engine/renderer/private/passes/gaussian_blur_pass.cpp:55:87: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

   55 |         const GPUImage* target = _context->Resources()->ImageResourceManager().Access(_targets[isVerticalPass]);
      |                                                                                       ^

engine/renderer/private/passes/gaussian_blur_pass.cpp:62:29: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
   62 |             descriptorSet = _targetDescriptorSets[horizontalTargetIndex][currentFrame];
      |                             ^
```

engine/renderer/private/passes/gaussian_blur_pass.cpp:63:91: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

   63 |             const GPUImage* source = _context->Resources()->ImageResourceManager().Access(_targets[horizontalTargetIndex]);
      |                                                                                           ^

engine/renderer/private/passes/gaussian_blur_pass.cpp:184:38: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  184 |         descriptorWrites[0].dstSet = _sourceDescriptorSets[i];
      |                                      ^
```

engine/renderer/private/passes/gaussian_blur_pass.cpp:196:99: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  196 |         util::VK_ASSERT(_context->VulkanContext()->Device().allocateDescriptorSets(&allocateInfo, _targetDescriptorSets[i].data()),
      |                                                                                                   ^

engine/renderer/private/passes/gaussian_blur_pass.cpp:203:83: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression

  203 |                 .imageView = _context->Resources()->ImageResourceManager().Access(_targets[i])->view,
      |                                                                                   ^

engine/renderer/private/passes/gaussian_blur_pass.cpp:208:42: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  208 |             descriptorWrites[0].dstSet = _targetDescriptorSets[i][frame];
      |                                          ^
```
engine/renderer/private/passes/ssao_pass.cpp:175:9: warning: [cppcoreguidelines-avoid-c-arrays]

do not declare C-style arrays, use std::array<> instead
```
  175 |         float components[4] = { color.r, color.g, color.b, color.a };
      |         ^
```

engine/renderer/private/passes/ssao_pass.cpp:178:37: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

  178 |         const std::byte* rawBytes = reinterpret_cast<const std::byte*>(components);
      |                                     ^

engine/renderer/private/passes/ssao_pass.cpp:179:60: warning: [cppcoreguidelines-pro-bounds-pointer-arithmetic]

do not use pointer arithmetic

  179 |         byteData.insert(byteData.end(), rawBytes, rawBytes + sizeof(components));
      |                                                            ^

engine/renderer/private/passes/tonemapping_pass.cpp:14:147: warning: [performance-unnecessary-value-param]

the parameter 'hdrTarget' is copied for each invocation but only used as a const reference; consider making it a const reference

   14 | TonemappingPass::TonemappingPass(const std::shared_ptr<GraphicsContext>& context, const Settings::Tonemapping& settings, ResourceHandle<GPUImage> hdrTarget, ResourceHandle<GPUImage> bloomTarget, ResourceHandle<GPUImage> outputTarget, const SwapChain& _swapChain, const BloomSettings& bloomSettings)
      |                                                                                                                                                   ^
      |                                                                                                                          const                   &

engine/renderer/private/passes/tonemapping_pass.cpp:14:183: warning: [performance-unnecessary-value-param]

the parameter 'bloomTarget' is copied for each invocation but only used as a const reference; consider making it a const reference

   14 | TonemappingPass::TonemappingPass(const std::shared_ptr<GraphicsContext>& context, const Settings::Tonemapping& settings, ResourceHandle<GPUImage> hdrTarget, ResourceHandle<GPUImage> bloomTarget, ResourceHandle<GPUImage> outputTarget, const SwapChain& _swapChain, const BloomSettings& bloomSettings)
      |                                                                                                                                                                                       ^
      |                                                                                                                                                              const                   &

engine/renderer/private/passes/tonemapping_pass.cpp:20:21: warning: [performance-unnecessary-value-param]

parameter 'outputTarget' is passed by value and only copied once; consider moving it to avoid unnecessary copies
```
    2 |     , _outputTarget(outputTarget)
      |                     ^           
      |                     std::move(  )
```

engine/renderer/private/renderer.cpp:47:1: warning: [cppcoreguidelines-pro-type-member-init]

constructor does not initialize these fields: _tracyContexts

   47 | Renderer::Renderer(ApplicationModule& application, Viewport& viewport, const std::shared_ptr<GraphicsContext>& context, ECSModule& ecs)
      | ^

engine/renderer/private/renderer.cpp:301:9: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  301 |         _tracyContexts[i] = TracyVkContextCalibrated(
      |         ^
```
engine/renderer/private/renderer.cpp:305:13: warning: [cppcoreguidelines-pro-bounds-constant-array-index]

do not use array subscript when the index is not an integer constant expression
```
  305 |             _commandBuffers[i],
      |             ^
```

engine/renderer/private/renderer.cpp:306:13: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

  306 |             reinterpret_cast<PFN_vkGetPhysicalDeviceCalibrateableTimeDomainsEXT>(_context->VulkanContext()->Instance().getProcAddr("vkGetPhysicalDeviceCalibrateableTimeDomainsEXT")),
      |             ^

engine/renderer/private/renderer.cpp:307:13: warning: [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

  307 |             reinterpret_cast<PFN_vkGetCalibratedTimestampsEXT>(_context->VulkanContext()->Instance().getProcAddr("vkGetCalibratedTimestampsEXT")));
      |             ^

Have any feedback or feature suggestions? Share it here.

mmzala

Aside from the settings being a singleton that we discussed, it looks good to me and it works.

Also I'll add the settings file to the github action to this PR, as we don't have a single file that tells the github action what it needs to package.

shaders/tonemapping.frag

…tyGames/Y2024-25-PR-BB into feature/data-store

mmzala

Nice work! LGTM

LarsJanssen21

Looks good, project runs well. Improvement can be observed in the performance. runs around 80/90 fps at 1440p on my old rtx 2070

MuskettaMan added 18 commits January 17, 2025 17:26

[ADDED] Simple data store for persistent data for settings.

61fb76c

[ADDED] Split up settings file

73642e1

[ADDED] Defines for easier versioning

[ADDED] Visit struct

2b84956

[UPDATED] Settings serialization with visit struct for reflection.

[ADDED] SSAO settings

fc06891

[ADDED] Added more settings

b72f235

[ADDED] Added tonemapping settings.

20fe495

[UDPATED] Removed specular term and added new HDR

925442e

[UDPATED] ImGui inputs are now captured

d27e834

[UDPATED] Removed emissive buffer

b71c1eb

[UDPATED] HDRI file

80f17af

[REMOVED] GBuffer for emissive [UPDATED] Octwrap normals

[UDPATED] Added depth reprojection and reorganized gbuffers.

660bf7f

[UDPATED] Packed rougness and metallic together

0c1e8d1

[UPDATE] Enabled mips for model textures

ff1bf1d

[UPDATE] Fixed validation errors

49eb8e8

[ADDED] Vignetting

e4efc79

[ADDED] More color grading options.

2aeaf06

[ADDED] Updated settings editor

7c570ec

[UDPATE] Clean up

a64367f

github-actions bot requested review from LarsJanssen21 and VengarvdWal January 19, 2025 15:42

github-actions bot assigned MuskettaMan Jan 19, 2025

MuskettaMan added 2 commits January 19, 2025 16:48

[UDPATE] Resolve compile errors

47bbe00

[UDPATE] Enabled fog and fixed shadow distance

1e057b8

mmzala reviewed Jan 20, 2025

View reviewed changes

shaders/tonemapping.frag Outdated Show resolved Hide resolved

shaders/tonemapping.frag Outdated Show resolved Hide resolved

mmzala and others added 4 commits January 20, 2025 11:48

[ADDED] Settings file to steam build automation

c5b35dd

[UPDATE] Removed singleton data structure

4c27855

Merge branch 'feature/data-store' of https://github.com/BredaUniversi…

a2f6c6e

…tyGames/Y2024-25-PR-BB into feature/data-store

[UPDATE] Split up data store from settings

c023f82

mmzala approved these changes Jan 20, 2025

View reviewed changes

[UPDATE] Undef for bool

4f09a5c

LarsJanssen21 approved these changes Jan 20, 2025

View reviewed changes

MuskettaMan merged commit cb1a255 into main Jan 20, 2025
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimizations and Settings #145

Optimizations and Settings #145

MuskettaMan commented Jan 19, 2025 •

edited by mmzala

Loading

github-actions bot commented Jan 19, 2025 •

edited

Loading

mmzala left a comment

mmzala left a comment

LarsJanssen21 left a comment

Optimizations and Settings #145

Optimizations and Settings #145

Conversation

MuskettaMan commented Jan 19, 2025 • edited by mmzala Loading

Reviewer steps

Description

Issues

Test criteria

github-actions bot commented Jan 19, 2025 • edited Loading

Cpp-Linter Report ⚠️

mmzala left a comment

Choose a reason for hiding this comment

mmzala left a comment

Choose a reason for hiding this comment

LarsJanssen21 left a comment

Choose a reason for hiding this comment

MuskettaMan commented Jan 19, 2025 •

edited by mmzala

Loading

github-actions bot commented Jan 19, 2025 •

edited

Loading