Skip to content

Extend extended loader to avoid state leakage across asset parses#32

Open
j1george wants to merge 2 commits intomainfrom
dev/george/ME-6870/load-flag
Open

Extend extended loader to avoid state leakage across asset parses#32
j1george wants to merge 2 commits intomainfrom
dev/george/ME-6870/load-flag

Conversation

@j1george
Copy link
Copy Markdown

@j1george j1george commented Apr 16, 2026

https://fieldwire.atlassian.net/browse/ME-6870

Resets extended-loader parse state so later asset parses do not reuse stale loaded-state.

This updates the extended loader to clear the loaded-state for buffer/decode work before a new asset parse begins, while still preserving the intended once-per-asset behavior within a single parse. The goal is to prevent later asset loads on the same loader from incorrectly skipping required work because state from an earlier parse was left behind.

This change is limited to the extended-loader state lifetime issue. It does not add Android-side multi-model integration, sample or harness support, or broader federated BIM behavior. It also does not change placement, framing, picking, transforms, or app lifecycle handling.

update: previously was using a set to track load. current implementation just keeps the boolean and resets it on each new asset

Screenshots

N/A

Testing Notes

What did you do to test this PR?

  • added regression coverage for repeated parses on the same extended loader
  • built and ran test_gltfio; the new repeated-parse test passed
  • ran local pre-Android multi-model validation using uncommitted gltf_viewer harness-only changes outside this PR
  • confirmed multiple models rendered in one scene in the local viewer harness
  • built new filament AARs from this change and validated them through the existing Android POC multi-model path
  • confirmed rendering, navigation, and picking worked in the Android POC
  • observed no discernable performance regression versus the POC filament changes during this validation

@github-actions
Copy link
Copy Markdown

Please add a release note line to NEW_RELEASE_NOTES.md. If this PR does not warrant a release note, add the 'internal' label to this PR.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a state-lifetime issue in the extended glTF loader where per-parse buffer/decode bookkeeping could leak across multiple asset parses performed by the same AssetLoader, causing later loads to incorrectly skip required work.

Changes:

  • Introduce per-parse bookkeeping in AssetLoaderExtended and reset it via a new beginAssetLoad() method.
  • Replace the previous single boolean “buffers loaded” flag with a set-based tracker and ensure failed loads don’t poison subsequent retries.
  • Invoke beginAssetLoad() from FAssetLoader::createInstancedAsset before starting a fresh parse.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
libs/gltfio/src/extended/AssetLoaderExtended.h Adds beginAssetLoad() and new per-parse tracking container for buffer-load bookkeeping.
libs/gltfio/src/extended/AssetLoaderExtended.cpp Gates loadCgltfBuffers + meshopt decode behind the new per-parse tracking.
libs/gltfio/src/AssetLoader.cpp Resets extended-loader per-parse state before creating a new asset from parsed cgltf data.

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

Comment thread libs/gltfio/src/AssetLoader.cpp
Comment thread libs/gltfio/src/extended/AssetLoaderExtended.cpp Outdated
Comment thread libs/gltfio/src/extended/AssetLoaderExtended.h Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.


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

Comment thread libs/gltfio/test/gltfio_test.cpp Outdated
Comment thread libs/gltfio/test/gltfio_test.cpp Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.


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

@j1george j1george marked this pull request as ready for review April 20, 2026 18:08
Comment thread libs/gltfio/src/extended/AssetLoaderExtended.cpp Outdated
Comment thread libs/gltfio/src/extended/AssetLoaderExtended.cpp Outdated
@j1george j1george marked this pull request as draft April 20, 2026 20:03
Comment thread libs/gltfio/src/extended/AssetLoaderExtended.cpp Outdated
@j1george j1george marked this pull request as ready for review April 20, 2026 20:20
@j1george j1george force-pushed the dev/george/ME-6870/load-flag branch from eed9496 to e6c4a70 Compare April 20, 2026 21:35
Comment thread libs/gltfio/src/extended/AssetLoaderExtended.h
Comment thread libs/gltfio/src/extended/AssetLoaderExtended.h Outdated
Co-authored-by: George Jone <g.george.jone@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants