Skip to content

Simplify shape calculations in CTabFolderRenderer#3172

Merged
HeikoKlare merged 1 commit intoeclipse-platform:masterfrom
HeikoKlare:ctabfolderrenderer-shape-simplification
Apr 2, 2026
Merged

Simplify shape calculations in CTabFolderRenderer#3172
HeikoKlare merged 1 commit intoeclipse-platform:masterfrom
HeikoKlare:ctabfolderrenderer-shape-simplification

Conversation

@HeikoKlare
Copy link
Copy Markdown
Contributor

With the removal of non-simple (curved) header in CTabFolders, the remaining calculations suited for variable corner shapes are overly complex. Most calculations now include coordinates from an EMPTY_CORNER dummy, which just contains a single point of values 0.

This change simplifies all calculations currently including EMPTY_CORNER.

See #3168 (comment)

Copy link
Copy Markdown
Contributor

@vogella vogella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good simplification — removing the EMPTY_CORNER loops is the right direction.

I did a similar cleanup in #3174 that goes a few steps further, in case it is useful as a reference:

What this PR does that #3174 does not

  • Nicely scoped: only removes the EMPTY_CORNER iteration, no other concerns mixed in ✓

What #3174 does additionally

  • Removes drawBorder(GC, int[] shape) and replaces drawLeft/RightUnselectedBorder with a single gc.drawLine() each (the polyline always described a straight vertical line)
  • Removes the Region-based polygon clipping from drawBackground() — since all shapes are now rectangles the region was always identical to the bounding rect, making the clipping a no-op
  • Removes the Region.subtract(shape) + fillRegion() block in drawTabArea that painted parent background into curved-corner cutouts (corners are square, so the subtracted region was always empty)

One potential issue to check

In drawSelected, the borderLeft == 0 && itemIndex == parent.firstIndex case uses:

shape[index++] = x;
shape[index++] = y + height;
if (borderLeft == 0 && itemIndex == parent.firstIndex) {
    shape[index - 2] += x;       // → x + x = 2x
    shape[index - 1] += y + height; // → (y+height) + (y+height) = 2(y+height)
}

This faithfully reproduces the original loop behaviour (x + left[0] where left = {x, y+height}), which only works correctly because x == 0 when borderLeft == 0. The intent might be clearer with an explicit inline value rather than the += mutation. The onBottom variant also loses the -1 offset that was present in the original (y + height + left[1] - 1), though with x=0 that edge case may not be visible in practice.

@vogella
Copy link
Copy Markdown
Contributor

vogella commented Apr 1, 2026

#3174 is the other approach to that.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 1, 2026

Test Results

  170 files  ±0    170 suites  ±0   26m 10s ⏱️ -12s
4 679 tests ±0  4 658 ✅ ±0   21 💤 ±0  0 ❌ ±0 
6 592 runs  ±0  6 437 ✅ ±0  155 💤 ±0  0 ❌ ±0 

Results for commit 5f4f1cd. ± Comparison against base commit 64c168e.

♻️ This comment has been updated with latest results.

@vogella vogella marked this pull request as ready for review April 2, 2026 07:33
@vogella
Copy link
Copy Markdown
Contributor

vogella commented Apr 2, 2026

LGTM, planning to merge after build verification and afterwards rework #3174 on top of this one considering also @HeikoKlare feedback

@akurtakov akurtakov force-pushed the ctabfolderrenderer-shape-simplification branch from 4a18900 to 7f99084 Compare April 2, 2026 09:06
With the removal of non-simple (curved) header in CTabFolders, the
remaining calculations suited for variable corner shapes are overly
complex. Most calculations now include coordinates from an EMPTY_CORNER
dummy, which just contains a single point of values 0.

This change simplifies all calculations currently including
EMPTY_CORNER.
@HeikoKlare HeikoKlare force-pushed the ctabfolderrenderer-shape-simplification branch from 7f99084 to 5f4f1cd Compare April 2, 2026 13:06
@HeikoKlare HeikoKlare merged commit 7d7570f into eclipse-platform:master Apr 2, 2026
24 checks passed
@HeikoKlare HeikoKlare deleted the ctabfolderrenderer-shape-simplification branch April 2, 2026 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants