Bug 465644 - [GTK3] Investigate a GTK+ specific fix outside CTabFolder for rendering issues with GTK3
Summary: [GTK3] Investigate a GTK+ specific fix outside CTabFolder for rendering issue...
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.5   Edit
Hardware: PC Linux
: P3 major (vote)
Target Milestone: 4.6 M3   Edit
Assignee: Niraj Modi CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on: 460744 477798
Blocks: 484794
  Show dependency tree
 
Reported: 2015-04-28 03:23 EDT by Arun Thondapu CLA
Modified: 2017-02-16 04:32 EST (History)
17 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Arun Thondapu CLA 2015-04-28 03:23:01 EDT
+++ This bug was initially created as a clone of Bug #460744 +++

The original fix for bug 421127 for CTabFolder rendering issues on GTK3 caused bug 460744 on all platforms which was a performance issue (adding an item to CTabFolder forces a relayout on ALL siblings of the CTabFolder).

The performance problem has been fixed for all platforms including GTK+ now, but we should investigate if a fix that is not in CTabFolder code (that is common to all platforms) can be found, this bug will track the same.
Comment 1 Niraj Modi CLA 2015-10-05 11:02:29 EDT
A related CTabFolder bug 477798, which attempts to fix the root cause of CTabFolder initial size issue for all platforms via below gerrit patch:
https://git.eclipse.org/r/#/c/56655/

Once above changes gets in, it seems we can also get rid of GTK only code which forces a re-layouting call on parent control of CTabFolder.
Will share a gerrit for the same.
Comment 2 Eclipse Genie CLA 2015-10-06 02:46:53 EDT
New Gerrit change created: https://git.eclipse.org/r/57479
Comment 4 Niraj Modi CLA 2015-10-06 05:24:37 EDT
Tested the patch on Ubuntu14 with GTK3.10, pushed the changes to master.
Comment 5 Alexander Kurtakov CLA 2016-05-12 09:23:19 EDT
This bug has caused bug 487757. Should we revert it?
Comment 6 Andrey Loskutov CLA 2016-05-12 09:38:10 EDT
(In reply to Alexander Kurtakov from comment #5)
> This bug has caused bug 487757. Should we revert it?

I would say yes, since this is a regression for GTK3 MultiPageEditor's, but I'm not SWT committer to vote for it :-).
Comment 7 Niraj Modi CLA 2016-05-13 03:09:25 EDT
(In reply to Niraj Modi from comment #1)
> A related CTabFolder bug 477798, which attempts to fix the root cause of
> CTabFolder initial size issue for all platforms via below gerrit patch:
> https://git.eclipse.org/r/#/c/56655/
> 
> Once above changes gets in, it seems we can also get rid of GTK only code
> which forces a re-layouting call on parent control of CTabFolder.

Hi Arun,
The performance optimization that we did, to get rid of the GTK only code which forces a re-layouting call on parent control of CTabFolder.. has caused bug 487757

Going by observation by Andrey in comment 6, we should revert this GTK optimization CTabFolder in NeonRC2 ??
Comment 8 David Williams CLA 2016-05-13 12:49:26 EDT
I noticed this re-opened bug was still targeted to Neon M3, but the last comment mentions Neon RC2, so I did not want it to "get lost". 

Please retarget if I have misunderstood your plans for this bug.
Comment 9 Arun Thondapu CLA 2016-05-16 16:19:12 EDT
(In reply to Niraj Modi from comment #7)
> (In reply to Niraj Modi from comment #1)
> > A related CTabFolder bug 477798, which attempts to fix the root cause of
> > CTabFolder initial size issue for all platforms via below gerrit patch:
> > https://git.eclipse.org/r/#/c/56655/
> > 
> > Once above changes gets in, it seems we can also get rid of GTK only code
> > which forces a re-layouting call on parent control of CTabFolder.
> 
> Hi Arun,
> The performance optimization that we did, to get rid of the GTK only code
> which forces a re-layouting call on parent control of CTabFolder.. has
> caused bug 487757
> 
> Going by observation by Andrey in comment 6, we should revert this GTK
> optimization CTabFolder in NeonRC2 ??

We should ideally be fixing the actual root cause of bug 487757 instead of reverting this fix as any fix in CTabFolder for the GTK3 specific issue is only temporary and should not be necessary in the first place.

Since Eric is attempting to fix the root cause via bug 487160, I think we should leave this one as it is and not revert any changes.
Comment 10 Arun Thondapu CLA 2016-05-16 16:24:25 EDT
Moving back to RESOLVED state as bug 487160 has a patch that is targeted for RC2 and we may not need to make any changes regarding this bug.
Comment 11 Arun Thondapu CLA 2016-05-26 03:46:03 EDT
(In reply to Arun Thondapu from comment #10)
> Moving back to RESOLVED state as bug 487160 has a patch that is targeted for
> RC2 and we may not need to make any changes regarding this bug.

Reopening. Bug 487160 deals with a different issue and we don't have a fix for bug 487757 yet. Since bug 487757 affects usability with GTK3, its important to fix that one even if it means reverting the change for this bug and causing a performance impact for GTK3.
Comment 12 Arun Thondapu CLA 2016-05-26 03:48:59 EDT
We should find a proper fix for bug 487757 in 4.6.1 which does not depend on this workaround in CTabFolder. Targeting this bug to 4.6.1 accordingly.
Comment 13 Leo Ufimtsev CLA 2016-05-26 11:11:37 EDT
(In reply to Arun Thondapu from comment #12)
> We should find a proper fix for bug 487757 in 4.6.1 which does not depend on
> this workaround in CTabFolder. Targeting this bug to 4.6.1 accordingly.

Hello Arun, 

You pointed it out well. The underlying bug is somewhere in ScrolledComposite, CTabFolder was just masking an underlying fault in SWT/Gtk3.

I'll work on Bug 487757 now / this week to see if we can fix the underlying cause rather than revering CTabFolder fix.

Please hang on.
Comment 14 Arun Thondapu CLA 2016-05-26 14:13:11 EDT
Moving this bug back to RESOLVED state as reverting this fix doesn't actually seem to fix bug 487757 and therefore is not necessary.
Comment 15 Lars Vogel CLA 2017-02-16 04:32:38 EST
Opened Bug 512270 to remove the now unsed IS_GTK flag.