Skip to content

Allocate all four SH textures for paged ext splats - #424

Open
oscarlorentzon wants to merge 1 commit into
sparkjsdev:mainfrom
oscarlorentzon:paged-ext-sh3
Open

Allocate all four SH textures for paged ext splats#424
oscarlorentzon wants to merge 1 commit into
sparkjsdev:mainfrom
oscarlorentzon:paged-ext-sh3

Conversation

@oscarlorentzon

Copy link
Copy Markdown
Collaborator

Loading an asset with SH degree 3 throws RangeError: offset is out of bounds when both paged and pagedExtSplats are on. It happens once for each page that has SH3 data in it. The page is left without one of its two SH3 textures, and the rest of that frame's loading is skipped, so the scene fills in slowly or not at all.

  • With pagedExtSplats, SH3 is stored in two textures instead of one, so an sh3 asset needs four SH textures where other assets need three. Only three were created, so the fourth upload had no texture to write into and failed.
  • The fix creates as many textures as the asset has SH data for. Assets with a lower SH degree, and assets loaded without pagedExtSplats, work as before.

The ext encoding uploads four SH arrays for sh3 while the pager allocated
three. A paged sh3 asset threw RangeError on every page upload and lost its
SH3 term. Assets below sh3 and the packed path are unaffected.
@asundqui

Copy link
Copy Markdown
Contributor

Nice find and fix! In the future we'll need to add some regression tests so this doesn't happen again.

I'm wondering about removing this.curSh though - although it's not used anywhere else in Spark, I'm wondering if we should keep it, so that users can tell what Sh degree is being used? Obviously the logic will have to be different so it doesn't just key off shArrays.length, which will be different depending on extSplats vs. not.

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.

2 participants