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

feat: Adjust freeimage to all platforms #2303

Merged
merged 2 commits into from
Jun 18, 2024

Conversation

Jklawreszuk
Copy link
Collaborator

PR Details

This PR adjust FreeImage loading for macos and linux distributions (these operating systems load this dependency from packages).
From now, FreeImage is handling image files instead of DirectXTex library (LoadWICFile method is only available for Windows) .

Types of changes

  • Docs change / refactoring / dependency upgrade - Additional dependency to install
  • 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.

@Eideren
Copy link
Collaborator

Eideren commented Jun 7, 2024

TC is complaining, might need to adjust those texture extensions you removed to get loaded by freeimage instead ?
https://teamcity.stride3d.net/buildConfiguration/Engine_Tests_WindowsSimple/25548?hideTestsFromDependencies=false&hideProblemsFromDependencies=false&expandBuildDeploymentsSection=false&expandPull+Request+Details=true&expandBuildChangesSection=true&expandBuildProblemsSection=true

@Jklawreszuk
Copy link
Collaborator Author

Jklawreszuk commented Jun 7, 2024

@Eideren Glad the tests caught the error 😅 According to the documentation, it seems that FreeImage has dedicated both Unicode and Non-Unicode methods that should be used accordingly on Windows or Unix systems.

FreeImage 3.18.0 documentation FreeImage_LoadU
Note that this function only works on MS Windows operating systems. On other systems, the
function does nothing and returns NULL.

@Jklawreszuk
Copy link
Collaborator Author

Jklawreszuk commented Jun 7, 2024

I started looking for inspiration and it looks like our friends at Monogame fixed this by using appropriate method depending on the system. I will try to go in this direction
Monogame :

[DllImport(NativeLibName, CharSet = CharSet.Unicode, EntryPoint = "FreeImage_LoadU")]
      public static extern IntPtr LoadU(FREE_IMAGE_FORMAT fif, string filename, int flags);

      [DllImport(NativeLibName, EntryPoint = "FreeImage_Load")]
      public static extern IntPtr LoadS(FREE_IMAGE_FORMAT fif, string filename, int flags);

      public static IntPtr Load(FREE_IMAGE_FORMAT fif, string filename, int flags)
      {
          if (CurrentPlatform.OS == OS.Windows)
              return LoadU(fif, filename, flags);
          else
              return LoadS(fif, filename, flags);
      }

@Jklawreszuk Jklawreszuk force-pushed the freeimage-xplat branch 3 times, most recently from f088f60 to 3d6c94d Compare June 11, 2024 00:47
@Jklawreszuk
Copy link
Collaborator Author

Done! @Eideren, Could you try to run tests again?

@Eideren
Copy link
Collaborator

Eideren commented Jun 11, 2024

We have a certificate issue right now that xen is looking into, I'll get back to you once it's done :)

@Eideren Eideren merged commit 15d7dc3 into stride3d:master Jun 18, 2024
11 of 12 checks passed
@Eideren
Copy link
Collaborator

Eideren commented Jun 18, 2024

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