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

Add set_interface for access by GDNative #46781

Merged
merged 1 commit into from
Mar 26, 2021

Conversation

BastiaanOlij
Copy link
Contributor

@BastiaanOlij BastiaanOlij commented Mar 8, 2021

Ok, I've completely redone this and added a few additional changes to this.

We can now call the set_interface through a new GDNative ARVR API call.

As I'm adding a new API level I've also added another often requested feature and that is to get access to the depth buffer.
This is important in XR for re-projection during frame drops and to enable stuff like LIV (https://www.liv.tv/) and other mixed reality solutions.

I've also added an option to provide an external depth buffer alongside the external color buffer.

The depth buffer changes are pretty experimental especially on GLES2 as there is no way to switch between render buffer and texture.

@BastiaanOlij BastiaanOlij requested review from a team as code owners March 8, 2021 05:06
@BastiaanOlij BastiaanOlij requested a review from akien-mga March 8, 2021 05:06
@BastiaanOlij BastiaanOlij force-pushed the gdn_set_interface branch 2 times, most recently from a6d13f6 to c98eee2 Compare March 8, 2021 08:46
@akien-mga akien-mga added this to the 3.2 milestone Mar 8, 2021
@akien-mga
Copy link
Member

Still crashing with:

modules/gdnative/arvr/arvr_interface_gdnative.cpp:84:20: runtime error: member access within misaligned address 0x00000000000a for type 'const struct godot_arvr_interface_gdnative', which requires 8 byte alignment
0x00000000000a: note: pointer points here
<memory cannot be printed>
handle_crash: Program crashed with signal 11
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] bin/godot.x11.tools.64s() [0x171b67a] (/home/runner/work/godot/godot/platform/x11/crash_handler_x11.cpp:54)
[2] /lib/x86_64-linux-gnu/libc.so.6(+0x46210) [0x7f053eab4210] (??:0)
[3] ARVRInterfaceGDNative::set_interface(godot_arvr_interface_gdnative const*) (/home/runner/work/godot/godot/modules/gdnative/arvr/arvr_interface_gdnative.cpp:84)
[4] ARVRInterfaceGDNative::_set_interface(long) (/home/runner/work/godot/godot/modules/gdnative/arvr/arvr_interface_gdnative.cpp:90)

For the reference, here's what that CI Linux build with sanitizers and test project does:

# We should always be explicit with our flags usage here since it's gonna be sure to always set those flags
- name: Compilation
env:
SCONS_CACHE: ${{github.workspace}}/.scons_cache/
run: |
scons tools=yes target=debug use_asan=yes use_ubsan=yes
ls -l bin/
# Download and test project to check leaks and invalid memory usage.
# CI has no audio device, so use the Dummy audio driver to avoid spurious error messages.
- name: Importing and running project project
run: |
wget2 https://github.com/qarmin/RegressionTestProject/archive/3.2.zip
unzip 3.2.zip
mv "RegressionTestProject-3.2" "test_project"
echo "----- Open editor to check for memory leaks -----"
DRI_PRIME=0 xvfb-run bin/godot.x11.tools.64s --audio-driver Dummy -e -q --path test_project 2>&1 | tee sanitizers_log.txt || true
misc/scripts/check_ci_log.py sanitizers_log.txt
echo "----- Run and test project -----"
DRI_PRIME=0 xvfb-run bin/godot.x11.tools.64s 30 --video-driver GLES3 --audio-driver Dummy --path test_project 2>&1 | tee sanitizers_log.txt || true
misc/scripts/check_ci_log.py sanitizers_log.txt

Base automatically changed from 3.2 to 3.x March 16, 2021 11:11
@aaronfranke aaronfranke modified the milestones: 3.2, 3.3 Mar 16, 2021
@BastiaanOlij
Copy link
Contributor Author

Note before this gets lost and forgotten. After discussion on godot chat about this we've decided to do this more proper and add the required function to the ARVR GDNative API. Will find some time for that soonish.

@BastiaanOlij BastiaanOlij requested a review from a team as a code owner March 25, 2021 11:15
@BastiaanOlij
Copy link
Contributor Author

Ok, so I redid this whole thing and just updated the opening text. Also added access to the depth buffer.

- add set_interface function
- add access to depth buffer
- add supplying a depth buffer from an ARVR plugin
@akien-mga akien-mga merged commit acbd1e8 into godotengine:3.x Mar 26, 2021
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants