Skip to content

refactor: Move FreeImage into separate project #2690

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

refactor: Move FreeImage into separate project #2690

wants to merge 1 commit into from

Conversation

@Jklawreszuk
Copy link
Collaborator

@Jklawreszuk Jklawreszuk commented Mar 28, 2025 •
edited
Loading

PR Details

As the name suggests my PR separates FreeImage wrapper from TextureWrapper project just for sake of #2689 to prevent circular dependencies error.

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.

@Jklawreszuk Jklawreszuk changed the title feat: Xplat support for image loading and saving from memory refactor: Move FreeImage into separate project Mar 28, 2025
@Jklawreszuk Jklawreszuk mentioned this pull request Mar 28, 2025
Open
8 tasks
@Jklawreszuk
Copy link
Collaborator Author

Jklawreszuk commented Mar 29, 2025 •
edited
Loading

I'm not sure but seems like my new Stride.FreeImage package doesn't include the runtimes folder with freeimage.dll :( . If anyone has an idea what is wrong I would appreciate it

@Jklawreszuk Jklawreszuk marked this pull request as draft March 30, 2025 11:19
@Jklawreszuk Jklawreszuk marked this pull request as ready for review March 30, 2025 17:56
@Jklawreszuk
Copy link
Collaborator Author

Pr is ready for review, I tested the changes using the JumpyJet project. No regression so far 😄

@Doprez
Copy link
Contributor

Doprez commented Mar 30, 2025

I did a quick rebuild with the following steps:

  • git clean -xfd
  • dotnet clean
  • rebuild

And it seems to have broken the Test projects but I can confirm the GameStudio seems to run as expected.

[AssetCompiler] Exception in command [DdsMaskBC3] Stride.Assets.Textures.TextureConvertParameters: Stride.TextureConverter.TextureToolsException: Loading dds file D:/dev/GitControlledProjects/stride/sources/data/tests/SmallDdsMaskBC3.dds failed: E_FAIL

Im wondering if there is a reference to the new project missed for the tests somewhere?

@Jklawreszuk
Copy link
Collaborator Author

Oh that's because my branch is not up-to-date. One moment please hah

@Jklawreszuk
Copy link
Collaborator Author

@Doprez There you go

@Doprez
Copy link
Contributor

Doprez commented Mar 30, 2025

That seems to have fixed! Looks like everything is working on my end.

I did get a new crash but it seems unrelated to your changes but Ill log it here just in case:

Exception: ObjectDisposedException: Cannot access a disposed object.
Object name: 'System.Threading.ThreadLocal`1[[Stride.Core.MicroThreading.MicroThread, Stride.Core.MicroThreading, Version=4.2.0.1, Culture=neutral, PublicKeyToken=null]]'.
   at System.Threading.ThreadLocal`1.GetValueSlow()
   at Stride.Core.MicroThreading.Scheduler.get_RunningMicroThread() in D:\dev\GitControlledProjects\stride\sources\core\Stride.Core.MicroThreading\Scheduler.cs:line 79
   at Stride.Core.MicroThreading.MicroThreadSynchronizationContext.Post(SendOrPostCallback d, Object state) in D:\dev\GitControlledProjects\stride\sources\core\Stride.Core.MicroThreading\MicroThreadSynchronizationContext.cs:line 29
   at System.Threading.Tasks.AwaitTaskContinuation.RunCallback(ContextCallback callback, Object state, Task& currentTask)
--- End of stack trace from previous location ---
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_1(Object state)
   at System.Threading.QueueUserWorkItemCallback.Execute()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading.PortableThreadPool.WorkerThread.WorkerThreadStart()

Log: 1: BasicCameraController(0,0): []: Fatal: The asset BasicCameraController is missing or incorrectly indexed in the package. Please report this issue.
2: MyGameApp(0,0): []: Fatal: The asset MyGameApp is missing or incorrectly indexed in the package. Please report this issue.

It looks like a threading issue due to incorrectly disposed assets so I think you're fine for this PR, likely just an existing issue.

@Jklawreszuk Jklawreszuk marked this pull request as draft March 31, 2025 11:17
@Jklawreszuk Jklawreszuk force-pushed the freeimage branch 2 times, most recently from ac2dad6 to 81f8ea5 Compare October 29, 2025 18:48
@Jklawreszuk Jklawreszuk marked this pull request as ready for review October 29, 2025 18:48
@Jklawreszuk
Copy link
Collaborator Author

Okay, so my PR is now ready for review. I also used git mv to make my changes more transparent 😅

@Kryptos-FR
Copy link
Member

Kryptos-FR commented Oct 29, 2025 •
edited
Loading

LGTM.

Just FYI git mv doesn't change in any way how files are renamed. It's just a nice wrapper that stages the file after renaming it.

Kryptos-FR
Kryptos-FR reviewed Oct 29, 2025
View reviewed changes
Copy link
Member

@Kryptos-FR Kryptos-FR left a comment

Choose a reason for hiding this comment

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

I actually have a few remarks.

build/Stride.sln Outdated
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Stride.BepuPhysics.Tests", "..\sources\engine\Stride.BepuPhysics\Stride.BepuPhysics.Tests\Stride.BepuPhysics.Tests.csproj", "{7B70C783-4085-4702-B3C6-6570FD85CB8F}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Stride.FreeImage", "..\sources\tools\Stride.FreeImage\Stride.FreeImage.csproj", "{03695F9B-10E9-4A10-93AE-6402E46F10B5}"
Copy link
Member

Choose a reason for hiding this comment

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

That seems to be the wrong Guid for the type of project we use.

It should be {9A19103F-16F7-4668-BE54-9A1E7A4F7556}.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Strange, I was adding a project through Rider IDE. I fixed that now :)

sources/tools/Stride.FreeImage/Stride.FreeImage.csproj
<Nullable>enable</Nullable>
</PropertyGroup>
<ItemGroup>
<StrideNativeLib Include="$(MSBuildThisFileDirectory)..\..\..\deps\FreeImage\Release\**\*.dll">
Copy link
Member

Choose a reason for hiding this comment

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

The same link in the original project should probably be removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks I forgot about that, fixed!

@Jklawreszuk
Copy link
Collaborator Author

Jklawreszuk commented Oct 29, 2025 •
edited by Kryptos-FR
Loading

Actually I had to move GetFormatParameters(...) method to avoid circular dependency issue for Stride.csproj project

@Kryptos-FR Kryptos-FR self-requested a review October 30, 2025 06:36
@Kryptos-FR
Copy link
Member

The last change is failing the build. Is it necessary?

@Jklawreszuk
Copy link
Collaborator Author

Looks like not necessarily, I have already rolled back the change

@Kryptos-FR Kryptos-FR self-requested a review October 31, 2025 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Kryptos-FR Kryptos-FR Kryptos-FR approved these changes

Assignees

No one assigned

Labels

area-Graphics

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Jklawreszuk @Doprez @Kryptos-FR