Safari: Create Screenshot Using Render Target broken

Looks like CreateScreenshotUsingRenderTarget is broken on Mac and iOS while using Safari.

I ran across this issue that seemed similar, however, the above function throws errors on both the latest version of macOS and iOS (with fairly new devices, that should definitely be WebGL 2 capable).

This used to work with no changes on our part. We updated from Babylon 5.38.0 to 6.9.0 and started seeing the issue, if that helps narrow down where this may of started.

We fixed an issue with dump tools and premultiplied alpha 8 months ago (Fix dump tools premultiplied alpha. by sebavan Ā· Pull Request #13251 Ā· BabylonJS/Babylon.js Ā· GitHub), but that change was already in 5.38.0 (first merged in 5.33.0).

There has been no changes in the 3 files involved (dumpData.ts, tools.ts and screenshotTools.ts) since then, so Iā€™m not sure what the problem could beā€¦

For testing purpose, I have updated your PG to use the old code before 5.33.0:

Does it work?

@Evgeni_Popov Yup, that does seem to work

Iā€™m not sure to understand why, as it seems nothing changes in the screenshot code between 5.33.0 and nowā€¦

Are you sure you werenā€™t using a version older than 5.33.0 previously?

Also, it could help to know what are the errors thrown by Safari with the current version.

We are trying to get a more specific version where it broke. So far we have determined that it is working up to 5.57.1.

image

@Evgeni_Popov Looks like we narrowed it down to 6.2.0, and the likely culprit is Dump Tools: Use an offscreen canvas by Popov72 Ā· Pull Request #13803 Ā· BabylonJS/Babylon.js Ā· GitHub

Ohhhh it would make sense as Safari does not support webgl in offscreen canvas before 17 :frowning:

@Evgeni_Popov any way to add a fallback if the creation of the context is failing ?

1 Like

Ah, I seem to have failed looking at the history of these files, obviously there have been changes over the last few monthsā€¦

Perhaps we could add a static boolean DumpTools.DisableOffscreenCanvas that users could set before using the screenshot APIs, to create a canvas instead of an OffscreenCanvas?

[EDIT] Or, if weā€™re sure that context creation fails and the ThinEngine constructor throws an exception (which seems to be the case in the screenshot), I could catch it and use a canvas instead, removing the need for the new staticā€¦

I do prefer to the second option as we could remove this code path when supported everywhere without breaking changes or leftovers APIs.

@EricB Can you test this PG?

If itā€™s ok, I will make a PR.

In case the fix is ok, hereā€™s the PR:

@Evgeni_Popov Yup, looks like that is working on Safari on both a Mac and iOS!

1 Like