Use OfFloat Point values to create precise highlight brackets

[ShahzaibIbrahim]

Because of rounding errors the bracket selection highlight is often inconsistent. Making use of residual values provides precise rectangle values and highlight is consistent across all zooms

Problem: At 250% (the problem is also there for all zooms, at 250 its just better visible), After every three line the fourth line x axis is bit too much on the left. (rounding error)

line 29: the width of highlight is 17px

line 30: the width of highlight is 16px

How to test

• Run the runtime workspace with monitor-specific scaling turned on
• Add some brackets in Java file { }
• Select the opening bracket, the closing bracket will be highlighted.
• See if the rectangle around highlighted bracket is surrounded the bracket correctly (no-cutoffs)

Result

Before Fix:

After Fix:

Requires:

• Add Rectangle.of(Point,Point) supporting OfFloat eclipse.platform.swt#2907

[github-actions]

Test Results

 3 018 files  ±0   3 018 suites  ±0   2h 17m 50s ⏱️ - 13m 32s
 8 229 tests ±0   7 981 ✅ +1  248 💤 ±0  0 ❌  - 1 
23 511 runs  ±0  22 720 ✅ +1  791 💤 ±0  0 ❌  - 1 

Results for commit 5be0542. ± Comparison against base commit 63d03fc.

♻️ This comment has been updated with latest results.

[HeikoKlare]

Is it intended that with removing the -1 from the height calculation, the highlight is now too high and exceeds the line?

Before:

[ShahzaibIbrahim]

Is it intended that with removing the -1 from the height calculation, the highlight is now too high and exceeds the line?

Before:

Yes, it is intended. We either cutoff the highlighted bracket or we go over the line.

[HeikoKlare]

So if I understand correctly, we have two issues with two solutions here:

1. The highlight is drawn over the bracket, which is solved by removing the -1.
2. The highlight is too small on higher zooms because of bad rounding, which is solved by using Rectangle.of().

If propose to make two separate PRs, one for each of the issues, out of this. While the usage of Rectangle.of() to solve the scaling issue should be without any discusionss, the -1 has been added intentionally to make the highlight fit to the line, so changing that behavior might be more of a discussion.

[ShahzaibIbrahim]

So if I understand correctly, we have two issues with two solutions here:

1. The highlight is drawn over the bracket, which is solved by removing the -1.
2. The highlight is too small on higher zooms because of bad rounding, which is solved by using Rectangle.of().

If propose to make two separate PRs, one for each of the issues, out of this. While the usage of Rectangle.of() to solve the scaling issue should be without any discusionss, the -1 has been added intentionally to make the highlight fit to the line, so changing that behavior might be more of a discussion.

This PR now keep the height as it was before. The other PR that increases the height is here: #3653

Reason for change request was resolved

[vogella]

Can this be merged? @HeikoKlare you did removed your stale review, does that mean you approve the change?

[akoch-yatta]

Can this be merged? @HeikoKlare you did removed your stale review, does that mean you approve the change?

I will have a look at it today, so hopefully, yes

[akoch-yatta]

Won't merge this one today. While it fixes some scenarios it break other scenarios now, e.g. overlapping the line in the bottom in some scenarios. We will revise if we can sync the highlight bracket calculation better with the line bounds calculation in StyledText overall.

[ShahzaibIbrahim]

@akoch-yatta with the latest commit, I got rid of textWidget.getBoundsAtOffset() and use getLocationAtOffset instead since we only need location from styledText and the latter gives us the location without text mining (more accurate). Then we have inconsistency in width and height if you move across different lines of code, for that I use Rectangle.of to so it uses the point OfFloat values and round them accordingly to give us consistent results.

I used this code to test the outline on brackets on different lines:

package test.pack;

public class TestClass {
		 
		private void book(String a) {
			
		}
		
		@Test
		private void bigbook() {
			book("dadadad");
		}
		
		{ <- thats where i start
			{
				{
					{
						{
							{
								{
									{
										{
											
										}
									}
								}
							}
						}
					}
				}
			}
		}
}

and printed out results between master and after my latest changes here. The results are as follow at 175%:

Master:
drawRectangleInPixels: x=110 y=840 width=13 height=25
drawRectangleInPixels: x=163 y=814 width=12 height=24
drawRectangleInPixels: x=215 y=788 width=13 height=24
drawRectangleInPixels: x=268 y=761 width=12 height=25
drawRectangleInPixels: x=322 y=735 width=12 height=25
drawRectangleInPixels: x=375 y=709 width=12 height=24
drawRectangleInPixels: x=427 y=683 width=12 height=24
drawRectangleInPixels: x=481 y=656 width=13 height=25
drawRectangleInPixels: x=534 y=630 width=12 height=25

After Changes:

drawRectangleInPixels: x=110 y=866 width=13 height=27
drawRectangleInPixels: x=163 y=840 width=13 height=27
drawRectangleInPixels: x=216 y=814 width=13 height=27
drawRectangleInPixels: x=269 y=788 width=13 height=27
drawRectangleInPixels: x=322 y=761 width=13 height=27
drawRectangleInPixels: x=375 y=735 width=13 height=27
drawRectangleInPixels: x=428 y=709 width=13 height=27
drawRectangleInPixels: x=481 y=683 width=13 height=27
drawRectangleInPixels: x=534 y=656 width=13 height=27