-
Notifications
You must be signed in to change notification settings - Fork 188
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
[macOS] some UI elements (e.g. list, tree views) seem to have more space between lines now #1674
[macOS] some UI elements (e.g. list, tree views) seem to have more space between lines now #1674
Comments
Have you tried any of the SWT snippets to see if the problem is like that there too? |
I think eclipse-platform/eclipse.platform.swt#771 is the case for this. And I had the impression that is was by intention. |
The underlying cause is the macOS SDK version that the Eclipse launcher is built with. In older versions of Eclipse the launcher was built using SDK 10. Later versions are built using SDK 11 and 13. This dictates how the UI elements are displayed. Taller tree and table items are part of SDK 11 and greater. eclipse-platform/eclipse.platform.swt#771 is a fix to keep things consistent. |
I tested this using I20240219-0600, which has |
Not sure what you mean? To be clear, since Eclipse 4.30 the launcher has been built using SDK 13 so taller row heights are the norm. eclipse-platform/eclipse.platform.swt#771 was merged on Jan 17 2024. All that patch does is to ensure that row heights stay as they should when a font is set. |
I got the impression that the mentioned PR fixes the issue that I illustrated in the screenshots in #1674 (comment), that is why I thought I should re-test the latest I-build and report back here... ;-) The screenshot above from using Eclipse 2023-12 (4.30) looks like the old behavior though (smaller row height), so it looks like something has changed for 4.31 builds that causes this change. |
Since Eclipse 4.30 all row heights on Mac are taller by default. Before eclipse-platform/eclipse.platform.swt#771 changing the font of a Table/Tree/List incorrectly added padding of 1 pixel to the row height (wrong). It now adds 8 pixels to match the increased row height. You are seeing smaller row heights in Eclipse 2023-12 because the patch is not present in that version. The correct row height is the larger one, because of the changes in the SDK. |
I'll try to make this clearer... Please take a look at eclipse-platform/eclipse.platform.swt#677
TL;DR - taller row heights is the new normal. |
If you want smaller row height you could use |
The above screenshot when running on Eclipse 2024-03 M2 is with |
If you remove |
I'll try to explain the situation again (with reference to eclipse-platform/eclipse.platform.swt#677)
So the new row heights are actually correct for the new macOS SDK. If
|
Not without creating inconsistent behaviour. If the row height padding in To see this, add |
The only thing I can think of to maintain backward compatibility would be to use a vertical padding of 1 pixel if |
- If small fonts are set with -Dorg.eclipse.swt.internal.carbon.smallFonts then use smaller vertical padding - This maintains backward compatibility - See discussion at eclipse-platform/eclipse.platform.ui#1674
Please comment and test eclipse-platform/eclipse.platform.swt#1055 The caveat with this is that |
Yeah, I had the same thought, that would be awesome. |
@martinlippert Can you test eclipse-platform/eclipse.platform.swt#1055? |
Yeah, happy to give this a try. Any recommendations about the easiest way to build this locally? |
You need to clone https://github.com/eclipse-platform/eclipse.platform.swt and import the |
Using https://github.com/eclipse-platform/.github/blob/main/CONTRIBUTING.md#creating-an-eclipse-development-environment fully automates the setup process. It's a bit of overkill because it includes everything in the SDK, but it lets you launch a runtime workspace (launcher provide) where you can see the impact in context. You can also go back: Deselect everything: And select only SWT: |
Thinking about this more, this workaround is not really true to the intent of the With the workaround patch:
So with this patch we're effectively changing the meaning of the |
I'm not on a Mac but if this issue were Windows I would be super annoyed to suddenly have a lower density of information content in all my views. Are other places (other native mac applications) also showing such less dense tables, tree, and lists? |
But this was actually the case for all native Mac apps when macOS Big Sur (11) was released. Since then native row heights on Mac are increased (if they target the later Mac SDK), as noted here. I've spent a lot of time thinking about, and experimenting with, this issue and have come to accept this as the new normal. In our RCP app, Archi, we don't set the |
Yes. Even the SWT-based app "SmartGit". Before: After: To be clear, if you don't use the |
I can only second that. The operating system dictates how the high a line in a tree is. We should not try to circumvent this. |
Newer versions of macOS introduced a new style of table rows with a padding that is larger than previously seen. This broke item height computations (eclipse-platform#677). An attempt to fix this had the unexpected side effect that now some UI elements in Eclipse IDE (and other apps with "-Dorg.eclipse.swt.internal.carbon.smallFonts" enabled), eclise.platform.ui #1674, and it also broke compatibility with older versions of macOS that did not have the additional padding. This change reverts the previous fix, and adds logic to enforce the native item height by default, without making assumption about the exact height (which varies with "smallFonts", for example). The setting can be controlled by a new System property, org.eclipse.swt.internal.cocoa.useNativeItemHeight, that, if set to false, overrules the default. This allows apps to bring brack the old macOS behavior. Fixes eclipse-platform/eclipse.platform.ui#1674 Fixes eclipse-platform#677
Newer versions of macOS introduced a new style of table rows with a padding that is larger than previously seen. This broke item height computations (eclipse-platform#677). An attempt to fix this had the unexpected side effect that now some UI elements in Eclipse IDE (and other apps with "-Dorg.eclipse.swt.internal.carbon.smallFonts" enabled), eclise.platform.ui #1674, and it also broke compatibility with older versions of macOS that did not have the additional padding. This change reverts the previous fix, and adds logic to enforce the native item height by default, without making assumption about the exact height (which varies with "smallFonts", for example). The setting can be controlled by a new System property, org.eclipse.swt.internal.cocoa.useNativeItemHeight, that, if set to false, overrules the default. This allows apps to bring brack the old macOS behavior. Fixes eclipse-platform/eclipse.platform.ui#1674 Fixes eclipse-platform#677
Newer versions of macOS introduced a new style of table rows with a padding that is larger than previously seen. This broke item height computations (eclipse-platform#677). An attempt to fix this had the unexpected side effect that now some UI elements in Eclipse IDE (and other apps with "-Dorg.eclipse.swt.internal.carbon.smallFonts" enabled), eclise.platform.ui #1674, and it also broke compatibility with older versions of macOS that did not have the additional padding. This change reverts the previous fix, and adds logic to enforce the native item height by default, without making assumption about the exact height (which varies with "smallFonts", for example). The setting can be controlled by a new System property, org.eclipse.swt.internal.cocoa.useNativeItemHeight, that, if set to false, overrules the default. This allows apps to bring brack the old macOS behavior. Fixes eclipse-platform/eclipse.platform.ui#1674 Fixes eclipse-platform#677
Just wanted to report back no issues with the hotfix initially! @Phillipus thank you for guiding the discussion onto the right track and @kohlschuetter for providing the code. Very much appreciated. |
Newer versions of macOS introduced a new style of table rows with a padding that is larger than previously seen. This broke item height computations (eclipse-platform#677). An attempt to fix this had the unexpected side effect that now some UI elements in Eclipse IDE (and other apps with "-Dorg.eclipse.swt.internal.carbon.smallFonts" enabled), eclise.platform.ui #1674, and it also broke compatibility with older versions of macOS that did not have the additional padding. This change reverts the previous fix, and adds logic to enforce the native item height by default, without making assumption about the exact height (which varies with "smallFonts", for example). The setting can be controlled by a new System property, org.eclipse.swt.internal.cocoa.useNativeItemHeight, that, if set to false, overrules the default. This allows apps to bring brack the old macOS behavior. Fixes eclipse-platform/eclipse.platform.ui#1674 Fixes eclipse-platform#677
Newer versions of macOS introduced a new style of table rows with a padding that is larger than previously seen. This broke item height computations (eclipse-platform#677). An attempt to fix this had the unexpected side effect that now some UI elements in Eclipse IDE (and other apps with "-Dorg.eclipse.swt.internal.carbon.smallFonts" enabled), eclise.platform.ui #1674, and it also broke compatibility with older versions of macOS that did not have the additional padding. This change reverts the previous fix, and adds logic to enforce the native item height by default, without making assumption about the exact height (which varies with "smallFonts", for example). The setting can be controlled by a new System property, org.eclipse.swt.internal.cocoa.useNativeItemHeight, that, if set to false, overrules the default. This allows apps to bring brack the old macOS behavior. Lastly, update two tests in Test_org_eclipse_swt_widgets_List that made assumptions about the item height being different for two small font sizes -- this is no longer true when the native item height is enforced. We now increase the font size of the second font such that this corner case no longer matters. Fixes eclipse-platform/eclipse.platform.ui#1674 Fixes eclipse-platform#677
Newer versions of macOS introduced a new style of table rows with a padding that is larger than previously seen. This broke item height computations (#677). An attempt to fix this had the unexpected side effect that now some UI elements in Eclipse IDE (and other apps with "-Dorg.eclipse.swt.internal.carbon.smallFonts" enabled), eclise.platform.ui #1674, and it also broke compatibility with older versions of macOS that did not have the additional padding. This change reverts the previous fix, and adds logic to enforce the native item height by default, without making assumption about the exact height (which varies with "smallFonts", for example). The setting can be controlled by a new System property, org.eclipse.swt.internal.cocoa.useNativeItemHeight, that, if set to false, overrules the default. This allows apps to bring brack the old macOS behavior. Lastly, update two tests in Test_org_eclipse_swt_widgets_List that made assumptions about the item height being different for two small font sizes -- this is no longer true when the native item height is enforced. We now increase the font size of the second font such that this corner case no longer matters. Fixes eclipse-platform/eclipse.platform.ui#1674 Fixes #677
So, 76 comments later, what is the conclusion for the just released 2024-03 ? |
I think the fix will be part of the |
Is there a way to have a fix for the x86_64 Mac processor ? |
I just wanted to note that putting arbitrary "hotfix jars" downloaded from the internet into your eclipse folder can impose server security risk to your eclipse installation, therefore I would recommend to either build it locally yourself or use the official I-builds: https://download.eclipse.org/eclipse/downloads/ The integration builds will also contain SWT jars you can replace then in your installation. |
Like to add: "Haste makes waste" |
I personally would not recommend anyone to upgrade their stable Eclipse release to an integration build with probably hundreds of unrelated changes. @merks @lshanmug Can we please get an official release for the fix, without having to update to an unstable integration build? Furthermore, I get the whole xz/liblzma story may cause some folks to go extra paranoid but I want to set things straight here: I by myself built and provided the jar built from the code I wrote myself in the PR. The PR and jar is hosted on github.com. I use my real name in these communications, and with that I hopefully still have a reputation to lose. I feel that you calling this work an "arbitrary hotfix jar downloaded from the internet" that "can impose server security risk to your eclipse installation", is an insult to outside contributors like me, and a disservice to all open source contributors that act in good faith. I have attached the jar to this patch solely to help people in the moment, and I think you know that. On that note: I may also remind you that before this fix I have also fixed eclipse-m2e/m2e-core#1511 which was broken by a change that you introduced yourself almost a year before, and it was probably not fixed earlier because you (erroneously) claimed that another upstream bug was to blame. Since I see a pattern here, maybe you should lower your accusing tone a bit? I have spent numerous hours chasing both bugs and patiently followed up getting these fixes in so others can also benefit from my findings. Please, without diverting into the general case, have at least the decency to respect my contribution in good faith, or show us objective proof that my hotfix jar is a security risk. As to this date, updating Eclipse from within its own app occasionally shows me unsigned plugins that either have to trust blindly or not install at all, because the existing landscape of Eclipse plugins, integration build update sites and the Eclipse marketplace are a shit show by themselves. I mean, even for the signed plugins, I can't really tell the quality of a release, especially when obviously visible bugs like this present one don't qualify for a rather immediate patch release. This is something you should look into before starting the scaremongering. |
The next official release is in June. So unfortunately folks need to wait until that release or consume integration/milestone builds to get it sooner. The release engineering effort to produce a release of the platform is already daunting. Backporting the fix to the 4.31 maintenance branch is feasible but you will notice that there are no published builds for that stream Providing a fix for just this issue for the IDE packages on the download server just doesn’t seem feasible logistically. You certainly have my gratitude and respect for helping to resolve this bug. It’s effort beyond the call of duty. If even 1 in 10,000 were this helpful, we’d be flooded in riches. ❤️ |
Thanks a lot @merks, I appreciate your kind words. |
No one needs to upgrade their whole "stable Eclipse Release", Eclipse installations can perfectly run side-by-side. Also this integration builds will become the next release so if people would test these beforehand one probably would identify the one of the hundreds maybe surprising or even harmful changes before the release what would prevent such "after the fact" discovery that is hard to fix as @merks has mentioned. Beside that I never claimed your patch / jar is harmful, nor did I insulted / addressed you in anyway. This simply bypasses even the most basic checks we have in Eclipse/P2 and has nothing to do with being paranoid or scaremongering. Also this does not mean your contribution is bad in anyway or not appreciated. |
I fully agree to this, but I also feel the need to mention that I discovered and reported this specific issue before the release (using M2). So this was not a discovery after the fact ;-) |
Yep and it was worked on so everything is fine for me, it was also more a general comment and directed to the "probably hundreds of unrelated changes", the more people test it the earlier they we detect problem, the more likely one gets a fix / better quality in release. So from my experience ibuilds are very stable or if there is a problem are fixed sometimes even withing hours, so if I'm affected by a bug introduced in a release I can either stay at the old or use the just fixed ibuild and see if something else will affect my workflow. |
Hey @kohlschuetter, can you explain to me how you've exactly built this hotfix? I would like to run that myself, so in case you have the command that you used at hand or can share the script (or whatever you used), that would be awesome. Many many many thanks upfront! |
@martinlippert If I remember correctly, it's just a matter of running Please work with code obtained via |
In case you are concerned that for example https://download.eclipse.org/eclipse/updates/4.32-I-builds/ is a changing repository, instead of using the composite one can just use one of the immutable children (but they vanish after the next release): |
Thanks for the tip! That could be another option. (It would be great though to know which build contains which commits — it's not obvious to me where to find that information) I think there's a more of a general problem with switching from a stable release to bleeding edge development. For example, I'm pretty sure my Eclipse workspace will become incompatible, so downgrading will suddenly become unnecessarily difficult. Then, there's potential unexpected fallout due to new bugs, etc. This is all nothing new, not to me, and not in general (I have contributed to and survived living on "next major OS" releases for several years, and dogfooding is necessary for any serious project). It's just that there are "regular" Eclipse developers that don't consider themselves developers of Eclipse but simply developers using Eclipse, and they should certainly be able to just stick to the regular stable channel. As @merks commented above, I now understand that shipping a patch for a botched release isn't something trivial at the moment for the Eclipse IDE project. I wonder if that situation merits some attention from project management. Any thoughts? (follow-up maybe on a separate issue?) |
For each I-build the commit build of each sub-project is tagged with
It's a somehow known issue but I don't think one that can be generally solved soon. But besides the man-power there are also technical difficulties for projects that are more or less a root of the decency tree like SWT. Due to the way for example features work a new version of SWT means that all features including it and the features including those features need new builds because versions are locked in features. |
FYI. On my todo list is to investigate how best to allow EPP packages, as on the main download page, to update to any new platform build, including I-builds. It’s annoying when a problem in a simrel milestone or a release cannot be updated until there is a new simrel milestone. And I think it’s an avoidable annoyance. |
Newer versions of macOS introduced a new style of table rows with a padding that is larger than previously seen. This broke item height computations (eclipse-platform#677). An attempt to fix this had the unexpected side effect that now some UI elements in Eclipse IDE (and other apps with "-Dorg.eclipse.swt.internal.carbon.smallFonts" enabled), eclise.platform.ui #1674, and it also broke compatibility with older versions of macOS that did not have the additional padding. This change reverts the previous fix, and adds logic to enforce the native item height by default, without making assumption about the exact height (which varies with "smallFonts", for example). The setting can be controlled by a new System property, org.eclipse.swt.internal.cocoa.useNativeItemHeight, that, if set to false, overrules the default. This allows apps to bring brack the old macOS behavior. Lastly, update two tests in Test_org_eclipse_swt_widgets_List that made assumptions about the item height being different for two small font sizes -- this is no longer true when the native item height is enforced. We now increase the font size of the second font such that this corner case no longer matters. Fixes eclipse-platform/eclipse.platform.ui#1674 Fixes eclipse-platform#677
Newer versions of macOS introduced a new style of table rows with a padding that is larger than previously seen. This broke item height computations (#677). An attempt to fix this had the unexpected side effect that now some UI elements in Eclipse IDE (and other apps with "-Dorg.eclipse.swt.internal.carbon.smallFonts" enabled), eclise.platform.ui #1674, and it also broke compatibility with older versions of macOS that did not have the additional padding. This change reverts the previous fix, and adds logic to enforce the native item height by default, without making assumption about the exact height (which varies with "smallFonts", for example). The setting can be controlled by a new System property, org.eclipse.swt.internal.cocoa.useNativeItemHeight, that, if set to false, overrules the default. This allows apps to bring brack the old macOS behavior. Lastly, update two tests in Test_org_eclipse_swt_widgets_List that made assumptions about the item height being different for two small font sizes -- this is no longer true when the native item height is enforced. We now increase the font size of the second font such that this corner case no longer matters. Fixes eclipse-platform/eclipse.platform.ui#1674 Fixes #677
Some UI elements seem to have a lot more space between lines on macOS since Eclipse 2024-03 builds, for example the package explorer, dialogs showing lists (switch to different perspective), etc. It looks like a very general issue (maybe even in SWT, not sure). Not sure if this was an intended change, but I would prefer the old behavior to save screen real estate.
Eclipse 2024-03-M2 on macOS Sonoma 14.3.1
Plain vanilla EPP package download, empty workspace.
The text was updated successfully, but these errors were encountered: