-
Notifications
You must be signed in to change notification settings - Fork 968
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
Unity related changes #690
base: main
Are you sure you want to change the base?
Conversation
@atteneder Thanks for the PR. I'm going to start by going over the GitHub actions and CMake stuff. Frank and l will look more deeply into the code over the next week or so-- things are pretty busy at present. Re the GH actions stuff: there are some internal policy items that necessitate some changes to our repo configuration before we can accept CI automation that touches third-party actions. This shouldn't impact you much, but it's slowing the process down because Google projects can't accept the addition of third-party actions without these changes. As I currently understand the only change on your side is that CI integration must use specific released versions of third-party actions, but beyond that nothing is jumping out at me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the slow turnaround on this. I've really only gone through the CI and build related changes. Overall they look fine excepting the packaging related stuff, which I don't think we can accept in the parent repo.
.github/workflows/unity.yml
Outdated
|
||
linux: | ||
|
||
runs-on: ubuntu-18.04 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use ubuntu-latest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've had a users report an issue when using a library built on ubuntu-latest (20.10 at the time) on ubuntu 18.04 (correct glibc version was not found).
Is there a better way to achieve backwards compatibility? Or cross distribution compatibility in general?
Thank you very much @tomfinegan for the excellent feedback and sorry I wasn't able to pick it back up for so long. I tried to implement all suggestions and returned some questions where things are not clear to me. Depending on your answer I'll remove the packaging (or not), test if the CI works and make the PR ready for review. |
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
…roceeds until all mesh and vertex attributes are known (This is required for clients to prepare resources like allocating buffers). Step two does the actual decoding of vertex attributes. This change allows for certain optimization in the Unity integration. Note: WIP Only tested on MeshEdgebreakerDecoder.
… to left-hand (required for glTF embed Draco data)
…lding better compression
…Windows/macOS/Linux, 64-bit variants)
…ripten/iOS builds
fix: Corrected Emscripten v1 destination chore: Unified naming a bit
chore: Updated geekyeggo/delete-artifact action
…nded coordinate system in Unity encoder wrappers. Useful for Unity glTF export purpose.
* chore: Fixed CI runner image to avoid future surprises * ci: Refactored/simplified Unity build script * ci: Disabled Emscripten builds, as those moved into other CI jobs * ci: Fixed deprecated parameter * fix: Adressed linker errors due to duplicated symbols in `draco` and `dracodec_unity`. - Consolidated dracodec_unity and dracoenc_unity into a single target named draco_unity * chore: Renamed iOS simulator library name to avoid name conflict with device SDK variant. * fix: Attempt to create universal binary * fix: Attempt to create universal binary by providing architectures * feat: Apple tvOS and Apple visionOS support * chore: Raised iOS/tvOS target to 11.0, since this is the minimum version for Unity 2020 LTS * feat: Added Windows ARM64 support * fix: visionOS required dynamic libraries * Revert "chore: Raised iOS/tvOS target to 11.0, since this is the minimum version for Unity 2020 LTS" It broke the build This reverts commit 1eb874b. --------- Co-authored-by: Andreas Atteneder <andreas.atteneder@gmail.com>
* feat: Initial version of the WebGL packages * feat(ci): Yamato CI scripts * feat: Unit test that sanity checks the native library file sizes
… as indirect dependency (#6)
* Unity WebGL packages 1.0.0-pre.2 * fix: Correct namespace for Editor tests * chore: Increased minimum required version to Unity 2020.3 * feat: Empty runtime tests
Hello dear Draco developers,
This PR mostly serves the purpose of suggesting additions/changes, kick off discussions and not primarily to be merged in the end.
The code can be tested in the related Unity package DracoUnity 2.0.0-preview.
Split up decode function
The decode function could be sped up for Unity (and maybe other systems as well) by splitting it into phases:
Encoding
I've created an additional library that allows encoding Unity meshes to draco data/files.
Others
The latter is nothing new, but I'm open to re-integrate this package into the original repository. We could create a GitHub action that builds the native libaries, adds it to the package and deploys it to a special branch. From there it could be deployed to a package manage server (like verdaccio or OpenUPM)
Let me know your thoughts,
Thanks