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

DebugDrawer improved. #2249

Merged
merged 4 commits into from Aug 20, 2014
Merged

DebugDrawer improved. #2249

merged 4 commits into from Aug 20, 2014

Conversation

@nooone
Copy link
Contributor

nooone commented Aug 20, 2014

I have added a few more implementations to the DebugDrawer:

  • reportErrorWarning
  • drawContactPoint
  • drawTriangle
  • draw3dText

The draw3dText implementation is probably the most interesting part as it's in need of the current glViewport parameters, which is why I added begin(Viewport). Another alternative (@xoppa's idea) would be to keep track of the current glViewport paramters in a static way, for example via Gdx.graphics.getGlViewport(). A more slow (but maybe for debugging purposes appropriate?) way would be to just read it like this:

private static final Rectangle viewportRect = new Rectangle();

public static Rectangle getGlViewport() {
    FloatBuffer buffer = BufferUtils.newFloatBuffer(16);

    Gdx.graphics.getGL20().glGetFloatv(GL20.GL_VIEWPORT, buffer);
    viewportRect.x = buffer.get(0);
    viewportRect.y = buffer.get(1);
    viewportRect.width = buffer.get(2);
    viewportRect.height = buffer.get(3);

    return viewportRect;
}

In both cases we could get rid of the Viewport completely here.

The way this PR draws 3D text is by projecting the destination position to the screen and then drawing it in 2D, centered on the position. This has the advantage of pixel perfection. However there are also some drawbacks:

  • Impossible to make use of the depth buffer
  • The text has always the same size, however far away it is

I've tried to implement it following kalle_h's advice and draw it via the spritebatch with manipulated matrices to "real" 3D, however I could not make it work completely. Something must be wrong with the object matrix's rotation:

public void draw3dText(Vector3 location, String textString) {
    shapeRenderer.end();

    Matrix4 lookAtMatrix = new Matrix4().setToLookAt(location, camera.position, Vector3.Y);
    Quaternion lookAtQuaternion = new Quaternion();
    lookAtMatrix.getRotation(lookAtQuaternion, true);

    Matrix4 objectMatrix = new Matrix4();
    objectMatrix.set(location, lookAtQuaternion);

    spriteBatch.setProjectionMatrix(camera.combined);
    spriteBatch.setTransformMatrix(objectMatrix);
    spriteBatch.begin();

    Gdx.graphics.getGL20().glEnable(GL20.GL_DEPTH_TEST);

    TextBounds bounds = font.getBounds(textString);
    font.draw(spriteBatch, textString, -bounds.width / 2, bounds.height / 2);

    spriteBatch.end();
    shapeRenderer.begin(ShapeType.Line);
}

If we could get this to work, there's no need for the glViewport parameters at all.

@xoppa
Copy link
Member

xoppa commented Aug 20, 2014

Note that the ShapeRenderer, BitmapFont and SpriteBatch have to be disposed when no longer needed. Could you make them private with setters and getters, along with a private boolean ownsFont etc? Then when the user provides one of those in the constructor or via the setter you set the ownsXXX boolean to false, otherwise to true. In the dispose method, when the ownsXXX field is true you dispose the corresponding member.

@nooone
Copy link
Contributor Author

nooone commented Aug 20, 2014

Good spot @xoppa! I've implemented it the way you described it.

}

private static final Vector3 tmp = new Vector3();

This comment has been minimized.

Copy link
@xoppa

xoppa Aug 20, 2014

Member

It looks like this is never used.

shapeRenderer.point(pointOnB.x, pointOnB.y, pointOnB.z);

to.set(pointOnB.x, pointOnB.y, pointOnB.z);
to.add(normalOnB.scl(distance));

This comment has been minimized.

Copy link
@xoppa

xoppa Aug 20, 2014

Member

this is the same as normalOnB.scl(distance).add(pointOnB);

@xoppa
Copy link
Member

xoppa commented Aug 20, 2014

I'm not sure about creating a font and spritebatch (and thus Mesh and ShaderProgram) by default. As we've seen the draw3dText method is by default only called for softbody simulation. So in most cases the font and spritebatch aren't even used. Perhaps lazy initialization would be a better approach.

@nooone
Copy link
Contributor Author

nooone commented Aug 20, 2014

@xoppa: It seems like the libgdx projects are configured to not display warnings about unused fields? That's why I didn't notice it. I removed them and added the lazy initialization.

/** Switches the {@link SpriteBatch}. The given sprite batch won't be disposed when {@link #dispose()} is called. */
public void setSpriteBatch (SpriteBatch spriteBatch) {
if (ownsSpriteBatch) {
this.spriteBatch.dispose();

This comment has been minimized.

Copy link
@xoppa

xoppa Aug 20, 2014

Member

I guess this would cause a NPE now :)

@nooone
Copy link
Contributor Author

nooone commented Aug 20, 2014

Haha, yeah I forgot that. Then tested it in my game and fixed it... but forgot to also apply the fix in my libgdx fork before commiting -.-

Genius!

xoppa added a commit that referenced this pull request Aug 20, 2014
@xoppa xoppa merged commit c0f2719 into libgdx:master Aug 20, 2014
@xoppa
Copy link
Member

xoppa commented Aug 20, 2014

Thanks!

@nooone nooone deleted the nooone:debugdrawer branch Aug 27, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.