Conversation
|
Pushed additional updates to #92 which now has CI running on macOS. Once we have dependency situation figured out, we could restore Linux clang as CI target? |
|
Builds except for the flight-paths example which depends on microsoft/vcpkg#11016. I changed GCC to 9 to match the other build script. |
|
Now ready for review. Main changes are:
Benefits of this PR:
|
ilijapuaca
left a comment
There was a problem hiding this comment.
Awesome, we should land this one with all the arrow stuff out of the way. A few questions:
- Does drawing work on Linux? You mentioned animometer example was running on using OpenGL, what about flight paths?
- Seeing quite a few warnings in travis build, is that also the case when you build locally? Just making sure it's consistent, most of them are type related and should be easy to fix once I get around to fixing gcc build on macOS
- Is this something we should be worried about?
==8479== LEAK SUMMARY:
==8479== definitely lost: 3,848 bytes in 35 blocks
==8479== indirectly lost: 832 bytes in 26 blocks
==8479== possibly lost: 0 bytes in 0 blocks
==8479== still reachable: 0 bytes in 0 blocks
==8479== suppressed: 0 bytes in 0 blocks
==8479== Rerun with --leak-check=full to see details of leaked memory
| // TODO(ilija@unfolded.ai): wrapLongitude and modelMatrix not set, set them to default values explicitly if they | ||
| // aren't relevant? | ||
| ViewportUniforms uniforms = {// Projection mode values | ||
| .viewProjectionMatrix = Matrix4<float>{matrixAndOffset.viewProjectionMatrix}, |
There was a problem hiding this comment.
Just curious, what kind of order does gcc try to enforce? This doesn't seem to match order within the actual struct
There was a problem hiding this comment.
GCC was complaining that the initializers weren't in the order the fields were declared in.
There was a problem hiding this comment.
The order they are currently in on this branch does not match what's on master, that's why it looked pretty random to me. Perhaps we need to rebase to make sure it works after we merge
At the moment it does not work. Same issue on flight-paths right now.
Yes, the messages are consistent with my local build.
It's not a big deal but I would prefer to eliminate leaks from the tests if possible. Usually this happens because when writing the test we might not bother to free some allocated memory, since the application will be torn down after the test. |
This indicates that it's trying to use Vulkan, and I'm not sure what kind of state that's in. Perhaps it's just a matter of drivers on your machine not being compatible, but we can just switch up the order in which they are prioritized and bump OpenGL up if that works |
I think that was how I previously got it to work - looks like at some point it reverted to trying Vulkan. Edit: Recompiling Dawn without Vulkan still gives this error. Possibly deck.gl is trying to use the wrong renderer, though, since Vulkan shouldn't have been used in the first place. |
This depends on a branch of the deps, which depends on a modified version of vcpkg. I was able to successfully build and run the tests and the animometer example.