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

Bug 312352

Summary: Performance Regression: children outside given clipping are painted
Product: [Tools] GEF Reporter: Syed Atif <syedatif>
Component: GEF-Legacy Draw2dAssignee: Alex Boyko <aboyko>
Status: RESOLVED FIXED QA Contact:
Severity: major    
Priority: P3 CC: aboyko, ahunter.eclipse, dmisic, nyssen
Version: unspecified   
Target Milestone: 3.6.0 (Helios) RC1   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Bug Depends on: 195527    
Bug Blocks:    
Attachments:
Description Flags
Patch that fixes the problem none

Description Syed Atif CLA 2010-05-10 18:47:22 EDT 
Build Identifier: v20100502-2050

A recent change in the Figure class has allowed clients to define a clipping strategy (Bug 195527).

There was a change in the paintChildren() method of Figure which no longer checks if the clipping rectangle(s) given by the clipping strategy actually intersect with the parent. In essence, for layers such as connection layer, the entire area gets painted no matter what size the viewport is.

Here's how to reproduce in logic diagrams:
1. Create a diagram with two LEDs and a connection between them.
2. Create another pair of LEDs far to the right of the diagram so that to view any pair of LEDs you will have to scroll.
3. Put a breakpoint in Polyline#paint(..) method (you may need to define a default method that just calls its super).
4. Close and reopen the diagram. You will see that all LEDs and connections are drawn (using the breakpoint), even though only one pair of LEDs is visible in the viewport.

This is a major regression that can cause significant performance degradation in normal diagram editing/viewing (in cases where painting can take quite some time, as in for connection routing). For example, you can replace step 4 above with any other operation that requires a Figure#paint(..)

Reproducible: Always
Comment 1 Syed Atif CLA 2010-05-10 19:47:33 EDT 
Created attachment 167846 [details]
Patch that fixes the problem

A little note about the patch: if you open the patch in a text file, you will see all the lines in the Figure class removed and readded, with some of my changes added as well. If you apply this patch with Eclipse, you will see only my changes, which is good.

I cannot, for the life of me, create a patch that shows only what has changed. I have tried all possible permutations of creating a patch. From creating a patch with different patch options from the wizard to creating a patch with different eclipse releases (3.4, 3.5, 3.6)...and they all give the same output.

I follow this procedure: http://wiki.eclipse.org/CVS_FAQ#How_do_I_send_someone_a_patch.3F

If someone knows whether I am doing something wrong or whether this is a defect, please let me know, since I have come across the same issue last time as well.


Just to let you know what's actually in the patch, the following lines have been added after the line 1101 in the Figure class from head:


Rectangle clip = graphics.getClip(Rectangle.SINGLETON);
Rectangle clippingRect = clipping[j];
if (!clip.intersects(clippingRect)) {
	continue;
}
Comment 2 Alexander Nyßen CLA 2010-05-11 09:21:29 EDT 
The patch seems looks ok. However, if we are speaking of a performance regression, I would also recommend to remove the unneeded local variable clippingRect and change it to something like:

Rectangle clip = graphics.getClip(Rectangle.SINGLETON);
if (!clip.intersects(clipping[j])) {
	continue;
}
Comment 3 Alexander Nyßen CLA 2010-05-15 05:42:59 EDT 
The latest patch I have added to bug #195527 already takes this into account, so adding a depends on here.
Comment 4 Alexander Nyßen CLA 2010-05-17 14:36:16 EDT 
 Patch to bug #195527 has been applied to cvs HEAD. Syed, please confirm that the regression is resolved by this.
Comment 5 Alex Boyko CLA 2010-05-17 16:43:49 EDT 
I'll mark it as fixed for 2,3 RC1 - the fix seems to work ok for me. Syed will report if there is still a performance problem.
Comment 6 Alexander Nyßen CLA 2015-03-20 12:14:47 EDT 
Comment on attachment 167846 [details]
Patch that fixes the problem

Making obsolete and removing iplog+ as this patch has not been applied.