Why async _readTexturePixels uses synchronous version of readTexturePixels?

Hello!

I see below that the async version of readTexturePixels, instead of using an async function from webgl/webgpu, just uses a synchronous version.

Why is that? Async version does not provide any benefits?

ThinEngine.prototype._readTexturePixels = function (
    texture: InternalTexture,
    width: number,
    height: number,
    faceIndex = -1,
    level = 0,
    buffer: Nullable<ArrayBufferView> = null,
    flushRenderer = true,
    noDataConversion = false,
    x = 0,
    y = 0
): Promise<ArrayBufferView> {
    return Promise.resolve(this._readTexturePixelsSync(texture, width, height, faceIndex, level, buffer, flushRenderer, noDataConversion, x, y));
};

readPixels is sync with webGL API and async with webGPU.
so, you have to allow sync and async versions working independently of the underlying API and route the calls/wrap them with a promise.

You can use Engine._readPixelsAsync to make async version of this function when WebGL 2.0 is available.
More than 30% of users use browsers with WebGL 2.0 and without WebGPU.

Hello,

Here is the working async function inside Babylonjs. Can you please explain why you do not use it? (it was done 4 year ago)

it depends on the scenario. And the portability

For existing API we used to use the sync version and we cannot switch them without breaking back compat

So to your question: in which scenario are you?

We used the async version for all possible clients that have WebGL 2, and it worked for millions of people; what do you mean by “cannot switch them without breaking back compact”?

well I do not understand what you want :slight_smile:
Can you elaborate?

Both versions (sync and async) are available so what is the problem?

The problem is that you avoid using Webgl 2 async API; even if you already have it in place in Babylon, you do not use it. Instead, you use sync Webgl API with fake async renaming.

I just want to understand why. Yes, it is extremely easy to use an already-ready-to-use async one, but you don’t.

Are there some problems with the async Webgl 2 API? Or why do you avoid using it and instead fake it using the sync one?

yes but where do you want to change it? all the functions using readPixel right now are sync. I can probably change that in the GPUPIcking for sure (hence my ask if you wanted to do a PR)

I have no problem to not use it if a function is async already.

I think what hes trying to say is that since its already wrapped in an async function, u could add a conditional caps check to call either the sync or async platform api inside the current function without breaking anything.

Yep! I agreed on that :slight_smile: I was simply wondering if someone wants to do the PR:)

Also the async version needs to be updated a bit to avoid too much GC by creating new buffer in each call

Ok I thought about this thread under the shower (yeah Inonow this is weird!).

I think the confusion comes from the fact that I thought @forux was asking to change all call to readpixel by the async version which was a no go as this will unleash hell

But now I realize that the ask was to replace the fake promise.resolve within the readpixelasync by a call to the webgl2 real async version

Now it makes sense:)

I’ll be happy to do the change if no one is motivated:)

2 Likes

Btw this might impact some gpu sync timing, idk i didnt look at the code, but worth sharing anyway because its weird… Promise.resolve(void) is actually a microtask and will return before timers check again, so it acts kind of like a deferred synchronous function that returns before any actual async functions. Only if it resolves to void, if it returns a value it’ll return after timer check with all the other async functions