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

Refactor #27

Merged
merged 7 commits into from
Sep 5, 2019
Merged

Refactor #27

merged 7 commits into from
Sep 5, 2019

Conversation

xissburg
Copy link

@xissburg xissburg commented Jun 28, 2019

Summary

  • Option to build static lib.
  • Remove pluginTest and move tests into a Samples folder.
  • Refactor tests/samples.
  • Move assets into a Media directory. Delete and ignore build dir.
  • Remove Ogre_glTF::loaderAdapter::getMesh and replace it with Ogre_glTF::loaderAdapter::getFirstSceneNode() which returns the hierarchy in the gltf file starting from the first root node found.
  • Create Ogre::TagPoint for items that are children of bones.
  • Do not use inverseBindMatrices to build skeleton. Use bone node transform instead.
  • Support skeletons with multiple roots.

Sorry for not making this a clean PR with small commits etc, but this is a separate branch I created and merged from the other one where I have added morph animation support and I removed all the morph animation stuff because it's not ready to be merged yet since my PR wasn't reviewed yet in the Ogre repo.

Let me know if there's anything you think should be changed.

Thanks

Remove pluginTest and move tests into a Samples folder.
Refactor tests/samples.
Move assets into a Media directory. Delete and ignore build dir.
Remove Ogre_glTF::loaderAdapter::getMesh and replace it with Ogre_glTF::loaderAdapter::getFirstSceneNode() which returns the hierarchy in the gltf file starting from the first root node found.
Create Ogre::TagPoint for items that are children of bones.
Do not use inverseBindMatrices to build skeleton. Use bone node transform instead.
Support skeletons with multiple roots.
@Ybalrid
Copy link
Owner

Ybalrid commented Jun 29, 2019

This sounds good!

Don't worry for the CI builds, my server is offline, and the scripts are pulling a pre-built Ogre V2 from it. (I'm in the middle of a server migration)

Most of your changes are things that I was meaning to do but never got around to, so this is super helpful, thanks! 🎉

@Ybalrid
Copy link
Owner

Ybalrid commented Jul 1, 2019

I'm happy about what you did to the bone loading routines, this seems much much cleaner.

Can you insert a mkdir build command inside the CI scripts?

Before line 37 in /.travis.yml .

For the windows builds currently it fails to copy the Hlms library. It's possible (probable?) that the CMake script cannot find the HLMS directory, as the path contains "OGRE_MEDIA_DIR-NOTFOUND".

I may need to rebuild the SDK package used for the windows builds. For now it's the content of the SDK as packaged by the INSTALL target. I am aware they are issues with it...

Have you validated the CMake changes you have made under a Windows/Visual Studio environment?

@xissburg
Copy link
Author

xissburg commented Jul 2, 2019

The OGRE_MEDIA_DIR variable is supposed to point to Samples/Media inside the Ogre dir. That's used to copy the Hlms dir into the build dir so the samples can load the shaders.

I have not tested it on Windows yet. Will have to do it later. I'll let you know.

@xissburg
Copy link
Author

I have tested it on Windows and fixed a thing to make it work which was to set the right working directory. Let me know if there's anything else.

Morph animation was merged into the official Ogre v2-1 branch so I'll create another PR to merge the glTF morph branch into here as soon as this one is merged.

I have also implemented support for the MSFT_lod extension. It required a few changes to Ogre so I will have to submit a PR in there later and then I'll also create a PR in here if you want to add MSFT_lod support. I have also added MSFT_lod support in the Blender 2.8 glTF exporter in my fork so it should be possible to export glTF files with LODs and import them straight into Ogre.

@Ybalrid
Copy link
Owner

Ybalrid commented Jul 11, 2019

I have tested it on Windows and fixed a thing to make it work which was to set the right working directory. Let me know if there's anything else.

👍 I'm gonna check the details and get back to you

Morph animation was merged into the official Ogre v2-1 branch so I'll create another PR to merge the glTF morph branch into here as soon as this one is merged.

This is great news! I'm going to rebuild the Ogre v2-1 used by AppVeyor and see what's going on over there 😉

I have also implemented support for the MSFT_lod extension. It required a few changes to Ogre so I will have to submit a PR in there later and then I'll also create a PR in here if you want to add MSFT_lod support. I have also added MSFT_lod support in the Blender 2.8 glTF exporter in my fork so it should be possible to export glTF files with LODs and import them straight into Ogre.

This is also really cool. LOD is useful. I'll be more than happy to be able to support it in Ogre_glTF (even tho it's a Microsoft vendor extension). Maybe you can try to get that feature merged into the Blender 2.8 exporter? I don't really know what their stance on vendor glTF extension is tho, so maybe that could only live as a fork... In any cases, keep me up-to-date on that!

@Ybalrid
Copy link
Owner

Ybalrid commented Jul 11, 2019

Well, it looks like the only error in AppVeyor is that it cannot copy the media directory (hlms shader library I suppose).

I'm going to check the state of everything, and I'll merge this later today. (I need to pull and build Ogre from sources again, I'll mess with that tonight)

@xissburg
Copy link
Author

If you build Ogre with samples and also install the samples, FindOGRE should be able to find OGRE_MEDIA_DIR, I believe.

@Ybalrid
Copy link
Owner

Ybalrid commented Jul 13, 2019

If you build Ogre with samples and also install the samples, FindOGRE should be able to find OGRE_MEDIA_DIR, I believe.

Yes, that's what I think too. I haven't got around to rebuild Ogre just yet

@Ybalrid
Copy link
Owner

Ybalrid commented Jul 23, 2019

@xissburg Sorry for taking that long on this, I just have all my free time eaten up by other project 🙃 I'll try to get this merged ASAP, I just haven't found the time to take care of it... 😅

@Ybalrid
Copy link
Owner

Ybalrid commented Sep 5, 2019

Okay, I'll merge that

@Ybalrid Ybalrid merged commit f83073b into Ybalrid:master Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants