Skip to content

Conversation

Doprez
Copy link
Contributor

@Doprez Doprez commented Mar 30, 2025

PR Details

The default build was setting itself to always be for arm even on non-arm systems. This change should allow users to build and have it detect whether or not they need the arm build enabled.

The architecture information was taken from https://learn.microsoft.com/en-us/dotnet/api/microsoft.build.utilities.processorarchitecture?view=msbuild-17-netcore although they did not include X64 which is required after some testing.

Related Issue

#2694
#2693

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

azeno and others added 2 commits March 30, 2025 18:11
The dlls were built using `sources\tools\Stride.TextureConverter.Wrappers\build-deps.bat` inside of developer command prompt of VS2022
@Doprez
Copy link
Contributor Author

Doprez commented Mar 30, 2025

The logic was taken from dotnet/msbuild#5065 (comment) which seems to be the only solution to detect architecture.

Sorry to ping you again @azeno but since you seem to have an arm system can you give this a try to make sure it works as I hope?

@Kryptos-FR
Copy link
Member

Kryptos-FR commented Mar 30, 2025

Can this PR be rebased? It seems to include the same change that was done in #2694.

@Doprez
Copy link
Contributor Author

Doprez commented Mar 30, 2025

Im trying to rebase it but it keeps lying to me saying its up to date...
{4A766ED5-E2AF-4C52-AEAF-20772BA70DD5}

I think the issue is I branched off of Azenos repo originally so I may just start fresh unless someone know a better command.

@Doprez
Copy link
Contributor Author

Doprez commented Mar 30, 2025

Thanks to the wonderful jkao for pointing out my silliness in Discord it should be good now.

@azeno
Copy link
Collaborator

azeno commented Mar 30, 2025

Condition="'$(StrideNativeWindowsArm64Enabled)' == ''" must stay there, otherwise it won't work when cross compiling. For example the build agent on TC will be win-x64, but we still want it to build the ARM64 assets, by passing StrideNativeWindowsArm64Enabled=true to the build / modify the Stride.build file accordingly.

@Doprez
Copy link
Contributor Author

Doprez commented Mar 30, 2025

Gotcha makes sense, in that case I think the below code change should be fine so that it only runs when the conditional is true:

  <Choose>
    <When Condition="'$(StrideNativeWindowsArm64Enabled)' == 'true'">
      <PropertyGroup>
        <StrideNativeWindowsArm64Enabled>True</StrideNativeWindowsArm64Enabled>
      </PropertyGroup>
    </When>
    <When Condition="'$(StrideNativeWindowsArm64Enabled)' == 'false'">
      <PropertyGroup>
        <StrideNativeWindowsArm64Enabled>False</StrideNativeWindowsArm64Enabled>
      </PropertyGroup>
    </When>
    <Otherwise>
      <PropertyGroup>
        <!-- Determining Arm builds based on OS and cpu architecture from https://learn.microsoft.com/en-us/dotnet/api/microsoft.build.utilities.processorarchitecture?view=msbuild-17-netcore -->
        <StrideNativeWindowsArm64Enabled Condition="$([MSBuild]::IsOSPlatform('Windows')) AND '$([System.Runtime.InteropServices.RuntimeInformation]::OSArchitecture)' == 'X64'">False</StrideNativeWindowsArm64Enabled>
        <StrideNativeWindowsArm64Enabled Condition="$([MSBuild]::IsOSPlatform('Windows')) AND '$([System.Runtime.InteropServices.RuntimeInformation]::OSArchitecture)' == 'AMD64'">False</StrideNativeWindowsArm64Enabled>
        <StrideNativeWindowsArm64Enabled Condition="$([MSBuild]::IsOSPlatform('Windows')) AND '$([System.Runtime.InteropServices.RuntimeInformation]::OSArchitecture)' == 'X86'">False</StrideNativeWindowsArm64Enabled>
        <StrideNativeWindowsArm64Enabled Condition="$([MSBuild]::IsOSPlatform('Windows')) AND '$([System.Runtime.InteropServices.RuntimeInformation]::OSArchitecture)' == 'ARM64'">True</StrideNativeWindowsArm64Enabled>
        <StrideNativeWindowsArm64Enabled Condition="$([MSBuild]::IsOSPlatform('Windows')) AND '$([System.Runtime.InteropServices.RuntimeInformation]::OSArchitecture)' == 'ARM'">True</StrideNativeWindowsArm64Enabled>
      </PropertyGroup>
    </Otherwise>
  </Choose>

And the default behaviour is just to use the hardware metadata to determine the result.

@Doprez
Copy link
Contributor Author

Doprez commented Mar 30, 2025

Another assumption that maybe Im misunderstanding is that I thought you would need an arm system to build the arm libraries. If that isnt true am I missing something to be able to do a full build for this? I already installed the arm related MSVC v143 from the VS installer.

@azeno
Copy link
Collaborator

azeno commented Mar 31, 2025

Did you install the ARM64 (not ARM) MSVC v143? I did it this afternoon on a win-x64 machine and it worked just fine.

@Doprez
Copy link
Contributor Author

Doprez commented Mar 31, 2025

oh my goodness that was exactly what I got wrong, thank you! I had the ARM version when I should have had MSVC v143 - VS 2022 C++ ARM64/ARM64EC build tools (latest)
{8CCFD025-811E-4611-A7BA-8379C6B6A96D}

It works perfectly fine with that, I guess this PR is useless in that case eh?

@Kryptos-FR
Copy link
Member

Kryptos-FR commented Mar 31, 2025

I haven't checked what the current state is of what this PR changes, but ideally we should have this by default:

  • TeamCity should be able to build the ARM64 target by default (DEBUG or RELEASE), along with all other targets.
  • Users shouldn't need to install the arm64 tools when building on a x64 machine.
  • Users with an arm64 machine shouldn't need the x64 build tools.

In other words, running msbuild manually on a user machine should only build what's necessary for that user machine, therefore detect the proper settings.

@Doprez
Copy link
Contributor Author

Doprez commented Mar 31, 2025

Ok, this PR should still be relevant then.

It adds a toggle to enable or disable arm builds and if no setting is provided it uses the developers host machine to determine if it should be enabled. The default for the Stride.Build file is to have it true so it should be on for TC builds.

@azeno
Copy link
Collaborator

azeno commented Mar 31, 2025

Your proposal doesn't include the other way round, where on an arm64 machine the x64 targets are optional. Since I'm currently sitting in front of a x64 machine and arm64 machine, I can have a look if you want and post the results here.

@Doprez
Copy link
Contributor Author

Doprez commented Mar 31, 2025

Is that because there are more configuration changes needed outside of StrideNativeWindowsArm64Enabled?

If you have time to test it that would be awesome as I dont have valid hardware to test natively.

@azeno
Copy link
Collaborator

azeno commented Mar 31, 2025

Ok, turns out the other way round (no x64/x86 on arm64 host machine) is not really an option as they are a requirement for other packages.
In the end I ended up with this condition:

  <!-- To keep MSVC build tools for Arm64 optional we enable it by default only on hosts running on Arm64 -->
  <Choose>
    <When Condition="'$(StrideNativeWindowsArm64Enabled)' == ''">
      <Choose>
        <When Condition="$([System.Runtime.InteropServices.RuntimeInformation]::ProcessArchitecture) == 'Arm64'">
          <PropertyGroup>
            <StrideNativeWindowsArm64Enabled>true</StrideNativeWindowsArm64Enabled>
          </PropertyGroup>
        </When>
        <Otherwise>
          <PropertyGroup>
            <StrideNativeWindowsArm64Enabled>false</StrideNativeWindowsArm64Enabled>
          </PropertyGroup>
        </Otherwise>
      </Choose>
    </When>
  </Choose>

@Doprez
Copy link
Contributor Author

Doprez commented Mar 31, 2025

Ah nice, that is a ton more clean then what I ended up with. Although is just ARM not an issue then and only ARM64? Not sure if there would be a need for backwards compatibility on this.

@azeno
Copy link
Collaborator

azeno commented Apr 1, 2025

As far as I know there's only ARM64 on Windows, ARM32 is getting phased out.

@Eideren
Copy link
Collaborator

Eideren commented Apr 6, 2025

Is this PR ready to be merged ? Looks like a no ?

@Doprez
Copy link
Contributor Author

Doprez commented Apr 6, 2025

I had to replace my PC so I completely forgot about this. I think I just need to test Azenos solution and update it and we should be good.

Ill finish this in the next hour.

@Doprez
Copy link
Contributor Author

Doprez commented Apr 6, 2025

Ok, this should be good to go now.

Tested with and without the ARM64 build tools and everything works as expected with Azenos above code.

@Doprez
Copy link
Contributor Author

Doprez commented Apr 13, 2025

I dont think any of my changes here should have affected OVR but I did notice the csproj for VR is the only one that contains false explicitly for arm builds?

<StrideNativeWindowsArm64Enabled>false</StrideNativeWindowsArm64Enabled>

No clue if thats actually the problem though as it works locally for me.

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.

4 participants