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

[macOS Silicon] Table and Tree item height gets smaller when setting the font #677

Closed
Phillipus opened this issue May 18, 2023 · 22 comments · Fixed by #771 or #1117
Closed

[macOS Silicon] Table and Tree item height gets smaller when setting the font #677

Phillipus opened this issue May 18, 2023 · 22 comments · Fixed by #771 or #1117

Comments

@Phillipus
Copy link
Contributor

Phillipus commented May 18, 2023

This occurs on Mac Silicon (aarch64) only, presumably because Eclipse on that platform uses a later Apple SDK. I've tested on Mac Mini M1 with Monterey and Ventura.

Edit - also occurs on Mac Intel. It depends on which macOS SDK is used to create the launcher binary (eclipse or java).

Setting a table or tree font to the same font reduces the item height.

  1. Create a simple table with some rows
  2. The item height of the rows is 25:
Before
  1. Set the table's font to its current font (table.setFont(table.getFont()))
  2. The item height of the rows is now 18:
After

Expected behaviour: should use the height of the target SDK. On aarch64 this is the 25 pixel item height. On Intel it is 18.

@Phillipus
Copy link
Contributor Author

Phillipus commented May 18, 2023

Snippet to reproduce (ensure not to use the argument -Dorg.eclipse.swt.internal.carbon.smallFonts):

import static org.eclipse.swt.events.SelectionListener.widgetSelectedAdapter;

import org.eclipse.jface.layout.GridDataFactory;
import org.eclipse.swt.SWT;
import org.eclipse.swt.layout.GridData;
import org.eclipse.swt.layout.GridLayout;
import org.eclipse.swt.widgets.Button;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Label;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.Table;
import org.eclipse.swt.widgets.TableItem;

public class TableFont {

    public static void main(String[] args) {
        Display display = new Display();
        Shell shell = new Shell(display);
        shell.setText("Table Font");
        shell.setLayout(new GridLayout());
        
        Table table = new Table(shell, SWT.BORDER | SWT.V_SCROLL | SWT.H_SCROLL);
        GridDataFactory.create(GridData.FILL_HORIZONTAL).applyTo(table);
        
        for(int i = 0; i < 11; i++) {
            TableItem item = new TableItem(table, 0);
            item.setText("Item " + i);
        }
        
        Label label1 = new Label(shell, SWT.NONE);
        Label label2 = new Label(shell, SWT.NONE);
        updateLabels(table, label1, label2);
        
        Button button = new Button(shell, SWT.PUSH);
        button.setText("Set font");
        button.addSelectionListener(widgetSelectedAdapter(e -> {
            // Set table font to the existing table font
            table.setFont(table.getFont());
            updateLabels(table, label1, label2);
        }));
        
        shell.setSize(500, 450);
        shell.open();
        
        while(!shell.isDisposed()) {
            if(!display.readAndDispatch()) display.sleep();
        }
        
        display.dispose();
    }

    private static void updateLabels(Table table, Label label1, Label label2) {
        label1.setText("Table font: " + table.getFont().getFontData()[0]);
        label2.setText("Table item height: " + table.getItemHeight());
    }
}

@elsazac
Copy link
Member

elsazac commented Jun 15, 2023

I am able to reproduce the issue in Apple M1 Pro Ventura 13.4

Eclipse SDK
Version: 2023-06 (4.28)
Build id: I20230605-0440
OS: Mac OS X, v.13.4, aarch64 / cocoa
Java vendor: Eclipse Adoptium
Java runtime version: 17.0.6+10
Java version: 17.0.6

@elsazac
Copy link
Member

elsazac commented Jun 30, 2023

@Phillipus
Can you also add what should be the expected behaviour?

@Phillipus
Copy link
Contributor Author

Can you also add what should be the expected behaviour?

Added to OP.

@elsazac
Copy link
Member

elsazac commented Jul 14, 2023

I am investigating on this issue and these are my findings.

if (set || widget.rowHeight () < height) {
widget.setRowHeight (height);

If I change the above condition either to one of the below , we see the expected behaviour but not sure about the exact purpose of set. Does someone has any idea on when exactly set needs to be evaluated?

if (widget.rowHeight () < height) {
		widget.setRowHeight (height);
if (set && widget.rowHeight () < height) {
		widget.setRowHeight (height);

elsazac added a commit to elsazac/eclipse.platform.swt that referenced this issue Aug 18, 2023
function named as getInset() is created in Table.java.This function
calculates the inset value of a cell in a table for aarch64
architecture. The inset value of a row in a table is calculated by
subtracting the height of a cell from a frame.This calculated inset
value is added to the height in setItemHeight() function.The inset value
of 1 is retained for x86_64 architecture.
elsazac added a commit to elsazac/eclipse.platform.swt that referenced this issue Aug 18, 2023
function named as getInset() is created in Table.java.This function
calculates the inset value of a cell in a table for aarch64
architecture. The inset value of a row in a table is calculated by
subtracting the height of a cell from a frame.This calculated inset
value is added to the height in setItemHeight() function.The inset value
of 1 is retained for x86_64 architecture.
elsazac added a commit to elsazac/eclipse.platform.swt that referenced this issue Aug 31, 2023
X_86_64 the hardcoded inset value was 1.It is retained as such.Since
aarch64 was introduced at a very later time the new table format was
introduced with extra paddings.So the appropriate value to get the same
font would be 8.
@Phillipus
Copy link
Contributor Author

The problem also occurs for the SWT List widget:

import static org.eclipse.swt.events.SelectionListener.widgetSelectedAdapter;

import org.eclipse.jface.layout.GridDataFactory;
import org.eclipse.swt.SWT;
import org.eclipse.swt.layout.GridData;
import org.eclipse.swt.layout.GridLayout;
import org.eclipse.swt.widgets.Button;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Label;
import org.eclipse.swt.widgets.List;
import org.eclipse.swt.widgets.Shell;

public class ListRowHeight {

    public static void main(String[] args) {
        Display display = new Display();
        Shell shell = new Shell(display);
        shell.setText("List Row Height");
        shell.setLayout(new GridLayout());
        
        List list = new List(shell, SWT.BORDER | SWT.V_SCROLL | SWT.H_SCROLL);
        GridDataFactory.create(GridData.FILL_HORIZONTAL).applyTo(list);
        
        String[] items = new String[10];
        for(int i = 0; i < 10; i++) {
            items[i] = "Item " + i;
        }
        list.setItems(items);
        
        Label label1 = new Label(shell, SWT.NONE);
        Label label2 = new Label(shell, SWT.NONE);
        updateLabels(list, label1, label2);
        
        Button button = new Button(shell, SWT.PUSH);
        button.setText("Set font");
        button.addSelectionListener(widgetSelectedAdapter(e -> {
            // Set list font to the existing table font
            list.setFont(list.getFont());
            updateLabels(list, label1, label2);
        }));
        
        shell.setSize(500, 450);
        shell.open();
        
        while(!shell.isDisposed()) {
            if(!display.readAndDispatch()) display.sleep();
        }
        
        display.dispose();
    }

    private static void updateLabels(List list, Label label1, Label label2) {
        label1.setText("List font: " + list.getFont().getFontData()[0]);
        label2.setText("List item height: " + list.getItemHeight());
    }
}

elsazac added a commit to elsazac/eclipse.platform.swt that referenced this issue Oct 5, 2023
@Phillipus
Copy link
Contributor Author

Table and Tree row heights are initially set to 25 on Mac aarch64 and then set to 18 when resetting the font. With a List widget the row height is initially 24 and then 17 when resetting the font.

I have a doubt about whether the initial row height is wrong and should be as it is on intel Mac. But there are these earlier bug reports:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=575696

https://bugs.eclipse.org/bugs/show_bug.cgi?id=574932

and this comment from @sravanlakkimsetti:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=574932#c2

This is because eclipse launcher SDK version
On x86_64 SDK used is 10.14
On aarch64 SDK used is 11
But the way forward is to use latest SDK and update SWT to MacOS 11 SDK.

So I guess the higher row height values are correct for aarch64?

@elsazac
Copy link
Member

elsazac commented Dec 8, 2023

@Phillipus

I have verified some tables in Mac and I can see the table height in Mac being aligned with table in the snippet(with height 25)

Screenshot 2023-12-06 at 1 11 26 PM

I have gone through the bugzilla links and I see that the increased item height is the new native behaviour on BigSur. row height is 24pt in BigSur and 17pt in macOS 10.15.
https://bugs.eclipse.org/bugs/show_bug.cgi?id=574932#c40

the snippet is initially set to item height 25 because the default item height in aarch is 25 unless we override and change it to 18

Also can you please try some tables in x86_64?

elsazac added a commit to elsazac/eclipse.platform.swt that referenced this issue Jan 10, 2024
X_86_64 the hardcoded inset value was 1.It is retained as such.Since
aarch64 was introduced at a very later time the new table format was
introduced with extra paddings.So the appropriate value to get the same
font would be 8.
elsazac added a commit to elsazac/eclipse.platform.swt that referenced this issue Jan 10, 2024
sravanlakkimsetti pushed a commit to elsazac/eclipse.platform.swt that referenced this issue Jan 12, 2024
X_86_64 the hardcoded inset value was 1.It is retained as such.Since
aarch64 was introduced at a very later time the new table format was
introduced with extra paddings.So the appropriate value to get the same
font would be 8.
sravanlakkimsetti pushed a commit to elsazac/eclipse.platform.swt that referenced this issue Jan 12, 2024
elsazac added a commit to elsazac/eclipse.platform.swt that referenced this issue Jan 17, 2024
X_86_64 the hardcoded inset value was 1.It is retained as such.Since
aarch64 was introduced at a very later time the new table format was
introduced with extra paddings.So the appropriate value to get the same
font would be 8.
elsazac added a commit to elsazac/eclipse.platform.swt that referenced this issue Jan 17, 2024
X_86_64 the hardcoded inset value was 1.It is retained as such.Since
aarch64 was introduced at a very later time the new table format was
introduced with extra paddings.So the appropriate value to get the same
font would be 8.
elsazac added a commit to elsazac/eclipse.platform.swt that referenced this issue Jan 17, 2024
X_86_64 the hardcoded inset value was 1.It is retained as such.Since
aarch64 was introduced at a very later time the new table format was
introduced with extra paddings.So the appropriate value to get the same
font would be 8.
merks pushed a commit that referenced this issue Jan 17, 2024
X_86_64 the hardcoded inset value was 1.It is retained as such.Since
aarch64 was introduced at a very later time the new table format was
introduced with extra paddings.So the appropriate value to get the same
font would be 8.
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 15, 2024
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 15, 2024
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, but only when smallFonts is not enabled.

The setting can also be controlled by a new System property,
org.eclipse.swt.internal.cocoa.enforceNativeItemHeightMinimum, that, if
set, overrules any default (smallFonts or not). This allows apps to
bring brack the old macOS behavior even when the smallFonts setting is
undesired.

Lastly, we add support to selectively enable/disable the native item
height minimum for Table, Tree and List on a per-instance basis, so
applications can selectively control the behavior for specific use
cases.

Fixes eclipse-platform/eclipse.platform.ui#1674
Fixes eclipse-platform#677
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 15, 2024
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, assuming the previous fix is reverted adds logic to enforce
the native item height by default, but only when smallFonts is not
enabled.

The setting can also be controlled by a new System property,
org.eclipse.swt.internal.cocoa.enforceNativeItemHeightMinimum, that, if
set, overrules any default (smallFonts or not). This allows apps to
bring brack the old macOS behavior even when the smallFonts setting is
undesired.

Lastly, we add support to selectively enable/disable the native item
height minimum for Table, Tree and List on a per-instance basis, so
applications can selectively control the behavior for specific use
cases.

Fixes eclipse-platform/eclipse.platform.ui#1674
Fixes eclipse-platform#677
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 15, 2024
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, but only when smallFonts is not enabled.

The setting can also be controlled by a new System property,
org.eclipse.swt.internal.cocoa.enforceNativeItemHeightMinimum, that, if
set, overrules any default (smallFonts or not). This allows apps to
bring brack the old macOS behavior even when the smallFonts setting is
undesired.

Lastly, we add support to selectively enable/disable the native item
height minimum for Table, Tree and List on a per-instance basis, so
applications can selectively control the behavior for specific use
cases.

Fixes eclipse-platform/eclipse.platform.ui#1674
Fixes eclipse-platform#677
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 15, 2024
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, but only when smallFonts is not enabled.

The setting can also be controlled by a new System property,
org.eclipse.swt.internal.cocoa.enforceNativeItemHeightMinimum, that, if
set, overrules any default (smallFonts or not). This allows apps to
bring brack the old macOS behavior even when the smallFonts setting is
undesired.

Lastly, we add support to selectively enable/disable the native item
height minimum for Table, Tree and List on a per-instance basis, so
applications can selectively control the behavior for specific use
cases.

Fixes eclipse-platform/eclipse.platform.ui#1674
Fixes eclipse-platform#677
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 15, 2024
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, but only when smallFonts is not enabled.

The setting can also be controlled by a new System property,
org.eclipse.swt.internal.cocoa.enforceNativeItemHeightMinimum, that, if
set, overrules any default (smallFonts or not). This allows apps to
bring brack the old macOS behavior even when the smallFonts setting is
undesired.

Lastly, we add support to selectively enable/disable the native item
height minimum for Table, Tree and List on a per-instance basis, so
applications can selectively control the behavior for specific use
cases.

Fixes eclipse-platform/eclipse.platform.ui#1674
Fixes eclipse-platform#677
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 16, 2024
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, but only when smallFonts is not enabled.

The setting can also be controlled by a new System property,
org.eclipse.swt.internal.cocoa.enforceNativeItemHeightMinimum, that, if
set, overrules any default (smallFonts or not). This allows apps to
bring brack the old macOS behavior even when the smallFonts setting is
undesired.

Lastly, we add support to selectively enable/disable the native item
height minimum for Table, Tree and List on a per-instance basis, so
applications can selectively control the behavior for specific use
cases.

Fixes eclipse-platform/eclipse.platform.ui#1674
Fixes eclipse-platform#677
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 16, 2024
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, but only when smallFonts is not enabled.

The setting can also be controlled by a new System property,
org.eclipse.swt.internal.cocoa.enforceNativeItemHeightMinimum, that, if
set, overrules any default (smallFonts or not). This allows apps to
bring brack the old macOS behavior even when the smallFonts setting is
undesired.

Lastly, we add support to selectively enable/disable the native item
height minimum for Table, Tree and List on a per-instance basis, so
applications can selectively control the behavior for specific use
cases.

Fixes eclipse-platform/eclipse.platform.ui#1674
Fixes eclipse-platform#677
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 19, 2024
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, but only when smallFonts is not enabled.

The setting can also be controlled by a new System property,
org.eclipse.swt.internal.cocoa.useNativeItemHeight, that, if set,
overrules any default (smallFonts or not). This allows apps to bring
brack the old macOS behavior even when the smallFonts setting is
undesired.

Fixes eclipse-platform/eclipse.platform.ui#1674
Fixes eclipse-platform#677
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 19, 2024
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
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 20, 2024
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
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 20, 2024
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
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 20, 2024
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
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 20, 2024
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
kohlschuetter added a commit to kohlschuetter/eclipse.platform.swt that referenced this issue Mar 21, 2024
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
merks pushed a commit that referenced this issue Mar 21, 2024
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
@tmssngr
Copy link
Contributor

tmssngr commented Mar 21, 2024

What is the expected result of the last commit 611accb? For our applications users complain that the line height is much higher with the latest SWT build than with previous versions. However, I don't see any change with this commit.

@merks
Copy link
Contributor

merks commented Mar 21, 2024

I assume that you need to ensure that this returns false:

Boolean.parseBoolean(System.getProperty("org.eclipse.swt.internal.cocoa.useNativeItemHeight", "true"));

@kohlschuetter
Copy link
Contributor

@tmssngr For apps that use -Dorg.eclipse.swt.internal.carbon.smallFonts (like Eclipse IDE), the change in 611accb brings back the old behavior, as an immediate remedy to the problem without the need of further configuration.

For apps that don't specify smallFonts, the appearance depends on their macOS version. Newer versions of macOS have a taller native height that matters here. To consistently bring back the old behavior, specify -Dorg.eclipse.swt.internal.cocoa.useNativeItemHeight=false when launching your app.

@tmssngr
Copy link
Contributor

tmssngr commented Mar 21, 2024

I'm not sure I fully understand all. Older versions of SWT (e.g. used in SmartGit 23.1.2) show table and tree rows quite narrow on my M1 Mac Mini with macOS 14.4. Newer versions of SWT (e.g. used in SmartGit 24.1 preview) show a much larger row height. We don't have the mentioned properties set. Do you mean, that the larger row height in newer SWT builds is now the correct behavior for macOS 14.4, and if a user wants to have a narrower row-height, he needs to set the mentioned properties?

@kohlschuetter
Copy link
Contributor

@tmssngr Yes, I'm afraid that's the case, but setting a property will do the trick.

Anyhow, I'm just a Eclipse IDE user whose eyes couldn't bear looking at these oversized list items, so I'm glad I could contribute to resolving this issue.

@Phillipus
Copy link
Contributor Author

Phillipus commented Mar 21, 2024

Older versions of SWT (e.g. used in SmartGit 23.1.2) show table and tree rows quite narrow on my M1 Mac Mini with macOS 14.4.

@tmssngr Take a look at SmartGit 23.1.2 Settings tree. You'll see bigger row heights. See eclipse-platform/eclipse.platform.ui#1674 (comment)

There always has been bigger row heights in SWT since the later macOS versions. However, when a call to setItemHeight is made (by setting the control's font or image or something else), the row heights shrink, as the snippet here demonstrates. The commit of @kohlschuetter corrects this anomaly.

@tmssngr
Copy link
Contributor

tmssngr commented Mar 21, 2024

@Phillipus Ah, so there was even another difference between plain and owner-drawn trees.
On MacOS we were setting the display's system font (a slightly different one) to have all digits the same width. So it looks like our problem of the increased row-heights is a slightly different problem than this one covered in this ticket. Sorry for the noise.

@Phillipus
Copy link
Contributor Author

Phillipus commented Mar 21, 2024

@tmssngr

It depends on how a Tree/Table/List is used.

As the snippet in this issue shows, when a Tree/Table/List control is first created it has the native row height (larger on later macOS). Thereafter, a change to the control's font/image/whatever triggers a re-calculation of the row height. This is where the row height shrinks because it doesn't take into account the initial native row height when calculating the row height based upon the current font.

Eclipse uses the -Dorg.eclipse.swt.internal.carbon.smallFonts property and this triggers the recalculation of row heights immediately after the control is created. This is why Eclipse users always saw smaller row heights.

Now that the recalculation anomaly has been fixed there is consistency between the initial row heights and the re-calculated ones, so no more unexpected row height shrinkage. However, this means that row heights are bigger by default because that's how they are initially created.

I think SmartGit Preview is seeing bigger row heights because of the erroneous padding added in this commit.

You could try using the latest SWT build with the new fix in and add the -Dorg.eclipse.swt.internal.cocoa.useNativeItemHeight=false setting to SmartGit's Info.plist file (I think that's where you set these properties?).

@Phillipus
Copy link
Contributor Author

Phillipus commented Mar 21, 2024

@tmssngr My assumptions are correct.

  1. I downloaded SmartGit Preview 2 for Mac aarch64 and saw the increased row heights (except for the owner draw ones)
  2. I replaced SmartGit.app/Contents/Resources/Java/org.eclipse.swt.cocoa.macosx.aarch64.jar with the latest SWT build
  3. I added -Dorg.eclipse.swt.internal.cocoa.useNativeItemHeight=false to the Info.plist file in the JVMOptions section
  4. Bingo! smaller row heights (and consistent ones, too, so no more anomalous large rows in the Settings tree in SmartGit 23.1.2)

Before:

Screenshot 2024-03-21 at 15 53 37

After:

Screenshot 2024-03-21 at 15 54 13

tmssngr added a commit to syntdev/swt-jar-cache that referenced this issue Mar 21, 2024
@tmssngr
Copy link
Contributor

tmssngr commented Mar 21, 2024

Thanks, @Phillipus, this helped to fix it. :)

elsazac pushed a commit to elsazac/eclipse.platform.swt that referenced this issue Apr 9, 2024
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
elsazac pushed a commit that referenced this issue Apr 9, 2024
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
@odrotbohm
Copy link

I might be the odd one out here, but is there a way to get both small fonts, but the bigger spacing back after the fix? 😇 I see my ini file having -Dorg.eclipse.swt.internal.carbon.smallFonts set. I've tried adding -Dorg.eclipse.swt.internal.cocoa.useNativeItemHeight=true (I also tried false) to get the bigger spacing back, but don't see any effect.

@Phillipus
Copy link
Contributor Author

is there a way to get both small fonts, but the bigger spacing back after the fix?

No, because the native item height is smaller when small fonts are set.

@odrotbohm
Copy link

That's a pity, but thanks! 😕

@marcleidner
Copy link

Issue is fixed in 2024-06 (4.32.0), for me at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment