Update From Dev by bluesky013 · Pull Request #79 · bluesky013/SkyEngine

bluesky013 · 2026-03-23T17:58:53Z

No description provided.

- Add header-only CmdParser (core/cmdline/CmdParser.h) with default value support - Remove cxxopts third-party dependency from CMake and thirdparty.json - Update all 10 source files to use sky::CmdOptions/CmdValue/CmdParseResult

Copilot

Pull request overview

This PR looks like a broad “engine integration/update” touching tools, plugin build configuration, shader/render pipeline behavior (including reverse-Z + pipeline variant keys), asset/build pipeline, and third-party/CMake helpers.

Changes:

Replace some third-party usages (PerlinNoise, cxxopts, crc32c) with in-engine equivalents and simplify third-party CMake via helper functions.
Introduce JSON-driven plugin configuration/switches and apply dependencies via plugin.json.
Extend render pipeline variant handling (pass options, pipeline keys) and start integrating reverse-Z depth behavior.

Reviewed changes

Copilot reviewed 234 out of 235 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
engine/framework/src/asset/AssetManager.cpp	Switch asset loading to memory-backed archive; adds error logging in async load path.
engine/test/render/StaticMeshTest.cpp	Updates static mesh test types/asserts; minor formatting change at end.
engine/render/backend/metal/src/Queue.mm	Adds `ReadImage` override (currently stubbed).
engine/render/shader/src/ShaderVariant.cpp	Fixes shader variant bit packing and adds `PipelineVariantSetter`.
engine/render/adaptor/src/pipeline/ForwardMSAAPass.cpp	Uses `DepthSettings` for depth clear; adds pass pipeline key list.
engine/render/backend/vulkan/src/DescriptorSet.cpp	Adjust descriptor update handling for samplers and binding lookup.
engine/render/core/include/render/RenderDepthSettings.h	Adds centralized depth helper utilities for reverse-Z.
engine/test/core/TreeTest.cpp	File header modified (now includes BOM).
engine/render/builder/render/src/ImageBuilder.cpp	Adds resizing step + logging; changes width/height assignment.
engine/render/builder/render/src/MeshBuilder.cpp	Adds logging/includes; alters dependency build behavior (commented build requests).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-23T18:03:29Z

engine/render/adaptor/src/pipeline/ForwardMSAAPass.cpp

@@ -48,7 +51,8 @@ namespace sky {
                Attachment{rdg::RasterAttachment{fwdColor, rhi::LoadOp::DONT_CARE, rhi::StoreOp::STORE}, rhi::ClearValue(0.f, 0.f, 0.f, 0.f)});

            depthStencil =
-                Attachment{rdg::RasterAttachment{fwdMSAADepthStencil, rhi::LoadOp::CLEAR, rhi::StoreOp::DONT_CARE}, rhi::ClearValue(1.f, 0)};
+                Attachment{rdg::RasterAttachment{fwdMSAADepthStencil, rhi::LoadOp::CLEAR, rhi::StoreOp::DONT_CARE},
+                    DepthSettings::DepthStencilClear(reverseZ)};


reverseZ is hard-coded to true here, but other passes (e.g. DepthPass) are using non-reverse-Z clear values. This will make the depth clear/compare conventions inconsistent across passes and can break depth testing. Please derive reverseZ from the active SceneView (or a pipeline-wide setting) and use that consistently.

Copilot · 2026-03-23T18:03:30Z

engine/render/backend/vulkan/src/DescriptorSet.cpp

+        // Find the VkWriteDescriptorSet entry for this binding
+        VkDescriptorType descriptorType = VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE;
+        for (const auto &entry : writeEntries) {
+            if (entry.dstBinding == binding) {
+                descriptorType = entry.descriptorType;
+                break;
+            }
+        }


If binding is not present in writeEntries, this code silently falls back to VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE, which can select the wrong image layout. Consider asserting when the binding isn't found (or query the descriptor type from the layout directly) so incorrect bindings fail loudly.

Copilot · 2026-03-23T18:03:30Z

engine/test/core/TreeTest.cpp

@@ -1,4 +1,4 @@
-//
+//


This file now begins with a UTF-8 BOM character before // (shows up as //). Please remove the BOM to avoid potential compiler/tooling issues and noisy diffs across platforms.

Copilot · 2026-03-23T18:03:30Z

engine/render/builder/render/src/ImageBuilder.cpp

+        globalConfig.maxWidth = 2048;
+        globalConfig.maxHeight = 2048;
+
+        // Limit resolution to global max
+        {
+            ImageResizer resizer(ImageResizer::Payload{image, globalConfig.maxWidth, globalConfig.maxHeight});
+            resizer.DoWork();
+        }


globalConfig.maxWidth/maxHeight are hard-coded here (2048×2048), which overrides any configuration set via SetGlobalConfig() and makes the resize behavior non-configurable. Prefer using the existing globalConfig values as provided by the caller (and only apply defaults if they are unset).

Copilot · 2026-03-23T18:03:30Z

engine/render/builder/render/src/MeshBuilder.cpp

+static const char* TAG = "MaterialBuilder";



TAG is set to "MaterialBuilder" in MeshBuilder.cpp, which makes logs misleading. Please rename it to something accurate (e.g., "MeshBuilder").

Copilot · 2026-03-23T18:03:31Z

engine/render/builder/render/src/ImageBuilder.cpp

+        LOG_I(TAG, "Build Imasge Success. %s", request.assetInfo->path.path.GetStr().c_str());
    }


Log message has a typo (Build Imasge Success). Please correct to "Build Image Success" to keep logs searchable/consistent.

Copilot · 2026-03-23T18:03:31Z

engine/render/builder/render/src/MeshBuilder.cpp

        const auto& matData = asset->Data();
-        for (auto& mat : matData.materials) {
-            AssetBuilderManager::Get()->BuildRequest(mat, request.target);
+        for (const auto& mat : matData.materials) {
+            // AssetBuilderManager::Get()->BuildRequest(mat, request.target);
            asset->AddDependencies(mat);
        }


The dependency build requests are commented out (BuildRequest(...)) but the assets are still added as dependencies. If this is intentional, please remove the commented-out calls or replace them with a clear flag/strategy (e.g., build dependencies in a separate stage) to avoid leaving dead code in the hot path.

Copilot · 2026-03-23T18:03:32Z

engine/framework/src/asset/AssetManager.cpp

+        auto  bin  = file->ReadBin();
+        IStreamArchivePtr archive = new IMemoryArchive(bin);
+


file->ReadBin() can fail and return a null BinaryDataPtr; IMemoryArchive(bin) immediately dereferences bin in its constructor. Add a null/size check after ReadBin() and return an error (or fall back to ReadAsArchive()) if the file read fails.

Copilot · 2026-03-23T18:03:32Z

engine/framework/src/asset/AssetManager.cpp

+                if (!asset)
+                {
+                    auto sourceAsset = AssetDataBase::Get()->FindAsset(uuid);
+                    LOG_E(TAG, "Asset %s : %s not found in database. Maybe deleted?", uuid.ToString().c_str(), sourceAsset->path.path.GetStr().c_str());
+                }


This error log dereferences sourceAsset without checking it for null (FindAsset(uuid) can return nullptr). If asset is missing, this can crash before the subsequent SKY_ASSERT(asset). Guard sourceAsset (and/or log only uuid) to avoid null deref in the failure path.

Copilot · 2026-03-23T18:03:32Z

engine/render/backend/metal/src/Queue.mm

+    rhi::TransferTaskHandle Queue::ReadImage(const rhi::ImagePtr& image, rhi::ReadCallBack&& callback)
+    {
+        return CreateTask([]() {
+
+        });
+    }


Queue::ReadImage currently creates an empty task and never performs a readback or invokes the callback. If this API is expected to work on Metal, implement the readback and call callback (including error signaling); otherwise consider returning a failed task or explicitly asserting/marking as unsupported to avoid silent no-ops.

bluesky013 added 29 commits March 8, 2026 16:37

[feat]: Image Resizer && fix test compilation

202d2fd

[feat]: Fix FileStream.

28bf65a

[feat]: ReverseZ

e57d1be

[feat]: update 3rdparty

571767b

[feat]: fix pipeline key && fix ibl.

716206a

[feat]: fix skeleton mesh.

c5413fa

[feat]: Transparent Mat.

9f4c9f5

[feat]: fix camera.

2376759

[3rd]: update 3rd cmake

945dc63

[doc]: project reference.

83810ca

[build]: plugin system driven by JSON config

61130ba

[feat]: crc32 replace google::crc32c

d2af04e

[feat]: UTF encode / decode.

316e3f9

[feat]: replace perlin noise.

4e329b5

[feat]: fix perlin noise.

8b540d6

refactor: replace cxxopts with built-in CmdParser

2f40fbc

- Add header-only CmdParser (core/cmdline/CmdParser.h) with default value support - Remove cxxopts third-party dependency from CMake and thirdparty.json - Update all 10 source files to use sky::CmdOptions/CmdValue/CmdParseResult

Remove nodeeditor dependency and related editor code

52a3031

fix: mac os build.

9130ec9

fix: Add DS_Store

461cd4a

[skill]: update engine skill

585a26d

[ops]: open spec init.

9e4d58e

[ops]: add coding rules.

74c5329

[feat]: Node to Generate ResourceGroup

6f9bd29

[fix]: Unicode and Warning.

f471083

[feat]: Update Shader Node

fcd2bce

[3rd]: update 3rd

5aae99e

[skill]: Update Build Skill.

64ae801

feat: add docs-generate skill

a15dc2d

add doc to opencode

19360be

bluesky013 self-assigned this Mar 23, 2026

bluesky013 assigned Codex Mar 23, 2026

Copilot AI review requested due to automatic review settings March 23, 2026 17:58

Copilot started reviewing on behalf of bluesky013 March 23, 2026 17:59 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

bluesky013 added 9 commits March 25, 2026 02:15

[feat]: Terrain Plugin.

bca76f4

feat: move plugin 3rd-party finds to each plugin CMakeLists

bcb73fe

tool: sync OpenCode skills to Copilot and add sync-from-opencode skill

3e4b0bd

doc: add C++ file creation skills

0c8f0f0

[feat]: Terminal Console.

1c7a6b2

[feat]: Console By ImGUI

f183571

[3rd]: update find 3rd.

6580f57

doc: refresh README build guide

03e5e08

[feat]: Text Align.

c5826cb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update From Dev#79

Update From Dev#79
bluesky013 wants to merge 38 commits intomainfrom
dev

bluesky013 commented Mar 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		LOG_I(TAG, "Build Imasge Success. %s", request.assetInfo->path.path.GetStr().c_str());
		}

		auto bin = file->ReadBin();
		IStreamArchivePtr archive = new IMemoryArchive(bin);

Conversation

bluesky013 commented Mar 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants