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

Can't render Object3D with zero position and scale #1784

Closed
rpicolet opened this issue Oct 3, 2016 · 5 comments
Closed

Can't render Object3D with zero position and scale #1784

rpicolet opened this issue Oct 3, 2016 · 5 comments

Comments

@rpicolet
Copy link
Collaborator

rpicolet commented Oct 3, 2016

Rajawali Version or Branch

1.1.610

Summary

Rendering Object3D with position and scale both 0 results in IllegalStateException from Matrix4.inverse(). This worked in older versions, prior to the introduction of the throws clause on Matrix4.inverse(). (The use case is adding an object to the scene which is to be made visible via a scaling animation.)

Not sure what the best/preferred fix is (handle the exception in setToNormalMatrix()?; remove the throws?; add a check for isInvertible()?), but I'm willing to take direction to create a PR for it.

Steps to Reproduce

Set position and scale of an Object3D to zero; next frame render fails.

Trace or Log Output

10-03 15:10:35.580 637-687/com.stupidhardgames.stuawki E/AndroidRuntime: FATAL EXCEPTION: GLThread 5017
Process: com.stupidhardgames.stuawki, PID: 637
java.lang.IllegalStateException: Matrix is singular and cannot be inverted.
at org.rajawali3d.math.Matrix4.inverse(Matrix4.java:348)
at org.rajawali3d.math.Matrix4.setToNormalMatrix(Matrix4.java:726)
at org.rajawali3d.materials.Material.setModelMatrix(Material.java:1076)
at org.rajawali3d.Object3D.render(Object3D.java:300)
at org.rajawali3d.Object3D.render(Object3D.java:343)
at org.rajawali3d.Object3D.render(Object3D.java:343)
at org.rajawali3d.Object3D.render(Object3D.java:343)
at org.rajawali3d.Object3D.render(Object3D.java:343)
at org.rajawali3d.Object3D.render(Object3D.java:343)
at org.rajawali3d.Object3D.render(Object3D.java:343)
at org.rajawali3d.Object3D.render(Object3D.java:343)
at org.rajawali3d.Object3D.render(Object3D.java:343)
at org.rajawali3d.Object3D.render(Object3D.java:343)
at org.rajawali3d.Object3D.render(Object3D.java:343)
at org.rajawali3d.Object3D.render(Object3D.java:184)
at org.rajawali3d.scene.Scene.render(Scene.java:1135)
at org.rajawali3d.scene.Scene.render(Scene.java:1002)
at org.rajawali3d.renderer.Renderer.render(Renderer.java:420)
at org.rajawali3d.renderer.Renderer.onRender(Renderer.java:410)
at org.rajawali3d.renderer.Renderer.onRenderFrame(Renderer.java:385)
at com.stupidhardgames.stuawki.control.scene3d.impl.Scene3DRenderer.onRenderFrame(Scene3DRenderer.java:64)
at org.rajawali3d.view.SurfaceView$RendererDelegate.onDrawFrame(SurfaceView.java:235)
at android.opengl.GLSurfaceView$GLThread.guardedRun(GLSurfaceView.java:1532)
at android.opengl.GLSurfaceView$GLThread.run(GLSurfaceView.java:1249)

@jwoolston
Copy link
Member

Im not sure I like the idea of allowing this...hence the addition of the throws in the first place. Why not just make the animation set the object to invisible?

@rpicolet
Copy link
Collaborator Author

rpicolet commented Oct 4, 2016

I don't object to the throws either, it makes good sense to disallow a non-invertible matrix from inverse(). But it doesn't seem inherently wrong to render a degenerate object, either, rather than having to check for degeneracy in every use case and make sure that never makes it into the render.

For example, I also use scale animations to remove objects from view; the ScaleAnimation3D.applyTransformation() would have to be modified to check on every frame (or the onAnimationUpdate() callback monitored) for a zero scale vector to set the visible flag accordingly, just to handle the last frame.

Wouldn't it be more generally useful for Material.setModelMatrix() not to assume an invertible model matrix, or perhaps for Matrix4.setToNormal() to handle it?

BTW, great job getting Bombshell out!

@jwoolston
Copy link
Member

But it doesn't seem inherently wrong to render a degenerate object, either, rather than having to check for degeneracy in every use case and make sure that never makes it into the render.

When you put it that way, I agree completely. I would love to see a PR for this. I think the appropriate thing here would be to add the throws clause to Matrix4#setToNormal() and make Material.setModelMatrix() handle the possibility of exception as you suggest. If you make a PR for this, I will push out another release.

BTW, great job getting Bombshell out!

Thanks. I always appreciate when people understand I can't just crank it out like its my day job (it actually used to be though). Hopefully you will like the changes coming for 2.0.

@rpicolet
Copy link
Collaborator Author

rpicolet commented Oct 4, 2016

I think the appropriate thing here would be to add the throws clause to Matrix4#setToNormal() and make Material.setModelMatrix() handle the possibility of exception as you suggest. If you make a PR for this, I will push out another release.

Will do. Do I need to base the fix branch on anything besides the usual master (e.g. the Bombshell tag)? If different in any way, guidance would be appreciated.

I always appreciate when people understand I can't just crank it out like its my day job (it actually used to be though).

Whaa...? You don't get paid to make all us Rajawali users happy??? ;-) Well then, have a beer, at least! 🍺

@jwoolston
Copy link
Member

master would be appropriate. We have had a few other bug fixes since 610 was released so they should be included as well.

Well then, have a beer, at least!

Oh many beers have been consumed over my years of working on Rajawali, I assure you. I have mulled over the idea of offering paid development services with Rajawali - something like I will develop any features needed by someone who is paying and/or develop apps using Rajawali - just havn't decided if I should or not yet and how much it would take to be worthwhile to me.

rpicolet added a commit to rpicolet/Rajawali that referenced this issue Oct 4, 2016
- adding throws IllegalStateException to Matrix4.setToNormal() as it depends on Matrix4.inverse()
- putting call to setToNormal() in Material.setModelMatrix() in try/catch block, thus enabling continued rendering of degenerate/zero-scale Object3Ds (which have singular model matrices)
rpicolet added a commit to rpicolet/Rajawali that referenced this issue Oct 5, 2016
- Adding debug log notice to Material.setModleMatrix() for IllegalStateException
- Adding ATransformable3D.isZeroScale()
- Adding isZeroScale() check at beginning of Object3D.render(), and skipping render if true
- Adding both isDestroyed() and isZeroScale() checks to Object3D.renderColorPicking() to match render()
jwoolston added a commit that referenced this issue Oct 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants