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

CMake samples #84

Open
wants to merge 2 commits into
base: cmake
Choose a base branch
from
Open

CMake samples #84

wants to merge 2 commits into from

Conversation

JayFoxRox
Copy link
Owner

Based on #74

Adds the samples, but, more importantly, shows what kind of problems have to be solved in nxdk before many large libs can be ported.

The changes from this should be polished and upstreamed, but I likely won't be doing that.

mkdir build
cd build
${NXDK_DIR}/usr/bin/nxdk-cmake ..
${NXDK_DIR}/usr/bin/nxdk-cmake ..
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it necessary to run cmake twice tho?

I did not check if this also happens when building for linux (with these flags)

git clone https://github.com/MADEAPPS/newton-dynamics.git --branch v3.14c --depth=1
mkdir build
cd build
${NXDK_DIR}/usr/bin/nxdk-cmake ..
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will also complain a bunch about libs being forced from SHARED to STATIC

-DEM_DENORMAL=1
-DEM_ZERODIVIDE=1
-D_clearfp\(...\)=0
-D_controlfp\(...\)=0
Copy link
Owner Author

@JayFoxRox JayFoxRox Apr 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up until this point is #76

-D_clearfp\(...\)=0
-D_controlfp\(...\)=0
-D_finite\(...\)=0
-D_isnan\(...\)=0
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is part of #76 yet, or whereelse it might be handled

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made upstream issue nxdk 452, so hopefully someone picks it up


glm::vec3 v = glm::vec3(0.0f, 1.0f, 2.0f);

debugPrint("Hello World! %d", int(1000.0 * glm::length(v)));
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be (int) or ideally static_cast<int>(...)


debugPrint("Hello World! %d", int(1000.0 * glm::length(v)));
while(1);
return 0;
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad practice to have this after an endless loop (other instance, too)

"${NXDK_DIR}/lib/net/nforceif/src/sys_arch.c"
)
target_link_libraries(net lwipcore pktdrv)
endif()
Copy link
Owner Author

@JayFoxRox JayFoxRox Apr 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Combine lwip and Xbox driver into libnet

This should be part of nxdk, not of this sample.
Reported as upstream issue 454.

Copy link
Owner Author

@JayFoxRox JayFoxRox Apr 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Teufelchen1: This is probably a good task for you
Maybe a challenge for @kosmas12 if @Teufelchen1 doesn't want to do this.

(nxdk issue 454: "Network stack should be a static library")

git clone https://github.com/guillaumeblanc/ozz-animation.git --branch 0.13.0 --depth=1
mkdir build
cd build
${NXDK_DIR}/usr/bin/nxdk-cmake ..
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found an older comment saying:

ozz needs CMake to run twice; also the SDL renderer was not tested on Xbox, because it hangs during startup (fails to load skeleton / animation)

Compare https://github.com/JayFoxRox/nxdk/pull/84/files#r611207045

@JayFoxRox JayFoxRox force-pushed the cmake-samples branch 2 times, most recently from 871b7c8 to f378d5f Compare April 11, 2021 16:50
@@ -0,0 +1,3 @@
# Dummy file, so the CI doesn't trip
# Ideally this should be building the CMake projects on CI
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO

@@ -0,0 +1,50 @@
if(NXDK)
# Hack for winsock2.h and mmsystem.h
include_directories(SYSTEM "${NXDK_DIR}/lib/winapi/ws2_32;${NXDK_DIR}/lib/winapi/winmm")
Copy link
Owner Author

@JayFoxRox JayFoxRox Apr 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be part of default includes; I believe this is an upstream nxdk bug which will have to be integrated in #82

(speaking about winmm; obviously ws2_32 is a separate issue as it's part of this PR)

@@ -1,4 +1,5 @@
include $(NXDK_DIR)/lib/winapi/winmm/Makefile
include $(NXDK_DIR)/lib/winapi/ws2_32/Makefile
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ws2_32 should be split from this PR, cleaned, polished, and upstreamed.

Maybe @dracc or @Teufelchen1 (also @thrimbor or @Ryzee119, but those got too many tasks already)

if(NXDK)
# Make enet depend on Xbox network
set_target_properties(enet
PROPERTIES LINK_LIBRARIES "${NXDK_DIR}/lib/ws2_32.lib;${NXDK_DIR}/lib/winmm.lib;net")
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why aren't ws2_32 and winmm already deps of enet or the net library?
Should be fixed (upstream potentially?)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


// We should never reach this
return 1;
}
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yikes, not sure why I had this file? This probably needs a menu or something

_PDCLIB_PUBLIC double atof( const char * nptr ) {
return 0.0f;
}
}
Copy link
Owner Author

@JayFoxRox JayFoxRox Apr 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearly this is a hack - must be fixed upstream in nxdk / nxdk-pdclib or somewhere.

if(NXDK)
# Hack for memory.h
file(TOUCH ${CMAKE_CURRENT_BINARY_DIR}/memory.h)
include_directories(SYSTEM "${CMAKE_CURRENT_BINARY_DIR}/")
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what <memory.h> is, but an empty file is enough.

Fix upstream in ozz-animation by removing the inclusion if it's really not necessary.
Fix in nxdk by adding a dummy file (not sure where? winapi?) if it's actually used in ozz-animation.

add_subdirectory(ozz-animation EXCLUDE_FROM_ALL)

target_compile_options(ozz_base PUBLIC
-Dcout=clog
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nxdk-libcxx issue 5: std::cout does not exist, but std::clog and std::cerr exist

# COMMAND ${CMAKE_COMMAND} -E make_directory media
# COMMAND ${CMAKE_COMMAND} -E copy "${ozz_media_directory}/bin/alain_skeleton.ozz" "./media/skeleton.ozz"
# COMMAND ${CMAKE_COMMAND} -E copy "${ozz_media_directory}/bin/alain_crossarms.ozz" "./media/animation.ozz"
# VERBATIM)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code?

add_executable(default
main.cpp
# "${CMAKE_CURRENT_BINARY_DIR}/media/skeleton.ozz"
# "${CMAKE_CURRENT_BINARY_DIR}/media/animation.ozz"
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code?

}

}
}
Copy link
Owner Author

@JayFoxRox JayFoxRox Apr 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I updated ozz earlier I had to fix a bunch of stuff in this sample. I wasn't sure where the ozz related code originated (I remember writing the SDL2 line-renderer stuff myself though), so I just patched some spots.

The ozz related code should be taken from a sample so it matches upstream again.

I know that this sample worked in the past.
However, I did not try running this sample since updating ozz.

# Add enet and fix a bug in enet where include folder is not known to itself
add_subdirectory(enet)
set_target_properties(enet
PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${CMAKE_SOURCE_DIR}/enet/include/")
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is broken due to https://github.com/lsalzman/enet/blame/master/CMakeLists.txt#L58 using the wrong variable?

However, it should not be setting include directories for all targets anyway

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.

1 participant