Skip to content

Commit

Permalink
cocoa: Enforce native item height for Table, Tree, List if necessary
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kohlschuetter committed Mar 15, 2024
1 parent 18a169d commit 0f1d659
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
*/
public abstract class Device implements Drawable {

public static final boolean SMALL_FONTS = System.getProperty("org.eclipse.swt.internal.carbon.smallFonts") != null;

/* Debugging */
public static boolean DEBUG;
boolean debug = DEBUG;
Expand Down Expand Up @@ -609,8 +611,7 @@ protected void init () {
tabs.release();

/* Initialize the system font slot */
boolean smallFonts = System.getProperty("org.eclipse.swt.internal.carbon.smallFonts") != null;
double systemFontSize = smallFonts ? NSFont.smallSystemFontSize() : NSFont.systemFontSize();
double systemFontSize = SMALL_FONTS ? NSFont.smallSystemFontSize() : NSFont.systemFontSize();
Point dpi = this.dpi = getDPI(), screenDPI = getScreenDPI();
NSFont font = NSFont.systemFontOfSize(systemFontSize * dpi.y / screenDPI.y);
font.retain();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ enum APPEARANCE {
NSFont textViewFont, tableViewFont, outlineViewFont, datePickerFont;
NSFont boxFont, tabViewFont, progressIndicatorFont;

boolean enforceNativeItemHeightMinimum;

Shell [] modalShells;
Dialog modalDialog;
NSPanel modalPanel;
Expand Down Expand Up @@ -2404,6 +2406,8 @@ protected void init () {
initFonts ();
setDeviceZoom ();

enforceNativeItemHeightMinimum = initDefaultEnforceNativeItemHeightMinimum();

/*
* Create an application delegate for app-level notifications. The AWT may have already set a delegate;
* if so, hold on to it so messages can be forwarded to it.
Expand Down Expand Up @@ -2502,6 +2506,26 @@ public void run() {
isPainting = isPainting.initWithCapacity(12);
}

/**
* Checks if the native item height should be enforced as a minimum.
*
* Newer version of macOS may use a default item height in Table, Tree and List
* controls that is larger than what is traditionally expected.
*
* Enforcing the default height as a minimum may break existing assumptions and
* render UI elements with a padding that may be considered too large.
*
* This is implicitly enabled unless {@code org.eclipse.swt.internal.carbon.smallFonts} is set.
*/
private boolean initDefaultEnforceNativeItemHeightMinimum() {
boolean enforce = !Device.SMALL_FONTS;
String v = System.getProperty("org.eclipse.swt.internal.cocoa.enforceNativeItemHeightMinimum");
if (v != null) {
enforce = "true".equals(v);
}
return enforce;
}

private static NSString getAwtRunLoopMode() {
// Special run loop mode mode used by AWT enters when it only wants related messages processed.
// The name of this mode is a defacto contract established by the JavaNativeFoundation (JNF) libary.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ public class List extends Scrollable {
int itemCount;
boolean ignoreSelect, didSelect, rowsChanged, mouseIsDown;

final int nativeItemHeight;
private boolean enforceNativeItemHeightMinimum;

static int NEXT_ID;

static final int CELL_GAP = 1;
Expand Down Expand Up @@ -81,6 +84,9 @@ public class List extends Scrollable {
*/
public List (Composite parent, int style) {
super (parent, checkStyle (style));

this.nativeItemHeight = (int)((NSTableView)view).rowHeight();
this.enforceNativeItemHeightMinimum = display.enforceNativeItemHeightMinimum;
}

@Override
Expand Down Expand Up @@ -1179,7 +1185,11 @@ void setFont (NSFont font) {
super.setFont (font);
double ascent = font.ascender ();
double descent = -font.descender () + font.leading ();
((NSTableView)view).setRowHeight ((int)Math.ceil (ascent + descent) + 1);
int height = (int)Math.ceil (ascent + descent) + 1;
if (enforceNativeItemHeightMinimum) {
height = Math.max (height, nativeItemHeight);
}
((NSTableView)view).setRowHeight (height);
setScrollWidth();
}

Expand Down Expand Up @@ -1544,4 +1554,13 @@ void updateRowCount() {
}
}

public boolean isEnforceNativeItemHeightMinimum() {
return enforceNativeItemHeightMinimum;
}

public void setEnforceNativeItemHeightMinimum(boolean enforce) {
this.enforceNativeItemHeightMinimum = enforce;
setFont(getFont()); // update height
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ public class Table extends Composite {
boolean shouldScroll = true;
boolean keyDown;

final int nativeItemHeight;
private boolean enforceNativeItemHeightMinimum;

static int NEXT_ID;

static final int FIRST_COLUMN_MINIMUM_WIDTH = 5;
Expand Down Expand Up @@ -129,6 +132,9 @@ public class Table extends Composite {
*/
public Table (Composite parent, int style) {
super (parent, checkStyle (style));

this.nativeItemHeight = (int)((NSTableView)view).rowHeight();
this.enforceNativeItemHeightMinimum = display.enforceNativeItemHeightMinimum;
}

@Override
Expand Down Expand Up @@ -2823,6 +2829,9 @@ void setItemHeight (Image image, NSFont font, boolean set) {
double ascent = font.ascender ();
double descent = -font.descender () + font.leading ();
int height = (int)Math.ceil (ascent + descent) + 1;
if (enforceNativeItemHeightMinimum) {
height = Math.max (height, nativeItemHeight);
}
Rectangle bounds = image != null ? image.getBounds () : imageBounds;
if (bounds != null) {
imageBounds = bounds;
Expand Down Expand Up @@ -3730,4 +3739,13 @@ void updateRowCount() {
setRedraw(true);
}

public boolean isEnforceNativeItemHeightMinimum() {
return enforceNativeItemHeightMinimum;
}

public void setEnforceNativeItemHeightMinimum(boolean enforce) {
this.enforceNativeItemHeightMinimum = enforce;
setItemHeight(null, null, true); // update height
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ public class Tree extends Composite {
/* Used to control drop feedback when DND.FEEDBACK_EXPAND and DND.FEEDBACK_SCROLL is set/not set */
boolean shouldExpand = true, shouldScroll = true;

final int nativeItemHeight;
private boolean enforceNativeItemHeightMinimum;

static int NEXT_ID;

/*
Expand Down Expand Up @@ -144,6 +147,9 @@ public class Tree extends Composite {
*/
public Tree (Composite parent, int style) {
super (parent, checkStyle (style));

this.nativeItemHeight = (int)((NSTableView)view).rowHeight();
this.enforceNativeItemHeightMinimum = display.enforceNativeItemHeightMinimum;
}

@Override
Expand Down Expand Up @@ -3171,6 +3177,9 @@ void setItemHeight (Image image, NSFont font, boolean set) {
double ascent = font.ascender ();
double descent = -font.descender () + font.leading ();
int height = (int)Math.ceil (ascent + descent) + 1;
if (enforceNativeItemHeightMinimum) {
height = Math.max (height, nativeItemHeight);
}
Rectangle bounds = image != null ? image.getBounds () : imageBounds;
if (bounds != null) {
imageBounds = bounds;
Expand Down Expand Up @@ -3575,5 +3584,14 @@ void updateCursorRects (boolean enabled) {
updateCursorRects (enabled, headerView);
}

public boolean isEnforceNativeItemHeightMinimum() {
return enforceNativeItemHeightMinimum;
}

public void setEnforceNativeItemHeightMinimum(boolean enforce) {
this.enforceNativeItemHeightMinimum = enforce;
setItemHeight(null, null, true); // update height
}

}

0 comments on commit 0f1d659

Please sign in to comment.