Skip to content

Conversation

@beaufortfrancois
Copy link
Collaborator

@beaufortfrancois beaufortfrancois commented May 7, 2025

An image flash occurs when switching between texture formats. I believe it happens because writeTexture may not be complete causing the canvas to use incomplete texture data.

@beaufortfrancois beaufortfrancois requested a review from kainino0x May 7, 2025 09:30
Copy link
Collaborator

@greggman greggman left a comment

Choose a reason for hiding this comment

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

To fix the flash,

In createVolumeTexture, move the call to createTexture to after the stream has been decompressed.

The reason it flashes is because

  1. it creates a blank texture
  2. it asynchronously waits for data to decompress - may take multiple frames during which the texture is transparent black
  3. it then finally writes the data into the texture

You should never need to call onScheduledWorkDone. It's sole purpose is ATM is throttling.

@beaufortfrancois
Copy link
Collaborator Author

To fix the flash,

In createVolumeTexture, move the call to createTexture to after the stream has been decompressed.

I think it's better to fail early with non-supported texture formats rather than downloading the resource and then fail.
Moreover, the user sees an error right away in the canvas.

The reason it flashes is because

  1. it creates a blank texture
  2. it asynchronously waits for data to decompress - may take multiple frames during which the texture is transparent black
  3. it then finally writes the data into the texture

You should never need to call onScheduledWorkDone. It's sole purpose is ATM is throttling.

I'm happy to remove onScheduledWorkDone(). I naively thought it was to make sure writeTexture() was effective.

@greggman
Copy link
Collaborator

greggman commented May 7, 2025

To fix the flash,
In createVolumeTexture, move the call to createTexture to after the stream has been decompressed.

I think it's better to fail early with non-supported texture formats rather than downloading the resource and then fail. Moreover, the user sees an error right away in the canvas.

The code already exits at line 147 if the format is not supported.

@beaufortfrancois
Copy link
Collaborator Author

To fix the flash,
In createVolumeTexture, move the call to createTexture to after the stream has been decompressed.

I think it's better to fail early with non-supported texture formats rather than downloading the resource and then fail. Moreover, the user sees an error right away in the canvas.

The code already exits at line 147 if the format is not supported.

Even though createTexture() can still fail for other reasons I agree it should be enough in this context. Thanks for the review! I've updated the PR.

@beaufortfrancois
Copy link
Collaborator Author

@mwyrzykowski FYI As you can see below, the canvas still behaves weirdly in Safari Tech Preview Release 218 (Safari 18.4, WebKit 20622.1.10.19.1)

safari-flash.mov

@beaufortfrancois beaufortfrancois merged commit 896c8d3 into webgpu:main May 8, 2025
1 check passed
@greggman
Copy link
Collaborator

greggman commented May 8, 2025

To fix the flash,
In createVolumeTexture, move the call to createTexture to after the stream has been decompressed.

I think it's better to fail early with non-supported texture formats rather than downloading the resource and then fail. Moreover, the user sees an error right away in the canvas.

The code already exits at line 147 if the format is not supported.

Even though createTexture() can still fail for other reasons I agree it should be enough in this context. Thanks for the review! I've updated the PR.

You could change the code to

temp = createTexture
fetch
decompress
writeData
volumeTexture = temp

If you wanted some other error related to texture creation to show up first.

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.

3 participants