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

Improvements over Image, Bitmap and Icon classes #9

Merged
merged 38 commits into from
Jul 1, 2024

Conversation

damiansalvia
Copy link
Collaborator

Several improvements over Image/Bitmap/Icon classes:

  • Make Bitmap class inherit from Image class
  • Base Image class on SKBitmap (instead of SKImage)
  • Implement missing methods and properties
  • Add PixelFormat, RotateFlipType, ImageLockMode, ImageFlags, GraphicUnit enums

Copy link
Member

@fedeazzato fedeazzato left a comment

Choose a reason for hiding this comment

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

Magnificent work! 👏

I just added a couple of notes that I believe may need small improvements.

src/Common/Image.cs Outdated Show resolved Hide resolved
src/Common/Image.cs Outdated Show resolved Hide resolved
src/Common/Image.cs Outdated Show resolved Hide resolved
src/Common/Bitmap.cs Outdated Show resolved Hide resolved
@damiansalvia
Copy link
Collaborator Author

I applied a fix for every suggestion except for the SvgImage proposal, I'm still considering the best way to implement it.

@damiansalvia
Copy link
Collaborator Author

I created an Svg class (sibling of Bitmap) to encapsulate the definition of SkiaSharp.Extended.Svg.SKSvg. This class also has a reference to XmlDocument for manipulating the SVG. Currently, it is used for transformations such as rotation and scaling provided by the RotateFlip method, but it could be useful for additional operations in the future.

This change also includes a refactor of the Image and Bitmap classes to remove the direct reference to SKBitmap in the Image class. Now, SKBitmap is only referenced by the Bitmap class.

Copy link
Member

@fedeazzato fedeazzato left a comment

Choose a reason for hiding this comment

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

This work is awesome!

I just left a single question in Icon class.

src/Common/Icon.cs Outdated Show resolved Hide resolved
@damiansalvia
Copy link
Collaborator Author

@fedeazzato Did you have time to check my replay on your comment? If you're agree, can we merge these changes into master?

@damiansalvia damiansalvia merged commit 065e57b into main Jul 1, 2024
1 check passed
@damiansalvia damiansalvia deleted the feature/bitmap branch July 1, 2024 14:40
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.

2 participants