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

Bug 218453

Summary:	Performance - Figure calls restoreState unnecessarily
Product:	[Tools] GEF	Reporter:	Randy Hudson <hudsonr>
Component:	GEF-Legacy Draw2d	Assignee:	gef-inbox <gef-inbox>
Status:	RESOLVED WONTFIX	QA Contact:
Severity:	trivial
Priority:	P3	CC:	nyssen
Version:	3.3
Target Milestone:	---
Hardware:	PC
OS:	Windows XP
Whiteboard:

Description Randy Hudson

2008-02-10 16:59:42 EST

There are a few occurrences of:
	graphics.popState();
	graphics.restoreState();
in the class Figure.  The second line is redundant and results in lots of unnecessary method calls and checks.

I haven't benchmarked the real impact, but seems trivial to delete the extra calls.

Comment 1 Alexander Nyßen

2011-03-10 17:01:04 EST

There are two occurrences of this kind within Figure#paintClientArea. However, if I remove the second call, in both cases, the circuit figure in the GEF logic example does not render correctly any more.

Comment 2 Alexander Nyßen

2011-03-10 17:30:41 EST

(In reply to comment #1)
> There are two occurrences of this kind within Figure#paintClientArea. However,
> if I remove the second call, in both cases, the circuit figure in the GEF logic
> example does not render correctly any more.

This is caused because CircuitFigure uses local coordinates. Within paintClientArea, only the second call clears the translation of the graphics that has been set directly before pushing the state.

  graphics.translate(getBounds().x + getInsets().left, getBounds().y
					+ getInsets().top);
  if (!optimizeClip)
    graphics.clipRect(getClientArea(PRIVATE_RECT));
  graphics.pushState();
  paintChildren(graphics);
  graphics.popState();
  graphics.restoreState();

Comment 3 Alexander Nyßen

2011-03-10 17:49:46 EST

If we are thus going to remove the second line in the two cases, we will have to add a restoreState() after the call to painClientArea() within paint as follows:

graphics.pushState();
try {
  paintFigure(graphics);
  graphics.restoreState();
  paintClientArea(graphics);
  graphics.restoreState(); // was not here before
  paintBorder(graphics);
} finally {
  graphics.popState();
}

As such the number of  calls to restoreState will be exactly the same. However, I would tend to do so, because the code will become a bit clearer.

Comment 4 Alexander Nyßen

2011-03-10 18:11:30 EST

But, changing this would be an API change, as the javadoc of paintClientArea currently states that should leave the graphics in its initial state upon returning. Resolving as wontfix because of this.