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

ShaderData::bindTexture texture diffuseTexture at loc=0 not ready #1848

Open
SteveGreatApe opened this issue Apr 23, 2018 · 11 comments
Open
Labels

Comments

@SteveGreatApe
Copy link

Bit of a strange one this, I've got a scene object where I can set it's material to either be a texture created from a bitmap, or I set the texture to null and the ambient and diffuse colours to a selected colour instead.

Setting the texture from a bitmap works without a problem, but when I try and switch from a texture to a solid colour by setting the diffuseTexture to null:

            material.setTexture("diffuseTexture", null);

I then get this error rapidly repeating in the log:

...
2018-04-23 15:20:05.734 27690-28066/com.greatape.avrgallery V/gvrf: ShaderData::bindTexture texture diffuseTexture at loc=0 not ready
2018-04-23 15:20:05.747 27690-28066/com.greatape.avrgallery V/gvrf: ShaderData::bindTexture texture diffuseTexture at loc=0 not ready
2018-04-23 15:20:05.751 27690-28066/com.greatape.avrgallery V/gvrf: ShaderData::bindTexture texture diffuseTexture at loc=0 not ready
2018-04-23 15:20:05.763 27690-28066/com.greatape.avrgallery V/gvrf: ShaderData::bindTexture texture diffuseTexture at loc=0 not ready
...

And bizarrely the object now appears to be showing a texture from another object in the scene. If I then later try and change the colour, which also happens to set the texture to null again, this seems to fix the problem and it changes to showing the fixed colour as expected.

@NolaDonato
Copy link
Contributor

I have tried to reproduce this with the PhongShader but I cannot. What shader are you using? Could you send me a code snippet?

NolaDonato added a commit to NolaDonato/GearVRf that referenced this issue Apr 23, 2018
When a texture is set to null, GearVRf was not always regenerating the shader. This patch fixes that - issue Samsung#1848
NolaDonato added a commit to NolaDonato/GearVRf that referenced this issue Apr 23, 2018
When a texture is set to null, GearVRf was not always regenerating the shader. This patch fixes that - issue Samsung#1848
@NolaDonato
Copy link
Contributor

I think you were using the Texture shader, I reproduced it with that. PR #1850 should fix it, please check.

@SteveGreatApe
Copy link
Author

Thanks, it's the Phong Shader I'm using, I've tried your fix and doesn't seem to have worked for me, I've added some logging to it to make sure I'm going through that code and it's definitely running your change.

I can reproduce it in the multilight demo app, see below for changes. When the texture is set to null I see it replaced by the texture being used for the 3D character.

diff --git a/gvr-multilight/app/src/main/java/org/gearvrf/multilight/MultiLightMain.java b/gvr-multilight/app/src/main/java/org/gearvrf/multilight/MultiLightMain.java
index 68d831f..faf8aa6 100644
--- a/gvr-multilight/app/src/main/java/org/gearvrf/multilight/MultiLightMain.java
+++ b/gvr-multilight/app/src/main/java/org/gearvrf/multilight/MultiLightMain.java
@@ -41,6 +41,8 @@ public class MultiLightMain extends GVRMain {
     private GVRSceneObject rotateObject;
     private GVRSceneObject backdrop;
     private GVRScene mScene;
+    private GVRTexture tex;
+    private boolean noTexture;
 
     @Override
     public void onInit(GVRContext gvrContext) {
@@ -70,8 +72,10 @@ public class MultiLightMain extends GVRMain {
     public void onStep() {
         FPSCounter.tick();
         theta += 0.005;
-        if (theta >= Math.PI / 4)
+        if (theta >= Math.PI / 4) {
             theta = -Math.PI / 4;
+            toggleTexture();
+        }
         if (rotateObject != null) {
             Quaternionf q = new Quaternionf();
             q.rotateAxis((float) theta, 0.0f, 1.0f, 0.0f);
@@ -141,7 +145,7 @@ public class MultiLightMain extends GVRMain {
      */
     private GVRSceneObject createBackdrop(GVRContext context)
     {
-        GVRTexture tex = context.getAssetLoader().loadTexture(new GVRAndroidResource(mGVRContext, R.drawable.gearvrflogo));
+        tex = context.getAssetLoader().loadTexture(new GVRAndroidResource(mGVRContext, R.drawable.gearvrflogo));
         GVRSceneObject backdrop = new GVRSceneObject(context, 10.0f, 4.0f, tex);
         GVRRenderData rdata = backdrop.getRenderData();
         GVRMaterial material = new GVRMaterial(context, GVRMaterial.GVRShaderType.Phong.ID);
@@ -158,4 +162,8 @@ public class MultiLightMain extends GVRMain {
     	return backdrop;
     }
 
+    private void toggleTexture() {
+        noTexture = !noTexture;
+        backdrop.getRenderData().getMaterial().setTexture("diffuseTexture", noTexture ? null : tex);
+    }
 }

@NolaDonato
Copy link
Contributor

I reproduced your problem in multilight on master. Then I switched to my pull request. After that gvr-multilight switched between the texture and no texture just fine. Are you sure you had my change? It is a one liner.

@SteveGreatApe
Copy link
Author

It's getting more complicated now, for the demo it seems your right and the fix does work, my demo project was still being built against 4.0.1-SNAPSHOT when I reproduced the problem.

But in my own code where I still see the problem I definitely am running the fix. What I've now noticed is that after setting the texture to null it depends where I'm looking to what problem I see. It seems that any texture visible can now appear on the scene object with the null texture. In fact if I tip my head at the right angle I can see different textures being displayed for each eye, even swinging the pointer in and out of view changes what texture appears in place of the null texture.

@SteveGreatApe
Copy link
Author

Here's a souped up mod to the multilight demo that shows some weirdness, not only does the astro boy texture appear in place of a the null texture, but I also get backdrops showing the wrong texture, or no texture when it should have one. Try tweaking the number of backdrops to get different behaviour.


diff --git a/gvr-multilight/app/src/main/java/org/gearvrf/multilight/MultiLightMain.java b/gvr-multilight/app/src/main/java/org/gearvrf/multilight/MultiLightMain.java
index 68d831f..857c05d 100644
--- a/gvr-multilight/app/src/main/java/org/gearvrf/multilight/MultiLightMain.java
+++ b/gvr-multilight/app/src/main/java/org/gearvrf/multilight/MultiLightMain.java
@@ -34,13 +34,16 @@ import android.util.Log;
 import android.view.MotionEvent;
 
 public class MultiLightMain extends GVRMain {
-
+    private static final int NumBackdrops = 3;
+    private final static int[] BackdropsIds = {0, R.drawable.gearvrflogo, R.drawable.__default_splash_screen__, R.drawable.ic_launcher, R.drawable.cursor};
     private static final float LIGHT_Z = 100.0f;
     private static final float LIGHT_ROTATE_RADIUS = 100.0f;
     private GVRContext mGVRContext;
     private GVRSceneObject rotateObject;
-    private GVRSceneObject backdrop;
+    private GVRSceneObject[] backdrops = new GVRSceneObject[NumBackdrops];
     private GVRScene mScene;
+    private GVRTexture[] textures = new GVRTexture[NumBackdrops];
+    private int step;
 
     @Override
     public void onInit(GVRContext gvrContext) {
@@ -52,12 +55,14 @@ public class MultiLightMain extends GVRMain {
         GVRSceneObject character = createCharacter(gvrContext);
         GVRSceneObject light1 = createLight(gvrContext, 1, 0, 0, 0.8f);
         GVRSceneObject light2 = createLight(gvrContext, 0, 1, 0, -0.8f);
-        
-        backdrop = createBackdrop(gvrContext);
+
         root.setName("root");
         root.getTransform().setPosition(0, 0, -zdist);
         mScene.addSceneObject(root);
-        root.addChildObject(backdrop);
+        for(int index = 0; index < NumBackdrops; index++) {
+            backdrops[index] = createBackdrop(gvrContext, index);
+            root.addChildObject(backdrops[index]);
+        }
         root.addChildObject(light1);
         root.addChildObject(light2);
         root.addChildObject(character);
@@ -70,8 +75,10 @@ public class MultiLightMain extends GVRMain {
     public void onStep() {
         FPSCounter.tick();
         theta += 0.005;
-        if (theta >= Math.PI / 4)
+        if (theta >= Math.PI / 4) {
             theta = -Math.PI / 4;
+            updateTextures();
+        }
         if (rotateObject != null) {
             Quaternionf q = new Quaternionf();
             q.rotateAxis((float) theta, 0.0f, 1.0f, 0.0f);
@@ -84,7 +91,7 @@ public class MultiLightMain extends GVRMain {
 
     public void onTouchEvent(MotionEvent event) {
         if ((event.getAction() & MotionEvent.ACTION_MASK) == MotionEvent.ACTION_UP) {
-            GVRRenderData rdata = backdrop.getRenderData();
+            GVRRenderData rdata = backdrops[0].getRenderData();
             if (lightEnabled)
                 rdata.disableLight();
             else
@@ -139,10 +146,12 @@ public class MultiLightMain extends GVRMain {
      * The multiple light shader uses the name "diffuseTexture" instead
      * of the name "main_texture".
      */
-    private GVRSceneObject createBackdrop(GVRContext context)
+    private GVRSceneObject createBackdrop(GVRContext context, int index)
     {
-        GVRTexture tex = context.getAssetLoader().loadTexture(new GVRAndroidResource(mGVRContext, R.drawable.gearvrflogo));
-        GVRSceneObject backdrop = new GVRSceneObject(context, 10.0f, 4.0f, tex);
+        int drawable = BackdropsIds[index];
+        GVRTexture tex = drawable != 0 ? context.getAssetLoader().loadTexture(new GVRAndroidResource(mGVRContext, drawable)) : null;
+        textures[index] = tex;
+        GVRSceneObject backdrop = new GVRSceneObject(context, 5.0f, 2.0f, tex);
         GVRRenderData rdata = backdrop.getRenderData();
         GVRMaterial material = new GVRMaterial(context, GVRMaterial.GVRShaderType.Phong.ID);
         
@@ -153,9 +162,17 @@ public class MultiLightMain extends GVRMain {
         material.setFloat("specular_exponent", 10.0f);
         material.setTexture("diffuseTexture", tex);
         backdrop.setName("Backdrop");
-        backdrop.getTransform().setPositionZ(-2.0f);
+        backdrop.getTransform().setPosition(0, index * 2f, -2.0f);
         rdata.setMaterial(material);
     	return backdrop;
     }
 
+    private void updateTextures() {
+        step++;
+        for(int index = 0; index < NumBackdrops; index++) {
+            GVRTexture texture = textures[(index + step) % NumBackdrops];
+            Log.d("ZZZ", "Setting texture for index " + index + " to: " + texture);
+            backdrops[index].getRenderData().getMaterial().setTexture("diffuseTexture", texture);
+        }
+    }
 }

@liaxim liaxim added the bug label May 2, 2018
@NolaDonato
Copy link
Contributor

I dug into this and figured out what is wrong. It is a timing issue because the app is switching textures in the framework thread and the GL thread doesn't correctly pick it up. I am investigating a fix for that.

For now, if you update the textures in the GL thread, it will work. I verified this by putting updateTextures() into a Runnable and then calling GVRContext.runOnGLThread. When I did that, those messages disappear and it seems to update the textures correctly.

@SteveGreatApe
Copy link
Author

Thanks, that work around is working for me okay.

@liaxim
Copy link
Contributor

liaxim commented Aug 21, 2018

@SteveGreatApe Assuming this issue has been resolved. Planning to close in few days.

@SteveGreatApe
Copy link
Author

I've removed my workaround and it seems okay. But a bit concerned there hasn't actually been a fix published for this problem. As it was a timing issue I might just be lucky at the moment to not be hitting the problem again.

@liaxim
Copy link
Contributor

liaxim commented Aug 22, 2018

Ok, will look into fixing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants