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

MathView and TextView width and height calculation does not work with non-100% DPI on Windows #136

Closed
MartinZikmund opened this issue Jun 21, 2020 · 22 comments
Labels
Resolution/Fixed The described issue is fixed. Status/5. Awaiting next release This issue has been resolved and is waiting for the next release/prerelease. Type/Bug

Comments

@MartinZikmund
Copy link

Describe the bug

When display has non-100 % DPI, the height of TextView is calculated as significantly bigger than expected on Windows.

To Reproduce

  1. Create a StackLayout with two TextView controls, both with the following content:
\textit{Napětí by se dalo téměř krájet. 
Skupina rytířů vkládá do velkého koše poslední zátěž a přesouvá se 
ke šlapacímu kolu. 
Na povel se začnou pomalu pohybovat a stroj se zachvěje. 
Rameno trebuchetu se jen neochotně sklání k zemi.}

\\

Opatrně ho zajistí mohutnou pojistkou.

\\

Několik mužů umístí těžký kámen do koženého vaku.

\\

Všichni ustoupí do bezpečné vzdálenosti od mohutné dřevěné konstrukce.

\\

Trhnutí lanem a kámen letí vstříc nepřátelské pevnosti.

\\

Desítky očí se nemohou odtrhnout od projektilu.

\\

Doletí?

\\

Pokud se těleso bude pohybovat v blízkosti Země (nebo jakékoliv jiné planety), bude na něj působit tíhová síla. Abychom si celou situaci zjednodušili, tak budeme předpokládat, že těleso se pohybuje v těsné blízkosti povrchu Země a jeho trajektorie bude velmi malá (oboje vzhledem k rozměrům Země).
  1. Set display scaling in Windows Settings to something higher than 100% (I have 150%).
    3.Observe the result has significant space between the texts.

Example from my app:

image

But on 100% scaling display:

image

Expected behavior

The height should be calculated appropriately regardless of display scaling.

Environment (please complete the following information):

  • Platform: CSharpMath.SkiaSharp
  • Package Version or Commit: latest
  • Device: Windows 10 PC
@MartinZikmund MartinZikmund added Status/0. New This issue is new and is awaiting confirmation from the maintainers. Type/Bug labels Jun 21, 2020
@Happypig375
Copy link
Collaborator

Also note that diacritics are not italicized properly. We need a way to convert ě to \v e and ů to \r u automatically because
image

@Happypig375
Copy link
Collaborator

I have no idea what went wrong. Smells like a Xamarin.Forms bug...

@Happypig375
Copy link
Collaborator

Should be a Xamarin.Forms bug.
image

@Happypig375
Copy link
Collaborator

Hmmm. The spacing increases with the height of the content.

@FoggyFinder
Copy link
Collaborator

Should be a Xamarin.Forms bug.

Does it persist across different XF versions?

@Happypig375
Copy link
Collaborator

Still investigating. I might be wrong on this being a Xamarin.Forms problem.

@MartinZikmund
Copy link
Author

@Happypig375 actually I discovered the spacing bug in my UWP version, so it is probably something in Painter.Measure 🤔

@charlesroddie
Copy link
Collaborator

charlesroddie commented Jun 21, 2020

In our Xamarin Forms app we have a function which is supposed to retrieve the pixels per XF view unit. It is defined in each platform project:

// Android
Resources.System.DisplayMetrics.ScaledDensity
// iOS
UIScreen.MainScreen.Scale
// UWP
Windows.Graphics.Display.DisplayInformation.GetForCurrentView().RawPixelsPerViewPixel
// Mac
NSScreen.MainScreen.BackingScaleFactor

This is then used in places (Skiasharp and CSharpMath.Skiasharp) that work with raw pixels.

I wonder if there is a better approach to this.

@Happypig375
Copy link
Collaborator

Xamarin.Essentials.DeviceDisplay.MainDisplayInfo.Density?

@Happypig375 Happypig375 added Status/1. Ready This issue has been confirmed and is ready to be worked on. and removed Status/0. New This issue is new and is awaiting confirmation from the maintainers. labels Jun 23, 2020
@Happypig375
Copy link
Collaborator

My theory is that on higher scales, a larger view box is allocated, but the scale is not taken into account, thus appearing to be smaller. With a fixed width/height, this leads to the view appearing to be scaled down, leading to #137.

@prepare
Copy link

prepare commented Jun 28, 2020

Hello,

I'm not sure but I think this may related about this issue.

How DPI effects the final pixels size calculation ?

"other than 96 dpi"

on Windows,

The Typography uses default DPI value on => 96 dpi, and PointsPerInch= 72
to calculate the scale_to_pixel value from CalculateScaleToPixel() and CalculateScaleToPixelFromPointSize() .


  const int s_pointsPerInch = 72;//point per inch, fix?        

        /// <summary>
        /// default dpi
        /// </summary>
        public static uint DefaultDpi { get; set; } = 96;

        /// <summary>
        /// convert from point-unit value to pixel value
        /// </summary>
        /// <param name="targetPointSize"></param>
        /// <param name="resolution">dpi</param>
        /// <returns></returns>
        public static float ConvPointsToPixels(float targetPointSize, int resolution = -1)
        {
            //http://stackoverflow.com/questions/139655/convert-pixels-to-points
            //points = pixels * 72 / 96
            //------------------------------------------------
            //pixels = targetPointSize * 96 /72
            //pixels = targetPointSize * resolution / pointPerInch

            if (resolution < 0)
            {
                //use current DefaultDPI
                resolution = (int)DefaultDpi;
            }

            return targetPointSize * resolution / s_pointsPerInch;
        }
        /// <summary>
        /// calculate scale to target pixel size based on current typeface's UnitsPerEm
        /// </summary>
        /// <param name="targetPixelSize">target font size in point unit</param>
        /// <returns></returns>
        public float CalculateScaleToPixel(float targetPixelSize)
        {
            //1. return targetPixelSize / UnitsPerEm
            return targetPixelSize / this.UnitsPerEm;
        }
        /// <summary>
        ///  calculate scale to target pixel size based on current typeface's UnitsPerEm
        /// </summary>
        /// <param name="targetPointSize">target font size in point unit</param>
        /// <param name="resolution">dpi</param>
        /// <returns></returns>
        public float CalculateScaleToPixelFromPointSize(float targetPointSize, int resolution = -1)
        {
            //1. var sizeInPixels = ConvPointsToPixels(sizeInPointUnit);
            //2. return sizeInPixels / UnitsPerEm

            if (resolution < 0)
            {
                //use current DefaultDPI
                resolution = (int)DefaultDpi;
            }
            return (targetPointSize * resolution / s_pointsPerInch) / this.UnitsPerEm;
        }

_from https://github.com/LayoutFarm/Typography/blob/master/Typography.OpenFont/Typeface.cs#L297


For DPI-aware app=> The DPI value should be changed to match current DPI value of your system.

(an incorrect value may lead to incorrect scale => incorrect heights... etc).

dpi_120

pic 1: DPI-aware WinForm on Win10, set DPI=120, Sarabun font, 20pt, [a] Rendered with PixelFarm no Hint, [b] Windows Notepad, [c] Rendered with PixelFarm + vertical only TrueType hint

@Happypig375
Copy link
Collaborator

I have noticed the settable resolution in Typography too but I haven't got around to experiment with it.

@prepare
Copy link

prepare commented Jul 3, 2020

I updated method CalculateScaleToPixel() and CalculateScaleToPixelFromPointSize() .
by changing default value of resolution to -1.

see => LayoutFarm/Typography@69f741e

@Happypig375
Copy link
Collaborator

Happypig375 commented Jul 14, 2020

A problem would be how to get the DPI in a cross-platform way. Is Density DPI?

@MartinZikmund
Copy link
Author

A problem would be how to get the DPI in a cross-platform way. Is Density DPI?

There is a cross platform implementation of display properties in Uno Platform (DisplayInformation class) and Xamarin.Essentials.

@Happypig375
Copy link
Collaborator

What happens when the view crosses display boundaries?

@Happypig375
Copy link
Collaborator

Also, MathViews have the same problem.

@Happypig375 Happypig375 changed the title TextView height calculation does not work with non-100% DPI on Windows MathView and TextView height calculation does not work with non-100% DPI on Windows Jul 14, 2020
@Happypig375
Copy link
Collaborator

This bug does not occur in CSharpMath.Avalonia.

@Happypig375
Copy link
Collaborator

I'll try to take a Xamarin.Essentials in CSharpMath.Forms and use the density.

@Happypig375 Happypig375 changed the title MathView and TextView height calculation does not work with non-100% DPI on Windows MathView and TextView width and height calculation does not work with non-100% DPI on Windows Jul 14, 2020
@Happypig375
Copy link
Collaborator

Using BackgroundColor as a reference, this bug affects width too.

@Happypig375 Happypig375 added Status/3. Reviewing The pull request associated with this issue is currently being reviewed. and removed Status/1. Ready This issue has been confirmed and is ready to be worked on. labels Jul 16, 2020
@Happypig375
Copy link
Collaborator

Should be fixed in latest commit. Try the nightly packages!

@Happypig375 Happypig375 added Status/4. Awaiting author confirmation Awaiting a confirmation from the author of this issue that this issue has been resolved as expected. and removed Status/3. Reviewing The pull request associated with this issue is currently being reviewed. labels Jul 18, 2020
@MartinZikmund
Copy link
Author

Tested on latest master and works great too :-) . Thanks!

@Happypig375 Happypig375 added Status/5. Awaiting next release This issue has been resolved and is waiting for the next release/prerelease. Resolution/Fixed The described issue is fixed. and removed Status/4. Awaiting author confirmation Awaiting a confirmation from the author of this issue that this issue has been resolved as expected. labels Jul 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution/Fixed The described issue is fixed. Status/5. Awaiting next release This issue has been resolved and is waiting for the next release/prerelease. Type/Bug
Projects
None yet
Development

No branches or pull requests

5 participants