Skip to content

Refactor from System.Drawing.Common to Skiasharp.#1538

Closed
Miepee wants to merge 22 commits intoUnderminersTeam:masterfrom
Miepee:skiasharp
Closed

Refactor from System.Drawing.Common to Skiasharp.#1538
Miepee wants to merge 22 commits intoUnderminersTeam:masterfrom
Miepee:skiasharp

Conversation

@Miepee
Copy link
Copy Markdown
Contributor

@Miepee Miepee commented Nov 15, 2023 •
edited
Loading

Description

This PR attempts to refactor from System.Drawing.Common to Skiasharp. Doing this successfully means, that we'll be finally able to bump the .NET version for the project and not make the project Windows only by doing so.
Fixes #875

Caveats

This is not finished yet

Notes

TODO:

  • Opening this, because I need help with the GUI part (CC @VladiStep ), as I'm unable to do meaningful changes there due to me using Linux.
  • Someone should test this with QOI games (I don't have any on me).
  • Ensure that basic graphic functionality didn't get lost (i.e. viewing embedded texture pages, texture page entries, rooms etc.) once the GUI part has been refactored
  • Make sure that all the graphic related scripts (importAllTilesets, importAllSprites, newTextureRepacker etc.) still work as they should
  • Check if these behave properly on different display scales

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 15, 2023 •
edited
Loading

Download the artifacts for this pull request here:

GUI:

CLI:

@mustafa-archived
Copy link
Copy Markdown
Contributor

mustafa-archived commented Nov 15, 2023

Opening this, because I need help with the GUI part (CC @VladiStep ), as I'm unable to do meaningful changes there due to me using Linux.

Can't you just use wine? I usually do my work on utmt with wine and it works just fine except for a few offs

@Miepee
Copy link
Copy Markdown
Contributor Author

Miepee commented Nov 15, 2023 •
edited
Loading

That works for the GUI, but I don't want to make the core library require the use of wine.
Wine only runs on certain machines, while if we can get it to run natively, it will run anywhere where the dotnet runtime is able to run. Which is to say, a lot more machines.

If you meant dev'ing using WINE - that's more painful, because i can't actually build the GUI. And running the whole build environment via wine is a huge PITA.

@mustafa-archived
Copy link
Copy Markdown
Contributor

mustafa-archived commented Nov 15, 2023

Oh ok.

Miepee and others added 21 commits June 2, 2024 16:07
@Miepee
Refactor from System.Drawing.Common to Skiasharp.
22ceca0
@VladiStep @Miepee
Some UI fixes.
c26eb75
@VladiStep @Miepee
Implement GMS 2 tile flag rendering.
9e841c6
@VladiStep @Miepee
A little optimization of CreateLayerSource()
9faf60b
@VladiStep @Miepee
Remove redundant DLL import and uncomment one line.
ce38b10
@VladiStep @Miepee
Simplify disposal of the disposable objects.
c55aeab
@VladiStep @Miepee
Minor code improvements for "TextureWorker.cs".
1067b9b
@VladiStep @Miepee
Improve performance of some methods in TextureWorker.
61b975b
@VladiStep @Miepee
Minor code improvements for "QoiConverter.cs".
ca437af
@VladiStep @Miepee
Fix a crash on exporting all frames of a sprite,
c677100
@VladiStep @Miepee
Remove all references of "System.Drawing" from the lib and GUI.
f74955a
@VladiStep @Miepee
Remove all references of "System.Drawing" from the scripts (WIP)
a8331c7
@VladiStep @Miepee
Revert scripts changes
0c21988
@VladiStep @Miepee
Add useful methods and one argument to TextureWorker.
77d5725
@VladiStep @Miepee
Add missing comments for tile transformations.
0ea44e8
@VladiStep @Miepee
Remove a redundant DLL import.
f693c8a
@VladiStep @Miepee
Add reminder comments for tile transformation part.
f843311
@VladiStep @Miepee
Add a comment with a warning.
98364cb
@VladiStep @Miepee
Remove unneccesary SKBitmap.SetImmutable() call.
a720cd7
@VladiStep @Miepee
Revert the QoiConverter.GetImageFromSpan() comment change.
a8337d0
@VladiStep @Miepee
Replace "System.Drawing" with "SkiaSharp" in all scripts. (#24)
2ba06eb 
* Restore WIP script changes

* Finish removing/replacing of "System.Drawing" in all scripts (not tested)

* Update MergeImages.csx

* Update ImportMasks.csx

---------

Co-authored-by: Miepee <[email protected]>
@Miepee Miepee marked this pull request as ready for review June 2, 2024 14:24
@Miepee
Copy link
Copy Markdown
Contributor Author

Miepee commented Jun 2, 2024

Opening this PR up for review. I did a very basic test via WINE and it worked as expected there.
But I would like to have someone on windows still test:

  • QOI games
  • the graphic exporter/importer scripts

@colinator27
Copy link
Copy Markdown
Member

colinator27 commented Aug 15, 2024

So I tested this a few days ago and found that (for sure) SkiaSharp doesn't losslessly process PNG images, so unfortunately this probably won't be able to merge. (And it'll probably be superseded by #1870 and the PRs that follow it.)

However, this is still really good reference for what CSX scripts we'll need to update after #1870 is merged, which I'm going to start working on.

@Miepee
Copy link
Copy Markdown
Contributor Author

Miepee commented Aug 22, 2024

I will close this PR as its superseded as mentioned above.
For script reference, this PR existing is enough reference.

@Miepee Miepee closed this Aug 22, 2024
@colinator27 colinator27 mentioned this pull request Aug 26, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Migrate away from System.Drawing.Common

4 participants

@Miepee @mustafa-archived @colinator27 @VladiStep