fix(asset): scene texture paths resolve against project root, not cwd (#887) - #916
Conversation
…project root, not cwd (#887) EditorAssetManager::ImportAsset resolved a relative input path (the "Assets/Textures/Foo.png" spelling scene YAML stores) via std::filesystem::absolute(), which resolves against the process's current working directory (OloEditor runs with cwd = OloEditor/, one level above the project directory). Whenever a same-named file happened to also exist under OloEditor's own generic assets/ tree — true for every shipped texture — that wrong resolution silently "succeeded", registering the asset under a bogus "../assets/..." relative key. TextureSerializer::TryLoadData then loaded straight from that bogus (still cwd-relative) key instead of prefixing Project::GetProjectDirectory() like every other AssetSerializer in the file already does, so the actual pixel load failed and a placeholder texture was substituted — non-null, but with an empty GetPath(). SceneSerializer wrote that empty path back into the scene (YAML), and a component that had already round-tripped an empty path once dropped the reference entirely on the next save (.scenebin), since an empty path fails to resolve back into a texture on reload. Fix: ImportAsset now tries the documented project-relative resolution first, falling back to the previous cwd-relative behaviour only when that fails (preserving several existing scenes that still carry the older working-directory-relative spelling). TextureSerializer now reads from Project::GetProjectDirectory() / metadata.FilePath while keeping the reported identity (GetPath()) project-relative, via a new optional identityPath parameter on Texture2D::Create threaded through both the OpenGL and Vulkan backends. Adds AssetSceneLoad.DecalAndSpriteTexturePathsSurviveRoundTrip, which loads DecalModeMatrixTest.olo and PinkCubeWithTextures.olo twice each (once via YAML, once via the resulting .scenebin sidecar) through a staged EditorAssetManager and asserts both texture paths survive non-empty and identical across both loads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 32 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
1 participant
Summary
Root-caused and fixed #887. The decal texture arm was the reported symptom; the
underlying bug affected any scene-authored, project-relative texture path
("Assets/Textures/Foo.png"), not just decals.
Root cause, found by driving the live editor over its MCP diagnostics server and
reading
OloEngine.log:EditorAssetManager::ImportAssetresolved a relative input path viastd::filesystem::absolute(), which resolves against the process's currentworking directory (
OloEditor/), not the project directory(
OloEditor/SandboxProject/) the path is documented to be relative to. BecauseOloEditor/also ships a copy of every generic texture under its ownassets/tree, this wrong resolution frequently "succeeded" anyway — but registered the
asset under a bogus
../assets/...relative key.TextureSerializer::TryLoadDatathen read straight from that bogus (stillcwd-relative) key instead of prefixing
Project::GetProjectDirectory(), the wayevery other
AssetSerializer::TryLoadDatain the file already does. The actualpixel load failed, and
AssetManager::ResolveAssetOrPlaceholdersilentlysubstituted a placeholder texture — non-null, but with an empty
GetPath().SceneSerializerthen wrote that empty path back into the scene YAML, and acomponent that had already round-tripped an empty path once dropped the reference
entirely on the next save (the
.scenebinsymptom) — an empty path fails toresolve back into a texture on reload, so nothing is written at all. This matches
the "double-silent, second-load" framing in the issue exactly.
Fix
EditorAssetManager::ImportAssetnow tries the documented project-relativeresolution first, falling back to the previous cwd-relative behaviour only when
that fails — several existing scenes (
PinkCubeWithTextures, the Sponza scenes,VehiclesTest,Drift) still carry the older working-directory-relativespelling and must keep resolving unchanged.
TextureSerializer::TryLoadData/TryLoadRawDatanow read fromProject::GetProjectDirectory() / metadata.FilePath, matching every otherserializer in the file, while keeping the reported identity (
GetPath())project-relative — via a new optional
identityPathparameter onTexture2D::Create, threaded through both the OpenGL and Vulkan backends(purely additive, default-valued, so no existing call site changes behaviour).
Verified live over the editor's MCP diagnostics server, before and after the fix,
across both the YAML and
.scenebinload paths (see log excerpts in the linkedissue investigation).
Test plan
AssetSceneLoad.DecalAndSpriteTexturePathsSurviveRoundTrip— loadsDecalModeMatrixTest.oloandPinkCubeWithTextures.olotwice each (once viaYAML, once via the resulting
.scenebinsidecar) through a stagedEditorAssetManager, asserting both texture paths survive non-empty andidentical across both loads.
OloEngine-Tests.exe --gtest_filter="AssetSceneLoad.*"— 3/3 passed.OloEngine-Tests.exe --gtest_filter="*Texture*:*Sprite*:*Decal*:ComponentRoundTrip.*:SceneBinarySidecar.*:*AssetContentValidity*"— 355/355 passed.olo_scene_open+olo://scene/current): bothDecalModeMatrixTest.olo'sEmissiveTexturePathandPinkCubeWithTextures.olo'sTexturePathresolvecorrectly and round-trip through both load paths.
🤖 Generated with Claude Code