Fix caret of raw insert mode of text editor on HiDPI screens

[wahlbrink]

Fixes: #3010

[github-actions]

Test Results

 3 018 files  ±0   3 018 suites  ±0   2h 4m 44s ⏱️ - 25m 10s
 8 270 tests ±0   8 021 ✅  - 1  248 💤 ±0  1 ❌ +1 
23 634 runs  ±0  22 842 ✅  - 1  791 💤 ±0  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 9b6afe9. ± Comparison against base commit 62f3974.

♻️ This comment has been updated with latest results.

[HeikoKlare]

Thank you for proposing this fix! I have one question regarding the proposed solution, as it appears more complex to me than required.

[HeikoKlare]

Is there any reason to use an ImageDataProvider rather than an ImageGcDrawer?
This requires to manually implement proper line width calculation and line rendering instead of just using gc.drawLine() like in the original code.

[wahlbrink]

This creates a 1-bit image as before.

[HeikoKlare]

What do you mean with "1-bit image"? Do you mean that the line is 1 pixel wide? That's the intended behavior of setLineWidth(0). Changing that to setLineWidth(1) should resolve it.

[wahlbrink]

1-bit image = image with 1-bit color palette = a pixels uses 1-bit = colloquially called black-and-white image

[HeikoKlare]

I see. But is it really necessary/beneficial to keep it like that?

[arunjose696]

I also experimented with using ImageGcDrawer. Since the image created this way is no longer 1-bit, I had to adapt the GC drawing slightly to get the desired result. The following variant worked reliably for me:

Display display = styledText.getDisplay();
		ImageGcDrawer imageGcDrawer = (gc, imageWidth, imageHeight) -> {
			gc.setForeground(display.getSystemColor(SWT.COLOR_BLACK));
			gc.setLineWidth(1);
			int h3 = imageHeight / 3;
			for (int i = 0; i < imageWidth; i++) {
				gc.drawLine(i, imageHeight - h3, i, imageHeight - 2 * h3);
			}
		};
		final Image image = new Image(display, imageGcDrawer, width, styledText.getLineHeight());

I don’t have a strong opinion on ImageDataProvider vs ImageGcDrawer. From my perspective, using ImageGcDrawer allows relying on gc.drawLine(), while the ImageDataProvider approach also seems to work fine.

@wahlbrink do you see any reason why ImageDataProvider might be the better option here, or any trade-offs between the two that we should consider?

[wahlbrink]

1. I don’t know if there are special requirements (color depth) for cursor images on any platform

2. The ImageDataProvider works on the real monitor resolution:

   • It takes advantage of higher resolutions (when dividing the height into thirds)
   • You get exactly what you want at any zoom level (e.g. no rounding by drawing methods)

3. If you want to use ImageGcDrawer, your solution is not yet mature

   • The two visible rectangles does not always have same height
   • The drawn rectangle is fragmented at some zoom levels

   Screenshot (2x): top ImageDataProvider, bottom ImageGcDrawer
   

[HeikoKlare]

2. The ImageDataProvider works on the real monitor resolution:

That's also the case for the ImageGcDrawer.

Couldn't the drawer simply be something like this?

gc.setBackground(display.getSystemColor(SWT.COLOR_BLACK));
gc.fillRectangle(0, imageHeight / 3, imageWidth, imageHeight / 3);

[wahlbrink]

That's also the case for the ImageGcDrawer

The image yes, but not the GC API (when using autoscale). It works with "zoom 100" int coordinates, therefore the positioning of elements using the GC's regular drawing methods is bound to the "zoom 100" grid.

[HeikoKlare]

The image yes, but not the GC API (when using autoscale). It works with "zoom 100" int coordinates, therefore the positioning of elements using the GC's regular drawing methods is bound to the "zoom 100" grid.

If I understand correctly, that refers to an issue to be addressed by eclipse-platform/eclipse.platform.swt#2913.

Is that relevant here? At a first glance, I don't see any drawing of the caret image with GC's drawImage() methods but only usages of OS methods taking the OS handles for drawing.

[arunjose696]

I’ve tested the changes in both dark and light themes, and they work as expected. and I can see the caret at multiple zooms.

[HeikoKlare]

I tested with the following implementation, which looks fine on Windows, but turns out that it at least looks weird on MacOS.

int width = getCaretWidthPreference();
Display display = styledText.getDisplay();
ImageGcDrawer imageGcDrawer = (gc, imageWidth, imageHeight) -> {
	gc.setBackground(display.getSystemColor(SWT.COLOR_BLACK));
	gc.fillRectangle(0, imageHeight / 3, imageWidth, imageHeight / 3);
};
return new Image(display, imageGcDrawer, width, styledText.getLineHeight());

For some reason MacOS does not rescaling and anti-aliasing of the caret image (without any need), which makes it look strange. This makes the current caret not look nice at all, with the implementation above it's a bit better, but with the one proposed in this PR it's the best we can have.

In consequence, I am in favor of using the current propsal as it properly fixes the issue for zooms != 100 and also improves the apperance on MacOS in general.

[HeikoKlare]

Failing test is unrelated:

• PartRenderingEngineTests.ensureCleanUpAddonCleansUp still fails randomly #3581