feat(graphic): add material usage#411
Conversation
…l buffer creation
…erial, mesh, and transform
There was a problem hiding this comment.
Pull request overview
This PR adds material support to the graphics rendering pipeline, enabling entities to have customizable material properties including texture support. The implementation includes GPU-side material management, default material fallback, and a new example demonstrating material usage.
- Introduces Material component with texture support and GPU material buffer system
- Adds automatic GPU component lifecycle management through templated setup functions
- Updates camera and transform forward vector calculations for consistency
Reviewed changes
Copilot reviewed 51 out of 52 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| xmake.lua | Adds build configuration option for the new graphic material usage example |
| src/plugin/object/src/component/Transform.hpp | Updates quaternion initialization order and adds GetForwardVector method |
| src/plugin/object/src/component/Material.hpp | Adds Material::Id type, default ambient value, and ambientTexName field |
| src/plugin/graphic/src/component/GPUMaterial.hpp | New GPU-side material component with buffer, texture, sampler, and bind group IDs |
| src/plugin/graphic/src/component/GPUCamera.hpp | Changes forward vector calculation to use negative Z direction |
| src/plugin/graphic/src/utils/EmptyTexture.hpp | Defines constants for empty texture (magenta/checkerboard pattern) |
| src/plugin/graphic/src/utils/DefaultTexture.hpp | Defines constants for default texture identifier |
| src/plugin/graphic/src/utils/DefaultSampler.hpp | Defines constants for default sampler identifier |
| src/plugin/graphic/src/utils/DefaultMaterial.hpp | Defines constants for default material identifier |
| src/plugin/graphic/src/utils/DefaultPipeline.hpp | Extends shader to include material uniforms, texture sampling, and material bind group support |
| src/plugin/graphic/src/system/preparation/UpdateGPUMaterials.hpp | Header for system that updates GPU material buffers each frame |
| src/plugin/graphic/src/system/preparation/UpdateGPUMaterials.cpp | Implements GPU material buffer updates for all entities with GPUMaterial components |
| src/plugin/graphic/src/system/initialization/CreateEmptyTexture.hpp | Header for system that creates the empty/fallback texture |
| src/plugin/graphic/src/system/initialization/CreateEmptyTexture.cpp | Creates magenta checkerboard pattern texture for missing textures |
| src/plugin/graphic/src/system/initialization/CreateDefaultTexture.hpp | Header for system that creates the default texture |
| src/plugin/graphic/src/system/initialization/CreateDefaultTexture.cpp | Creates default brown texture for materials |
| src/plugin/graphic/src/system/initialization/CreateDefaultSampler.hpp | Header for system that creates the default sampler |
| src/plugin/graphic/src/system/initialization/CreateDefaultSampler.cpp | Creates default sampler resource for texture sampling |
| src/plugin/graphic/src/system/initialization/CreateDefaultMaterial.hpp | Header for system that creates the default material |
| src/plugin/graphic/src/system/initialization/CreateDefaultMaterial.cpp | Creates default material with buffer and bind group for entities without custom materials |
| src/plugin/graphic/src/system/GPUComponentManagement/OnTransformDestruction.hpp | Header for transform component cleanup on destruction |
| src/plugin/graphic/src/system/GPUComponentManagement/OnTransformDestruction.cpp | Cleans up GPU transform buffer and bind group when transform is destroyed |
| src/plugin/graphic/src/system/GPUComponentManagement/OnTransformCreation.hpp | Updates signature to accept Engine::Entity instead of entt::entity |
| src/plugin/graphic/src/system/GPUComponentManagement/OnTransformCreation.cpp | Removes local entity wrapper construction |
| src/plugin/graphic/src/system/GPUComponentManagement/OnMeshDestruction.hpp | Header for mesh component cleanup on destruction |
| src/plugin/graphic/src/system/GPUComponentManagement/OnMeshDestruction.cpp | Cleans up GPU mesh buffers when mesh is destroyed |
| src/plugin/graphic/src/system/GPUComponentManagement/OnMeshCreation.hpp | Updates signature to accept Engine::Entity instead of entt::entity |
| src/plugin/graphic/src/system/GPUComponentManagement/OnMeshCreation.cpp | Removes local entity wrapper construction |
| src/plugin/graphic/src/system/GPUComponentManagement/OnMaterialDestruction.hpp | Header for material component cleanup on destruction |
| src/plugin/graphic/src/system/GPUComponentManagement/OnMaterialDestruction.cpp | Cleans up GPU material buffer and bind group when material is destroyed |
| src/plugin/graphic/src/system/GPUComponentManagement/OnMaterialCreation.hpp | Header for material GPU component creation |
| src/plugin/graphic/src/system/GPUComponentManagement/OnMaterialCreation.cpp | Creates GPU material resources including buffer, texture loading, sampler, and bind group |
| src/plugin/graphic/src/system/GPUComponentManagement/OnCameraDestruction.hpp | Header for camera component cleanup on destruction |
| src/plugin/graphic/src/system/GPUComponentManagement/OnCameraDestruction.cpp | Cleans up GPU camera buffer and bind group when camera is destroyed |
| src/plugin/graphic/src/resource/buffer/MaterialGPUBuffer.hpp | New GPU buffer class for managing material uniform data transfer to GPU |
| src/plugin/graphic/src/resource/Texture.hpp | Adds const correctness and changes default texture format to RGBA8UnormSrgb |
| src/plugin/graphic/src/resource/Sampler.cpp | Simplifies sampler creation by relying on WebGPU defaults |
| src/plugin/graphic/src/resource/DeviceContext.hpp | Adds missing optional header include |
| src/plugin/graphic/src/resource/BindGroup.hpp | Uses GetOrDefault for texture retrieval to support fallback textures |
| src/plugin/graphic/src/plugin/PluginGraphic.cpp | Adds templated GPU component setup and registers material systems |
| src/plugin/graphic/src/Graphic.hpp | Includes all new material-related headers |
| src/plugin/graphic/tests/ShaderTest.cpp | Renames setSamplerType to setType for consistency |
| src/plugin/graphic/tests/RenderPassTest.cpp | Updates texture format to RGBA8UnormSrgb |
| src/plugin/graphic/tests/BindGroupTest.cpp | Renames setSamplerType to setType for consistency |
| src/plugin/graphic/src/utils/shader/SamplerBindGroupLayoutEntry.hpp | Renames method from setSamplerType to setType for API consistency |
| examples/graphic_usage_with_physics/src/main.cpp | Adjusts camera Z position to negative value for correct viewing direction |
| examples/graphic_usage/src/main.cpp | Adjusts camera Z position to negative value for correct viewing direction |
| examples/graphic_material_usage/xmake.lua | Build configuration for the new material usage example |
| examples/graphic_material_usage/src/main.cpp | Complete example demonstrating material usage with camera controls and multiple material configurations |
| examples/graphic_material_usage/README.md | Documentation for building and running the material usage example |
| examples/graphic_material_usage/.gitignore | Ignores build artifacts and IDE files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis pull request introduces a comprehensive material system for the graphics plugin, including GPU resource lifecycle management (textures, samplers, material buffers, bind groups), per-entity material creation and destruction hooks, default texture and sampler initialization, and updates to rendering pipeline math. Additional changes include const-correctness refinements for texture construction and camera transformation calculations. Changes
Sequence Diagram(s)sequenceDiagram
participant Core as Engine Core
participant CompSys as Component System
participant OnMat as OnMaterialCreation
participant Containers as Resource Containers
participant BindGroup as BindGroup Manager
Core->>CompSys: Material component added to entity
CompSys->>OnMat: Trigger creation hook
activate OnMat
OnMat->>Containers: Get TextureContainer, SamplerContainer, GPUBufferContainer
OnMat->>Containers: Create/register Texture from ambientTexName
OnMat->>Containers: Create/register Sampler
OnMat->>Containers: Create/register MaterialGPUBuffer with material data
OnMat->>BindGroup: Create BindGroup with material buffer, texture, sampler
OnMat->>BindGroup: Register BindGroup and store ID in GPUMaterial component
deactivate OnMat
OnMat-->>CompSys: GPUMaterial component attached (resource IDs stored)
Note over Core,BindGroup: Material is now ready for rendering
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/graphic_usage/src/main.cpp (1)
126-134: Critical: Throwing C++ exception from C callback is undefined behavior.The lambda at line 132 throws a C++ exception from a WebGPU error callback, which is a C API boundary. Propagating C++ exceptions through C stack frames is undefined behavior and can cause crashes, resource leaks, or
std::terminate().🔎 Recommended fix
Instead of throwing directly, set a flag or store the error, then check and handle it outside the callback:
// Add member or resource to track errors struct GraphicErrorState { std::atomic<bool> hasError{false}; std::string lastError; }; // In the callback: core.RegisterSystem<RenderingPipeline::Init>([](Engine::Core &core) { core.GetResource<Graphic::Resource::GraphicSettings>().SetOnErrorCallback( [](WGPUDevice const *, WGPUErrorType type, WGPUStringView message, WGPU_NULLABLE void *, WGPU_NULLABLE void *) { Log::Error(fmt::format("Custom uncaptured device error: type {:x} ({})", static_cast<uint32_t>(type), std::string(message.data, message.length))); // Set error flag instead of throwing // (Would need error state tracking mechanism in your architecture) }); }); // Then check the error state in your main loop or after operationsAlternatively, if this example is strictly for demonstration and immediate termination on error is acceptable, consider calling
std::terminate()orstd::abort()directly instead of throwing.src/plugin/graphic/src/Graphic.hpp (1)
76-76: Duplicate include detected.
OnCameraCreation.hppis included twice — here at line 76 and again at line 95. Remove one of the duplicates.🔎 Proposed fix
#include "plugin/PluginGraphic.hpp" -#include "system/GPUComponentManagement/OnCameraCreation.hpp" - // Systems #include "system/initialization/ConfigureSurface.hpp"
♻️ Duplicate comments (1)
src/plugin/graphic/src/system/GPUComponentManagement/OnMaterialCreation.cpp (1)
27-28: Sampler ID derived from texture name creates 1:1 coupling.Using
material.ambientTexNamefor bothtextureIdandsamplerIdmeans each texture gets its own sampler, preventing sampler reuse. This was flagged in a previous review. Consider using a shared default sampler or a separate sampler naming scheme.
🧹 Nitpick comments (14)
examples/graphic_usage/src/main.cpp (2)
25-65: Consider camera-relative movement for more intuitive controls.The camera currently moves in world space directions (W always moves in world -Z) rather than relative to its facing direction. This can feel unintuitive for users expecting FPS-style controls where W moves forward in the camera's view direction.
💡 Optional: Transform movement by camera rotation
To make movement relative to camera facing:
void CameraTranslationSystem(Engine::Core &core) { const float cameraTranslationSpeed = 1.f; const float deltaTime = core.GetScheduler<Engine::Scheduler::FixedTimeUpdate>().GetTickRate(); auto &inputManager = core.GetResource<Input::Resource::InputManager>(); Engine::Entity camera = core.GetRegistry().view<Object::Component::Camera>().front(); auto &transform = camera.GetComponents<Object::Component::Transform>(core); glm::vec3 direction{0.0f}; if (inputManager.IsKeyPressed(GLFW_KEY_W)) direction += glm::vec3(0.0f, 0.0f, 1.0f); // Forward in local space if (inputManager.IsKeyPressed(GLFW_KEY_S)) direction += glm::vec3(0.0f, 0.0f, -1.0f); // Backward in local space if (inputManager.IsKeyPressed(GLFW_KEY_A)) direction += glm::vec3(-1.0f, 0.0f, 0.0f); // Left in local space if (inputManager.IsKeyPressed(GLFW_KEY_D)) direction += glm::vec3(1.0f, 0.0f, 0.0f); // Right in local space if (inputManager.IsKeyPressed(GLFW_KEY_SPACE)) direction += glm::vec3(0.0f, 1.0f, 0.0f); if (inputManager.IsKeyPressed(GLFW_KEY_LEFT_SHIFT)) direction += glm::vec3(0.0f, -1.0f, 0.0f); if (glm::length(direction) > 0.0f) { direction = glm::normalize(direction); // Transform direction by camera's rotation direction = transform.GetRotation() * direction; } transform.SetPosition(transform.GetPosition() + direction * cameraTranslationSpeed * deltaTime); }This assumes Transform provides a
GetRotation()returning a quaternion or similar.
92-95: Consider separate yaw/pitch transforms to avoid gimbal lock.The current rotation applies both pitch and yaw in the camera's local space (
current * delta), which can lead to unexpected roll and gimbal lock issues when the camera pitches beyond ±90 degrees.For more robust FPS-style controls, consider maintaining separate yaw (world Y-axis) and pitch (local X-axis) values:
// Store as separate floats in a resource or component struct CameraController { float yaw = 0.0f; float pitch = 0.0f; }; void CameraRotationSystem(Engine::Core &core) { const float cameraRotationSpeed = 45.f; const float deltaTime = core.GetScheduler<Engine::Scheduler::FixedTimeUpdate>().GetTickRate(); auto &inputManager = core.GetResource<Input::Resource::InputManager>(); auto &controller = core.GetResource<CameraController>(); Engine::Entity camera = core.GetRegistry().view<Object::Component::Camera>().front(); glm::vec2 rotation{0.0f}; if (inputManager.IsKeyPressed(GLFW_KEY_UP)) rotation.x += 1.0f; if (inputManager.IsKeyPressed(GLFW_KEY_DOWN)) rotation.x -= 1.0f; if (inputManager.IsKeyPressed(GLFW_KEY_LEFT)) rotation.y += 1.0f; if (inputManager.IsKeyPressed(GLFW_KEY_RIGHT)) rotation.y -= 1.0f; controller.pitch += glm::radians(cameraRotationSpeed * deltaTime * rotation.x); controller.yaw += glm::radians(cameraRotationSpeed * deltaTime * rotation.y); // Clamp pitch to avoid flipping controller.pitch = glm::clamp(controller.pitch, glm::radians(-89.0f), glm::radians(89.0f)); // Build rotation: yaw around world Y, then pitch around local X glm::quat rotation = glm::angleAxis(controller.yaw, glm::vec3(0.0f, 1.0f, 0.0f)) * glm::angleAxis(controller.pitch, glm::vec3(1.0f, 0.0f, 0.0f)); camera.GetComponents<Object::Component::Transform>(core).SetRotation(rotation); }This prevents gimbal lock and keeps the camera upright.
src/plugin/graphic/src/resource/buffer/MaterialGPUBuffer.hpp (1)
12-12: Consider makingprefixaconstexprstring literal.Using
static inline std::stringallocates on the heap. Aconstexprstring view would be more efficient for a compile-time constant.🔎 Proposed fix
- static inline std::string prefix = "MaterialGPUBuffer_"; + static constexpr std::string_view prefix = "MaterialGPUBuffer_";Then update line 28 to use
std::string(prefix)in the concatenation.src/plugin/object/src/component/Transform.hpp (1)
9-10: Remove unused includes.
glm/gtx/string_cast.hppand<iostream>appear unused in this file. If they were added for debugging, consider removing them.🔎 Proposed fix
#include "glm/gtx/quaternion.hpp" -#include <glm/gtx/string_cast.hpp> -#include <iostream> - namespace Object::Component {src/plugin/graphic/src/resource/Texture.hpp (1)
221-221: Consider type-aware texture format handling.The sRGB format is correct for the material ambient textures currently loaded via this path (color/image data), as it ensures proper gamma correction for rendered colors. However, the generic
Imageclass provides no distinction between color textures and data textures (normal maps, height maps), so this format applies unconditionally to all textures created this way. If non-color texture types are added later, they would incorrectly use sRGB format. Consider adding texture type parameters or separate constructors to support different formats based on intended use.src/plugin/graphic/src/system/GPUComponentManagement/OnTransformCreation.cpp (1)
27-33: Consider extracting magic values to named constants.The shader name
"DEFAULT_RENDER_PASS_SHADER"and binding group index1are hardcoded. For maintainability, consider defining these as constants similar to howDEFAULT_TEXTURE_NAMEandDEFAULT_TEXTURE_IDare defined in the utils headers.src/plugin/graphic/src/system/GPUComponentManagement/OnMeshDestruction.cpp (1)
1-4: Remove unused include.The
BindGroupManager.hppheader is included but not used in this implementation.🔎 Proposed fix
#include "system/GPUComponentManagement/OnMeshDestruction.hpp" #include "component/GPUMesh.hpp" -#include "resource/BindGroupManager.hpp" #include "resource/GPUBufferContainer.hpp"src/plugin/graphic/src/system/initialization/CreateDefaultSampler.cpp (1)
10-15: Consider handling potential null device.
GetDevice().value()will throwstd::bad_optional_accessif the device is not initialized. While this may be guaranteed by the initialization order, consider adding a check or usingvalue_orwith appropriate error handling for robustness.- Resource::Sampler defaultSampler(context.deviceContext.GetDevice().value()); + auto device = context.deviceContext.GetDevice(); + if (!device.has_value()) + { + throw std::runtime_error("Device not initialized when creating default sampler"); + } + Resource::Sampler defaultSampler(device.value());src/plugin/graphic/src/component/GPUMaterial.hpp (1)
3-6: Remove unused includes.
Camera.hpp,Transform.hpp, andglm/glm.hppare not used in this file. Onlyentt/core/hashed_string.hppis required for theentt::hashed_stringtype alias.#pragma once -#include "component/Camera.hpp" -#include "component/Transform.hpp" -#include "glm/glm.hpp" #include <entt/core/hashed_string.hpp>src/plugin/graphic/src/system/GPUComponentManagement/OnMeshCreation.cpp (1)
11-12: Unused variablemesh.The
meshvariable is retrieved on line 12 but does not appear to be used in this function. If it's not needed, consider removing it to avoid confusion.Graphic::Component::GPUMesh &GPUMesh = entity.AddComponent<Graphic::Component::GPUMesh>(core); - const auto &mesh = entity.GetComponents<Object::Component::Mesh>(core);src/plugin/graphic/src/utils/DefaultPipeline.hpp (1)
105-116: TODO noted: Per-frame component check overhead.The TODO correctly identifies that checking
HasComponents<GPUMaterial>every frame has overhead. A pre-render system that ensures all renderable entities have a GPUMaterial would eliminate this branch.Would you like me to open an issue to track this optimization?
examples/graphic_material_usage/src/main.cpp (3)
22-26: Unused struct members.
scrollSensitivityandinertiaare defined but never used in this example. Consider removing them if not planned for future use, or adding the corresponding functionality.
79-79: Remove unused variable.
inputManageris fetched but never used in this function—GetMovementForceobtains it independently.🔎 Proposed fix
void CameraTranslationSystem(Engine::Core &core) { const float cameraTranslationSpeed = 1.f; const float deltaTime = core.GetScheduler<Engine::Scheduler::Update>().GetDeltaTime(); - auto &inputManager = core.GetResource<Input::Resource::InputManager>(); glm::vec3 movementForce = GetMovementForce(core);
86-88: Minor: Reuse the transform reference.Line 88 fetches the transform again when
transformis already available from line 86.🔎 Proposed fix
Engine::Entity camera = core.GetRegistry().view<Object::Component::Camera>().front(); auto &transform = camera.GetComponents<Object::Component::Transform>(core); - glm::vec3 forwardDir = camera.GetComponents<Object::Component::Transform>(core).GetForwardVector(); + glm::vec3 forwardDir = transform.GetForwardVector(); glm::vec3 up{0.0f, 1.0f, 0.0f};
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
examples/graphic_material_usage/asset/texture.pngis excluded by!**/*.png
📒 Files selected for processing (54)
.gitattributesexamples/graphic_material_usage/.gitignoreexamples/graphic_material_usage/README.mdexamples/graphic_material_usage/src/main.cppexamples/graphic_material_usage/xmake.luaexamples/graphic_usage/src/main.cppexamples/graphic_usage_with_physics/src/main.cppsrc/plugin/graphic/src/Graphic.hppsrc/plugin/graphic/src/component/GPUCamera.hppsrc/plugin/graphic/src/component/GPUMaterial.hppsrc/plugin/graphic/src/plugin/PluginGraphic.cppsrc/plugin/graphic/src/resource/BindGroup.hppsrc/plugin/graphic/src/resource/DeviceContext.hppsrc/plugin/graphic/src/resource/Image.hppsrc/plugin/graphic/src/resource/Sampler.cppsrc/plugin/graphic/src/resource/Shader.hppsrc/plugin/graphic/src/resource/ShaderDescriptor.hppsrc/plugin/graphic/src/resource/Texture.hppsrc/plugin/graphic/src/resource/buffer/MaterialGPUBuffer.hppsrc/plugin/graphic/src/system/GPUComponentManagement/OnCameraDestruction.cppsrc/plugin/graphic/src/system/GPUComponentManagement/OnCameraDestruction.hppsrc/plugin/graphic/src/system/GPUComponentManagement/OnMaterialCreation.cppsrc/plugin/graphic/src/system/GPUComponentManagement/OnMaterialCreation.hppsrc/plugin/graphic/src/system/GPUComponentManagement/OnMaterialDestruction.cppsrc/plugin/graphic/src/system/GPUComponentManagement/OnMaterialDestruction.hppsrc/plugin/graphic/src/system/GPUComponentManagement/OnMeshCreation.cppsrc/plugin/graphic/src/system/GPUComponentManagement/OnMeshCreation.hppsrc/plugin/graphic/src/system/GPUComponentManagement/OnMeshDestruction.cppsrc/plugin/graphic/src/system/GPUComponentManagement/OnMeshDestruction.hppsrc/plugin/graphic/src/system/GPUComponentManagement/OnTransformCreation.cppsrc/plugin/graphic/src/system/GPUComponentManagement/OnTransformCreation.hppsrc/plugin/graphic/src/system/GPUComponentManagement/OnTransformDestruction.cppsrc/plugin/graphic/src/system/GPUComponentManagement/OnTransformDestruction.hppsrc/plugin/graphic/src/system/initialization/CreateDefaultMaterial.cppsrc/plugin/graphic/src/system/initialization/CreateDefaultMaterial.hppsrc/plugin/graphic/src/system/initialization/CreateDefaultSampler.cppsrc/plugin/graphic/src/system/initialization/CreateDefaultSampler.hppsrc/plugin/graphic/src/system/initialization/CreateDefaultTexture.cppsrc/plugin/graphic/src/system/initialization/CreateDefaultTexture.hppsrc/plugin/graphic/src/system/initialization/CreateEmptyTexture.cppsrc/plugin/graphic/src/system/initialization/CreateEmptyTexture.hppsrc/plugin/graphic/src/system/preparation/UpdateGPUMaterials.cppsrc/plugin/graphic/src/system/preparation/UpdateGPUMaterials.hppsrc/plugin/graphic/src/utils/DefaultMaterial.hppsrc/plugin/graphic/src/utils/DefaultPipeline.hppsrc/plugin/graphic/src/utils/DefaultSampler.hppsrc/plugin/graphic/src/utils/DefaultTexture.hppsrc/plugin/graphic/src/utils/EmptyTexture.hppsrc/plugin/graphic/src/utils/shader/SamplerBindGroupLayoutEntry.hppsrc/plugin/graphic/tests/BindGroupTest.cppsrc/plugin/graphic/tests/RenderPassTest.cppsrc/plugin/graphic/tests/ShaderTest.cppsrc/plugin/object/src/component/Material.hppsrc/plugin/object/src/component/Transform.hpp
💤 Files with no reviewable changes (2)
- src/plugin/graphic/src/resource/Image.hpp
- src/plugin/graphic/src/resource/Sampler.cpp
🧰 Additional context used
🧬 Code graph analysis (17)
src/plugin/graphic/src/system/preparation/UpdateGPUMaterials.hpp (1)
src/plugin/graphic/src/resource/buffer/MaterialGPUBuffer.hpp (10)
core(34-40)core(34-34)core(41-48)core(41-41)core(50-50)core(50-50)core(51-66)core(51-51)core(68-77)core(68-68)
src/plugin/graphic/src/system/GPUComponentManagement/OnTransformCreation.cpp (2)
src/plugin/graphic/src/system/GPUComponentManagement/OnTransformCreation.hpp (1)
OnTransformCreation(8-8)src/plugin/graphic/src/utils/DefaultPipeline.hpp (1)
entity(100-100)
src/plugin/graphic/src/resource/BindGroup.hpp (1)
src/plugin/graphic/tests/TextureTest.cpp (1)
texture(14-14)
src/plugin/graphic/src/system/GPUComponentManagement/OnCameraDestruction.cpp (1)
src/plugin/graphic/src/system/GPUComponentManagement/OnCameraDestruction.hpp (1)
OnCameraDestruction(8-8)
src/plugin/graphic/src/system/GPUComponentManagement/OnMeshDestruction.cpp (1)
src/plugin/graphic/src/system/GPUComponentManagement/OnMeshDestruction.hpp (1)
OnMeshDestruction(8-8)
src/plugin/graphic/src/resource/buffer/MaterialGPUBuffer.hpp (2)
src/engine/src/entity/Entity.hpp (2)
static_cast(65-65)_entity(70-70)src/plugin/graphic/src/utils/webgpu.hpp (1)
StringView(619-619)
examples/graphic_usage_with_physics/src/main.cpp (1)
src/plugin/graphic/src/component/GPUCamera.hpp (2)
camera(21-28)camera(21-21)
src/plugin/graphic/src/system/preparation/UpdateGPUMaterials.cpp (2)
src/plugin/graphic/src/system/preparation/UpdateGPUMaterials.hpp (1)
UpdateGPUMaterials(7-7)src/plugin/graphic/src/resource/buffer/MaterialGPUBuffer.hpp (10)
core(34-40)core(34-34)core(41-48)core(41-41)core(50-50)core(50-50)core(51-66)core(51-51)core(68-77)core(68-68)
src/plugin/graphic/src/system/GPUComponentManagement/OnTransformDestruction.cpp (1)
src/plugin/graphic/src/system/GPUComponentManagement/OnTransformDestruction.hpp (1)
OnTransformDestruction(8-8)
examples/graphic_material_usage/src/main.cpp (1)
examples/graphic_usage/src/main.cpp (8)
EscapeKeySystem(15-23)EscapeKeySystem(15-15)CameraTranslationSystem(25-65)CameraTranslationSystem(25-25)CameraRotationSystem(67-96)CameraRotationSystem(67-67)Setup(98-113)Setup(98-98)
src/plugin/graphic/src/system/GPUComponentManagement/OnMaterialDestruction.cpp (1)
src/plugin/graphic/src/system/GPUComponentManagement/OnMaterialDestruction.hpp (1)
OnMaterialDestruction(8-8)
src/plugin/graphic/src/utils/DefaultPipeline.hpp (4)
src/plugin/graphic/src/resource/BindGroup.hpp (6)
core(46-50)core(46-46)core(53-57)core(53-53)core(59-68)core(59-59)src/plugin/graphic/src/resource/buffer/MaterialGPUBuffer.hpp (12)
core(34-40)core(34-34)core(41-48)core(41-41)core(50-50)core(50-50)core(51-66)core(51-51)core(68-77)core(68-68)materialComponent(94-98)materialComponent(94-94)src/plugin/graphic/src/resource/SingleExecutionRenderPass.hpp (1)
renderPass(50-50)src/plugin/graphic/src/utils/shader/SamplerBindGroupLayoutEntry.hpp (4)
SamplerBindGroupLayoutEntry(9-12)SamplerBindGroupLayoutEntry(9-9)SamplerBindGroupLayoutEntry(13-13)SamplerBindGroupLayoutEntry(15-15)
src/plugin/graphic/src/system/GPUComponentManagement/OnMeshCreation.cpp (2)
src/plugin/graphic/src/system/GPUComponentManagement/OnMeshCreation.hpp (1)
OnMeshCreation(8-8)src/plugin/graphic/src/utils/DefaultPipeline.hpp (1)
entity(100-100)
src/plugin/object/src/component/Material.hpp (4)
src/plugin/graphic/src/resource/Texture.hpp (2)
other(99-122)other(99-99)src/plugin/graphic/src/utils/shader/SamplerBindGroupLayoutEntry.hpp (2)
other(16-16)other(16-16)src/plugin/graphic/src/resource/Sampler.cpp (2)
other(28-39)other(28-28)src/plugin/object/src/component/Mesh.hpp (4)
other(48-48)other(48-48)other(53-53)other(53-53)
src/plugin/graphic/src/plugin/PluginGraphic.cpp (22)
src/plugin/graphic/src/resource/buffer/MaterialGPUBuffer.hpp (10)
core(34-40)core(34-34)core(41-48)core(41-41)core(50-50)core(50-50)core(51-66)core(51-51)core(68-77)core(68-68)src/plugin/graphic/src/system/GPUComponentManagement/OnCameraCreation.cpp (2)
OnCameraCreation(14-48)OnCameraCreation(14-14)src/plugin/graphic/src/system/GPUComponentManagement/OnCameraDestruction.cpp (2)
OnCameraDestruction(6-22)OnCameraDestruction(6-6)src/plugin/graphic/src/system/GPUComponentManagement/OnMeshCreation.cpp (2)
OnMeshCreation(9-29)OnMeshCreation(9-9)src/plugin/graphic/src/system/GPUComponentManagement/OnMeshDestruction.cpp (2)
OnMeshDestruction(6-21)OnMeshDestruction(6-6)src/plugin/graphic/src/system/GPUComponentManagement/OnTransformCreation.cpp (2)
OnTransformCreation(10-36)OnTransformCreation(10-10)src/plugin/graphic/src/system/GPUComponentManagement/OnTransformDestruction.cpp (2)
OnTransformDestruction(6-22)OnTransformDestruction(6-6)src/plugin/graphic/src/system/GPUComponentManagement/OnMaterialCreation.cpp (2)
OnMaterialCreation(16-67)OnMaterialCreation(16-16)src/plugin/graphic/src/system/GPUComponentManagement/OnMaterialDestruction.cpp (2)
OnMaterialDestruction(6-22)OnMaterialDestruction(6-6)src/plugin/graphic/src/system/initialization/CreateInstance.cpp (2)
CreateInstance(8-18)CreateInstance(8-8)src/plugin/graphic/src/system/initialization/ReleaseInstance.cpp (2)
ReleaseInstance(5-14)ReleaseInstance(5-5)src/plugin/graphic/src/system/initialization/RequestCapabilities.cpp (2)
RequestCapabilities(7-17)RequestCapabilities(7-7)src/plugin/graphic/src/system/initialization/CreateDevice.cpp (2)
CreateDevice(29-40)CreateDevice(29-29)src/plugin/graphic/src/system/initialization/CreateQueue.cpp (2)
CreateQueue(7-18)CreateQueue(7-7)src/plugin/graphic/src/system/initialization/ConfigureSurface.cpp (2)
ConfigureSurface(7-72)ConfigureSurface(7-7)src/plugin/graphic/src/system/initialization/ReleaseAdapter.cpp (2)
ReleaseAdapter(6-11)ReleaseAdapter(6-6)src/plugin/graphic/src/system/initialization/CreateEmptyTexture.cpp (2)
CreateEmptyTexture(8-23)CreateEmptyTexture(8-8)src/plugin/graphic/src/system/initialization/CreateDefaultTexture.cpp (2)
CreateDefaultTexture(8-16)CreateDefaultTexture(8-8)src/plugin/graphic/src/system/initialization/CreateDefaultSampler.cpp (2)
CreateDefaultSampler(8-16)CreateDefaultSampler(8-8)src/plugin/graphic/src/system/initialization/CreateDefaultRenderPipeline.cpp (2)
CreateDefaultRenderPipeline(11-38)CreateDefaultRenderPipeline(11-11)src/plugin/graphic/src/system/preparation/UpdateGPUTransforms.cpp (2)
UpdateGPUTransforms(5-13)UpdateGPUTransforms(5-5)src/plugin/graphic/src/system/preparation/UpdateGPUMaterials.cpp (2)
UpdateGPUMaterials(5-13)UpdateGPUMaterials(5-5)
src/plugin/graphic/src/system/initialization/CreateDefaultMaterial.cpp (4)
src/plugin/graphic/src/system/initialization/CreateDefaultMaterial.hpp (1)
CreateDefaultMaterial(7-7)src/plugin/graphic/src/resource/BindGroup.hpp (6)
core(46-50)core(46-46)core(53-57)core(53-53)core(59-68)core(59-59)src/plugin/graphic/src/resource/buffer/MaterialGPUBuffer.hpp (10)
core(34-40)core(34-34)core(41-48)core(41-41)core(50-50)core(50-50)core(51-66)core(51-51)core(68-77)core(68-68)src/plugin/graphic/src/resource/Texture.hpp (8)
Texture(53-57)Texture(59-63)Texture(65-69)Texture(71-75)Texture(77-84)Texture(86-86)Texture(89-97)Texture(211-211)
examples/graphic_usage/src/main.cpp (1)
src/plugin/graphic/src/component/GPUCamera.hpp (2)
camera(21-28)camera(21-21)
🪛 Cppcheck (2.19.0)
examples/graphic_material_usage/src/main.cpp
[error] 187-187: Unhandled exception thrown in function that is an entry point.
(throwInEntryPoint)
🪛 LanguageTool
examples/graphic_material_usage/README.md
[grammar] ~3-~3: Ensure spelling is correct
Context: ...Graphic usage ## How to run / debug on vscode 1. Remove includes("../../xmake.lua") fro...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (55)
.gitattributes (1)
1-32: LGTM! Well-structured .gitattributes configuration.The file properly establishes line-ending conventions (LF), exhaustively marks binary file types, and disambiguates language classification for a mixed C/C++/Lua codebase. Using
.h/.cfor C and.hpp/.cppfor C++ follows established convention and helps GitHub's linguist correctly identify file languages.examples/graphic_usage_with_physics/src/main.cpp (1)
126-126: LGTM!The camera position adjustment places the camera at a reasonable viewing angle for observing the physics simulation. The change is consistent with the left-handed coordinate system and camera movement controls.
examples/graphic_usage/src/main.cpp (1)
107-107: The camera position is correct. With the Transform component's default forward vector being(0, 0, 1)(+Z direction), a camera at(0, 0, -2)with identity rotation looks toward the cube at the origin.examples/graphic_material_usage/.gitignore (1)
1-3: LGTM!Standard ignore patterns for an xmake-based C++ example project.
src/plugin/graphic/src/resource/DeviceContext.hpp (1)
4-4: LGTM!Necessary include for
std::optional<wgpu::Device>used in this header.src/plugin/graphic/src/system/initialization/CreateDefaultMaterial.hpp (1)
1-9: LGTM!Clean header declaration with proper include guard, namespace, and closing comment.
src/plugin/graphic/src/Graphic.hpp (1)
38-40: LGTM!Well-organized additions for default resource utilities, initialization systems, GPU component lifecycle management, and material preparation. The includes align with the material system feature.
Also applies to: 81-106
src/plugin/graphic/src/resource/buffer/MaterialGPUBuffer.hpp (3)
34-48: LGTM on buffer lifecycle.The
CreateandDestroymethods correctly manage the GPU buffer lifecycle with proper state tracking via_isCreated.
94-98: LGTM on buffer update logic.The
_UpdateBuffercorrectly writesCPUSizebytes (12) to the buffer. The buffer was created withGPUSize(16 bytes) to satisfy WGSL uniform alignment requirements forvec3<f32>.
58-61: The comparison_entity == Engine::Entity::entity_null_idis valid. TheEntityclass defines an explicitoperator==(const entity_id_type &rhs)member function that allows comparison withentity_null_id(which is of typeentity_id_type). This is not implicit conversion—it's an overloaded operator specifically designed for this comparison and is used consistently throughout the codebase.src/plugin/graphic/src/resource/Texture.hpp (1)
59-75: LGTM on const-correctness improvements.Good refactor to accept
const Context&in constructors and methods. This properly signals that these operations don't modify the context.Also applies to: 127-148, 150-152
src/plugin/object/src/component/Transform.hpp (1)
73-80: Use GLM's built-in quaternion rotation instead of manual calculation.The GetForwardVector formula uses a non-standard approach to quaternion-to-vector rotation that differs from the typical formula for rotating (0, 0, 1) by quaternion (w, x, y, z). The signs on some terms don't match the standard rotation matrix. While GPUCamera uses GetForwardVector (not a hardcoded direction as mentioned), the manual calculation is error-prone and harder to verify for correctness.
Replace with GLM's operator* for clarity and safety:
glm::vec3 GetForwardVector() const { return glm::normalize(_rotation * glm::vec3(0.0f, 0.0f, 1.0f)); }Additionally, GetForwardVector currently has no test coverage, which would help ensure the rotation behavior is correct.
src/plugin/graphic/src/resource/BindGroup.hpp (1)
123-123: LGTM: Graceful texture fallback.The change from
GettoGetOrDefaultprovides graceful degradation when a texture is missing by falling back to a default texture, which aligns well with the PR's introduction of default resource support.src/plugin/graphic/src/system/GPUComponentManagement/OnMeshDestruction.hpp (1)
1-10: LGTM: Consistent destruction hook API.The declaration follows the same pattern as other GPU component destruction hooks in this PR (OnCameraDestruction, OnTransformDestruction), maintaining API consistency across the component lifecycle management system.
src/plugin/graphic/src/system/initialization/CreateDefaultTexture.hpp (1)
1-7: LGTM: Simple initialization API.Clean forward declaration consistent with other default resource initialization functions in the PR (CreateDefaultSampler, CreateDefaultMaterial, CreateEmptyTexture).
src/plugin/graphic/src/system/initialization/CreateEmptyTexture.hpp (1)
1-7: LGTM: Simple initialization API.Clean forward declaration consistent with other resource initialization functions in the PR.
src/plugin/graphic/src/system/GPUComponentManagement/OnCameraDestruction.hpp (1)
1-10: LGTM: Consistent destruction hook API.The declaration follows the established pattern for GPU component destruction hooks, maintaining consistency across the component lifecycle management system.
src/plugin/graphic/src/system/GPUComponentManagement/OnTransformDestruction.hpp (1)
1-10: LGTM: Consistent destruction hook API.The declaration follows the established pattern for GPU component destruction hooks, maintaining consistency across the component lifecycle management system.
src/plugin/graphic/tests/RenderPassTest.cpp (2)
44-45: LGTM: Vertex order updated for back-face culling.The vertex order has been updated to use clockwise winding, which correctly aligns with the new default back-face culling behavior (
CullMode::Back) introduced inShaderDescriptor.hpp. This ensures the quad's front faces are rendered properly.
67-67: LGTM: Color space correction.The texture format change from
RGBA8UnormtoRGBA8UnormSrgbapplies proper gamma correction for sRGB color space, which is typically the correct choice for standard color rendering and display.src/plugin/graphic/src/resource/ShaderDescriptor.hpp (1)
213-213: Remove concern about changing default cull mode—ShaderDescriptor was created with Back culling as the intended design.This file was newly created with
_cullMode = wgpu::CullMode::Backas the initial default (not changed fromNone). The test file explicitly sets this same mode with a comment noting "// This is done by default," confirming the developers intentionally designed it this way and validated compatibility. No evidence of breaking changes or incompatible code exists in the codebase.src/plugin/graphic/src/system/initialization/CreateDefaultTexture.cpp (1)
7-16: LGTM!The implementation correctly creates a 2x2 solid-color default texture and registers it in the texture container. The unused
posparameter in the lambda is appropriate here since the texture is uniform.src/plugin/graphic/src/system/GPUComponentManagement/OnTransformCreation.cpp (1)
10-10: LGTM!The signature update from
entt::entitytoEngine::Entityaligns with the header declaration and the broader API consistency changes across GPU component management hooks.src/plugin/graphic/src/system/GPUComponentManagement/OnMaterialCreation.hpp (1)
1-10: LGTM!The header follows the established pattern for GPU component management hooks, with appropriate includes and the
Engine::Entityparameter type consistent with other hooks in this namespace.src/plugin/graphic/src/system/initialization/CreateEmptyTexture.cpp (1)
7-23: LGTM!The implementation correctly creates a 2x2 magenta/black checkerboard texture using a position-based lambda and registers it as the container's default texture. The checkerboard pattern logic is correct.
src/plugin/graphic/src/system/GPUComponentManagement/OnTransformCreation.hpp (1)
8-8: LGTM!The signature update to
Engine::Entityis consistent with the implementation and aligns with the API standardization across GPU component management hooks.src/plugin/graphic/tests/ShaderTest.cpp (1)
72-75: LGTM!The test correctly uses the renamed
setTypemethod, consistent with the API change inSamplerBindGroupLayoutEntry.src/plugin/graphic/src/utils/EmptyTexture.hpp (1)
6-9: LGTM!The constants follow the established pattern from
DefaultTexture.hppandDefaultSampler.hpp. Thestatic inlinespecifiers correctly prevent ODR violations when the header is included in multiple translation units.src/plugin/graphic/src/system/preparation/UpdateGPUMaterials.hpp (1)
1-9: LGTM!Clean header declaration following project conventions. The function signature is clear and properly namespaced.
src/plugin/graphic/src/utils/shader/SamplerBindGroupLayoutEntry.hpp (1)
30-35: LGTM! API rename improves consistency.The method rename from
setSamplerTypetosetTypealigns with the naming convention used inBufferBindGroupLayoutEntry::setType(as shown in BufferBindGroupLayoutEntry.hpp), improving API consistency across bind group layout entry types.src/plugin/graphic/tests/BindGroupTest.cpp (1)
103-106: LGTM!Test correctly updated to use the renamed
setTypeAPI.src/plugin/graphic/src/system/initialization/CreateDefaultSampler.hpp (1)
1-7: LGTM!Clean header declaration consistent with the graphics initialization workflow.
src/plugin/graphic/src/system/GPUComponentManagement/OnMeshCreation.hpp (1)
8-8: LGTM!Parameter type migration from
entt::entitytoEngine::Entityis consistent with the broader API refactoring across GPU component management hooks.src/plugin/graphic/src/system/GPUComponentManagement/OnMeshDestruction.cpp (1)
6-21: LGTM!The destruction logic is correct with proper defensive checks using
Containsbefore removing GPU buffers, and appropriately cleans up the GPUMesh component.src/plugin/graphic/src/system/GPUComponentManagement/OnMaterialDestruction.hpp (1)
1-10: LGTM!Clean header declaration consistent with other GPU component destruction hooks in the system.
src/plugin/graphic/src/system/preparation/UpdateGPUMaterials.cpp (1)
5-13: Defensive check not required, but consider adding for robustness.The code relies on system lifecycle guarantees: GPUMaterial components are only created in OnMaterialCreation.cpp, where
materialBuffer->Create(core)is called immediately (line 46). UpdateGPUMaterials runs in the Preparation stage after component creation, so all GPU buffers should be created before this update phase runs.That said, the Update method (MaterialGPUBuffer.hpp, lines 53-56) does throw UpdateBufferError if the buffer is not created. While the current architecture appears to guarantee creation, adding an
if (!gpuBuffer->IsCreated(core))check would make the dependency explicit and protect against future system ordering changes.examples/graphic_material_usage/xmake.lua (1)
1-28: Build configuration looks reasonable.The target setup with dependencies, packages, and include paths follows the expected pattern for xmake projects. The Windows-specific warning flags and compile commands generation are good additions.
src/plugin/graphic/src/system/initialization/CreateDefaultMaterial.cpp (1)
13-27: Material buffer creation and registration looks correct.The flow of creating the material buffer, setting default ambient values, and registering it in the container is well-structured.
src/plugin/graphic/src/component/GPUMaterial.hpp (1)
8-16: Simple and clean component definition.The struct appropriately uses
entt::hashed_stringfor resource IDs, enabling efficient hash-based lookups in resource containers.src/plugin/graphic/src/utils/DefaultSampler.hpp (1)
1-9: LGTM!The constant definitions follow a consistent pattern with other default resource headers (
DefaultTexture.hpp,DefaultMaterial.hpp). Usingstatic inline constexprfor the string view andstatic inline constfor the hashed string is appropriate.src/plugin/graphic/src/utils/DefaultTexture.hpp (1)
1-9: LGTM!Consistent with the other default resource constant headers.
src/plugin/graphic/src/system/GPUComponentManagement/OnMeshCreation.cpp (1)
9-9: Signature update aligns with broader refactor.The change from
entt::entitytoEngine::Entityis consistent with other system hooks in this PR.src/plugin/graphic/src/utils/DefaultMaterial.hpp (1)
1-12: LGTM!The header follows the established pattern for default resource identifiers. The additional bind group constants are well-organized alongside the material identifiers.
src/plugin/graphic/src/component/GPUCamera.hpp (1)
23-23: Multiplying forward vector by scale before normalization has no effect for uniform scales.The expression
transform.GetForwardVector() * transform.GetScale()followed byglm::normalize()will produce the same direction asglm::normalize(transform.GetForwardVector())when scale is uniform. If the intent is to handle negative scale for mirroring, this works, but for positive uniform scales, the multiplication is redundant. Consider clarifying the intent or simplifying if mirroring isn't needed.src/plugin/graphic/src/system/GPUComponentManagement/OnCameraDestruction.cpp (1)
6-22: LGTM!The implementation correctly follows the established destruction pattern: early return if component is absent, cleanup of associated GPU resources (buffer and bind group), and component removal.
src/plugin/graphic/src/system/GPUComponentManagement/OnTransformDestruction.cpp (1)
6-22: LGTM!The implementation correctly handles GPU resource cleanup for transforms, following the same pattern as other destruction handlers.
src/plugin/graphic/src/plugin/PluginGraphic.cpp (2)
10-18: Well-designed helper template for GPU component lifecycle management.The
SetupGPUComponenttemplate cleanly consolidates the creation/destruction wiring. Connecting destruction to bothCPUComponentandGPUComponentensures cleanup regardless of which is removed first, and the early-return guard in destruction functions prevents double-cleanup issues.
33-40: LGTM!The new material component wiring follows the same pattern as Camera, Mesh, and Transform, maintaining consistency across the codebase.
src/plugin/graphic/src/utils/DefaultPipeline.hpp (1)
147-161: LGTM!The material bind group layout correctly defines the uniform buffer, texture, and sampler bindings matching the WGSL shader declarations at group 2.
src/plugin/object/src/component/Material.hpp (1)
41-67: LGTM!The additions of
Idtype alias,ambientTexNamefield, and defaultambientvalue align well with the GPU material integration. The copy/move semantic declarations are correct and the previously noted parameter naming issue (mesh→other) has been addressed.examples/graphic_material_usage/src/main.cpp (5)
29-37: LGTM!Clean and consistent with the established pattern from other examples.
39-73: LGTM!Good extraction of input handling into a reusable static helper. The direction vectors are correctly oriented for the camera-relative movement system.
98-139: LGTM!The callback registration pattern is appropriate since this system is registered with
Engine::Scheduler::Startup, ensuring callbacks are registered only once.
141-174: LGTM!Well-structured setup demonstrating three material usage patterns: default (no component), with texture, and without texture. The system registrations use appropriate schedulers.
176-179: LGTM!Standard exception class pattern.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Related to no issues. <img width="912" height="944" alt="Capture d’écran 2025-12-26 à 15 44 18" src="https://github.com/user-attachments/assets/45cc6cb9-b4e6-4742-9052-bfa1d6587923" /> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Graphic Material Usage example demonstrating material and texture functionality. * Implemented default material, sampler, and texture system initialization for graphics. * Enhanced camera system with improved vector calculations and transform handling. * Added material lifecycle management with automatic resource cleanup. * **Improvements** * Refined render pipeline with improved front-face culling settings. * Better fallback behavior for missing textures using default resources. * Updated material component to support texture names and default values. * **Documentation** * Added README for Graphic Material Usage example with build and debug instructions. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Master_Laplace <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>



Related to no issues.
Summary by CodeRabbit
New Features
Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.