Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: [#2359] work around image decode limits in chromium #2361

Merged
merged 7 commits into from
Jun 25, 2022

Conversation

eonarheim
Copy link
Member

@eonarheim eonarheim commented Jun 23, 2022

===:clipboard: PR Checklist :clipboard:===

  • 📌 issue exists in github for these changes
  • 🔬 existing tests still pass
  • 🙈 code conforms to the style guide
  • 📐 new tests written and passing / old tests updated with new scenario(s)
  • 📄 changelog entry added (or not needed)

==================

Closes #2359

This PR switches away from Image.decode() to use the older method of Image.onload this method is more reliable for large or numerous images due to a chromium bug around decode.

@HxShard provided a great codesandbox illustrating the issue https://codesandbox.io/s/happy-gagarin-j1vjr

Changes:

  • Creates a new Future type for working with native Promises' resolve/reject at any time
  • Creates a new Semaphore type that can limit async calls in between enter and exit
  • Fixes a small clock schedule bug as well

@github-actions github-actions bot added the bug This issue describes undesirable, incorrect, or unexpected behavior label Jun 23, 2022
@HxShard
Copy link

HxShard commented Jun 24, 2022

@eonarheim I just did some checks and noticed one not very cool thing. In my case with this error, Image#decode was fired only 20 times.
I think, buffer size (or something similar) matters there too.

@eonarheim
Copy link
Member Author

@HxShard Good call out, I'll do some experimentation to see how the buffer size plays into this.

Roughly how large were your images? Dimensions and disk size?

@HxShard
Copy link

HxShard commented Jun 24, 2022

@eonarheim https://gist.github.com/HxShard/376b82855f076e4f6e8b51fc40e85f7e
UPD: I modified that codesandbox to show me byte sizes, and it gives me another result
Image#decode falls when I trying to decode a little more than 8000000 bytes (2 times more)

@eonarheim
Copy link
Member Author

eonarheim commented Jun 25, 2022

@HxShard With some experimentation, looks like using Image.decode() will be problematic for just a handful of large textures in chrome, tuning down the semaphore super low to 2-3 "works" but really kills load speed.

I checked using the old school Image.onload and that seems to work (and fast) just fine without issue or image flickering in webgl, I think because the GPU needs to decode the image into whatever internal format it needs for graphics. I think Image.decode() may only be necessary for DOM related images... more research is required but experimentally seems to be the case.

I'm going to explore removing the semaphore and switching to the Image.onload method instead 👍

@eonarheim eonarheim merged commit a374da8 into main Jun 25, 2022
@eonarheim eonarheim deleted the fix/2359-image-decode-limits-in-chromium branch June 25, 2022 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes undesirable, incorrect, or unexpected behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chromium's Image#decode has limited calls
2 participants