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

[wahlbrink]

Fixes: #3010

[github-actions]

Test Results

 3 015 files  ±0   3 015 suites  ±0   2h 10m 4s ⏱️ - 15m 7s
 8 264 tests ±0   8 016 ✅ ±0  248 💤 ±0  0 ❌ ±0 
23 616 runs  ±0  22 825 ✅ ±0  791 💤 ±0  0 ❌ ±0 

Results for commit 65aeb70. ± Comparison against base commit 8a64d23.

♻️ 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.