Android black textures

[Kernael]

As discussed here #16 , textures loaded with gl.getExtension('RN').loadTexture() sometimes fails to load on Android (rendering a black mesh)

export const loadThreeJSTexture = async (gl, src, texture, renderer) => {
  var properties = renderer.properties.get(texture);
  await gl.getExtension('RN')
    .loadTexture({ yflip: false, image: src })
    .then(({ texture }) => {
      properties.__webglTexture = texture;
      properties.__webglInit = true;
      texture.needsUpdate = true;
    });
};

export const loadTexture = async (gl, src, renderer) => {
  const texture = new THREE.Texture();
  const material = new THREE.MeshBasicMaterial({ map: texture, transparent: true, overdraw: 0.5 });
  await loadThreeJSTexture(gl, src, texture, renderer);
  return material;
};

export const createPlayer = async (gl, renderer) => {
  const geometry = new THREE.PlaneBufferGeometry(0.6, 1);
  const material = await loadTexture(gl, images.player, renderer);
  const mesh = new THREE.Mesh(geometry, material);

  mesh.material.side = THREE.DoubleSide;
  mesh.rotation.z = Math.PI;
  mesh.rotation.y = Math.PI;

  return mesh;
};

[epochrion]

I have encountered the same issue where textures sporadically fail to load, thereby rendering a black texture instead of the desired image. Please find linked sample code that helps further illustrate this issue.

The code creates a 5 x 5 grid of meshes, using three.js and the react-native-webgl libraries. Each mesh consists of a plane geometry and material created from an image texture. To eliminate the possibility of the issue being related to image size or format, every texture references the same image. The expected output would be 25 properly rendered images, placed adjacent to one another. However, the actual output consists of some meshes having a black texture instead, as shown in the linked image.

Testing has been conducted on the following platforms:

Device	Operating System
Nexus 5 (Physical Device)	Android 6.0.1
Emulated Nexus 5X (x86)	Android 7.1.1
Pixel (Physical Device)	Android 8.0.0

Testing was also conducted on an iPhone 6S (running iOS 10.3.3), but the issue was not present on it.

For reference, the following relevant libraries were linked to the project when testing:

• NDK Version: android-ndk-r10e
• Three JS Version: 0.85.0

Please let me know if you require any further information. If you have any ideas regarding what may be causing this issue, let me know and I can help update the code / submit a pull request afterwards.

[gre]

ok guys, I plan to investigate the hell out of this tomorrow :) #motivation
EDIT: tomorrow of tomorrow xD

[epochrion]

I tested the latest commits on Android, and it looks like the issue is still present. Using the earlier grid example, some images still sporadically show up as black textures while others appear to render correctly.

[gre]

yes exactly. see my comment #16 (comment) It's hard for me to see where things are wrong, if you want to investigate on your side feel free. I also have no idea why sometimes the image appears twice, this feels to be a bad concurrency issue, I wonder if it's in the CPP low level code itself. I hope @nikki93 can help on this, maybe it would be great if we could try to replicate this example in Expo EXGLView itself (that have slightly different API). i'll try to port things over Snack

[gre]

I have this https://snack.expo.io/HJE_kof3- that seems to work on Android (I can't repro the present bug. so maybe it means it's a concurrency issue on react-native-webgl side, the image loader impl)

however it renders white on iOS (same as in react-native-webgl NineTextures example) and I have no idea why – it looks like iOS have trouble supporting many textures at the same time?

[Kernael]

Are there other ways to load images on Android so we can test it ?

[gre]

for the iOS bug, i'm delegating the investigation to expo/expo#728 – for Android, i'll investigate

[gre]

@Kernael i'm not sure to follow what you mean?

[Kernael]

Just wondering if there is another way than gl.getExtension('RN').loadTexture({ yflip: false, image: src }) to load the texture so that we can verify that this is the implementation of this function that is inducing the bug.

[gre]

for the Android bug: you can also use a typed array, but at this point I don't think there will be issues with typed array since it seems to works ok in expo. probably just an issue in the current implementation. going to investigate this afternoon

[gre]

in my current investigation the bug seems around this call that never succeed in some random cases.. https://github.com/react-community/react-native-webgl/blob/b7aa3cfc18ebcc2aa226d2dfa41c9eb5a064ff9a/android/src/main/java/fr/greweb/rnwebgl/RNWebGLTextureBitmap.java#L26

To me it looks like a classical "race condition" issue. between 2 part of the code:

• https://github.com/react-community/react-native-webgl/blob/b7aa3cfc18ebcc2aa226d2dfa41c9eb5a064ff9a/android/src/main/java/fr/greweb/rnwebgl/RNWebGLView.java#L89 that adds to the queue. this is exec, usually, but the main thread.
• https://github.com/react-community/react-native-webgl/blob/b7aa3cfc18ebcc2aa226d2dfa41c9eb5a064ff9a/android/src/main/java/fr/greweb/rnwebgl/RNWebGLView.java#L65-L68 that clear and exec the queue. this is exec by the GL thread.

there are some synchronized method but I think it's not enough to cover this.. This is from the fork of android impl of expo EXGLView, which I believe is affected by the bug too (except in this current fork, I use runonGLThread much more). Actually I had to protect an exception to raise by this commit: b7aa3cf but that bug makes me think it was NOT the correct fix, this was just me hiding things under the carpet..

I don't have a lot of experience into dealing with Java race condition, do you? I'll try to document myself.

beside this first issue, that does not explain to me why one image would get dup into 2 "independent" textures..

[Kernael]

for the Android bug: you can also use a typed array

Could you provide an example on how to do so, based on the snippet I provided on the first post of this issue ?
If there is a workaround to this issue, this would make it less critical, because textures not rendering are a bit of a problem.

Thank you for your awesome work !

[gre]

so as far as I understand, it's not correct to use a ArrayList, needs to use one of java.concurrent.* list/queue

[gre]

I (think to) have fixed the concurrent issue, but still some cases it fails. continuing investigating...

[epochrion]

Any particular reason why you have to invoke mEventQueue.clear() and instead couldn't call mEventQueue.remove(r) within the for loop itself? I did a quick test using the latter approach, and it appears to be an improvement. Instead of seeing sporadic black textures every reload, I see them maybe 2/10 reloads now. Apologies though if the clear method is required and if I've overlooked something else.

[odusseys]

I'm going to try to help since I have quite a bit of experience with Java concurrency.
I'm going to clone this and look, but just by looking at the RNWebGLView file I notice the onDrawGLFrame method is not synchronized, which could cause the queue to be cleared -

[odusseys]

Also you really shouldn't ever leak "this" to other scopes in constructors (RNWebGLView, RNWebGLTextureBitmap). This can cause the object to be used while it still hasn't been properly created. Only pass this to other objects or methods outside the constructor.

[gre]

thanks for your help :) this project is definitely looking for more Java experts, it is backed with many langs (c++ / objc / java / js) there are probably a lot to improve in each lang paradigm :)

interesting about "this" to not use in constructor 👍

[gre]

summary of the problem

BTW here is a simple way to replicate the bug of this issue:

open the project react-native-webgl/example/android under Android Studio (after a npm install from that /example), hit Run. on the example app, you can open the example "Nine Textures"
it is supposed to render this:

but instead it tends to render black image AND/OR twice the image in 2 different textures. there might be multiple bugs involved by this. it sounds to be a race condition in Java, but it might be a race condition in C++ too (across the GL calls).

[gre]

I'm getting closer to the problem I think with this commit: 17857f5

GL is stateful so I had to "restore" the bounded texture to not affect user's code.. I think the twice-the-same-image issue is gone however there are still some black case.. I wonder if this black case is related to that problem as well (like maybe I just restore to the null value now?). still maybe that means we don't schedule that code at the correct time..?
it should be correct because the queue is flushed before the C++ flush (which unqueue the user's GL code), mmh...

[gre]

Hi again everyone. I think I have FINALLY fixed the problem.
it was obviously a combination of many issues , that's why it was tricky to understand. I used a synchronized on the onDrawFrame, like suggested by @odusseys and this obviously seem to have fixed the last bit.

Feel free to comment here if you experience the bug is not fixed. otherwise, we'll schedule a new release soon.

[odusseys]

Please do not closed the issue without confirmation that this is actually fixed.
I am testing too and adding synchronized does not fix everything :

• the textures are indeed loaded
• but they are NOT loaded in the right order. Look at your own example, you have a duplicated image.

I think I may have found a bug : the documentation for Facebook's BaseBitmapDataSubscriber states that the Bitmap object in the callback is only available in the scope of the method and may be reused later. But you pass it to your RNWebGLTextureBitmap in RNWebGLTextureImageLoader.loadWithConfig
This means that the bitmap can be rewritten when loading an ulterior texture and therefore be written twice when you actually write them to the GL buffer.

I wrote
bitmap = bitmap.copy(bitmap.getConfig(), bitmap.isMutable());

But this does not solve the problem. It is necessary though

[odusseys]

Yet another potential concurrency bug : in RNWebGLTextureLoader.loadWithConfigAndWaitAttached :
You first check if the object is attached, if so call the callback, otherwise listen for attachment. But in the meantime the object has been attached, the callback will be fired twice. So in RNWebGLTexture.listenAttached :

• attached should be an AtomicBoolean
• every method accessing attached should be synchronized
• listenAttached should read :

public synchronized boolean listenAttached (Runnable r) {
    if(attached.get()){
            r.run();
            return true;
        } else {
            return mAttachEventQueue.add(r);
        }
    }

[gre]

yeah sorry, my example image was actually before the bug was fixed*. i am not able to have the bug anymore (a few try, not meaning it's 100% guarantee it's fixed).

oh yeah I was wondering if I had or not to .copy, I think it's definitely safer to copy, very good point.

ok i'll try to address these points, thanks a lot for these feedback!!

[gre]

@odusseys I added two commits to address these points, thanks a lot for your valuable feedbacks :)

[gre]

@Kernael have you tried the master version to see if it fixes the issue (at least on Android)

[epochrion]

The issue appears to be fixed based on preliminary testing. Thank you.

[Kernael]

I did not have the time to thoroughly test this but based on what I could see, the issue seems to be fixed !