This patch (committed) fixes a few problems in our JSlider implementation:
(1) the tick marks don't align exactly with the slider thumb (noticeable at the end
points);
(2) the labels are clipped;
(3) the label table can be regenerated when it shouldn't be,
(4) toggling the paintTicks flag doesn't update the component correctly,
(5) the buffer area at each end of the slider isn't calculated the same way as the
reference implementation,
(6) alignment of the slider is incorrect for some layouts.
The ChangeLog entry is:
2006-03-29 David Gilbert <[EMAIL PROTECTED]>
* javax/swing/JSlider.java
(setPaintLabels): Only create standard labels if labelTable is null,
* javax/swing/plaf/basic/BasicSliderUI.java
(PropertyChangeHandler.propertyChange): Recalculate geometry for
"paintTicks" property change,
(calculateThumbSize): Updated API docs,
(calculateContentRect): Likewise,
(calculateTrackBuffer): Take into account the lowest and highest
labels when calculating buffer space,
(calculateTrackRect): Include labels, if visible, in the calculation of
the trackRect position,
(calculateTickRect): Height is zero if ticks are not painted,
(calculateLabelRect): Use max dimensions of actual labels,
(getWidthOfHighValueLabel): Use preferred size,
(getWidthOfLowValueLabel): Likewise,
(getHeightOfHighValueLabel): Likewise,
(getHeightOfLowValueLabel): Likewise,
(drawInverted): Just return slider setting,
(getHighestValueLabel): Updated API docs,
(paintTicks): Removed redundant (and buggy) code, replaced with calls
to xPositionForValue() and yPositionForValue(),
(paintHorizontalLabel): Removed full qualification of class name,
(paintVerticalLabel): Likewise,
(xPositionForValue): Reimplemented,
(yPositionForValue): Reimplemented,
* javax/swing/plaf/metal/MetalSliderUI.java
(paintTrack): Made track one pixel longer.
Regards,
Dave
Index: javax/swing/JSlider.java
===================================================================
RCS file: /sources/classpath/classpath/javax/swing/JSlider.java,v
retrieving revision 1.25
diff -u -r1.25 JSlider.java
--- javax/swing/JSlider.java 27 Mar 2006 14:33:10 -0000 1.25
+++ javax/swing/JSlider.java 29 Mar 2006 21:12:12 -0000
@@ -1061,7 +1061,7 @@
if (paint != paintLabels)
{
paintLabels = paint;
- if (paint && majorTickSpacing > 0)
+ if (paint && majorTickSpacing > 0 && labelTable == null)
labelTable = createStandardLabels(majorTickSpacing);
firePropertyChange("paintLabels", !paint, paint);
}
Index: javax/swing/plaf/basic/BasicSliderUI.java
===================================================================
RCS file:
/sources/classpath/classpath/javax/swing/plaf/basic/BasicSliderUI.java,v
retrieving revision 1.26
diff -u -r1.26 BasicSliderUI.java
--- javax/swing/plaf/basic/BasicSliderUI.java 29 Mar 2006 09:13:46 -0000
1.26
+++ javax/swing/plaf/basic/BasicSliderUI.java 29 Mar 2006 21:12:16 -0000
@@ -248,6 +248,8 @@
slider.getModel().addChangeListener(changeListener);
calculateThumbLocation();
}
+ else if (e.getPropertyName().equals("paintTicks"))
+ calculateGeometry();
// elif the componentOrientation changes (this is a bound property,
// just undocumented) we change leftToRightCache. In Sun's
@@ -1135,8 +1137,8 @@
}
/**
- * This method calculates the size but not the position of the thumbRect. It
- * must take into account the orientation of the slider.
+ * Sets the width and height of the <code>thumbRect</code> field, using the
+ * dimensions returned by [EMAIL PROTECTED] #getThumbSize()}.
*/
protected void calculateThumbSize()
{
@@ -1150,8 +1152,9 @@
}
/**
- * This method calculates the size and position of the contentRect. This
- * method does not need to be called if the orientation changes.
+ * Updates the <code>contentRect</code> field to an area inside the
+ * <code>focusRect</code>. This method does not need to be called if the
+ * orientation changes.
*/
protected void calculateContentRect()
{
@@ -1189,15 +1192,25 @@
}
/**
- * Calculates the gap size between the left edge of the contentRect and the
- * left edge of the trackRect.
+ * Calculates the gap size between the edge of the <code>contentRect</code>
+ * and the edge of the <code>trackRect</code>, storing the result in the
+ * <code>trackBuffer</code> field. Sufficient space needs to be reserved
+ * for the slider thumb and/or the labels at each end of the slider track.
*/
protected void calculateTrackBuffer()
{
if (slider.getOrientation() == JSlider.HORIZONTAL)
- trackBuffer = thumbRect.width / 2;
+ {
+ int w = Math.max(getWidthOfLowValueLabel(),
getWidthOfHighValueLabel());
+ trackBuffer = Math.max(thumbRect.width / 2, w / 2);
+
+ }
else
- trackBuffer = thumbRect.height / 2;
+ {
+ int h = Math.max(getHeightOfLowValueLabel(),
+ getHeightOfHighValueLabel());
+ trackBuffer = Math.max(thumbRect.height / 2, h / 2);
+ }
}
/**
@@ -1231,6 +1244,8 @@
if (slider.getPaintTicks() && (slider.getMajorTickSpacing() > 0
|| slider.getMinorTickSpacing() > 0))
h += getTickLength();
+ if (slider.getPaintLabels())
+ h += getHeightOfTallestLabel();
trackRect.y = contentRect.y + (contentRect.height - h) / 2 - 1;
trackRect.width = contentRect.width - 2 * trackBuffer;
trackRect.height = thumbRect.height;
@@ -1241,6 +1256,8 @@
if (slider.getPaintTicks() && (slider.getMajorTickSpacing() > 0
|| slider.getMinorTickSpacing() > 0))
w += getTickLength();
+ if (slider.getPaintLabels())
+ w += getWidthOfWidestLabel();
trackRect.x = contentRect.x + (contentRect.width - w) / 2 - 1;
trackRect.y = contentRect.y + trackBuffer;
trackRect.width = thumbRect.width;
@@ -1274,7 +1291,7 @@
tickRect.x = trackRect.x;
tickRect.y = trackRect.y + trackRect.height;
tickRect.width = trackRect.width;
- tickRect.height = getTickLength();
+ tickRect.height = (slider.getPaintTicks() ? getTickLength() : 0);
if (tickRect.y + tickRect.height > contentRect.y + contentRect.height)
tickRect.height = contentRect.y + contentRect.height - tickRect.y;
@@ -1283,7 +1300,7 @@
{
tickRect.x = trackRect.x + trackRect.width;
tickRect.y = trackRect.y;
- tickRect.width = getTickLength();
+ tickRect.width = (slider.getPaintTicks() ? getTickLength() : 0);
tickRect.height = trackRect.height;
if (tickRect.x + tickRect.width > contentRect.x + contentRect.width)
@@ -1302,13 +1319,13 @@
labelRect.x = contentRect.x;
labelRect.y = tickRect.y + tickRect.height;
labelRect.width = contentRect.width;
- labelRect.height = contentRect.height - labelRect.y;
+ labelRect.height = getHeightOfTallestLabel();
}
else
{
labelRect.x = tickRect.x + tickRect.width;
labelRect.y = contentRect.y;
- labelRect.width = contentRect.width - labelRect.x;
+ labelRect.width = getWidthOfWidestLabel();
labelRect.height = contentRect.height;
}
}
@@ -1371,38 +1388,42 @@
}
/**
- * This method returns the width of the label whose key has the highest
- * value.
+ * Returns the width of the label whose key has the highest value, or 0 if
+ * there are no labels.
*
- * @return The width of the high value label or 0 if no label table exists.
+ * @return The width of the label whose key has the highest value.
+ *
+ * @see #getHighestValueLabel()
*/
protected int getWidthOfHighValueLabel()
{
Component highValueLabel = getHighestValueLabel();
if (highValueLabel != null)
- return highValueLabel.getWidth();
+ return highValueLabel.getPreferredSize().width;
else
return 0;
}
/**
- * This method returns the width of the label whose key has the lowest
- * value.
+ * Returns the width of the label whose key has the lowest value, or 0 if
+ * there are no labels.
*
- * @return The width of the low value label or 0 if no label table exists.
+ * @return The width of the label whose key has the lowest value.
+ *
+ * @see #getLowestValueLabel()
*/
protected int getWidthOfLowValueLabel()
{
Component lowValueLabel = getLowestValueLabel();
if (lowValueLabel != null)
- return lowValueLabel.getWidth();
+ return lowValueLabel.getPreferredSize().width;
else
return 0;
}
/**
- * This method returns the height of the label whose key has the highest
- * value.
+ * Returns the height of the label whose key has the highest value, or 0 if
+ * there are no labels.
*
* @return The height of the high value label or 0 if no label table exists.
*/
@@ -1410,14 +1431,14 @@
{
Component highValueLabel = getHighestValueLabel();
if (highValueLabel != null)
- return highValueLabel.getHeight();
+ return highValueLabel.getPreferredSize().height;
else
return 0;
}
/**
- * This method returns the height of the label whose key has the lowest
- * value.
+ * Returns the height of the label whose key has the lowest value, or 0 if
+ * there are no labels.
*
* @return The height of the low value label or 0 if no label table exists.
*/
@@ -1425,19 +1446,20 @@
{
Component lowValueLabel = getLowestValueLabel();
if (lowValueLabel != null)
- return lowValueLabel.getHeight();
+ return lowValueLabel.getPreferredSize().height;
else
return 0;
}
/**
- * This method returns whether the slider is to be drawn inverted.
+ * Returns <code>true</code> if the slider scale is to be drawn inverted,
+ * and <code>false</code> if not.
*
- * @return True is the slider is to be drawn inverted.
+ * @return <code>true</code> if the slider is to be drawn inverted.
*/
protected boolean drawInverted()
{
- return ! (slider.getInverted() ^ leftToRightCache);
+ return slider.getInverted();
}
/**
@@ -1470,9 +1492,10 @@
}
/**
- * This method returns the label whose key has the highest value.
+ * Returns the label whose key has the highest value.
*
- * @return The high value label or null if no label table exists.
+ * @return The label whose key has the highest value or <code>null</code> if
+ * no label table exists.
*/
protected Component getHighestValueLabel()
{
@@ -1669,38 +1692,16 @@
{
if (slider.getOrientation() == JSlider.HORIZONTAL)
{
- double loc = tickRect.x + 0.5;
- double increment = (max == min) ? 0
- : majorSpace * (double) (tickRect.width - 1) / (max - min);
- if (drawInverted())
- {
- loc += tickRect.width;
- increment *= -1;
- }
g.translate(0, tickRect.y);
for (int i = min; i <= max; i += majorSpace)
- {
- paintMajorTickForHorizSlider(g, tickRect, (int) loc);
- loc += increment;
- }
+ paintMajorTickForHorizSlider(g, tickRect, xPositionForValue(i));
g.translate(0, -tickRect.y);
}
- else
+ else // JSlider.VERTICAL
{
- double loc = tickRect.height + tickRect.y + 0.5;
- double increment = (max == min) ? 0
- : -majorSpace * (double) (tickRect.height - 1) / (max - min);
- if (drawInverted())
- {
- loc = tickRect.y + 0.5;
- increment *= -1;
- }
g.translate(tickRect.x, 0);
for (int i = min; i <= max; i += majorSpace)
- {
- paintMajorTickForVertSlider(g, tickRect, (int) loc);
- loc += increment;
- }
+ paintMajorTickForVertSlider(g, tickRect, yPositionForValue(i));
g.translate(-tickRect.x, 0);
}
}
@@ -1708,38 +1709,16 @@
{
if (slider.getOrientation() == JSlider.HORIZONTAL)
{
- double loc = tickRect.x + 0.5;
- double increment = (max == min) ? 0
- : minorSpace * (double) (tickRect.width - 1) / (max - min);
- if (drawInverted())
- {
- loc += tickRect.width;
- increment *= -1;
- }
g.translate(0, tickRect.y);
for (int i = min; i <= max; i += minorSpace)
- {
- paintMinorTickForHorizSlider(g, tickRect, (int) loc);
- loc += increment;
- }
+ paintMinorTickForHorizSlider(g, tickRect, xPositionForValue(i));
g.translate(0, -tickRect.y);
}
else
{
- double loc = tickRect.height + tickRect.y + 0.5;
- double increment = (max == min) ? 0
- : -minorSpace * (double) (tickRect.height - 1) / (max - min);
- if (drawInverted())
- {
- loc = tickRect.y + 0.5;
- increment *= -1;
- }
g.translate(tickRect.x, 0);
for (int i = min; i <= max; i += minorSpace)
- {
- paintMinorTickForVertSlider(g, tickRect, (int) loc);
- loc += increment;
- }
+ paintMinorTickForVertSlider(g, tickRect, yPositionForValue(i));
g.translate(-tickRect.x, 0);
}
}
@@ -1938,8 +1917,7 @@
h = labelRect.height;
label.setBounds(xpos, ypos, w, h);
- javax.swing.SwingUtilities.paintComponent(g, label, null,
- label.getBounds());
+ SwingUtilities.paintComponent(g, label, null, label.getBounds());
}
/**
@@ -1978,8 +1956,7 @@
w = labelRect.width;
label.setBounds(xpos, ypos, w, h);
- javax.swing.SwingUtilities.paintComponent(g, label, null,
- label.getBounds());
+ SwingUtilities.paintComponent(g, label, null, label.getBounds());
}
/**
@@ -2151,53 +2128,60 @@
}
/**
- * This method returns the X coordinate for the value passed in.
+ * Returns the x-coordinate (relative to the component) for the given slider
+ * value. This method assumes that the <code>trackRect</code> field is
+ * set up.
*
- * @param value The value to calculate an x coordinate for.
+ * @param value the slider value.
*
- * @return The x coordinate for the value.
+ * @return The x-coordinate.
*/
protected int xPositionForValue(int value)
{
- int min = slider.getMinimum();
- int max = slider.getMaximum();
- int len = trackRect.width - 1;
-
- int xPos = (max == min) ? 0 : (value - min) * len / (max - min);
+ double min = slider.getMinimum();
+ if (value < min)
+ value = (int) min;
+ double max = slider.getMaximum();
+ if (value > max)
+ value = (int) max;
+ double len = trackRect.width;
+ if ((max - min) <= 0.0)
+ return 0;
+ int xPos = (int) ((value - min) / (max - min) * len + 0.5);
- if (! drawInverted())
- xPos += trackRect.x;
+ if (drawInverted())
+ return trackRect.x + Math.max(trackRect.width - xPos - 1, 0);
else
- {
- xPos = len - xPos;
- xPos += trackRect.x;
- }
- return xPos;
+ return trackRect.x + Math.min(xPos, trackRect.width - 1);
}
/**
- * This method returns the y coordinate for the value passed in.
+ * Returns the y-coordinate (relative to the component) for the given slider
+ * value. This method assumes that the <code>trackRect</code> field is
+ * set up.
*
- * @param value The value to calculate a y coordinate for.
+ * @param value the slider value.
*
- * @return The y coordinate for the value.
+ * @return The y-coordinate.
*/
protected int yPositionForValue(int value)
{
- int min = slider.getMinimum();
- int max = slider.getMaximum();
- int len = trackRect.height - 1;
+ double min = slider.getMinimum();
+ if (value < min)
+ value = (int) min;
+ double max = slider.getMaximum();
+ if (value > max)
+ value = (int) max;
+ int len = trackRect.height;
+ if ((max - min) <= 0.0)
+ return 0;
- int yPos = (max == min) ? 0 : (value - min) * len / (max - min);
+ int yPos = (int) ((value - min) / (max - min) * len + 0.5);
if (! drawInverted())
- {
- yPos = len - yPos;
- yPos += trackRect.y;
- }
+ return trackRect.y + trackRect.height - Math.max(yPos, 1);
else
- yPos += trackRect.y;
- return yPos;
+ return trackRect.y + Math.min(yPos, trackRect.height - 1);
}
/**
Index: javax/swing/plaf/metal/MetalSliderUI.java
===================================================================
RCS file:
/sources/classpath/classpath/javax/swing/plaf/metal/MetalSliderUI.java,v
retrieving revision 1.8
diff -u -r1.8 MetalSliderUI.java
--- javax/swing/plaf/metal/MetalSliderUI.java 14 Mar 2006 16:28:52 -0000
1.8
+++ javax/swing/plaf/metal/MetalSliderUI.java 29 Mar 2006 21:12:17 -0000
@@ -1,5 +1,5 @@
/* MetalSliderUI.java
- Copyright (C) 2005 Free Software Foundation, Inc.
+ Copyright (C) 2005, 2006, Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -210,7 +210,7 @@
{
int trackX = trackRect.x;
int trackY = trackRect.y + (trackRect.height - getTrackWidth()) / 2;
- int trackW = trackRect.width - 1;
+ int trackW = trackRect.width;
int trackH = getTrackWidth();
// draw border
@@ -264,7 +264,7 @@
int trackX = trackRect.x + (trackRect.width - getTrackWidth()) / 2;
int trackY = trackRect.y;
int trackW = getTrackWidth();
- int trackH = trackRect.height - 1;
+ int trackH = trackRect.height;
if (slider.isEnabled())
BasicGraphicsUtils.drawEtchedRect(g, trackX, trackY, trackW, trackH,
darkShadowColor, shadowColor, darkShadowColor, highlightColor);
