Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[UWP] Min viable FontImageSource #4817

Merged
merged 3 commits into from
Dec 19, 2018
Merged

[UWP] Min viable FontImageSource #4817

merged 3 commits into from
Dec 19, 2018

Conversation

paymicro
Copy link
Contributor

@paymicro paymicro commented Dec 18, 2018

Description of Change

see #4747

Issues Resolved

API Changes

None

Platforms Affected

  • UWP

Before/After Screenshots

screenshot_1

Testing Procedure

Run Font ImageSource - Legacy

PR Checklist

  • Has automated tests
  • Rebased on top of the target branch at time of PR
  • Changes adhere to coding standard

Copy link
Member

@StephaneDelcroix StephaneDelcroix left a comment

Choose a reason for hiding this comment

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

🔥

Xamarin.Forms.Platform.UAP/FontImageSourceHandler.cs Outdated Show resolved Hide resolved
{
var iconcolor = fontsource.Color != Color.Default ? fontsource.Color : Color.White;
var device = CanvasDevice.GetSharedDevice();
var localDpi = Windows.Graphics.Display.DisplayInformation.GetForCurrentView().LogicalDpi;
Copy link
Contributor

Choose a reason for hiding this comment

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

Android and iOS are not so much, but UWP supports multi-scale and multi-monitor. The common case would be to move the app from one screen to another, potentially changing the resolution. We might want to handle this case to avoid distorted images.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally, yes. But here we return the bitmap. And if it is redrawn at each DPI measurement, then this greatly affects the performance. With a large number of icons, the application freezes for a couple of seconds. Therefore, I added a minimum dpi equal to 300. This should be enough so that the icon does not look soapy in most cases.

else
{
throw new InvalidCastException($"\"{Control.Source.GetType().FullName}\" is not supported.");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would maybe put this into an extension method because I don't think this is the only place we might use this. I am thinking image buttons and other bits that use images.

@kingces95 kingces95 merged commit 7ca231e into xamarin:font-image-source Dec 19, 2018
@kingces95
Copy link
Contributor

Long live @paymicro!

samhouts pushed a commit that referenced this pull request Jan 3, 2019
* min viable fontImageSource

* [UWP] Min viable FontImageSource (#4817)

* [UWP] font image source

* address comment

* [UWP] performance optimization of the rendering of icons font
- the minimum Dpi of the icon is 300

* char -> string; CR feedback
PureWeen pushed a commit that referenced this pull request Jan 24, 2019
* min viable fontImageSource

* [UWP] Min viable FontImageSource (#4817)

* [UWP] font image source

* address comment

* [UWP] performance optimization of the rendering of icons font
- the minimum Dpi of the icon is 300

* char -> string; CR feedback
andreinitescu pushed a commit to andreinitescu/Xamarin.Forms that referenced this pull request Jan 29, 2019
* min viable fontImageSource

* [UWP] Min viable FontImageSource (xamarin#4817)

* [UWP] font image source

* address comment

* [UWP] performance optimization of the rendering of icons font
- the minimum Dpi of the icon is 300

* char -> string; CR feedback
@samhouts samhouts modified the milestones: 4.0.0, 3.6.0 Feb 6, 2019
@samhouts
Copy link
Member

It looks like the Win2D.uwp didn't get installed automatically for this customer. #5188. Let's investigate why.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants