Skip to content

One texture pipeline for every model viewer, thumbnails by URI - #112

Merged
Frotty merged 4 commits into
refactor/port-editors-and-cleanupfrom
refactor/texture-thumbnail-transport
Sep 5, 2026
Merged

One texture pipeline for every model viewer, thumbnails by URI#112
Frotty merged 4 commits into
refactor/port-editors-and-cleanupfrom
refactor/texture-thumbnail-transport

Conversation

@Frotty

@Frotty Frotty commented Sep 5, 2026

Copy link
Copy Markdown
Member

Summary

Stacked on #111. Third slice of the audit list: the duplicated texture pipeline and the base64 thumbnail transport.

Texture pipeline

  • The standalone model viewer (blpPreview.ts) resolves textures through postTexturesToWebview, the same code path as the object editor's inline preview and the asset browser: local-first resolver, payload and missing caches, and a concurrency limit of 6, replacing its own lookup with an unbounded Promise.all.
  • Texture messages carry resolvedFsPath so the viewer's "open texture" link keeps working.
  • The BLP-only findLocalTexture helper is removed. Behaviour difference: the old helper also walked up to four parent directories of the model file; the shared resolver uses the document folder, imports/-style subfolders, workspace roots and the game cache. For models inside a map folder or workspace this is a superset; for a lone model in an unrelated directory tree with textures in a grandparent folder it is not.

Thumbnail transport

  • Cached thumbnails are sent as vscode-webview resource URIs instead of base64 data URLs; ~/.wurst/model_thumbs is in localResourceRoots for the object editor and the asset browser. Test doubles without asWebviewUri still get a data URL.
  • The asset browser fetches model bytes by URI (as the object editor already did), so neither model nor thumbnail bytes travel through postMessage as base64 any more.

Validation

  • tsc (real siblings and CI mocks), npm run lint, npm test: clean.
  • npm run test:e2e: all map-data editor specs pass; object-editor failures match the master baseline apart from the two known flaky specs, which pass on rerun.

- The standalone model viewer (blpPreview) resolves textures through
  postTexturesToWebview like the object editor's inline preview and the asset
  browser: same local-first resolver, payload/missing caches and concurrency
  limit, instead of its own lookup with unbounded Promise.all. Texture messages
  carry resolvedFsPath so the viewer's "open texture" link still works. The
  BLP-only findLocalTexture helper is removed.
- Cached model thumbnails are handed to webviews as resource URIs instead of
  base64 data URLs; the thumb cache directory is admitted through
  localResourceRoots for the object editor and the asset browser. The asset
  browser also fetches model bytes by URI like the object editor does, so
  neither the model nor the thumbnail travels through postMessage as base64.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-05T15:55:41.451171Z e64f528 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 12fc3fd5a5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/features/assetLinks.ts
Comment thread src/features/blpPreview.ts
…or model textures

- The asset browser's CSP admits the extension's cspSource for img-src and
  connect-src, which URI-mode model fetches and cached thumbnail images need.
- getCandidateRoots(..., { includeAncestors: true }) adds up to four ancestor
  directories of the document; postTexturesToWebview uses it so a model nested
  below a non-workspace map/project root still finds textures relative to that
  root, as the removed BLP-only lookup did. Scans (gatherImportedAssets) keep
  the plain root list and never walk ancestors.
@Frotty

Frotty commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d1bc57a85c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/features/blpPreview.ts
…changes

The shared texture resolver remembers misses so an absent texture is not
re-probed on every render, but nothing ever forgot them. The viewer's Refresh
(and its file watcher), the object editor's reload, and a wurst.wc3path change
now clear that cache so a texture added or made reachable later is picked up.
@Frotty

Frotty commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8f0e5aab9e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/features/assetLinks.ts
With models delivered as webview resource URIs, the asset browser's thumbnail
renderer still only decoded base64, so every cache miss rendered an empty
buffer. It now fetches the URI (falling back to base64 for test doubles) and
ignores the result if a newer job superseded it.
@Frotty

Frotty commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: e64f528d50

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Frotty
Frotty merged commit 1b82af6 into refactor/port-editors-and-cleanup Sep 5, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant