Reorganization: GLM Integration, SDL2 on macOS Build Fix, and Structural Enhancements #11

ZzzhHe · 2024-08-07T00:39:13Z

Description

This pull request reorganizes the project structure and and includes several important refactors. The changes include:

Resolved the issue with SDL2 failing to build on macOS.
Reorganized directory structure.
Enhanced the CMake configuration to achieve better modularity and build efficiency.
Replaced the Eigen library with GLM for vector and matrix operations.

Removed the scene.cpp file as as its corresponding header file doesn't exist.

Test Platform

macOS 14.5
Linux (Ubuntu 20.04)

TODO

refactor the model loading by integrating the assimp lib.
Refactor the data buffer to support additional information.

MRNIU · 2024-08-09T13:42:25Z

CMakeLists.txt

@@ -7,6 +7,9 @@
 # 设置最小 cmake 版本
 cmake_minimum_required(VERSION 3.27 FATAL_ERROR)

+set(CMAKE_CXX_STANDARD 20)
+set(CMAKE_CXX_STANDARD_REQUIRED True)
+


这两个在 CMakePresets.json 里面设置过了，可以删掉

但是删掉之后，就会在std::span的地方报错

In file included from /Users/hezhohao/Programming/GitRepo/SimpleRenderer/src/color.cpp:22: /Users/hezhohao/Programming/GitRepo/SimpleRenderer/src/include/log_system.h:75:25: warning: unused parameter 'args' [-Wunused-parameter] 75 | void debug(TARGS &&...args) { | ^ /Users/hezhohao/Programming/GitRepo/SimpleRenderer/src/color.cpp:33:24: error: no member named 'span' in namespace 'std' 33 | auto data_ptr = std::span(reinterpret_cast<uint8_t *>(&data), 4); | ~~~~~^ /Users/hezhohao/Programming/GitRepo/SimpleRenderer/src/color.cpp:101:23: error: no member named 'span' in namespace 'std' 101 | auto ret_ptr = std::span(reinterpret_cast<uint8_t *>(&ret), 4); | ~~~~~^ 1 warning and 2 errors generated. make[2]: *** [src/CMakeFiles/SimpleRenderer.dir/color.cpp.o] Error 1 make[1]: *** [src/CMakeFiles/SimpleRenderer.dir/all] Error 2 make: *** [all] Error 2

参考这个链接

我发现了，是因为CMakePresets.json这个文件我没设置好，导致在跑命令cmake --list-presets 的时候没有输出列表，然后就没办法用这个preset，我简单调整了一下preset的代码，现在可以正常使用preset跑了。

set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED True)

&

set(LOG_FILE_PATH "${PROJECT_SOURCE_DIR}/build/logs/SimpleRendererLog.log")

这两个地方都可以按照原本的代码直接跑，我下个commit会删掉我的更改

MRNIU · 2024-08-09T13:43:37Z

CMakeLists.txt

@@ -39,15 +42,15 @@ set(FONT_FILE_PATH "${wqy_font_SOURCE_DIR}/wqy-zenhei.ttc")
 include(ProcessorCount)
 ProcessorCount(NPROC)
 # 日志文件路径
-set(LOG_FILE_PATH "${EXECUTABLE_OUTPUT_PATH}/logs/SimpleRendererLog.log")
+set(LOG_FILE_PATH "${PROJECT_SOURCE_DIR}/build/logs/SimpleRendererLog.log")


EXECUTABLE_OUTPUT_PATH 这个值同样在 CMakePresets.json 中，如果构建目录不在 build 下可能有问题，建议使用 CMakePresets.json 下的值

MRNIU · 2024-08-09T13:45:55Z

.gitignore

@@ -14,4 +14,4 @@ tools/opensbi/build
 .idea
 3rd
 Doxyfile
-src/include/config.h
+include/config.h


为啥要把 include 放在 src 外面呢？
我的理解是 src 下包括源码+头文件，include 与 src 同级的话感觉会与 tools/cmake 等辅助文件搞混

MRNIU · 2024-08-09T13:49:53Z

CMakeLists.txt

 # 日志文件大小
 set(LOG_FILE_MAX_SIZE 1024*1024*4)
 # 日志文件数量
 set(LOG_FILE_MAX_COUNT 8)
 # 生成配置头文件
 configure_file(
        "${PROJECT_SOURCE_DIR}/cmake/config.h.in"
-        "${PROJECT_SOURCE_DIR}/src/include/config.h"
+        "${PROJECT_SOURCE_DIR}/include/config.h"


这里还是 include 目录的问题，后面不再专门说了~

MRNIU · 2024-08-09T13:53:49Z

cmake/3rd.cmake

+        "SDL_WERROR OFF"
+)
+find_package(SDL2 REQUIRED)
+


239 行还有一个 find_package(SDL2 REQUIRED)，是不是重复了？

MRNIU · 2024-08-09T14:03:32Z

src/CMakeLists.txt

 )

 target_link_libraries(${PROJECT_NAME} PRIVATE
-        ${DEFAULT_LINK_LIB}
+    ${DEFAULT_LINK_LIB}


cmakelist 的对齐你是用的什么？我是 clion 默认的对齐方案，这样比较省事，如果你有好用的工具可以推荐下

MRNIU · 2024-08-09T14:04:57Z

src/model.cpp

@@ -15,6 +15,7 @@
 */

 #include "model.hpp"
+#include <glm/gtc/matrix_transform.hpp>


头文件顺序可以交给 clang-format 处理，vscode 里面一键格式化

MRNIU · 2024-08-09T14:06:15Z

src/model.cpp


-  auto res4 = Vector4f(coord_.x(), coord_.y(), coord_.z(), 1);
-  auto ret4 = Vector4f(tran * res4);
+    // Convert the 3D coordinate to a 4D vector by adding a 1.0 w-component


注释可以顺手写个中文的，当然有些术语用中文很别扭的话除外

MRNIU · 2024-08-09T14:11:36Z

src/model.cpp

-    normal_ = (v0.normal_ + v1.normal_ + v2.normal_).normalized();
+  if (glm::normalize(v0.normal_) != glm::vec3(0.0f) && glm::normalize(v1.normal_) != glm::vec3(0.0f) &&
+      glm::normalize(v2.normal_) != glm::vec3(0.0f)) {
+    normal_ = glm::normalize((v0.normal_ + v1.normal_ + v2.normal_));


我想起来为啥当时用了 eigen 没用 glm 了。。
eigen 面向对象的能力比较好，能与 c++ 无缝接轨，glm 就不太能优雅封装，在代码里耦合太多了。
这个问题我们先搁置，等我们 c++ 版本稳定后考虑用 rust 重写

MRNIU · 2024-08-09T14:13:49Z

test/CMakeLists.txt

+endif()
+
+
+# add_subdirectory(unit_test)


这种预定义可以放在 CMakePresets.json 里面

Signed-off-by: ZhuohaoHe <[email protected]>

ZzzhHe · 2024-08-09T20:28:39Z

已按照review内容一一修改，build部分的CI已通过，但是这个publish部分的error不知道要怎么处理

MRNIU · 2024-08-26T02:43:34Z

src/include/model.hpp

@@ -40,7 +40,7 @@ class Model {
  /// 法向量
  using Normal = Vector3f;
  /// 贴图
-  using TextureCoord = Vector2f;
+  using TextureCoord = glm::vec2;


我们不是有 Vector2f 嘛，直接用呗

ZzzhHe added 6 commits August 6, 2024 12:42

run on MacOS successfully

cebca18

update log_system

78ba9f3

replace Eigen by glm

cebb066

update 3rd.cmake

049e985

update structure

ecdb542

update CMakeLists.txt

c877b4d

ZzzhHe requested a review from MRNIU August 9, 2024 13:28

MRNIU requested changes Aug 9, 2024

View reviewed changes

MRNIU mentioned this pull request Aug 9, 2024

Refactor model loading #12

Merged

ZzzhHe added 3 commits August 9, 2024 15:39

update according to the reviews

890c15c

Signed-off-by: ZhuohaoHe <[email protected]>

update 3rd.cmake to pass the CI

b502e5c

Signed-off-by: ZhuohaoHe <[email protected]>

update the CMakeLists.txt to make the log path correct

b380f8f

Signed-off-by: ZhuohaoHe <[email protected]>

MRNIU reviewed Aug 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorganization: GLM Integration, SDL2 on macOS Build Fix, and Structural Enhancements #11

Reorganization: GLM Integration, SDL2 on macOS Build Fix, and Structural Enhancements #11

ZzzhHe commented Aug 7, 2024

MRNIU Aug 9, 2024

ZzzhHe Aug 9, 2024 •

edited

Loading

ZzzhHe Aug 9, 2024

MRNIU Aug 9, 2024

MRNIU Aug 9, 2024

ZzzhHe Aug 9, 2024

MRNIU Aug 9, 2024

MRNIU Aug 9, 2024

MRNIU Aug 9, 2024

MRNIU Aug 9, 2024

MRNIU Aug 9, 2024

ZzzhHe Aug 9, 2024

MRNIU Aug 9, 2024

MRNIU Aug 9, 2024

ZzzhHe commented Aug 9, 2024 •

edited

Loading

MRNIU Aug 26, 2024

		endif()


		# add_subdirectory(unit_test)

Reorganization: GLM Integration, SDL2 on macOS Build Fix, and Structural Enhancements #11

Are you sure you want to change the base?

Reorganization: GLM Integration, SDL2 on macOS Build Fix, and Structural Enhancements #11

Conversation

ZzzhHe commented Aug 7, 2024

Description

Test Platform

TODO

Choose a reason for hiding this comment

ZzzhHe Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZzzhHe commented Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

ZzzhHe Aug 9, 2024 •

edited

Loading

ZzzhHe commented Aug 9, 2024 •

edited

Loading