Skip to content

Headless renderer #14

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

Merged
merged 50 commits into from
Mar 19, 2023
Merged

Headless renderer #14

merged 50 commits into from
Mar 19, 2023

Conversation

neverix
Copy link
Member

@neverix neverix commented Dec 8, 2022

Add compact, short information about your PR for easier understanding:

  • Add headless renderer
  • Changes a few render targets

To do

This PR is a Work in Progress

  • Most of the work
  • Hide the window

How to test

make -j8 && python hacking_testing/test_loop.py

@AI-WAIFU AI-WAIFU marked this pull request as ready for review February 9, 2023 16:46
@@ -392,6 +392,8 @@ def _write_config(self):
config_file.write("mute_sound = true\n")
config_file.write("show_debug = false\n")
config_file.write("enable_client_modding = true\n")
config_file.write("video_driver = null\n")
Copy link

Choose a reason for hiding this comment

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

I think these config changes shouldn't be included because I was just adding them to get the test loop to run in colab. but then it turned out that only empty frames were sent from minetest.

@rk1a
Copy link

rk1a commented Feb 9, 2023

Overall I am not quite sure why we would want to merge this now. We already have the semi headless mode using Xvfb.
The code in this branch hides the window, but it turned out this is only possible with a running XServer again requiring Xvfb (see #46).
So from what I know this would not add any useful functionality while adding the additional SDL dependency.

What would be good to know is if there is a performance difference between the current state of develop and this branch.
Also maybe this somehow does work without Xvfb on the TPU server? But seems like #46 is a general problem.

${LUA_INCLUDE_DIR}
${GMP_INCLUDE_DIR}
${JSON_INCLUDE_DIR}
${LUA_BIT_INCLUDE_DIR}
${SDL2_INCLUDE_DIR}

Choose a reason for hiding this comment

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

Not needed.

@@ -528,10 +538,12 @@ include_directories(SYSTEM
${ZMQ_INCLUDE_DIR}
${ZMQPP_INCLUDE_DIR}
${SQLITE3_INCLUDE_DIR}
${SDL2_INCLUDE_DIR}

Choose a reason for hiding this comment

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

Not needed.

minetest.conf Outdated
@@ -1,4 +1,5 @@
name = MinetestAgent
update_last_checked = 1670529488

Choose a reason for hiding this comment

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

Probably shouldn't be committed.

${ZSTD_LIBRARY}
${ZMQ_LIBRARY}
${ZMQPP_LIBRARY}
${X11_LIBRARIES}
${SOUND_LIBRARIES}
${SQLITE3_LIBRARY}
${SDL2_LIBRARY}

Choose a reason for hiding this comment

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

Not needed.

@@ -133,7 +133,8 @@ compile_commands.json
.vs/

# Optional user provided library folder
lib/irrlichtmt
# TODO: don't add irrlichtmt, apply patch from CMakeLists.txt

Choose a reason for hiding this comment

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

Don't merge TODO comments.

CMakeLists.txt Outdated
@@ -68,6 +69,19 @@ set(ENABLE_UPDATE_CHECKER (NOT ${DEVELOPMENT_BUILD}) CACHE BOOL
set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/cmake/Modules/")



if(BUILD_CLIENT AND BUILD_HEADLESS)

Choose a reason for hiding this comment

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

This should not be in both CMakeLists.txt and src/CMakeLists.txt.

@@ -36,6 +36,17 @@ set(CMAKE_BUILD_TYPE "${CMAKE_BUILD_TYPE}" CACHE STRING
# Set some random things default to not being visible in the GUI
mark_as_advanced(EXECUTABLE_OUTPUT_PATH LIBRARY_OUTPUT_PATH)

if(BUILD_CLIENT AND BUILD_HEADLESS)

Choose a reason for hiding this comment

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

This should not be in both CMakeLists.txt and src/CMakeLists.txt.

@@ -1834,6 +1836,7 @@ void Client::makeScreenshot()
if (!raw_image)
return;

// warningstream << "got data" << raw_image->getDimension().Width << std::endl;

Choose a reason for hiding this comment

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

Don't merge commented out code.

@AI-WAIFU AI-WAIFU merged commit 34a44b2 into develop Mar 19, 2023
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.

5 participants