Adjust selection filling the curve for CTabFolder

[ShahzaibIbrahim]

When CTabFolder is selected there is a visible gap on top as well as on the top left where there is a curve (in RCP when theming is disabled). Also the image was too close to selection highlight which is adjusted here as well.

Results

Before Changes at 200%:

After Changes at 200%:

[github-actions]

Test Results

  111 files  ±0    111 suites  ±0   17m 20s ⏱️ -21s
4 597 tests ±0  4 583 ✅ +2  14 💤  - 1  0 ❌  - 1 
  282 runs  ±0    281 ✅ ±0   1 💤 ±0  0 ❌ ±0 

Results for commit bfa6a2e. ± Comparison against base commit 63d5582.

♻️ This comment has been updated with latest results.

[ShahzaibIbrahim]

I have made some more changes (with the help of co-pilot), resolving some comments.

1. Images stays where they were.
2. Thinner highlight line.
3. Outline is drawn after (previous behavior)

This is the closest result I could achieve with all this calculations.

[HeikoKlare]

I have to admit that I do not get why we need to add another complex implementation for calculating a shape, which then needs to be kept consistent with the existing shape(s) for the background and the outline. There is already the shape calculated in the touched method, which is used to draw the background of the tab header and the outline. The same shape is used for both of them and adapted in between where necessary. Can't we do something similar for the highlight line? Then we would not add another calculation logic that we need to comprehend and maintain independently.

[ShahzaibIbrahim]

I tried using the shape at first but using shape fills the whole tab because it defines the entire tab’s outline like this:

while computed polygon only defines the thin highlight region that we see on the top only. There is a lot of operation performed on shape as well which I don't understand and I don't know how can I extract out that specific curve from it. That's my reason to just draw a highlight on top of it.

[HeikoKlare]

while computed polygon only defines the thin highlight region that we see on the top only. There is a lot of operation performed on shape as well which I don't understand and I don't know how can I extract out that specific curve from it. That's my reason to just draw a highlight on top of it.

But the polygon is not different from the shape (or if it is, it's wrong) except that the area it fills is larger (as it fills the whole header instead of only the topmost part of it). Why should the operation performed on the polygon be more simple than the ones on the existing shape? For people that have to maintain the code, it will become even more difficult when there are two implementations (that somehow need to be consistent) that they need to understand rather than one. If the current code is too hard to comprehend, we should first refactor it to improve comprehensibility. But replicating an implementation because we do not understand the existing one it not a good idea. And without understanding the existing shape implementation, how could we be sure that the added polygon one fits to it at all in every scenario?

[HeikoKlare]

I quickly hacked this demonstration (just adopted what is done for the background, removing all unnecessary stuff and adapted the bottom left and bottom right coordinates of the shape):

// draw highlight marker of selected tab
if (parent.selectionHighlightBarThickness > 0 && parent.simple) {
	int[] shape2 = Arrays.copyOf(shape, shape.length);
	shape2[1] = 3;
	shape2[3] = 3;
	shape2[shape2.length - 1] = 3;
	shape2[shape2.length - 3] = 3;
	Color defaultBackground = item.getDisplay().getSystemColor(parent.shouldHighlight() ? SWT.COLOR_LIST_SELECTION : SWT.COLOR_WIDGET_DISABLED_FOREGROUND);
	xx = x;
	yy = parent.onBottom ? y -1 : y + 1;
	ww = width;
	hh = height;
	if (!parent.single && !parent.simple) ww += curveWidth - curveIndent;
	drawBackground(gc, shape2, xx, yy, ww, hh, defaultBackground, null, null, null, false);
}

Which would already give you:

This of course needs to be improved to deal with all the different cases (such as highlight at bottom/bottom) and has to be simplified, but the PoC makes me think even more that it should be quite easy to adapt and resue the existing shape to draw a properly fitting highlight.
Unfortunately, the line still touched the icons on higher monitor zooms, so the values probably need some tweaking, but they were just random numbers for testing anyway.

[ShahzaibIbrahim]

I quickly hacked this demonstration (just adopted what is done for the background, removing all unnecessary stuff and adapted the bottom left and bottom right coordinates of the shape):

I used your snippet as a template to draw the highlight. Which in my opinion now looks better in both cases (top and bottom). Attaching the screenshot in order to compare

[HeikoKlare]

Reusing the existing shape is definitely a step forward. But with the current state the highlight line now becomes much thinner than it was before. At 100% is looks like this:

Current state:

With this PR:

In addition, the code needs to be cleaned up, as it currently (re)uses some random and badly named variables like xx, yy, shape2 etc. It's unclear why the GC background is set and reset while passing the color to some utility method as well. And is the call to drawBackground with so many null values necessary at all? I don't think there is much logic from drawBackground being used. It seems to be mostly about setting clipping and drawing a rectangle, so why not inline exactly the required code? Or maybe instead of setting clipping and drawing a rectangle, a drawPolygon could be used with the shape data.

[ShahzaibIbrahim]

Reusing the existing shape is definitely a step forward. But with the current state the highlight line now becomes much thinner than it was before. At 100% is looks like this:

These are the results after my latest changings. I think now thickness for the selection is ideal and in no zoom that I tested, the highlight line doesn't touches icon.

[HeikoKlare]

Please note that you have to use tabs with icons that start at the topmost pixel to have them touch the highlight line, such as the mentioned Manifest editor icon. With Java editor icons the behavior was always fine. This is how the current state looks at 150 with the Manifest editor:

The overall appearance of the highlight line is of course still better, but it still touches the icon.