View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0031037 | Lazarus | Widgetset | public | 2016-11-29 21:04 | 2018-11-30 19:18 |
Reporter | CudaText man | Assigned To | Ondrej Pokorny | ||
Priority | normal | Severity | major | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Platform | MacOS | ||||
Target Version | 1.8 | ||||
Summary | 0031037: Mac: fixed dpi 72 should be changed to Windows-value 96 | ||||
Description | Carbon 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 Information | function 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 | ||||
Tags | No tags attached. | ||||
Fixed in Revision | 53712, 53714, 54453, 54464, 54486 | ||||
LazTarget | 1.8 | ||||
Widgetset | |||||
Attached Files |
|
parent of | 0031912 | new | Lazarus | Some controls are shown with smaller font size | |
related to | 0031598 | resolved | Ondrej Pokorny | Lazarus | IDE comp-palette has icons w/out gaps, so icons overlap |
related to | 0031601 | closed | Ondrej Pokorny | Lazarus | Font and Image scaling issues in IDE |
related to | 0033597 | resolved | Ondrej Pokorny | Lazarus | Lazarus with cocoa widgetset on 64 bit darwin: source editor font very small |
related to | 0034461 | closed | Zeljan Rikalo | Patches | TreeItem text is centered instead of left justified |
related to | 0034625 | closed | Zeljan Rikalo | Lazarus | Qt/Qt5: Wrong PPI calculation on Mac after r57679 |
Not all the children of this issue are yet resolved or closed. |
|
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. |
|
Yes, this should be fixed. Otherwise the high DPI features will deliver nonsense on Mac/Carbon. It's important for the IDE as well. |
|
IMO, it's safe to add 96 as default for modern macs, especially for cocoa ws. |
|
I changed all 72 values to 96 in both Carbon and Cocoa. Please test and report issues, I didn't test it myself. |
|
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) |
|
Did you suggest changing 72 to 96 without testing? What now, revert? |
|
I did't sugget without testing. No pls don't revert. (not sure, maybe need to change default Font.Height on Mac) |
|
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: |
|
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? |
|
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. |
|
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.. |
|
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. |
|
>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.. |
|
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.) |
|
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 |
|
> 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%. |
|
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 |
|
> 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. |
|
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. |
|
What changed with r54453? |
|
I see change, which added IFDEF Darwin to application.inc |
|
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? |
|
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. |
|
> 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. |
|
> 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. |
|
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. |
|
I hope I could convince you all :) Documented: http://wiki.freepascal.org/Lazarus_1.8.0_release_notes#LCL_incompatibilities |
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 |