Adjust selection bounds for CTabRenderring

[ShahzaibIbrahim]

Selected tabs highlight look slightly off and more noticeable when zoom is not 100%. These adjusted value shows no gaps from top and better aligned highlights for tabs (Theme is enabled)

Results:

[github-actions]

Test Results

 3 018 files  ±0   3 018 suites  ±0   2h 10m 14s ⏱️ - 10m 17s
 8 270 tests ±0   8 022 ✅ ±0  248 💤 ±0  0 ❌ ±0 
23 634 runs  ±0  22 843 ✅ ±0  791 💤 ±0  0 ❌ ±0 

Results for commit e8ba2fa. ± Comparison against base commit 34498a4.

♻️ This comment has been updated with latest results.

[HeikoKlare]

With this change, the highlight line looks very thin (on 100%):

This is how it used to look:

[ShahzaibIbrahim]

@HeikoKlare I found the problem. Here were drawing the tab from (0,0) pixel but still using initial item location (1,1) and hence the gap was visible. I also updated the height to 3 when we draw from (0,0) so we have enough height for the selection marker.

The code that caused the gap:
https://github.com/vi-eclipse/eclipse.platform.ui/blob/8be04038fd7a1dd5cf4f7972e7813d0925181315/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/CTabRendering.java#L572C2-L577

[HeikoKlare]

The appearance is in my opinion much better than before. I have some questions/remarks on the current propose.
And the appearance with round tabs now became worse (than it already was before):

We may probably do something similar than in SWT to draw a proper shape. The current adjustments by 1 point based on cornerSize do not look sufficient.

[ShahzaibIbrahim]

The appearance is in my opinion much better than before. I have some questions/remarks on the current propose. And the appearance with round tabs now became worse (than it already was before):

We may probably do something similar than in SWT to draw a proper shape. The current adjustments by 1 point based on cornerSize do not look sufficient.

While I agree the rounded tabs should be adapted. But I wouldn't say it looks "worse" than master. Here's the SS comparing this PR and master state.

I would suggest that we fix the shape in another PR. This PR fixes the gap that we had from the top.

[amartya4256]

I pulled the branch and ran it on 250%. This is how it looks for me:

I can see a small gap in the left and at the top. Is it supposed to look like this?

[HeikoKlare]

I can see a small gap in the left and at the top. Is it supposed to look like this?

No, it should be fully aligned at the top, left and right without any gaps at every zoom.

[ShahzaibIbrahim]

I pulled the branch and ran it on 250%. This is how it looks for me:

I can see a small gap in the left and at the top. Is it supposed to look like this?

Are you sure you checked out master-517 from platform.ui? We also have master-517 for SWT. I checked again and there is no gap for the highlight.

[HeikoKlare]

@amartya4256 can you please retest this? Unfortunately, I have currently monitor available that I can set to 250%. Everything up to 225% looks fine to me.

[HeikoKlare]

I would suggest that we fix the shape in another PR. This PR fixes the gap that we had from the top.

@ShahzaibIbrahim But maybe that would require another readaptation of the calculations. Shouldn't we just fix it right away? Then it would also make more sense to take into account the outlineBoundsForOutline as those are used by the calculations for the (potentially rounded) borders.

[ShahzaibIbrahim]

I would suggest that we fix the shape in another PR. This PR fixes the gap that we had from the top.

@ShahzaibIbrahim But maybe that would require another readaptation of the calculations. Shouldn't we just fix it right away? Then it would also make more sense to take into account the outlineBoundsForOutline as those are used by the calculations for the (potentially rounded) borders.

I have added a commit to put highlight on rounded tabs. Although it looks big jagged as original one (non-selected rounded tabs) also bit jagged. I have reused the shape.

Here's the comparison of the selected and unselected tabs:

[eclipse-platform-bot]

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

bundles/org.eclipse.e4.ui.workbench.renderers.swt/META-INF/MANIFEST.MF

Warning

🚧 This PR cannot be modified by maintainers because edits are disabled or it is created from an organization repository. To obtain the required changes apply the git patch manually as an additional commit.

Git patch

From d1e7f2d1fabc7ac074af0a8ef8219a8b09b14361 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <platform-bot@eclipse.org>
Date: Mon, 15 Dec 2025 12:58:37 +0000
Subject: [PATCH] Version bump(s) for 4.39 stream


diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/META-INF/MANIFEST.MF b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/META-INF/MANIFEST.MF
index 249542c521..ca4f9ad426 100644
--- a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/META-INF/MANIFEST.MF
@@ -1,7 +1,7 @@
 Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-SymbolicName: org.eclipse.e4.ui.workbench.renderers.swt;singleton:=true
-Bundle-Version: 0.16.1000.qualifier
+Bundle-Version: 0.16.1100.qualifier
 Bundle-Name: %pluginName
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
-- 
2.52.0

Further information are available in Common Build Issues - Missing version increments.

[ShahzaibIbrahim]

After latest adaptation, there is no more "Bleeding" on the very left and right. The selection highlight looks fine to me in different zoom levels. Also addressed the comments from you @HeikoKlare. Let me know if I miss something.

[HeikoKlare]

We seem to be coming closer. The code became much simpler and many scenarios now look good.
But with rounded corner, the highlight is still far too large. At 150%, it looks like this (compare to the "correct" hight for the non-focused "problem" view below):

When reducing the hight by 1, I see the rendering artifacts mentioned before again.
The screenshots in the previous post seem to be taken with some custom auto-scaling settings. The tab icons are not scaled according to the text, so probably monitor-specific scaling is disabled or some custom auto-scale configuration is active.

[ShahzaibIbrahim]

We seem to be coming closer. The code became much simpler and many scenarios now look good. But with rounded corner, the highlight is still far too large. At 150%, it looks like this (compare to the "correct" hight for the non-focused "problem" view below):

I reduced the height by 1 and fixed the bleeding and this is how it looks now. For me its too small now. But if it feels okay to you I can push the changes.

[ShahzaibIbrahim]

As discussed earlier, the highlight height for the curved and non-curved tab folders should be same. I have adjusted the height now. This is the final result.

[HeikoKlare]

I looks almost fine now. But the lines still do not have the same height, such that the highlight bar with rounded corners appear a bit too thick and prominent. These are the heights I get:

• 100%: round corners 3px, square corners 2px
• 175%: round corners 5px, square corners 4px

Here is the comparison of the highlight at 100% with round corners before (top) and after (bottom) this change:

The overall appearance is definitely much better, but the line is thicker now.

[HeikoKlare]

Now with the latest changes it is too small again when placed at the bottom. In this screenshot, the left is the state now and the right is the state with this PR (at 150% zoom):

This is also somehow expected as the offset calculation used bounds.height - (highlightHeight - 1) before andnow uses bounds.height - (highlightHeight).

May I also ask to test every combination (rounded/square, bottom/top, different zooms) for the next review iteration yourself first?

[ShahzaibIbrahim]

Now with the latest changes it is too small again when placed at the bottom. In this screenshot, the left is the state now and the right is the state with this PR (at 150% zoom):

This is also somehow expected as the offset calculation used bounds.height - (highlightHeight - 1) before andnow uses bounds.height - (highlightHeight).

If you look the screenshot, the current state highlight is below the outline

And the after change, the size is as per the chart that I mentioned here in the comment (i.e. 3px at 150%): #3533 (comment)

[HeikoKlare]

Okay, I see the misplacement in the existing behavior. But it's misplaced with this PR as well, just in the opposite direction (it is placed too high instead too low). Note that the zoom I posted (150%) seems to actually have been incorrect for the screenshot. It must have been taken at 125%.

And the after change, the size is as per the chart that I mentioned here in the comment (i.e. 3px at 150%): #3533 (comment)

So why do we do it like that? While the highlight at the bottom was just misplaced before (at 125%), it is now misplaced and inconsistent to other highlights being show at the top (if not using round tabs, which is rather uncommon). Couldn't we draw an additional line (in addition to filling the rect) to compensate for the rounding error here? Since the outline is drawn at the y coordinate where the highlight is missing at 125%, it might be possible to compensate with that.

[ShahzaibIbrahim]

Okay, I see the misplacement in the existing behavior. But it's misplaced with this PR as well, just in the opposite direction (it is placed too high instead too low). Note that the zoom I posted (150%) seems to actually have been incorrect for the screenshot. It must have been taken at 125%.

And the after change, the size is as per the chart that I mentioned here in the comment (i.e. 3px at 150%): #3533 (comment)

So why do we do it like that? While the highlight at the bottom was just misplaced before (at 125%), it is now misplaced and inconsistent to other highlights being show at the top (if not using round tabs, which is rather uncommon). Couldn't we draw an additional line (in addition to filling the rect) to compensate for the rounding error here? Since the outline is drawn at the y coordinate where the highlight is missing at 125%, it might be possible to compensate with that.

I have now drawn a line on very bottom (the filler space), although It will make it look slightly darker (I have updated the values in the chart, see description). Also I have tested on every zoom which to me looks good now. The highlight is right on the line. The latest push doesn't affect top highlights (square or rounded). Attaching the screenshot for bottom highlight for all the zoom:

Bottom Highlight with rounded tabs on top

[HeikoKlare]

Let's take a step back here, as we seem to be running in circles. In my understanding, the goal of this change should be something like:

• Preserve the perfectly fine behavior at 100%
• Ensure that there are no unintended gaps under/over the highlight line at other zooms and ensure that they have a consistent appearance (consistent height for bottom-/top-placed highlights) at each zoom

With the current state, I see this on 100% (left is existing, right is with this PR), so changed behavior for 100%:

In particular, the behavior is also inconsistent, as you can see in the highlight on top being larger than the highlight at the bottom:

[ShahzaibIbrahim]

After discussion, I have made changes for this iteration which will make highlight consistent for all kind of tabs (Rounded, Square and bottom). The base size will be 2pts and it will increase as per zoom (2pts * (zoom/100)). The bottom highlight do have a bit of issue of outline at zoom >= 175 but that's how it currently is. So no change for that. Here are the result of each tab highlight in different zooms.

[HeikoKlare]

The current proposal seems to work decently fine when tab headers are placed at the top. The only limitation we see is that the highlights that are draw at the bottom end of a tab header (at the side of the tab contents) are a bit off on some zoom.

Unfortunately, the current implementation is fully incompatible with CTabFolders that have their tab headers placed at the bottom. I simply tested by adapting the CTabFolder constructor to always use onBottom = true and get this:

Even though the current state is not that great either (in terms of the highlights not having consistent heights), this PR would currently break the highlighting for such tab folders completely (top/bottom is swapped, placement is wrong and sometimes even the whole header is highlighted).

[HeikoKlare]

@ShahzaibIbrahim I have created #3656 to overcome the current limitations (in particular with headers placed at the bottom) of this proposal without messing up this PR. Can you please validate the PR in itself and against this one?

[ShahzaibIbrahim]

@ShahzaibIbrahim I have created #3656 to overcome the current limitations (in particular with headers placed at the bottom) of this proposal without messing up this PR. Can you please validate the PR in itself and against this one?

I have overcome the current problems with onBottom = true, and adapted few changes from your PR. Here are the results.

[HeikoKlare]

With the latest changes, onBottom looks like this:

100%:

150%:

Both don't look in any way as intended.

Note that the screenshots in the previous post don't show any onBottom CTabFolder. They just show folders that are "on the bottom" of a workbench window.

[ShahzaibIbrahim]

With the latest changes, onBottom looks like this:

100%:

150%:

Both don't look in any way as intended.

Note that the screenshots in the previous post don't show any onBottom CTabFolder. They just show folders that are "on the bottom" of a workbench window.

I misunderstood how onBottom works. So this time I have added a commit fixing the onBottom case. Here are the result

i have kept the commit separate for now just for you to see what I have done to fix it without affecting the case when onBottom==false. Hopefully now all the cases are covered correctly. The issue initially arose due to gc.drawPolyline cutting through the highlight and you see that gap.

[HeikoKlare]

I've tested this again and so far found two issues:

First, in case the highlight is not on top of a header, the highlight looks slightly cut off at the left end on zoom higher than 100%:

This is caused by the highlight starting at the same x pixel coordinate than the outline, but since the highlight is drawn one pixel too low, the outline does not cover the highlight at than one pixel below the outline.
Since we found that we cannot easily fix this issue of being one pixel too low, we may just also start a bit more to the right (like done now) via something like:

// When the rectangle is drawn at the bottom, the outline may not fully cover
// the left pixel such that we should start one point to the right (even though
// we produce a slight gap then)
int xOffset = highlightOnTop ? 0 : 1;
gc.fillRectangle(outlineBoundsForOutline.x + xOffset, highlightY,
		outlineBoundsForOutline.width - xOffset, highlightHeight);

One more severe observation is that when using onBottom the highlight on top "flickers" (i.e., the line become one pixel smaller/larger) on 125% when changing the position of the header by dragging the according part larger and smaller:

Edit: the latter one is also present in the current master state, so even though we do not improve that, I would not make anything worse. So we can probably ignore that for now.

I don't see severe blockers for this PR anymore. The one issue mentioned in my last review should be resolved before merging.

[ShahzaibIbrahim]

@HeikoKlare the mentioned issue with xOffset has been resolved. This PR is now ready for merging.

[akoch-yatta]

I tested it on Windows with and without rounded corner as well as with onBottom=true and false on the standard zooms in windows. Except the mentioned (pre-existing) issue, the behavior looks consistent over all zooms for me.