Possible GLView bug ?

  1. SDK Version: 38
  2. Platforms(Android/iOS/web/all): Android

Regardless of what value of n I specify in the following,

gl.bufferSubData(gl.ARRAY_BUFFER, 0, myFloat32Array, 0, n);

the whole contents of myFloat32Array is uploaded to the GPU, but it should only upload the first n elements.

A work around should be to use DataView(myFloat32Array, 0, n) because it’s backed by the same buffer as myFloat32Array but explicitly uses a new offset (0) and length (n). But this doesn’t seem to work either, e.g.

let n = 32;
let a = new DataView(myFloat32ArrayData.buffer, 0, n);
gl.bufferSubData(gl.ARRAY_BUFFER, 0, a, 0);  // this version takes an `ArrayBufferView` (which includes `DataView`)

Our gl implementation is not fully compliant with the specs, signature with source offset and length is not supported there. We usually add features like that case by case, but I would prefer not to add anything there because we are in process of rewriting that module for Hermes

Your workaround should work with any type except DataView, try using

new Float32Array(oldBuffer, 0, n) 
1 Like

Thanks for your reply.

What you suggest does work, but the resulting Float32Array is not backed by the same buffer as the original array. This is a real waste of CPU time as it copies each value over to a new buffer !

Is there any possibility you would reconsider and add this little extra capability? Currently I’m effectively copying data three times:

  1. Once to populate the sprite data (required)
  2. Once using new Float32Array(oldBuffer, 0, n)
  3. Once via gl.bufferSubData (required)

where step 2 should not be necessary. In my case it adds an extra 7 milliseconds to the frame and for games this could be used to more effect in game logic.

support for that is relatively simple, but it would be available in the next sdk or require ejecting.

If we support that change call to jsValueToSharedArray here https://github.com/expo/expo/blob/master/packages/expo-gl-cpp/cpp/EXGLNativeMethods.cpp#L329 would copy entire buffer instead of first n bytes. I’m not sure how this would affect your performance(it depends on relative sizes).

The solution that does not require additional copies would require to much refactor, but we might consider optimizing it in a hermes rewrite.

1 Like

Sorry I’m a bit new to ejecting - so far I’ve done everything in managed.

Are you saying that I can eject now and use the updated EXGLNativeMethods.cpp somehow ?

I’ve not used ejection before.

Am I right in thinking that if I eject I can edit the C/C++ files relating the GLView in packages/expo-gl-cpp/cpp/, recompile, and run with Expo all by myself… so I could just refactor all the Expo code myself?

Are you saying that I can eject now and use the updated EXGLNativeMethods.cpp somehow ?

I meant that if we would make those changes and publish updated version expo-gl-cpp to npm you would need to wait for the next sdk or eject.

Am I right in thinking that if I eject I can edit the C/C++ files relating the GLView in packages/expo-gl-cpp/cpp/ , recompile, and run with Expo all by myself… so I could just refactor all the Expo code myself?

technically you can do this, but setup might be complicated. I did not use that outside expo, so I’m not sure how much work would be to set it up

1 Like

Thanks for clarifying.

I must have made a mistake in my earlier tests because it seems that new Float32Array(oldArray.buffer, 0, n) is indeed backed by oldBuffer so there is no copying going on there actually. My bad.

But it would still be good to remove the malloc going on due to the jsValueToSharedArray C/C++ function though to help optimise mobile graphics performance.

I think traditionally this is solved with the following flow:

x=createByteBuffer(1000, 4) // mallocs 1000*4 bytes once and for all
x.rewind(); // sets x.position=0
x.putFloat32(0.3) // sets x.buffer[0]=0.3, x.position=1
x.putFloat32(1.2) // sets x.buffer[1] = 1.2, x.position=2
...

At the end x.position=2. Finally pass x to Expo gl.bufferSubData and use x.buffer directly (no malloc required).

Anyway, thanks for your help.

This topic was automatically closed 20 days after the last reply. New replies are no longer allowed.