Use OS#Polygon in GC#drawRectangleInPixels

[amartya4256]

This PR addresses the following issue: vi-eclipse/Eclipse-Platform#555

To analyse the problem, consider a zoom factor of 175%.

Context

Before the introduction of ImageGcDrawer, SWT created all images at 100% zoom and scaled them afterwards. Since scaling happened post-drawing, the image data preserved the expected geometry and visual alignment, even when consumers used two slightly different conventions (width vs width - 1).

After ImageGcDrawer was added, SWT now creates the image at the target zoom directly, rather than scaling from 100%. This produces sharper results but exposes a long-standing behavioural mismatch in how rectangles are drawn.

Root Cause

GC.drawRectangle delegates to OS.Rectangle, whose Win32 documentation states:
The supplied right/bottom coordinates represent the limits of the interior; the bottom/right border pixels are excluded.
Example: drawing (0, 0, 210, 210) produces:

• Left border: 1 px
• Interior width: 209 px
• Right border: excluded

However, the actual behaviour differs.
Passing (0,0,210,210) results in:

• Left border: 1 px
• Interior: 208 px
• Right border: 1 px

The total painted width becomes 210 px, contrary to the documentation.

This discrepancy was masked before because the rectangle was drawn at 100% and later scaled. But now that we draw directly at scaled resolutions:

• Consumers often write:
  gc.drawRectangle(x, y, width - 1, height - 1);
  intending to compensate for border thickness.
• After scaling, this subtraction (-1) becomes incorrect, because it represents points, not pixels. For example: (120 - 1) * 1.75 = 208.25, truncated to 208, causing visible shifts.

Observation Snippet

public class SnippetGC {
	public static void main(String[] args) {
		Display display = new Display();

		Shell shell = new Shell(display);

		Table table = new Table(shell, SWT.FILL);
		table.setBounds(0, 0, 1000, 1000);

		Image image = new Image(display, 120, 120);
		GC gc = new GC(image);
		gc.setAdvanced(true);
		Color color = new Color(255, 0, 0);
		gc.setBackground(color);
		Rectangle rect = image.getBounds();
		gc.fillRectangle(rect.x, rect.y, rect.width, rect.height);
		if (color.equals(display.getSystemColor(SWT.COLOR_BLACK))) {
			gc.setForeground(display.getSystemColor(SWT.COLOR_WHITE));
		}
		gc.drawRectangle(rect.x, rect.y, rect.width - 1, rect.height - 1);
		gc.dispose();

		TableItem item = new TableItem(table, SWT.FILL);
		item.setImage(image);

		shell.open();
		while (!shell.isDisposed()) {
			if (!display.readAndDispatch())
				display.sleep();
		}
		image.dispose();
		display.dispose();
	}
}

With this snippet, if you run it on master, you'll see the outline is inside the filled red rectangle. This happens because when we do (120 - 1) * 1.75, we get approximately 208 and then GC#drawRectangle applies x + width + 1 leading to 209. According to the OS.Rectangle documentation it should add a right border after 209 making it overall 210 px wide — but it does not. Hence OS.Rectangle is not behaving as documented in this context and we have the right and the bottom border a pixel extra inside.

Proposed Fix

Since a rectangle is also a polygon, use OS.Polygon by calling GCdrawPolygon instead of OS.Rectangle. The polygon approach aligns with the intended semantics and produces the correct visual result under zoom.

Notes and Limitations

• This is a workaround rather than a perfect solution: consumers are still commonly calling the API with width - 1 (which is incorrect for logical/point coordinates) instead of width.

• The polygon workaround improves behaviour for the observed cases (e.g., 175% zoom) but may not be perfect at very high zoom levels (e.g., 350%).

• A full, robust fix would require adjusting consumers to pass the correct values (i.e., use width/height as logical sizes) or otherwise revisiting the API semantics.

[github-actions]

Test Results (win32)

   34 files  ±0     34 suites  ±0   4m 22s ⏱️ -19s
4 627 tests ±0  4 554 ✅ ±0  73 💤 ±0  0 ❌ ±0 
  167 runs  ±0    164 ✅ ±0   3 💤 ±0  0 ❌ ±0 

Results for commit 7274167. ± Comparison against base commit 407a403.

[laeubi]

I think there's a fundamental misunderstanding here. The consumers are not doing anything wrong - they're correctly using the API as designed. The issue isn't about subtracting "one pixel" but rather about properly accounting for the stroke/border width, which could be any value (1pt, 5pt, etc.).

The Correct Model

When drawing a rectangle with a border/stroke, the standard graphics model works as follows:

For a drawing area of 100x100 points with a stroke width of 1pt:

• Drawing a rectangle at (0,0) with dimensions 99x99 will produce a 100x100 point rectangle
• This is because the stroke is centered on the path, so 0.5pt extends outward on each side
• The consumers are correctly specifying width - strokeWidth to fit within bounds

Scaling Considerations

With a 175% scaling factor, the transformation should be:

• Drawing area: 100pt × 1.75 = 175 pixels
• Stroke width: 1pt × 1.75 = 1.75 pixels (must be rounded, typically to 2px)
• Rectangle dimensions: 99pt × 1.75 = 173.25 pixels

The rounding here is where complexity arises:

• The stroke width rounds to 2px
• The rectangle dimension could round to either 173px or 174px
• With a 2px stroke centered on the path, we need 1px on each side
• Final rendered size: 173px + 2px = 175px or 174px + 2px = 176px (overflow!)

The Real Issue

The problem isn't that consumers are using the API incorrectly. The issue is that SWT needs to handle the fractional coordinate transformations and stroke width scaling consistently across different DPI settings. This includes:

1. Properly scaling stroke widths to pixel boundaries
2. Ensuring that drawRectangle(0, 0, width-strokeWidth, height-strokeWidth) always fits within the bounds regardless of scaling
3. Handling the centering of strokes on paths correctly at all scale factors

Rather than changing how consumers use the API, SWT should internally handle these transformations to maintain consistent behavior across all DPI settings.

[amartya4256]

The rounding here is where complexity arises:

• The stroke width rounds to 2px
• The rectangle dimension could round to either 173px or 174px
• With a 2px stroke centered on the path, we need 1px on each side
• Final rendered size: 173px + 2px = 175px or 174px + 2px = 176px (overflow!)

OS.Rectangle, when called on let's say (0, 0, 209, 209), doesn't create a bigger rectangle as mentioned in the documentation.
The rectangle that is drawn excludes the bottom and right edges. I would expect the rectangle to be 210 in width, including the border.
All the borders are considered inside the defined area. If you run the snippet, you'll find the rectangle border (drawn from gc#drawImage) has a smaller area than the filled red rectangle (drawn using gc#fillRectangle), which contradicts the documentation.

Also, In this snippet's case, the lineWidth is set to 0.0. When I set it to 1, half the pixel grows outwards each side and when I set it to 2, then 1 pixel grows outwards and 1 inwards. In any case, any idea why there is a +1 in this call?

	OS.Rectangle (handle, x, y, x + width + 1, y + height + 1);

[laeubi]

Your observation about the line width behavior is indeed correct - this is how most graphics APIs work, including the Windows GDI+ that SWT uses. The key issue here is understanding SWT's intended behavior and ensuring consistency across platforms.

I think we need a definition of how SWT should handle rectangle drawing with line widths to decide what is "right" way forward here as we have different possibilities:

1. Inset model: Visual rectangle = specified dimensions, stroke grows inward
2. Center model: Stroke centered on path, growing equally inward/outward (Windows GDI+ default)
3. Outset model: Visual rectangle includes stroke, growing outward

SWT appears to follow the center model (option 2), which aligns with the Windows GDI+ behavior where the stroke is centered on the geometric path.

Test Snippet for Line Width Analysis

import org.eclipse.swt.SWT;
import org.eclipse.swt.events.PaintEvent;
import org.eclipse.swt.events. PaintListener;
import org. eclipse.swt.graphics.*;
import org.eclipse.swt.layout.*;
import org.eclipse.swt.widgets.*;

public class LineWidthTestSnippet {
    
    public static void main(String[] args) {
        Display display = new Display();
        Shell shell = new Shell(display);
        shell.setText("Line Width Rectangle Comparison");
        shell.setLayout(new GridLayout(2, false));
        
        // Canvas for drawing
        Canvas canvas = new Canvas(shell, SWT. BORDER);
        GridData canvasData = new GridData(400, 400);
        canvas.setLayoutData(canvasData);
        canvas.setBackground(display.getSystemColor(SWT.COLOR_WHITE));
        
        // Control panel
        Composite controlPanel = new Composite(shell, SWT. NONE);
        controlPanel. setLayout(new GridLayout(2, false));
        controlPanel. setLayoutData(new GridData(SWT.FILL, SWT. FILL, true, true));
        
        // Line width control
        Label lineWidthLabel = new Label(controlPanel, SWT.NONE);
        lineWidthLabel.setText("Line Width:");
        
        Spinner lineWidthSpinner = new Spinner(controlPanel, SWT.BORDER);
        lineWidthSpinner.setMinimum(0);
        lineWidthSpinner.setMaximum(20);
        lineWidthSpinner.setSelection(1);
        lineWidthSpinner.setLayoutData(new GridData(SWT.FILL, SWT. CENTER, true, false));
        
        // Show grid checkbox
        Button showGrid = new Button(controlPanel, SWT.CHECK);
        showGrid.setText("Show pixel grid");
        showGrid.setSelection(true);
        showGrid.setLayoutData(new GridData(SWT. FILL, SWT.CENTER, true, false, 2, 1));
        
        // Show measurements checkbox
        Button showMeasurements = new Button(controlPanel, SWT.CHECK);
        showMeasurements.setText("Show measurements");
        showMeasurements.setSelection(true);
        showMeasurements.setLayoutData(new GridData(SWT.FILL, SWT. CENTER, true, false, 2, 1));
        
        // Info display
        Text infoText = new Text(controlPanel, SWT.MULTI | SWT. BORDER | SWT.READ_ONLY | SWT.V_SCROLL);
        GridData infoData = new GridData(SWT.FILL, SWT.FILL, true, true, 2, 1);
        infoData.heightHint = 200;
        infoText.setLayoutData(infoData);
        
        // Paint listener
        canvas.addPaintListener(new PaintListener() {
            @Override
            public void paintControl(PaintEvent e) {
                GC gc = e.gc;
                int lineWidth = lineWidthSpinner.getSelection();
                
                // Get canvas dimensions
                Rectangle bounds = canvas.getClientArea();
                int centerX = bounds.width / 2;
                int centerY = bounds.height / 2;
                
                // Rectangle dimensions
                int rectWidth = 100;
                int rectHeight = 100;
                int rectX = centerX - rectWidth / 2;
                int rectY = centerY - rectHeight / 2;
                
                // Draw pixel grid if enabled
                if (showGrid.getSelection()) {
                    gc. setLineWidth(1);
                    gc.setLineStyle(SWT.LINE_DOT);
                    gc.setForeground(display. getSystemColor(SWT.COLOR_GRAY));
                    gc.setAlpha(50);
                    
                    // Draw grid lines every 10 pixels
                    for (int x = 0; x < bounds.width; x += 10) {
                        gc.drawLine(x, 0, x, bounds.height);
                    }
                    for (int y = 0; y < bounds. height; y += 10) {
                        gc.drawLine(0, y, bounds.width, y);
                    }
                    
                    gc.setAlpha(255);
                    gc.setLineStyle(SWT.LINE_SOLID);
                }
                
                // Draw filled rectangle (red)
                gc.setBackground(display.getSystemColor(SWT.COLOR_RED));
                gc.setAlpha(128); // Semi-transparent to see overlap
                gc.fillRectangle(rectX, rectY, rectWidth, rectHeight);
                gc.setAlpha(255);
                
                // Draw outline rectangle (black)
                gc. setForeground(display.getSystemColor(SWT.COLOR_BLACK));
               gc.setAlpha(125);
                gc.setLineWidth(lineWidth);
                gc.drawRectangle(rectX, rectY, rectWidth, rectHeight);
                gc.setAlpha(255);
                
                // Draw measurements if enabled
                if (showMeasurements.getSelection()) {
                    gc.setLineWidth(1);
                    gc.setForeground(display. getSystemColor(SWT.COLOR_BLUE));
                    Font font = new Font(display, "Courier", 9, SWT.NORMAL);
                    gc.setFont(font);
                    
                    // Draw measurement lines and labels
                    int measureY = rectY - 20;
                    
                    // Horizontal measurement
                    gc.drawLine(rectX, measureY, rectX + rectWidth, measureY);
                    gc.drawLine(rectX, measureY - 5, rectX, measureY + 5);
                    gc.drawLine(rectX + rectWidth, measureY - 5, rectX + rectWidth, measureY + 5);
                    
                    String widthText = String.format("%dpx", rectWidth);
                    Point textExtent = gc.textExtent(widthText);
                    gc.drawText(widthText, rectX + (rectWidth - textExtent.x) / 2, measureY - textExtent.y - 2, true);
                    
                    // Vertical measurement
                    int measureX = rectX - 20;
                    gc.drawLine(measureX, rectY, measureX, rectY + rectHeight);
                    gc.drawLine(measureX - 5, rectY, measureX + 5, rectY);
                    gc.drawLine(measureX - 5, rectY + rectHeight, measureX + 5, rectY + rectHeight);
                    
                    // Draw coordinates at corners
                    gc.setForeground(display.getSystemColor(SWT.COLOR_DARK_GREEN));
                    gc.drawText(String.format("(%d,%d)", rectX, rectY), rectX - 50, rectY - 15, true);
                    gc. drawText(String.format("(%d,%d)", rectX + rectWidth, rectY + rectHeight), 
                               rectX + rectWidth + 5, rectY + rectHeight + 5, true);
                    
                    font.dispose();
                }
                
                // Update info text
                StringBuilder info = new StringBuilder();
                info.append("Platform: ").append(SWT.getPlatform()).append("\n");
                info.append("DPI: ").append(display.getDPI().x).append(" x ").append(display.getDPI().y).append("\n");
                info.append("\nRectangle Properties:\n");
                info.append("  Position: (").append(rectX).append(", ").append(rectY).append(")\n");
                info.append("  Size: ").append(rectWidth).append(" x ").append(rectHeight).append("\n");
                info.append("  Line Width: ").append(lineWidth).append("\n");
                info.append("\nExpected Behavior (center model):\n");
                if (lineWidth > 0) {
                    double halfWidth = lineWidth / 2.0;
                    info. append("  Outer bounds: (").append(String.format("%.1f", rectX - halfWidth))
                        .append(", ").append(String.format("%.1f", rectY - halfWidth)).append(") to (")
                        .append(String.format("%.1f", rectX + rectWidth + halfWidth))
                        .append(", ").append(String.format("%.1f", rectY + rectHeight + halfWidth)).append(")\n");
                    info.append("  Inner bounds: (").append(String.format("%.1f", rectX + halfWidth))
                        .append(", ").append(String.format("%.1f", rectY + halfWidth)).append(") to (")
                        .append(String.format("%.1f", rectX + rectWidth - halfWidth))
                        . append(", ").append(String.format("%.1f", rectY + rectHeight - halfWidth)).append(")\n");
                }
                
                infoText. setText(info.toString());
            }
        });
        
        // Redraw on spinner change
        lineWidthSpinner.addListener(SWT.Selection, e -> canvas.redraw());
        showGrid.addListener(SWT. Selection, e -> canvas.redraw());
        showMeasurements.addListener(SWT.Selection, e -> canvas.redraw());
        
        shell.pack();
        shell.open();
        
        while (!shell.isDisposed()) {
            if (! display.readAndDispatch()) {
                display.sleep();
            }
        }
        display.dispose();
    }
}

In GC#drawRectangle, the Windows implementation directly calls GDI+ methods without adjusting for line width, suggesting it relies on GDI+'s default center-stroke behavior. using OS#Polygon might introduce inconsistencies because polygon drawing and rectangle drawing might handle strokes differently in GDI+ but I'm not an expert in that area..

Some ideas

1. Run the test snippet on Windows, Linux, and macOS to document the current behavior with different line widths.

2. Document the expected behavior in the GC class Javadoc, specifically stating that strokes are centered on the geometric path (if that's the confirmed behavior).

3. For the PR: Consider whether using OS#Polygon maintains the same stroke positioning behavior as the current rectangle drawing implementation. The polygon approach might need coordinate adjustments to match the expected behavior.

4. Future adding SWT constants (like SWT.STROKE_CENTER, SWT.STROKE_INNER, SWT.STROKE_OUTER) if different stroke positioning modes are needed for different use cases in LineAttributes but might be overkill here.

Under my Linux it looks like this:

Given that I think one can not really paint a rectangle with 1px border reliable with the integer API and something like

gc.drawRectangle(rect.x, rect.y, rect.width - 1, rect.height - 1);

only works by chance because it has some rounding effects and actually has to be

gc.drawRectangle(rect.x-0.5, rect.y+0.5, rect.width - 0.5, rect.height - 0.5);

what in fact can only be done currently with a Path that allows float coordinates.