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

API Cleanup: Use PixelSize for device-dependent sizes. #1889

Merged
merged 9 commits into from
Oct 29, 2018

Conversation

grokys
Copy link
Member

@grokys grokys commented Sep 11, 2018

This is something I've been meaning to do for years... Our API was inconsistent about accepting pixel sizes and DPI. For example:

  • RenderTargetBitmap accepted a DPI but WriteableBitmap didn't
  • IBitmap didn't expose the DPI so once it was put into RenderTargetBitmap it couldn't be read again
  • Sometimes DPI was represented by a Vector and sometimes by a pair of doubles
  • etc.

This PR introduces a new struct PixelSize which has an int width and height, and also methods for converting to/from a Size with a specified DPI (instead of having to do size = new Size(width / (dpi / 96, height / (dpi / 96)); which isn't so easy to remember). Hopefully this helps make the distinction between device-independent pixels and device pixels clearer.

Wherever a pixel size is passed to a constructor to create a bitmap, a DPI is also passed as a Vector.

Unfortunately Skia doesn't provide an API for getting an image's DPI, so images loaded there are always 96dpi. Not sure what to do about this. For this reason Image still always uses the pixel size as the size to render, rather than the pixel size scaled by DPI.

@grokys grokys requested a review from a team September 11, 2018 21:00
@jkoritzinsky
Copy link
Collaborator

Contributes to #1292.

@ncarrillo
Copy link
Contributor

This looks good to me. Hooray for using types to convey units :).

@jmacato
Copy link
Member

jmacato commented Oct 29, 2018

This is yet another breaking change that should be added to wiki :)

@jmacato jmacato merged commit 022fa47 into master Oct 29, 2018
@jmacato jmacato deleted the refactor/pixelsize branch October 29, 2018 11:11
@grokys grokys added this to the 0.7.0 milestone Apr 3, 2019
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.

5 participants