threads / patch / 15734

patch, 8 partsSet table row height for the glog JTable

Subject: [EGIT PATCH 1/8] Set table row height for the glog JTable

## tl;dr

13 messages between Sep 30, 2008 and Oct 1, 2008. Diffs are folded; open one to read it.

replies: 12people: 2as markdown or json

Robin Rosenberg· Sep 30, 2008, 23:53 UTC · lore

For some obscure reason JTable has a fixed default row size of 16 pixels. This doesn't work well outside the default look-and-feels shipped with the JRE, e.g. the GTK look and feel for Linux.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../org/spearce/jgit/awtui/CommitGraphPane.java    |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)
Show changes to org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java +12 −1
diff --git a/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java b/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java
index 2be0e95..4ab2136 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java
@@ -52,6 +52,7 @@
 import javax.swing.table.AbstractTableModel;
 import javax.swing.table.DefaultTableCellRenderer;
 import javax.swing.table.JTableHeader;
+import javax.swing.table.TableCellRenderer;
 import javax.swing.table.TableColumn;
 import javax.swing.table.TableColumnModel;
 import javax.swing.table.TableModel;
@@ -83,8 +84,18 @@ public CommitGraphPane() {
 		allCommits = new SwingCommitList();
 		configureHeader();
 		setShowHorizontalLines(false);
-		setRowMargin(0);
 		setSelectionMode(ListSelectionModel.SINGLE_SELECTION);
+		configureRowHeight();
+	}
+
+	private void configureRowHeight() {
+		int h = 0;
+		for (int i = 0; i<getColumnCount(); ++i) {
+			TableCellRenderer renderer = getDefaultRenderer(getColumnClass(i));
+			Component c = renderer.getTableCellRendererComponent(this, "ÅOj", false, false, 0, i);
+			h = Math.max(h, c.getPreferredSize().height);
+		}
+		setRowHeight(h + getRowMargin());
 	}
 
 	/**
-- 
1.6.0.1.310.gf789d0.dirty
Robin Rosenberg· Sep 30, 2008, 23:53 UTC · re: Robin Rosenberg · lore

[EGIT PATCH 2/8] Move AWTPlotRenderer to its own file.

This is mostly a convenience issue as it allows the use of the JVM hotswap feature while debugging.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../org/spearce/jgit/awtui/AWTPlotRenderer.java    |  104 ++++++++++++++++++++
 .../org/spearce/jgit/awtui/CommitGraphPane.java    |   92 -----------------
 2 files changed, 104 insertions(+), 92 deletions(-)
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
Show changes to 2 files +104 −92

org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java, org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java

diff --git a/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java b/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
new file mode 100644
index 0000000..a9933a4
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
@@ -0,0 +1,104 @@
+/**
+ * 
+ */
+package org.spearce.jgit.awtui;
+
+import java.awt.Color;
+import java.awt.Graphics;
+import java.awt.Graphics2D;
+import java.awt.Polygon;
+
+import org.spearce.jgit.awtui.CommitGraphPane.GraphCellRender;
+import org.spearce.jgit.awtui.SwingCommitList.SwingLane;
+import org.spearce.jgit.lib.Ref;
+import org.spearce.jgit.revplot.AbstractPlotRenderer;
+import org.spearce.jgit.revplot.PlotCommit;
+
+final class AWTPlotRenderer extends AbstractPlotRenderer<SwingLane, Color> {
+
+	final GraphCellRender cell;
+
+	Graphics2D g;
+
+	AWTPlotRenderer(final GraphCellRender c) {
+		cell = c;
+	}
+
+	void paint(final Graphics in, final PlotCommit<SwingLane> commit) {
+		g = (Graphics2D) in.create();
+		try {
+			final int h = cell.getHeight();
+			g.setColor(cell.getBackground());
+			g.fillRect(0, 0, cell.getWidth(), h);
+			if (commit != null)
+				paintCommit(commit, h);
+		} finally {
+			g.dispose();
+			g = null;
+		}
+	}
+
+	@Override
+	protected void drawLine(final Color color, int x1, int y1, int x2,
+			int y2, int width) {
+		if (y1 == y2) {
+			x1 -= width / 2;
+			x2 -= width / 2;
+		} else if (x1 == x2) {
+			y1 -= width / 2;
+			y2 -= width / 2;
+		}
+
+		g.setColor(color);
+		g.setStroke(CommitGraphPane.stroke(width));
+		g.drawLine(x1, y1, x2, y2);
+	}
+
+	@Override
+	protected void drawCommitDot(final int x, final int y, final int w,
+			final int h) {
+		g.setColor(Color.blue);
+		g.setStroke(CommitGraphPane.strokeCache[1]);
+		g.fillOval(x, y, w, h);
+		g.setColor(Color.black);
+		g.drawOval(x, y, w, h);
+	}
+
+	@Override
+	protected void drawBoundaryDot(final int x, final int y, final int w,
+			final int h) {
+		g.setColor(cell.getBackground());
+		g.setStroke(CommitGraphPane.strokeCache[1]);
+		g.fillOval(x, y, w, h);
+		g.setColor(Color.black);
+		g.drawOval(x, y, w, h);
+	}
+
+	@Override
+	protected void drawText(final String msg, final int x, final int y) {
+		final int texty = g.getFontMetrics().getHeight()
+				- g.getFontMetrics().getDescent();
+		g.setColor(cell.getForeground());
+		g.drawString(msg, x, texty - (cell.getHeight() - y * 2));
+	}
+
+	@Override
+	protected Color laneColor(final SwingLane myLane) {
+		return myLane != null ? myLane.color : Color.black;
+	}
+
+	void paintTriangleDown(final int cx, final int y, final int h) {
+		final int tipX = cx;
+		final int tipY = y + h;
+		final int baseX1 = cx - 10 / 2;
+		final int baseX2 = tipX + 10 / 2;
+		final int baseY = y;
+		final Polygon triangle = new Polygon();
+		triangle.addPoint(tipX, tipY);
+		triangle.addPoint(baseX1, baseY);
+		triangle.addPoint(baseX2, baseY);
+		g.fillPolygon(triangle);
+		g.drawPolygon(triangle);
+	}
+
+}
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java b/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java
index 4ab2136..d35bd5e 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java
@@ -38,11 +38,8 @@
 package org.spearce.jgit.awtui;
 
 import java.awt.BasicStroke;
-import java.awt.Color;
 import java.awt.Component;
 import java.awt.Graphics;
-import java.awt.Graphics2D;
-import java.awt.Polygon;
 import java.awt.Stroke;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
@@ -59,7 +56,6 @@
 
 import org.spearce.jgit.awtui.SwingCommitList.SwingLane;
 import org.spearce.jgit.lib.PersonIdent;
-import org.spearce.jgit.revplot.AbstractPlotRenderer;
 import org.spearce.jgit.revplot.PlotCommit;
 import org.spearce.jgit.revplot.PlotCommitList;
 
@@ -251,92 +247,4 @@ static Stroke stroke(final int width) {
 		return new BasicStroke(width);
 	}
 
-	final class AWTPlotRenderer extends AbstractPlotRenderer<SwingLane, Color> {
-
-		final GraphCellRender cell;
-
-		Graphics2D g;
-
-		AWTPlotRenderer(final GraphCellRender c) {
-			cell = c;
-		}
-
-		void paint(final Graphics in, final PlotCommit<SwingLane> commit) {
-			g = (Graphics2D) in.create();
-			try {
-				final int h = cell.getHeight();
-				g.setColor(cell.getBackground());
-				g.fillRect(0, 0, cell.getWidth(), h);
-				if (commit != null)
-					paintCommit(commit, h);
-			} finally {
-				g.dispose();
-				g = null;
-			}
-		}
-
-		@Override
-		protected void drawLine(final Color color, int x1, int y1, int x2,
-				int y2, int width) {
-			if (y1 == y2) {
-				x1 -= width / 2;
-				x2 -= width / 2;
-			} else if (x1 == x2) {
-				y1 -= width / 2;
-				y2 -= width / 2;
-			}
-
-			g.setColor(color);
-			g.setStroke(stroke(width));
-			g.drawLine(x1, y1, x2, y2);
-		}
-
-		@Override
-		protected void drawCommitDot(final int x, final int y, final int w,
-				final int h) {
-			g.setColor(Color.blue);
-			g.setStroke(strokeCache[1]);
-			g.fillOval(x, y, w, h);
-			g.setColor(Color.black);
-			g.drawOval(x, y, w, h);
-		}
-
-		@Override
-		protected void drawBoundaryDot(final int x, final int y, final int w,
-				final int h) {
-			g.setColor(cell.getBackground());
-			g.setStroke(strokeCache[1]);
-			g.fillOval(x, y, w, h);
-			g.setColor(Color.black);
-			g.drawOval(x, y, w, h);
-		}
-
-		@Override
-		protected void drawText(final String msg, final int x, final int y) {
-			final int texty = g.getFontMetrics().getHeight()
-					- g.getFontMetrics().getDescent();
-			g.setColor(cell.getForeground());
-			g.drawString(msg, x, texty - (cell.getHeight() - y * 2));
-		}
-
-		@Override
-		protected Color laneColor(final SwingLane myLane) {
-			return myLane != null ? myLane.color : Color.black;
-		}
-
-		void paintTriangleDown(final int cx, final int y, final int h) {
-			final int tipX = cx;
-			final int tipY = y + h;
-			final int baseX1 = cx - 10 / 2;
-			final int baseX2 = tipX + 10 / 2;
-			final int baseY = y;
-			final Polygon triangle = new Polygon();
-			triangle.addPoint(tipX, tipY);
-			triangle.addPoint(baseX1, baseY);
-			triangle.addPoint(baseX2, baseY);
-			g.fillPolygon(triangle);
-			g.drawPolygon(triangle);
-		}
-	}
-
 }
-- 
1.6.0.1.310.gf789d0.dirty
Robin Rosenberg· Sep 30, 2008, 23:53 UTC · re: Robin Rosenberg · lore

[EGIT PATCH 3/8] Dispose of allocated colors on finalize()

---
 .../egit/ui/internal/history/SWTPlotRenderer.java  |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
Show changes to org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java +11 −0
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
index 23ec255..a58b3bf 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
@@ -45,6 +45,17 @@ SWTPlotRenderer(final Display d) {
 		sys_darkblue = d.getSystemColor(SWT.COLOR_DARK_BLUE);
 	}
 
+	@Override
+	protected void finalize() throws Throwable {
+		sys_black.dispose();
+		sys_blue.dispose();
+		sys_gray.dispose();
+		sys_darkblue.dispose();
+		sys_yellow.dispose();
+		sys_green.dispose();
+		sys_white.dispose();
+	}
+	
 	void paint(final Event event) {
 		g = event.gc;
 		cellX = event.x;
-- 
1.6.0.1.310.gf789d0.dirty
Robin Rosenberg· Sep 30, 2008, 23:53 UTC · re: Robin Rosenberg · lore

[EGIT PATCH 4/8] Align commit text properly in jgit glog

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../egit/ui/internal/history/SWTPlotRenderer.java  |    3 ---
 .../org/spearce/jgit/awtui/AWTPlotRenderer.java    |    7 +++----
 2 files changed, 3 insertions(+), 7 deletions(-)
Show changes to 2 files +3 −7

org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java, org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
index a58b3bf..7c286a4 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
@@ -51,9 +51,6 @@ protected void finalize() throws Throwable {
 		sys_blue.dispose();
 		sys_gray.dispose();
 		sys_darkblue.dispose();
-		sys_yellow.dispose();
-		sys_green.dispose();
-		sys_white.dispose();
 	}
 	
 	void paint(final Event event) {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java b/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
index a9933a4..5090ec9 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
@@ -10,7 +10,6 @@
 
 import org.spearce.jgit.awtui.CommitGraphPane.GraphCellRender;
 import org.spearce.jgit.awtui.SwingCommitList.SwingLane;
-import org.spearce.jgit.lib.Ref;
 import org.spearce.jgit.revplot.AbstractPlotRenderer;
 import org.spearce.jgit.revplot.PlotCommit;
 
@@ -76,10 +75,10 @@ protected void drawBoundaryDot(final int x, final int y, final int w,
 
 	@Override
 	protected void drawText(final String msg, final int x, final int y) {
-		final int texty = g.getFontMetrics().getHeight()
-				- g.getFontMetrics().getDescent();
+		final int texth = g.getFontMetrics().getHeight();
+		final int y0 = y - texth/2 + (cell.getHeight() - texth)/2;
 		g.setColor(cell.getForeground());
-		g.drawString(msg, x, texty - (cell.getHeight() - y * 2));
+		g.drawString(msg, x, y0 + texth - g.getFontMetrics().getDescent());
 	}
 
 	@Override
-- 
1.6.0.1.310.gf789d0.dirty
Robin Rosenberg· Oct 1, 2008, 00:02 UTC · re: Robin Rosenberg · lore

Re: [EGIT PATCH 4/8] Align commit text properly in jgit glog

The Series actually ends here for now. The rest are not ready yet but can be reviews if you are curious on my tagdecoration branch. These bug fixes came out as a side effect.

-- robin
Shawn O. Pearce· Oct 1, 2008, 14:38 UTC · re: Robin Rosenberg · lore

Re: [EGIT PATCH 4/8] Align commit text properly in jgit glog

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
Show 18 quoted lines
> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> ---
>  .../egit/ui/internal/history/SWTPlotRenderer.java  |    3 ---
>  .../org/spearce/jgit/awtui/AWTPlotRenderer.java    |    7 +++----
>  2 files changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
> index a58b3bf..7c286a4 100644
> --- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
> +++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
> @@ -51,9 +51,6 @@ protected void finalize() throws Throwable {
>  		sys_blue.dispose();
>  		sys_gray.dispose();
>  		sys_darkblue.dispose();
> -		sys_yellow.dispose();
> -		sys_green.dispose();
> -		sys_white.dispose();
>  	}

Can't you squash this hunk to the prior commit, assuming the prior commit is still a valid change?

-- 
Shawn.
Robin Rosenberg· Oct 1, 2008, 19:31 UTC · re: Robin Rosenberg · lore

[EGIT PATCH 1/3] Set table row height for the glog JTable

For some obscure reason JTable has a fixed default row size of 16 pixels. This doesn't work well outside the default look-and-feels shipped with the JRE, e.g. the GTK look and feel for Linux.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../org/spearce/jgit/awtui/CommitGraphPane.java    |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)
Show changes to org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java +12 −1
diff --git a/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java b/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java
index 2be0e95..d778821 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java
@@ -52,6 +52,7 @@
 import javax.swing.table.AbstractTableModel;
 import javax.swing.table.DefaultTableCellRenderer;
 import javax.swing.table.JTableHeader;
+import javax.swing.table.TableCellRenderer;
 import javax.swing.table.TableColumn;
 import javax.swing.table.TableColumnModel;
 import javax.swing.table.TableModel;
@@ -83,8 +84,18 @@ public CommitGraphPane() {
 		allCommits = new SwingCommitList();
 		configureHeader();
 		setShowHorizontalLines(false);
-		setRowMargin(0);
 		setSelectionMode(ListSelectionModel.SINGLE_SELECTION);
+		configureRowHeight();
+	}
+
+	private void configureRowHeight() {
+		int h = 0;
+		for (int i = 0; i<getColumnCount(); ++i) {
+			TableCellRenderer renderer = getDefaultRenderer(getColumnClass(i));
+			Component c = renderer.getTableCellRendererComponent(this, "\u00c5Oj", false, false, 0, i);
+			h = Math.max(h, c.getPreferredSize().height);
+		}
+		setRowHeight(h + getRowMargin());
 	}
 
 	/**
-- 
1.6.0.1.310.gf789d0.dirty
Robin Rosenberg· Oct 1, 2008, 19:31 UTC · re: Robin Rosenberg · lore

[EGIT PATCH 2/3] Move AWTPlotRenderer to its own file.

This is mostly a convenience issue as it allows the use of the JVM hotswap feature while debugging.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../org/spearce/jgit/awtui/AWTPlotRenderer.java    |  137 ++++++++++++++++++++
 .../org/spearce/jgit/awtui/CommitGraphPane.java    |   92 -------------
 2 files changed, 137 insertions(+), 92 deletions(-)
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
Show changes to 2 files +137 −92

org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java, org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java

diff --git a/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java b/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
new file mode 100644
index 0000000..dc785ec
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.awtui;
+
+import java.awt.Color;
+import java.awt.Graphics;
+import java.awt.Graphics2D;
+import java.awt.Polygon;
+
+import org.spearce.jgit.awtui.CommitGraphPane.GraphCellRender;
+import org.spearce.jgit.awtui.SwingCommitList.SwingLane;
+import org.spearce.jgit.revplot.AbstractPlotRenderer;
+import org.spearce.jgit.revplot.PlotCommit;
+
+final class AWTPlotRenderer extends AbstractPlotRenderer<SwingLane, Color> {
+
+	final GraphCellRender cell;
+
+	Graphics2D g;
+
+	AWTPlotRenderer(final GraphCellRender c) {
+		cell = c;
+	}
+
+	void paint(final Graphics in, final PlotCommit<SwingLane> commit) {
+		g = (Graphics2D) in.create();
+		try {
+			final int h = cell.getHeight();
+			g.setColor(cell.getBackground());
+			g.fillRect(0, 0, cell.getWidth(), h);
+			if (commit != null)
+				paintCommit(commit, h);
+		} finally {
+			g.dispose();
+			g = null;
+		}
+	}
+
+	@Override
+	protected void drawLine(final Color color, int x1, int y1, int x2,
+			int y2, int width) {
+		if (y1 == y2) {
+			x1 -= width / 2;
+			x2 -= width / 2;
+		} else if (x1 == x2) {
+			y1 -= width / 2;
+			y2 -= width / 2;
+		}
+
+		g.setColor(color);
+		g.setStroke(CommitGraphPane.stroke(width));
+		g.drawLine(x1, y1, x2, y2);
+	}
+
+	@Override
+	protected void drawCommitDot(final int x, final int y, final int w,
+			final int h) {
+		g.setColor(Color.blue);
+		g.setStroke(CommitGraphPane.strokeCache[1]);
+		g.fillOval(x, y, w, h);
+		g.setColor(Color.black);
+		g.drawOval(x, y, w, h);
+	}
+
+	@Override
+	protected void drawBoundaryDot(final int x, final int y, final int w,
+			final int h) {
+		g.setColor(cell.getBackground());
+		g.setStroke(CommitGraphPane.strokeCache[1]);
+		g.fillOval(x, y, w, h);
+		g.setColor(Color.black);
+		g.drawOval(x, y, w, h);
+	}
+
+	@Override
+	protected void drawText(final String msg, final int x, final int y) {
+		final int texty = g.getFontMetrics().getHeight()
+				- g.getFontMetrics().getDescent();
+		g.setColor(cell.getForeground());
+		g.drawString(msg, x, texty - (cell.getHeight() - y * 2));
+	}
+
+	@Override
+	protected Color laneColor(final SwingLane myLane) {
+		return myLane != null ? myLane.color : Color.black;
+	}
+
+	void paintTriangleDown(final int cx, final int y, final int h) {
+		final int tipX = cx;
+		final int tipY = y + h;
+		final int baseX1 = cx - 10 / 2;
+		final int baseX2 = tipX + 10 / 2;
+		final int baseY = y;
+		final Polygon triangle = new Polygon();
+		triangle.addPoint(tipX, tipY);
+		triangle.addPoint(baseX1, baseY);
+		triangle.addPoint(baseX2, baseY);
+		g.fillPolygon(triangle);
+		g.drawPolygon(triangle);
+	}
+
+}
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java b/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java
index d778821..c8c5a06 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/awtui/CommitGraphPane.java
@@ -38,11 +38,8 @@
 package org.spearce.jgit.awtui;
 
 import java.awt.BasicStroke;
-import java.awt.Color;
 import java.awt.Component;
 import java.awt.Graphics;
-import java.awt.Graphics2D;
-import java.awt.Polygon;
 import java.awt.Stroke;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
@@ -59,7 +56,6 @@
 
 import org.spearce.jgit.awtui.SwingCommitList.SwingLane;
 import org.spearce.jgit.lib.PersonIdent;
-import org.spearce.jgit.revplot.AbstractPlotRenderer;
 import org.spearce.jgit.revplot.PlotCommit;
 import org.spearce.jgit.revplot.PlotCommitList;
 
@@ -251,92 +247,4 @@ static Stroke stroke(final int width) {
 		return new BasicStroke(width);
 	}
 
-	final class AWTPlotRenderer extends AbstractPlotRenderer<SwingLane, Color> {
-
-		final GraphCellRender cell;
-
-		Graphics2D g;
-
-		AWTPlotRenderer(final GraphCellRender c) {
-			cell = c;
-		}
-
-		void paint(final Graphics in, final PlotCommit<SwingLane> commit) {
-			g = (Graphics2D) in.create();
-			try {
-				final int h = cell.getHeight();
-				g.setColor(cell.getBackground());
-				g.fillRect(0, 0, cell.getWidth(), h);
-				if (commit != null)
-					paintCommit(commit, h);
-			} finally {
-				g.dispose();
-				g = null;
-			}
-		}
-
-		@Override
-		protected void drawLine(final Color color, int x1, int y1, int x2,
-				int y2, int width) {
-			if (y1 == y2) {
-				x1 -= width / 2;
-				x2 -= width / 2;
-			} else if (x1 == x2) {
-				y1 -= width / 2;
-				y2 -= width / 2;
-			}
-
-			g.setColor(color);
-			g.setStroke(stroke(width));
-			g.drawLine(x1, y1, x2, y2);
-		}
-
-		@Override
-		protected void drawCommitDot(final int x, final int y, final int w,
-				final int h) {
-			g.setColor(Color.blue);
-			g.setStroke(strokeCache[1]);
-			g.fillOval(x, y, w, h);
-			g.setColor(Color.black);
-			g.drawOval(x, y, w, h);
-		}
-
-		@Override
-		protected void drawBoundaryDot(final int x, final int y, final int w,
-				final int h) {
-			g.setColor(cell.getBackground());
-			g.setStroke(strokeCache[1]);
-			g.fillOval(x, y, w, h);
-			g.setColor(Color.black);
-			g.drawOval(x, y, w, h);
-		}
-
-		@Override
-		protected void drawText(final String msg, final int x, final int y) {
-			final int texty = g.getFontMetrics().getHeight()
-					- g.getFontMetrics().getDescent();
-			g.setColor(cell.getForeground());
-			g.drawString(msg, x, texty - (cell.getHeight() - y * 2));
-		}
-
-		@Override
-		protected Color laneColor(final SwingLane myLane) {
-			return myLane != null ? myLane.color : Color.black;
-		}
-
-		void paintTriangleDown(final int cx, final int y, final int h) {
-			final int tipX = cx;
-			final int tipY = y + h;
-			final int baseX1 = cx - 10 / 2;
-			final int baseX2 = tipX + 10 / 2;
-			final int baseY = y;
-			final Polygon triangle = new Polygon();
-			triangle.addPoint(tipX, tipY);
-			triangle.addPoint(baseX1, baseY);
-			triangle.addPoint(baseX2, baseY);
-			g.fillPolygon(triangle);
-			g.drawPolygon(triangle);
-		}
-	}
-
 }
-- 
1.6.0.1.310.gf789d0.dirty
Robin Rosenberg· Oct 1, 2008, 19:31 UTC · re: Robin Rosenberg · lore

[EGIT PATCH 3/3] Align commit text properly in jgit glog

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 .../org/spearce/jgit/awtui/AWTPlotRenderer.java    |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
Show changes to org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java +3 −3
diff --git a/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java b/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
index dc785ec..b6b715c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
@@ -109,10 +109,10 @@ protected void drawBoundaryDot(final int x, final int y, final int w,
 
 	@Override
 	protected void drawText(final String msg, final int x, final int y) {
-		final int texty = g.getFontMetrics().getHeight()
-				- g.getFontMetrics().getDescent();
+		final int texth = g.getFontMetrics().getHeight();
+		final int y0 = y - texth/2 + (cell.getHeight() - texth)/2;
 		g.setColor(cell.getForeground());
-		g.drawString(msg, x, texty - (cell.getHeight() - y * 2));
+		g.drawString(msg, x, y0 + texth - g.getFontMetrics().getDescent());
 	}
 
 	@Override
-- 
1.6.0.1.310.gf789d0.dirty
Shawn O. Pearce· Oct 1, 2008, 14:37 UTC · re: Robin Rosenberg · lore

Re: [EGIT PATCH 3/8] Dispose of allocated colors on finalize()

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> ---
Missing SBO.
Show 21 quoted lines
>  .../egit/ui/internal/history/SWTPlotRenderer.java  |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
> index 23ec255..a58b3bf 100644
> --- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
> +++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/history/SWTPlotRenderer.java
> @@ -45,6 +45,17 @@ SWTPlotRenderer(final Display d) {
>  		sys_darkblue = d.getSystemColor(SWT.COLOR_DARK_BLUE);
>  	}
>  
> +	@Override
> +	protected void finalize() throws Throwable {
> +		sys_black.dispose();
> +		sys_blue.dispose();
> +		sys_gray.dispose();
> +		sys_darkblue.dispose();
> +		sys_yellow.dispose();
> +		sys_green.dispose();
> +		sys_white.dispose();
> +	}

I think this is wrong. Any color that we get from Display.getSystemColor() must not be disposed of by the application, its in use by the Display and/or other parts of the application.

>From the Javadocs:
  This color should not be free'd because it was allocated by the
  system, not the application.
I take that to mean that the color should not be disposed of.

Though reading the Gtk SWT sources it seems that the system color objects aren't pooled, each call to getSystemColor causes a new Color instace to be allocated. So it may be harmless on Gtk.

What was the rationale for disposing of these resources? Did you identify that this is a resource leak somewhere? Because I'd like to make sure I actually understand the SWT resource model better so I don't commit mistakes in the future.

-- 
Shawn.
Robin Rosenberg· Oct 1, 2008, 17:48 UTC · re: Shawn O. Pearce · lore

Re: [EGIT PATCH 3/8] Dispose of allocated colors on finalize()

onsdagen den 1 oktober 2008 16.37.18 skrev Shawn O. Pearce:
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
[..]
> I think this is wrong.  Any color that we get from
[...]
> What was the rationale for disposing of these resources?  Did you
> identify that this is a resource leak somewhere?  Because I'd like
> to make sure I actually understand the SWT resource model better
> so I don't commit mistakes in the future.

I read the Color javadoc, but not the getSystemColor one. You are right, drop this patch and the "dispose" hunk in the next patch (which was also a

-- robin
Shawn O. Pearce· Oct 1, 2008, 14:32 UTC · re: Robin Rosenberg · lore

Re: [EGIT PATCH 2/8] Move AWTPlotRenderer to its own file.

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
Show 20 quoted lines
> This is mostly a convenience issue as it allows the
> use of the JVM hotswap feature while debugging.
> 
> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> ---
>  .../org/spearce/jgit/awtui/AWTPlotRenderer.java    |  104 ++++++++++++++++++++
>  .../org/spearce/jgit/awtui/CommitGraphPane.java    |   92 -----------------
>  2 files changed, 104 insertions(+), 92 deletions(-)
>  create mode 100644 org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
> 
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java b/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
> new file mode 100644
> index 0000000..a9933a4
> --- /dev/null
> +++ b/org.spearce.jgit/src/org/spearce/jgit/awtui/AWTPlotRenderer.java
> @@ -0,0 +1,104 @@
> +/**
> + * 
> + */
> +package org.spearce.jgit.awtui;

Missing copyright from the old sources. Please carry over the existing copyright header.

-- 
Shawn.

← back to recent threads