View Issue Details

IDProjectCategoryView StatusLast Update
0031037LazarusWidgetsetpublic2018-11-30 19:18
ReporterCudaText man Assigned ToOndrej Pokorny  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
PlatformMacOS 
Target Version1.8 
Summary0031037: Mac: fixed dpi 72 should be changed to Windows-value 96
DescriptionCarbon sets OS's pixels per inch to 72.
It is not correct, coz it gives problems with font sizes in many apps.
e.g., CudaText scales a window from its design dpi 96, to OS dpi (for Mac it's 72).
It gives wrong scaled window - it's smaller on Mac, while it must not be smaller

Wish: set to 96
Additional Informationfunction TCarbonWidgetSet.GetDeviceCaps(DC: HDC; Index: Integer): Integer;
begin
  Result := 0;

  {$IFDEF VerboseWinAPI}
    DebugLn('TCarbonWidgetSet.GetDeviceCaps DC: ' + DbgS(DC) + ' Index: ' + DbgS(Index));
  {$ENDIF}
 
  if not CheckDC(DC, 'GetDeviceCaps') then Exit;

  case Index of
  LOGPIXELSX,
  LOGPIXELSY:
    // logical is allways 72 dpi, although physical can differ
    Result := 72; // TODO: test scaling and magnification
===
Mac users write this-
Aha! Looking with the correct phrase, "os x dpi", I get lots of hits. As expected, the general answer is no, the Mac OS still does not have a global DPI setting. It's all application independent. Each app does whatever it does and is unlikely to be the same as another app. So trying to find a magic point size that will always be xxx pixels tall would be a lesson in frustration.

https://discussions.apple.com/thread/5153667?start=0&tstart=0
TagsNo tags attached.
Fixed in Revision53712, 53714, 54453, 54464, 54486
LazTarget1.8
Widgetset
Attached Files

Relationships

parent of 0031912 new Lazarus Some controls are shown with smaller font size 
related to 0031598 resolvedOndrej Pokorny Lazarus IDE comp-palette has icons w/out gaps, so icons overlap 
related to 0031601 closedOndrej Pokorny Lazarus Font and Image scaling issues in IDE 
related to 0033597 resolvedOndrej Pokorny Lazarus Lazarus with cocoa widgetset on 64 bit darwin: source editor font very small 
related to 0034461 closedZeljan Rikalo Patches TreeItem text is centered instead of left justified 
related to 0034625 closedZeljan Rikalo Lazarus Qt/Qt5: Wrong PPI calculation on Mac after r57679 
Not all the children of this issue are yet resolved or closed.

Activities

Juha Manninen

2016-11-30 16:47

developer   ~0096425

Number 72 can be found also in TCocoaWidgetSet.GetDeviceCaps.
The request sounds sane. Macs also have higher DPIs now than they used to have.
Mac users, please check.

Ondrej Pokorny

2016-12-10 12:59

developer   ~0096638

Yes, this should be fixed. Otherwise the high DPI features will deliver nonsense on Mac/Carbon. It's important for the IDE as well.

Zeljan Rikalo

2016-12-10 14:45

developer   ~0096644

IMO, it's safe to add 96 as default for modern macs, especially for cocoa ws.

Ondrej Pokorny

2016-12-18 11:12

developer   ~0096882

I changed all 72 values to 96 in both Carbon and Cocoa. Please test and report issues, I didn't test it myself.

CudaText man

2016-12-18 21:29

reporter   ~0096917

Now my app, and IDE too-- show fonts bigger by 20-30%.
e.g. was: setting in my app "font size 12", now user must set "font size 9".
maybe not bad?
IDE: in all SynEdit's (editors, all option dialogs)

Ondrej Pokorny

2016-12-18 21:37

developer   ~0096918

Did you suggest changing 72 to 96 without testing?
What now, revert?

CudaText man

2016-12-18 23:28

reporter   ~0096921

I did't sugget without testing.
No pls don't revert. (not sure, maybe need to change default Font.Height on Mac)

Ondrej Pokorny

2017-03-18 09:36

developer  

MacOSX-HighDPI-1.patch (2,549 bytes)   
Index: lcl/include/application.inc
===================================================================
--- lcl/include/application.inc	(revision 54435)
+++ lcl/include/application.inc	(working copy)
@@ -437,6 +437,14 @@
     raise Exception.Create(rsNoWidgetSet);
   end;
   WidgetSet.AppInit(ScreenInfo);
+  {$IFDEF DARWIN}
+  // Mac OSX (all WS) return 72 PPI for 100% scaling.
+  // The LCL needs 96 PPI for correct
+  //   a) font size scaling (Height/Size ratio)
+  //   b) High-DPI scaling (Application.Scaled)
+  ScreenInfo.PixelsPerInchX := MulDiv(ScreenInfo.PixelsPerInchX, 96, 72);
+  ScreenInfo.PixelsPerInchY := MulDiv(ScreenInfo.PixelsPerInchY, 96, 72);
+  {$ENDIF}
   ScreenInfo.Initialized := True;
   Screen.UpdateScreen;
   // set that we are initialized => all exceptions will be handled by our HandleException
Index: lcl/interfaces/carbon/carbonwinapi.inc
===================================================================
--- lcl/interfaces/carbon/carbonwinapi.inc	(revision 54435)
+++ lcl/interfaces/carbon/carbonwinapi.inc	(working copy)
@@ -1455,8 +1455,8 @@
   case Index of
   LOGPIXELSX,
   LOGPIXELSY:
-    // logical is always 96 dpi (to correspond with 100% scaling on Windows), although physical can differ
-    Result := 96;
+    // logical is allways 72 dpi, although physical can differ
+    Result := 72; // TODO: test scaling and magnification
   BITSPIXEL:  Result := CGDisplayBitsPerPixel(CGMainDisplayID);
   else
     DebugLn('TCarbonWidgetSet.GetDeviceCaps TODO Index: ' + DbgS(Index));
Index: lcl/interfaces/cocoa/cocoawinapi.inc
===================================================================
--- lcl/interfaces/cocoa/cocoawinapi.inc	(revision 54435)
+++ lcl/interfaces/cocoa/cocoawinapi.inc	(working copy)
@@ -1875,9 +1875,9 @@
   // todo: change implementation for printers
   case Index of
     HORZSIZE:
-      Result := Round(NSScreen.mainScreen.frame.size.width / 96 * 25.4);
+      Result := Round(NSScreen.mainScreen.frame.size.width / 72 * 25.4);
     VERTSIZE:
-      Result := Round(NSScreen.mainScreen.frame.size.height / 96 * 25.4);
+      Result := Round(NSScreen.mainScreen.frame.size.height / 72 * 25.4);
     HORZRES:
       Result := Round(NSScreen.mainScreen.frame.size.width);
     BITSPIXEL:
@@ -1887,9 +1887,9 @@
     SIZEPALETTE:
       Result := 0;
     LOGPIXELSX:
-      Result := 96;
+      Result := 72;
     LOGPIXELSY:
-      Result := 96;
+      Result := 72;
     VERTRES:
       Result := Round(NSScreen.mainScreen.frame.size.height);
     NUMRESERVED:
MacOSX-HighDPI-1.patch (2,549 bytes)   

Ondrej Pokorny

2017-03-18 09:40

developer   ~0098999

From the discussion on the mailing list: the new font height is correct in regard to other OS. It means that when upgrading a LCL 1.6 project to LCL 1.8 that uses Font.Size, the font height will increase.

---

I'd like to fix the issue in a different way - see MacOSX-HighDPI-1.patch. I'd like to keep the low-level WS functions intact and fix it on LCL level for all WS in one place. Any objections / suggestions?

Ondrej Pokorny

2017-03-21 20:30

developer   ~0099115

No objections raised: I applied the patch and moved the 72->96 PPI fix to LCL level. Please retest all.

To-Do: document in LCL changes.

CudaText man

2017-03-22 10:37

reporter   ~0099128

Objection:
Juha tells always to add Mac things to WS level, and you have
+ {$IFDEF DARWIN}
+ // Mac OSX (all WS) return 72 PPI for 100% scaling.
+ // The LCL needs 96 PPI for correct
+ // a) font size scaling (Height/Size ratio)
+ // b) High-DPI scaling (Application.Scaled)
+ ScreenInfo.PixelsPerInchX := MulDiv(ScreenInfo.PixelsPerInchX, 96, 72);
+ ScreenInfo.PixelsPerInchY := MulDiv(ScreenInfo.PixelsPerInchY, 96, 72);
+ {$ENDIF}

in appliction.inc now..

Ondrej Pokorny

2017-03-22 10:46

developer   ~0099129

AlexeyT: there is a difference between OS Mac ({$IFDEF DARWIN}) and WS Carbon/Cocoa ({$IFDEF LCLCarbon} / {$IFDEF LCLCocoa}).

The former can go everywhere, whereas the latter only to WS-level and up.

CudaText man

2017-03-22 10:55

reporter   ~0099130

>I'd like to fix the issue in a different way - see MacOSX-HighDPI-1.patch. I'd like to keep the low-level WS functions intact and fix it on LCL level for all WS in one place

Why to do it in new way? prev way, where 96 sitted in WS level, was nice code. new way, with 72, is not nice code, and you have IFDEF Darwin, not nice too..

Ondrej Pokorny

2017-03-22 11:04

developer   ~0099131

With previous code, I would have {$IFDEF Darwin} in Qt and possible also all other widgetsets that are supported on the Mac. So the fix would spread all over different WS at several places. Now the change is at exactly 1 place.

Furthermore, the WS code returns the DPI for different canvas handles - which can be different! (It isn't supported yet in LCL Carbon/Cocoa but in Qt and Win32 it is.)

CudaText man

2017-03-22 11:35

reporter   ~0099133

So it is to fix Qt dpi<96, which is on Mac. (or not only Mac.)
But better is to fix Qt WS, it must _not_ report 72, and if it gets 72, WS should ignore it and use anyway 96. So all WSes have 96, and ignore <96
IMO

Ondrej Pokorny

2017-03-22 12:08

developer   ~0099134

> So it is to fix Qt dpi<96, which is on Mac. (or not only Mac.)

No, it is to cope with the fact that Mac uses 72 DPI for 100% scaling, whereas the LCL needs 96 DPI as 100%.

CudaText man

2017-03-22 12:20

reporter   ~0099135

But i've read Mac forums, Mac dont use 72: it dont have any fixed value:

Aha! Looking with the correct phrase, "os x dpi", I get lots of hits. As expected, the general answer is no, the Mac OS still does not have a global DPI setting. It's all application independent. Each app does whatever it does and is unlikely to be the same as another app. So trying to find a magic point size that will always be xxx pixels tall would be a lesson in frustration.
https://discussions.apple.com/thread/5153667?start=0&tstart=0

Ondrej Pokorny

2017-03-22 12:34

developer   ~0099136

> But i've read Mac forums, Mac dont use 72: it dont have any fixed value

That's why we have the right to change 72 DPI to 96. We decided to use 96 DPI on Mac as well. I don't see your point.

CudaText man

2017-03-22 12:52

reporter   ~0099138

Point is:
revert this diff, because prev code (WS have 96) works ok, font size is ok,
only apps need to update settings in new LCL.

Ondrej Pokorny

2017-03-22 12:56

developer   ~0099139

What changed with r54453?

CudaText man

2017-03-22 20:08

reporter   ~0099150

I see change, which added IFDEF Darwin to application.inc

Ondrej Pokorny

2017-03-22 20:18

developer   ~0099151

Last edited: 2017-03-22 20:19

View 2 revisions

Alexey, in 0031037:0099138 you write:

Point is:
revert this diff, because prev code (WS have 96) works ok, font size is ok,
only apps need to update settings in new LCL.

--> Has something changed about the statement above?

And now in 0031037:0099150 you write again about IFDEF DARWIN...

So what is your problem? The presence of "IFDEF DARWIN" or functional regression?

CudaText man

2017-03-22 20:23

reporter   ~0099152

Last edited: 2017-03-22 20:24

View 2 revisions

I write "revert this diff", meaning that "diff with IFDEF Darwin, in application.inc, with value 72" is not okay. I want code before this diff. Which had values 96 in WS levels.
It was ok, only with Qt on Mac it was not ok?
(I wrote how to solve Qt on Mac dpi)

>Has something changed about the statement above?
not changed.

Ondrej Pokorny

2017-03-22 20:29

developer   ~0099153

> I want code before this diff

You brought only one argument "I don't like IFDEF DARWIN in LCL level", which is very poor.

> I wrote how to solve Qt on Mac dpi

Your suggestion cannot be applied. What if somebody changes his DPI settings to 72 PPI on Linux in order to get bigger text?

> > Has something changed about the statement above?
> not changed.

Then there's no problem.

Felipe Monteiro de Carvalho

2017-03-23 15:45

reporter   ~0099169

> You brought only one argument "I don't like IFDEF DARWIN in LCL level", which is very poor.

Its not a poor argument, its a design guideline for the LCL. I think it is better to keep it like that, as it keeps the LCL code cleaner.

Ondrej Pokorny

2017-03-23 16:12

developer   ~0099171

No no no :) The design guide for the LCL is that WS-dependent code goes to the WS-LCL part. But this is not a WS-specific problem, but OS-specific. You find IFDEF DARWIN and IFDEF MACOS even in FPC sources - and they have no idea about LCL WidgetSets.

If I revert r54453 and do what you suggest:
1) I have to use the same check with IFDEF DARWIN in all WS that support OSX: Qt, Qt5, Gtk*. + Carbon and Cocoa without the IFDEF, obviously.
2) I also have to check the HDC in the GetDeviceCaps if it belongs to screen or monitor and do the check only for them. Because different canvas objects can have different DPI assigned (e.g. printers).
3) The programmer doesn't have the chance to retrieve the real DPI from the system because he cannot pass the check. Now he can if he directly calls GetDeviceCaps.

So no, the approach you both suggest is not cleaner at all, IMO.

Ondrej Pokorny

2017-03-27 20:07

developer   ~0099250

I hope I could convince you all :)

Documented: http://wiki.freepascal.org/Lazarus_1.8.0_release_notes#LCL_incompatibilities

Issue History

Date Modified Username Field Change
2016-11-29 21:04 CudaText man New Issue
2016-11-30 16:47 Juha Manninen Note Added: 0096425
2016-12-10 12:57 Ondrej Pokorny LazTarget => -
2016-12-10 12:57 Ondrej Pokorny Status new => acknowledged
2016-12-10 12:58 Ondrej Pokorny LazTarget - => 1.8
2016-12-10 12:58 Ondrej Pokorny Target Version => 1.8
2016-12-10 12:59 Ondrej Pokorny Note Added: 0096638
2016-12-10 13:00 Ondrej Pokorny Severity minor => major
2016-12-10 14:45 Zeljan Rikalo Note Added: 0096644
2016-12-18 11:11 Ondrej Pokorny Assigned To => Ondrej Pokorny
2016-12-18 11:11 Ondrej Pokorny Status acknowledged => assigned
2016-12-18 11:12 Ondrej Pokorny Fixed in Revision => 53712, 53714
2016-12-18 11:12 Ondrej Pokorny Note Added: 0096882
2016-12-18 11:12 Ondrej Pokorny Status assigned => resolved
2016-12-18 11:12 Ondrej Pokorny Resolution open => fixed
2016-12-18 21:29 CudaText man Note Added: 0096917
2016-12-18 21:29 CudaText man Status resolved => assigned
2016-12-18 21:29 CudaText man Resolution fixed => reopened
2016-12-18 21:37 Ondrej Pokorny Note Added: 0096918
2016-12-18 23:28 CudaText man Note Added: 0096921
2017-03-18 09:36 Ondrej Pokorny File Added: MacOSX-HighDPI-1.patch
2017-03-18 09:40 Ondrej Pokorny Note Added: 0098999
2017-03-21 20:30 Ondrej Pokorny Note Added: 0099115
2017-03-21 20:31 Ondrej Pokorny Fixed in Revision 53712, 53714 => 53712, 53714, 54453
2017-03-22 10:37 CudaText man Note Added: 0099128
2017-03-22 10:46 Ondrej Pokorny Note Added: 0099129
2017-03-22 10:47 Ondrej Pokorny Fixed in Revision 53712, 53714, 54453 => 53712, 53714, 54453, 54464
2017-03-22 10:55 CudaText man Note Added: 0099130
2017-03-22 11:04 Ondrej Pokorny Note Added: 0099131
2017-03-22 11:35 CudaText man Note Added: 0099133
2017-03-22 12:08 Ondrej Pokorny Note Added: 0099134
2017-03-22 12:20 CudaText man Note Added: 0099135
2017-03-22 12:34 Ondrej Pokorny Note Added: 0099136
2017-03-22 12:52 CudaText man Note Added: 0099138
2017-03-22 12:56 Ondrej Pokorny Note Added: 0099139
2017-03-22 20:08 CudaText man Note Added: 0099150
2017-03-22 20:18 Ondrej Pokorny Note Added: 0099151
2017-03-22 20:19 Ondrej Pokorny Note Edited: 0099151 View Revisions
2017-03-22 20:23 CudaText man Note Added: 0099152
2017-03-22 20:24 CudaText man Note Edited: 0099152 View Revisions
2017-03-22 20:29 Ondrej Pokorny Note Added: 0099153
2017-03-23 15:45 Felipe Monteiro de Carvalho Note Added: 0099169
2017-03-23 16:12 Ondrej Pokorny Note Added: 0099171
2017-03-26 20:13 Ondrej Pokorny Relationship added related to 0031598
2017-03-26 20:13 Ondrej Pokorny Relationship added related to 0031601
2017-03-26 20:34 Ondrej Pokorny Fixed in Revision 53712, 53714, 54453, 54464 => 53712, 53714, 54453, 54464, 54486
2017-03-27 20:07 Ondrej Pokorny Note Added: 0099250
2017-03-27 20:07 Ondrej Pokorny Status assigned => resolved
2017-03-27 20:07 Ondrej Pokorny Resolution reopened => fixed
2017-07-27 22:09 Ondrej Pokorny Relationship added parent of 0031912
2018-04-19 08:01 Ondrej Pokorny Relationship added related to 0033597
2018-11-30 19:18 Zeljan Rikalo Relationship added related to 0034461
2018-11-30 19:18 Zeljan Rikalo Relationship added related to 0034625