Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Fragment-Host: org.eclipse.swt;bundle-version="[3.128.0,4.0.0)"
Bundle-Name: %fragmentName
Bundle-Vendor: %providerName
Bundle-SymbolicName: org.eclipse.swt.cocoa.macosx.aarch64; singleton:=true
Bundle-Version: 3.131.100.qualifier
Bundle-Version: 3.132.0.qualifier
Bundle-ManifestVersion: 2
Bundle-Localization: fragment
Export-Package:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Fragment-Host: org.eclipse.swt;bundle-version="[3.128.0,4.0.0)"
Bundle-Name: %fragmentName
Bundle-Vendor: %providerName
Bundle-SymbolicName: org.eclipse.swt.cocoa.macosx.x86_64; singleton:=true
Bundle-Version: 3.131.100.qualifier
Bundle-Version: 3.132.0.qualifier
Bundle-ManifestVersion: 2
Bundle-Localization: fragment
Export-Package:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Fragment-Host: org.eclipse.swt;bundle-version="[3.128.0,4.0.0)"
Bundle-Name: %fragmentName
Bundle-Vendor: %providerName
Bundle-SymbolicName: org.eclipse.swt.gtk.linux.aarch64; singleton:=true
Bundle-Version: 3.131.100.qualifier
Bundle-Version: 3.132.0.qualifier
Bundle-ManifestVersion: 2
Bundle-Localization: fragment
Export-Package:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Fragment-Host: org.eclipse.swt;bundle-version="[3.128.0,4.0.0)"
Bundle-Name: %fragmentName
Bundle-Vendor: %providerName
Bundle-SymbolicName: org.eclipse.swt.gtk.linux.loongarch64; singleton:=true
Bundle-Version: 3.131.0.qualifier
Bundle-Version: 3.132.0.qualifier
Bundle-ManifestVersion: 2
Bundle-Localization: fragment
Export-Package:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Fragment-Host: org.eclipse.swt;bundle-version="[3.128.0,4.0.0)"
Bundle-Name: %fragmentName
Bundle-Vendor: %providerName
Bundle-SymbolicName: org.eclipse.swt.gtk.linux.ppc64le;singleton:=true
Bundle-Version: 3.131.100.qualifier
Bundle-Version: 3.132.0.qualifier
Bundle-ManifestVersion: 2
Bundle-Localization: fragment
Export-Package:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Fragment-Host: org.eclipse.swt;bundle-version="[3.128.0,4.0.0)"
Bundle-Name: %fragmentName
Bundle-Vendor: %providerName
Bundle-SymbolicName: org.eclipse.swt.gtk.linux.riscv64; singleton:=true
Bundle-Version: 3.131.100.qualifier
Bundle-Version: 3.132.0.qualifier
Bundle-ManifestVersion: 2
Bundle-Localization: fragment
Export-Package:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Fragment-Host: org.eclipse.swt;bundle-version="[3.128.0,4.0.0)"
Bundle-Name: %fragmentName
Bundle-Vendor: %providerName
Bundle-SymbolicName: org.eclipse.swt.gtk.linux.x86_64; singleton:=true
Bundle-Version: 3.131.100.qualifier
Bundle-Version: 3.132.0.qualifier
Bundle-ManifestVersion: 2
Bundle-Localization: fragment
Export-Package:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Fragment-Host: org.eclipse.swt;bundle-version="[3.128.0,4.0.0)"
Bundle-Name: %fragmentName
Bundle-Vendor: %providerName
Bundle-SymbolicName: org.eclipse.swt.win32.win32.aarch64; singleton:=true
Bundle-Version: 3.131.100.qualifier
Bundle-Version: 3.132.0.qualifier
Bundle-ManifestVersion: 2
Bundle-Localization: fragment
Export-Package:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Fragment-Host: org.eclipse.swt;bundle-version="[3.128.0,4.0.0)"
Bundle-Name: %fragmentName
Bundle-Vendor: %providerName
Bundle-SymbolicName: org.eclipse.swt.win32.win32.x86_64; singleton:=true
Bundle-Version: 3.131.100.qualifier
Bundle-Version: 3.132.0.qualifier
Bundle-ManifestVersion: 2
Bundle-Localization: fragment
Export-Package:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,13 @@ public void setY(float y) {
this.y = Math.round(y);
this.residualY = y - this.y;
}

public static Point.OfFloat from(Point point) {
if (point instanceof Point.OfFloat pointOfFloat) {
return new Point.OfFloat(pointOfFloat.getX(), pointOfFloat.getY());
}
return new Point.OfFloat(point.x, point.y);
}
Comment on lines +162 to +167
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a duplication of the method in FloatAwareGeometryFactory, isn't it? Shouldn't that be streamlined in an OS-independent way?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With vi-eclipse/Eclipse-Platform#320 we wanted to provide such utility methods directly inside rectangle and point since, they are platform independent. FloatAwareGeometryFactory is a private class inside Win32DPIUtils. Do you think it makes sense to keep using that or maybe we should move towards Point.from and Rectangle.from.

For now I can use FloatAwareGeometryFactory and create a PR following the refactoring as suggested. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean that we should not introduce duplicated code unnecessarily. If we introduce this method as proposed, we can remove the according method from the FloatAwareGeometryFactory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do that separately. I would also need to adapt other places. For now I'll remove this from here and use the Factory method. And I'll refactor everything in the next PR. Sounds good?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that work? The factory is win32-specific while this is OS-independent code.

Copy link
Contributor

@laeubi laeubi Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get what you mean.

I mean why is it important where GridLayout is located for the part about FloatAwareGeometryFactory and Win32DPIUtils? A Layout Manger shold never need to know anything about DPI scaling and alike...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please read my comment above. Everything should be in there: it does not make sense (and technically you cannot even) reference OS-specific code from an OS-agnostic class (like the layout-related classes).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HeikoKlare I read the comments but they do not explain why GridLayout now needs to use Point.OfFloat and be rewritten in using internal API now, so for me it means EVERY LayoutManger now needs to be potentially be rewritten as well (what could be code outside of SWT as well).

As LayoutMangers should already work on the "points" and not the "pixels" so something seems fundamentally wrong here!

And while I can understand that there might be some slight rounding errors, the example above shows a noticeable cut-of that can not be explained by a sub-pixel rounding error to me!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HeikoKlare I read the comments but they do not explain why GridLayout now needs to use Point.OfFloat and be rewritten in using internal API now, so for me it means EVERY LayoutManger now needs to be potentially be rewritten as well (what could be code outside of SWT as well).

Yes, that might be the case.

As LayoutMangers should already work on the "points" and not the "pixels" so something seems fundamentally wrong here!

As we know, points are imprecise because of their limitation to integers and according rounding errors.

And while I can understand that there might be some slight rounding errors, the example above shows a noticeable cut-of that can not be explained by a sub-pixel rounding error to me!

It is such an off-by-one rounding error.

Copy link
Contributor

@laeubi laeubi Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is such an off-by-one rounding error.

It does not really look like being of by a few pixels. For me it looks like more the reported size of the label is smaller than it actually should, so the Label class needs to be fixed (or the Point/Rectangle) to not round down but round up instead so in such case we get a slightly larger size (and maybe show a tiny little extra whitespace) than one that is too small. This would then fix the issue for many other cases as well instead of trying to "fix" all callers.

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,13 @@ public void setHeight(float height) {
this.residualHeight = height - this.height;
}

public static Rectangle.OfFloat from(Rectangle rectangle) {
if (rectangle instanceof Rectangle.OfFloat rectangleOfFloat) {
return new Rectangle.OfFloat(rectangleOfFloat.getX(), rectangleOfFloat.getY(), rectangleOfFloat.getWidth(), rectangleOfFloat.getHeight());
}
return new Rectangle.OfFloat(rectangle.x, rectangle.y, rectangle.width, rectangle.height);
}

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,9 +398,9 @@ public final class GridData {
*/
public static final int FILL_BOTH = FILL_VERTICAL | FILL_HORIZONTAL;

int cacheWidth = -1, cacheHeight = -1;
int defaultWhint, defaultHhint, defaultWidth = -1, defaultHeight = -1;
int currentWhint, currentHhint, currentWidth = -1, currentHeight = -1;
float cacheWidth = -1, cacheHeight = -1;
int defaultWhint, defaultHhint, currentWhint, currentHhint;
float defaultWidth = -1, defaultHeight = -1, currentWidth = -1, currentHeight = -1;

/**
* Constructs a new instance of GridData using
Expand Down Expand Up @@ -493,19 +493,20 @@ void computeSize (Control control, int wHint, int hHint, boolean flushCache) {
Point size = control.computeSize (wHint, hHint, flushCache);
defaultWhint = wHint;
defaultHhint = hHint;
defaultWidth = size.x;
defaultHeight = size.y;
Point.OfFloat defaultSize = Point.OfFloat.from(size);
defaultWidth = defaultSize.getX();
defaultHeight = defaultSize.getY();
}
cacheWidth = defaultWidth;
cacheHeight = defaultHeight;
return;
}
if (currentWidth == -1 || currentHeight == -1 || wHint != currentWhint || hHint != currentHhint) {
Point size = control.computeSize (wHint, hHint, flushCache);
Point.OfFloat size = Point.OfFloat.from(control.computeSize (wHint, hHint, flushCache));
currentWhint = wHint;
currentHhint = hHint;
currentWidth = size.x;
currentHeight = size.y;
currentWidth = size.getX();
currentHeight = size.getY();
}
cacheWidth = currentWidth;
cacheHeight = currentHeight;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,16 +292,16 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
/* Column widths */
int availableWidth = width - horizontalSpacing * (columnCount - 1) - (marginLeft + marginWidth * 2 + marginRight);
int expandCount = 0;
int [] widths = new int [columnCount];
int [] minWidths = new int [columnCount];
float [] widths = new float [columnCount];
float [] minWidths = new float [columnCount];
boolean [] expandColumn = new boolean [columnCount];
for (int j=0; j<columnCount; j++) {
for (int i=0; i<rowCount; i++) {
GridData data = getData (grid, i, j, rowCount, columnCount, true);
if (data != null) {
int hSpan = Math.max (1, Math.min (data.horizontalSpan, columnCount));
if (hSpan == 1) {
int w = data.cacheWidth + data.horizontalIndent;
float w = data.cacheWidth + data.horizontalIndent;
widths [j] = Math.max (widths [j], w);
if (data.grabExcessHorizontalSpace) {
if (!expandColumn [j]) expandCount++;
Expand Down Expand Up @@ -330,27 +330,23 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
expandCount++;
expandColumn [j] = true;
}
int w = data.cacheWidth + data.horizontalIndent - spanWidth - (hSpan - 1) * horizontalSpacing;
float w = data.cacheWidth + data.horizontalIndent - spanWidth - (hSpan - 1) * horizontalSpacing;
if (w > 0) {
if (makeColumnsEqualWidth) {
int equalWidth = (w + spanWidth) / hSpan;
int remainder = (w + spanWidth) % hSpan, last = -1;
float equalWidth = (w + spanWidth) / hSpan;
for (int k = 0; k < hSpan; k++) {
widths [last=j-k] = Math.max (equalWidth, widths [j-k]);
widths [j-k] = Math.max (equalWidth, widths [j-k]);
}
if (last > -1) widths [last] += remainder;
} else {
if (spanExpandCount == 0) {
widths [j] += w;
} else {
int delta = w / spanExpandCount;
int remainder = w % spanExpandCount, last = -1;
float delta = w / spanExpandCount;
for (int k = 0; k < hSpan; k++) {
if (expandColumn [j-k]) {
widths [last=j-k] += delta;
widths [j-k] += delta;
}
}
if (last > -1) widths [last] += remainder;
}
}
}
Expand All @@ -361,14 +357,12 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
if (spanExpandCount == 0) {
minWidths [j] += w;
} else {
int delta = w / spanExpandCount;
int remainder = w % spanExpandCount, last = -1;
float delta = w / spanExpandCount;
for (int k = 0; k < hSpan; k++) {
if (expandColumn [j-k]) {
minWidths [last=j-k] += delta;
minWidths [j-k] += delta;
}
}
if (last > -1) minWidths [last] += remainder;
}
}
}
Expand All @@ -377,8 +371,8 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
}
}
if (makeColumnsEqualWidth) {
int minColumnWidth = 0;
int columnWidth = 0;
float minColumnWidth = 0;
float columnWidth = 0;
for (int i=0; i<columnCount; i++) {
minColumnWidth = Math.max (minColumnWidth, minWidths [i]);
columnWidth = Math.max (columnWidth, widths [i]);
Expand Down Expand Up @@ -424,20 +418,18 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
spanWidth += widths [j-k];
if (expandColumn [j-k]) spanExpandCount++;
}
int w = !data.grabExcessHorizontalSpace || data.minimumWidth == SWT.DEFAULT ? data.cacheWidth : data.minimumWidth;
float w = !data.grabExcessHorizontalSpace || data.minimumWidth == SWT.DEFAULT ? data.cacheWidth : data.minimumWidth;
w += data.horizontalIndent - spanWidth - (hSpan - 1) * horizontalSpacing;
if (w > 0) {
if (spanExpandCount == 0) {
widths [j] += w;
} else {
int delta2 = w / spanExpandCount;
int remainder2 = w % spanExpandCount, last2 = -1;
float delta2 = w / spanExpandCount;
for (int k = 0; k < hSpan; k++) {
if (expandColumn [j-k]) {
widths [last2=j-k] += delta2;
widths [j-k] += delta2;
}
}
if (last2 > -1) widths [last2] += remainder2;
}
}
}
Expand Down Expand Up @@ -499,16 +491,16 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
/* Row heights */
int availableHeight = height - verticalSpacing * (rowCount - 1) - (marginTop + marginHeight * 2 + marginBottom);
expandCount = 0;
int [] heights = new int [rowCount];
int [] minHeights = new int [rowCount];
float [] heights = new float [rowCount];
float [] minHeights = new float [rowCount];
boolean [] expandRow = new boolean [rowCount];
for (int i=0; i<rowCount; i++) {
for (int j=0; j<columnCount; j++) {
GridData data = getData (grid, i, j, rowCount, columnCount, true);
if (data != null) {
int vSpan = Math.max (1, Math.min (data.verticalSpan, rowCount));
if (vSpan == 1) {
int h = data.cacheHeight + data.verticalIndent;
float h = data.cacheHeight + data.verticalIndent;
heights [i] = Math.max (heights [i], h);
if (data.grabExcessVerticalSpace) {
if (!expandRow [i]) expandCount++;
Expand Down Expand Up @@ -537,19 +529,17 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
expandCount++;
expandRow [i] = true;
}
int h = data.cacheHeight + data.verticalIndent - spanHeight - (vSpan - 1) * verticalSpacing;
float h = data.cacheHeight + data.verticalIndent - spanHeight - (vSpan - 1) * verticalSpacing;
if (h > 0) {
if (spanExpandCount == 0) {
heights [i] += h;
} else {
int delta = h / spanExpandCount;
int remainder = h % spanExpandCount, last = -1;
float delta = h / spanExpandCount;
for (int k = 0; k < vSpan; k++) {
if (expandRow [i-k]) {
heights [last=i-k] += delta;
heights [i-k] += delta;
}
}
if (last > -1) heights [last] += remainder;
}
}
if (!data.grabExcessVerticalSpace || data.minimumHeight != 0) {
Expand All @@ -559,14 +549,12 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
if (spanExpandCount == 0) {
minHeights [i] += h;
} else {
int delta = h / spanExpandCount;
int remainder = h % spanExpandCount, last = -1;
float delta = h / spanExpandCount;
for (int k = 0; k < vSpan; k++) {
if (expandRow [i-k]) {
minHeights [last=i-k] += delta;
minHeights [i-k] += delta;
}
}
if (last > -1) minHeights [last] += remainder;
}
}
}
Expand Down Expand Up @@ -609,20 +597,18 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
spanHeight += heights [i-k];
if (expandRow [i-k]) spanExpandCount++;
}
int h = !data.grabExcessVerticalSpace || data.minimumHeight == SWT.DEFAULT ? data.cacheHeight : data.minimumHeight;
float h = !data.grabExcessVerticalSpace || data.minimumHeight == SWT.DEFAULT ? data.cacheHeight : data.minimumHeight;
h += data.verticalIndent - spanHeight - (vSpan - 1) * verticalSpacing;
if (h > 0) {
if (spanExpandCount == 0) {
heights [i] += h;
} else {
int delta2 = h / spanExpandCount;
int remainder2 = h % spanExpandCount, last2 = -1;
float delta2 = h / spanExpandCount;
for (int k = 0; k < vSpan; k++) {
if (expandRow [i-k]) {
heights [last2=i-k] += delta2;
heights [i-k] += delta2;
}
}
if (last2 > -1) heights [last2] += remainder2;
}
}
}
Expand Down Expand Up @@ -651,7 +637,7 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
if (data != null) {
int hSpan = Math.max (1, Math.min (data.horizontalSpan, columnCount));
int vSpan = Math.max (1, data.verticalSpan);
int cellWidth = 0, cellHeight = 0;
float cellWidth = 0, cellHeight = 0;
for (int k=0; k<hSpan; k++) {
cellWidth += widths [j+k];
}
Expand All @@ -660,7 +646,7 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
}
cellWidth += horizontalSpacing * (hSpan - 1);
int childX = gridX + data.horizontalIndent;
int childWidth = Math.min (data.cacheWidth, cellWidth);
float childWidth = Math.min (data.cacheWidth, cellWidth);
switch (data.horizontalAlignment) {
case SWT.CENTER:
case GridData.CENTER:
Expand All @@ -677,7 +663,7 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
}
cellHeight += verticalSpacing * (vSpan - 1);
int childY = gridY + data.verticalIndent;
int childHeight = Math.min (data.cacheHeight, cellHeight);
float childHeight = Math.min (data.cacheHeight, cellHeight);
switch (data.verticalAlignment) {
case SWT.CENTER:
case GridData.CENTER:
Expand All @@ -694,7 +680,7 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
}
Control child = grid [i][j];
if (child != null) {
child.setBounds (childX, childY, childWidth, childHeight);
child.setBounds (new Rectangle.OfFloat(childX, childY, childWidth, childHeight));
}
}
gridX += widths [j] + horizontalSpacing;
Expand All @@ -708,8 +694,8 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
flush [i].cacheWidth = flush [i].cacheHeight = -1;
}

int totalDefaultWidth = 0;
int totalDefaultHeight = 0;
float totalDefaultWidth = 0;
float totalDefaultHeight = 0;
for (int i=0; i<columnCount; i++) {
totalDefaultWidth += widths [i];
}
Expand All @@ -718,7 +704,7 @@ Point layout (Composite composite, boolean move, int x, int y, int width, int he
}
totalDefaultWidth += horizontalSpacing * (columnCount - 1) + marginLeft + marginWidth * 2 + marginRight;
totalDefaultHeight += verticalSpacing * (rowCount - 1) + marginTop + marginHeight * 2 + marginBottom;
return new Point (totalDefaultWidth, totalDefaultHeight);
return new Point.OfFloat (totalDefaultWidth, totalDefaultHeight);
}

String getName () {
Expand Down
Loading
Loading