Skip to content

Conversation

@Ruben2776
Copy link
Contributor

Fixes #1780

@CLAassistant
Copy link

CLAassistant commented Jan 8, 2025

CLA assistant check
All committers have signed the CLA.

@dlemstra
Copy link
Owner

dlemstra commented Jan 9, 2025

Thanks for creating this PR but I am not sure if this would be the best solution. Maybe we could add an extra ToWriteableBitmapWithDensity method that uses the image.Density and change ToWriteableBitmap to use a density of 96? I can also make these changes myself if you don't want to do this. This should probably also include some tests to check if the dpi on the WriteableBitmap has the correct value. If that is even available in the api.

@Ruben2776
Copy link
Contributor Author

Yeah, okay, that sounds good.

@dlemstra
Copy link
Owner

dlemstra commented Jan 9, 2025

Will you be going to make these changes or do you want me to do this?

@Ruben2776
Copy link
Contributor Author

I don't know anything about how that dpi test would work. I think it would be better if you did it.

@dlemstra
Copy link
Owner

dlemstra commented Jan 9, 2025

In the unit test you would need to check if the Dpi property of the WriteableBitmap that was created has the correct value.

/// <returns>A <see cref="WriteableBitmap"/>.</returns>
public static unsafe WriteableBitmap ToWriteableBitmap<TQuantumType>(this IMagickImage<TQuantumType> self)
where TQuantumType : struct, IConvertible
{
Copy link
Owner

@dlemstra dlemstra Jan 9, 2025

Choose a reason for hiding this comment

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

I would create a private method that has the current implementation and specify either new Vector(96, 96) or new Vector(self.Density.X, self.Density.Y) as an argument to that method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated it with ToWriteableBitmapInternal and sending argument between the two.

@dlemstra
Copy link
Owner

Thanks for helping me. I will add the unit tests myself 😁

@dlemstra dlemstra merged commit a80eb05 into dlemstra:main Jan 10, 2025
1 check passed
This was referenced Dec 10, 2025
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.

Incorrect Size for ToWriteableBitmap Magick.NET.AvaloniaMediaImaging when DPI is not 96

3 participants