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

Cannot build templates for 32-bit Windows #80559

Closed
MBCX opened this issue Aug 12, 2023 · 4 comments · Fixed by #80656
Closed

Cannot build templates for 32-bit Windows #80559

MBCX opened this issue Aug 12, 2023 · 4 comments · Fixed by #80656

Comments

@MBCX
Copy link
Contributor

MBCX commented Aug 12, 2023

Godot version

v4.2.dev.custom_build [4714e95]

System information

Godot v4.2.dev (4714e95) - Windows 10.0.22621 - Vulkan (Compatibility) - Radeon RX 580 Series (Advanced Micro Devices, Inc.; 31.0.21023.2010) - Intel(R) Core(TM) i5-9600K CPU @ 3.70GHz (6 Threads)

Issue description

When trying to compile the export templates for 32 bit windows (using MSVC), it fails and then hangs, until Ctrl+C is pressed (or until it terminates itself). Below is a screenshot of the issue. This does not happen if you compile the templates for 64-bit windows.

Screenshot 2023-08-12 154227

Steps to reproduce

  1. Go to the root of the folder where the godot source is at.
  2. Run scons platform=windows target=template_debug/template_release arch=x86_32
  3. Wait until it tries to compile modules\openxr\extensions\openxr_extension_wrapper_extension.cpp

However, after looking at the lines of code in question:

// Below code is on line 129
void OpenXRExtensionWrapperExtension::on_instance_created(const XrInstance p_instance) {
	uint64_t instance = reinterpret_cast<uint64_t>(p_instance);
	GDVIRTUAL_CALL(_on_instance_created, instance);
}
// Below code is on line 138
void OpenXRExtensionWrapperExtension::on_session_created(const XrSession p_session) {
	uint64_t session = reinterpret_cast<uint64_t>(p_session);
	GDVIRTUAL_CALL(_on_session_created, session);
}

I would think the uint64_t type is causing it to fail, since is not available in a 32-bit environment (according to the screenshot).

Minimal reproduction project

N/A

@bruvzg
Copy link
Member

bruvzg commented Aug 12, 2023

I would think the uint64_t type is causing it to fail, since is not available in a 32-bit environment (according to the screenshot).

uint64_t is available on any platform. It's failing since XrInstance is defined as pointer on 64-bit platforms and as integer on 32-bit, therefore the cast should be C-cast to work in both cases.

For the reference, I had to fix this in ANGLE PR to test it - https://github.com/godotengine/godot/pull/72831/files#diff-df0c644d78a5bb40bc44f5210989217e2643d6cdad64ee034c538b2c87eef69a

@BastiaanOlij
Copy link
Contributor

cc @konczg was there a specific reason for casting it the way you did? We probably want to keep it exporting a 64bit value, even on 32bit because that is what GDExtension expects, but just insure it casts properly.

@konczg
Copy link
Contributor

konczg commented Aug 15, 2023

@BastiaanOlij With 32-bit platforms in mind, the best solution is probably to use C-style casts (as noted by @bruvzg above). The only reason I used reinterpret_cast was to make the specific type of cast applied clear, but 32-bit builds were not taken into account
cc @kisg

@konczg
Copy link
Contributor

konczg commented Aug 15, 2023

@BastiaanOlij @bruvzg @kisg I've created a PR for the required changes: #80656

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

Successfully merging a pull request may close this issue.

6 participants