Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 562183

Summary: CTabRendering uses cornerSize in computeTrim but it should not.
Product: [Eclipse Project] Platform Reporter: Mike Marchand <mmarchand>
Component: UIAssignee: Mike Marchand <mmarchand>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: Lars.Vogel, philippe.dul
Version: 4.16   
Target Milestone: 4.16 M3   
Hardware: PC   
OS: Windows 10   
See Also: https://git.eclipse.org/r/161023
https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=40500009cce062f9a81533e94f1e310fdc9f76de
https://git.eclipse.org/r/161062
https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=382e93b1405079871ad2353e870e954e60422c57
Whiteboard:
Bug Depends on:    
Bug Blocks: 538740    
Attachments:
Description Flags
16px corners
none
20px corners
none
30px corners
none
40px corners
none
Tab outline
none
Post-fix 16px corner with 0 top padding
none
Post-fix 16px corner with 2px top padding none

Description Mike Marchand CLA 2020-04-15 16:35:39 EDT 
While working on Bug 538740, I noticed that the CTabRendering.computeTrim() function uses the cornerSize in the calculation.  

I've got a series of screenshots that I will attach to the bug that show different corner sizes and how the view rendering is affected.

16px
20px
30px
40px

As you will be able to see, the padding for the internal contents of view grows as the corner radius does.  Similarly, the controls in the header are also shifted to the left as the radius grows.

If we stop considering the cornerSize in the computeTrim function, we do need to consider that there is a tab outline (see screenshot of the outline in red) of 1px that is not accounted for in the trim calculations.  

The other thing that we will need to do is use CSS to specify that there is supposed to be padding at the top, currently we specify bottom, right and left padding, but we do not specify the top padding because of this bug (not sure if anyone noticed that the top padding was set to 0px but it was never ACTUALLY drawn as 0px).

Attaching an image to show the final results, with 0px top padding and with padding that matches the bottom, left and right with 2px.
Comment 1 Mike Marchand CLA 2020-04-15 16:36:20 EDT 
Created attachment 282455 [details]
16px corners
Comment 2 Mike Marchand CLA 2020-04-15 16:36:36 EDT 
Created attachment 282456 [details]
20px corners
Comment 3 Mike Marchand CLA 2020-04-15 16:36:58 EDT 
Created attachment 282457 [details]
30px corners
Comment 4 Mike Marchand CLA 2020-04-15 16:43:01 EDT 
Created attachment 282458 [details]
40px corners
Comment 5 Mike Marchand CLA 2020-04-15 16:44:49 EDT 
So, to reiterate, the 4 images demonstrate how corner radius affects more than just the radius in the trim computation.  It is important for the trim computation to be independent of the corner radius so that the view can always be drawn in a consistent manner.  This is going to be important for square tabs, which are corner radius 0.
Comment 6 Mike Marchand CLA 2020-04-15 16:45:37 EDT 
Created attachment 282459 [details]
Tab outline

This image shows the tab outline that is never accounted for in the trim computation.
Comment 7 Mike Marchand CLA 2020-04-15 16:47:23 EDT 
Created attachment 282460 [details]
Post-fix 16px corner with 0 top padding

This shows what it looks like after my fix, without changing the top padding in the css to match the left, right, bottom padding.  Finally, the padding for the contents of the view is 0px, as specified.
Comment 8 Mike Marchand CLA 2020-04-15 16:48:38 EDT 
Created attachment 282461 [details]
Post-fix 16px corner with 2px top padding

And at last, we have our view with a 2px top padding, matching the left, right and bottom padding.
Comment 9 Eclipse Genie CLA 2020-04-15 16:54:29 EDT 
New Gerrit change created: https://git.eclipse.org/r/161023
Comment 11 Philippe Dul CLA 2020-04-16 06:29:18 EDT 
Can you update eclipse.platform.ui\bundles\org.eclipse.ui.themes\css\e4_basestyle.css too ?

Thanks
Comment 12 Lars Vogel CLA 2020-04-16 06:29:57 EDT 
(In reply to Philippe Dul from comment #11)
> Can you update
> eclipse.platform.ui\bundles\org.eclipse.ui.themes\css\e4_basestyle.css too ?
> 
> Thanks

Philippe, please push Gerrit.
Comment 13 Eclipse Genie CLA 2020-04-16 08:37:41 EDT 
New Gerrit change created: https://git.eclipse.org/r/161062