Bug 312352 - Performance Regression: children outside given clipping are painted
Summary: Performance Regression: children outside given clipping are painted
Status: RESOLVED FIXED
Alias: None
Product: GEF
Classification: Tools
Component: GEF-Legacy Draw2d (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows XP
: P3 major (vote)
Target Milestone: 3.6.0 (Helios) RC1   Edit
Assignee: Alex Boyko CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 195527
Blocks:
  Show dependency tree
 
Reported: 2010-05-10 18:47 EDT by Syed Atif CLA
Modified: 2015-03-20 12:14 EDT (History)
4 users (show)

See Also:


Attachments
Patch that fixes the problem (105.79 KB, patch)
2010-05-10 19:47 EDT, Syed Atif CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.