mosra / magnum-integration

Integration libraries for the Magnum C++11 graphics engine
https://magnum.graphics/
Other
99 stars 44 forks source link

Implement btIDebugDraw for Magnum #21

Closed Squareys closed 8 years ago

Squareys commented 8 years ago

Hi @mosra !

As promised via gitter, here is the PR for the bullet physics visualization ( #11 ).

There is one // TODO in draw3dText(...), you might want to look at how to specify/format it instead. I could also adapt the bullet example and whilst I'm at it remove the Shapes dependecy from it (since it will be deprecated) once you merged this.

Regards, Squareys.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 0.0% when pulling be5509b375d247e49eb9e43c22af059458eea13e on Squareys:physics-debug-draw into cdace8de771096025156c4899b4d1ae6ff3075a4 on mosra:master.

Squareys commented 8 years ago

Coverage remained the same at 0.0%

Uh... I guess it's not set up for magnum-integration yet?

mosra commented 8 years ago

No. If you remember, that's STILL because of this: https://github.com/travis-ci/apt-package-whitelist/issues/1190

I'm getting angry just seeing that issue.

The Bullet integration lib is built for OSX, but there I don't have the code coverage enabled. Maybe I should give up on waiting and build Bullet from source in this case.. :/

Squareys commented 8 years ago

No. If you remember, that's STILL

Right, forgot about it, it's been a year since.

Any other comments on the PR? For once it is not WIP, was simple enough ;) (Not pushing, would be great to have this upstream, though.)

mosra commented 8 years ago

Review done :)

The OSX build fails because of missing Shaders dependency. Can you update the CI files accordingly?

Regarding the text rendering -- yes, as you suggest, i think the better option long-term would be to use the "debug text" functionality once it's in. The TODO is fine like this. Or, maybe, put it in a Doxygen @todo block.

Until then, if you feel like doing it (just a suggestion!), even just copypasting the guts of the Text example and having the constructor take an optional Text::AbstractFont* font pointer with an opened font -- i.e. assert on it like this:

CORRADE_INTERNAL_ASSERT(!font || font->isOpened());

If the user won't supply the parameter (the default), it will not render any text. If the user provides the text, it will render it.

Eh, scratch that, now realized that this will be kinda hard to do with this "immediate" approach of the interface, as there is no flushText() or anything similar. Would need to think about it a bit more.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 0.0% when pulling 908642c85c01ab46d31143c981b37826e212a967 on Squareys:physics-debug-draw into cdace8de771096025156c4899b4d1ae6ff3075a4 on mosra:master.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 0.0% when pulling 404c308ccb123d1b3a93e532f5db9986481f71ab on Squareys:physics-debug-draw into cdace8de771096025156c4899b4d1ae6ff3075a4 on mosra:master.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 0.0% when pulling 404c308ccb123d1b3a93e532f5db9986481f71ab on Squareys:physics-debug-draw into cdace8de771096025156c4899b4d1ae6ff3075a4 on mosra:master.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 0.0% when pulling f9d17161f8ac567202d75e59ad80b620b1804a73 on Squareys:physics-debug-draw into b59c764e05a8fec1c85d33bfa911b4ff8aeacf41 on mosra:master.

mosra commented 8 years ago

Cherry-picked as f8eb01d66d44569b4ffb8000cc51922dbb28a43e, 28eb3c7d0c02d7d79fa7f2d5d1d5c03ff93bb96c and de3b54e3f10662d1b57db73fafa6e2f2ae7bf33b. On Linux the test failed the same way as on OSX, so I added an explicit cast to UnsignedInt before casting to void* to avoid sign extension.

Squareys commented 8 years ago

What's with https://github.com/mosra/magnum-integration/pull/21/commits/a08be624805ffe37df27f39f0ea6d114c06e4ce5? (Ah, I see that was also squashed into one of the other commits)

mosra commented 8 years ago

Into both, actually :)

I'm leaving the cleanup commits only in case they are not by the same author (so people can see the changes others made to their code).