Hi
Many thanks for your work on QCustomPlot!
I have noticed a few things that could be improved, which I am sharing here:
- The text and icon of QCPPlottableLegendItem are not properly aligned.
- TickLabels can overlap
- default values are not indicated in the documentation
The text and icon of QCPPlottableLegendItem are not properly aligned:
If you change void QCPPlottableLegendItem::draw(QCPPainter *painter) into this, then they become properly aligned. Note that this could be further improved by allowing the user to set the alignment itself, here I just put vertical and horizontal alignment by default.
void QCPPlottableLegendItem::draw(QCPPainter *painter)
{
if (!mPlottable) return;
painter->setFont(getFont());
painter->setPen(QPen(getTextColor()));
QSize iconSize = mParentLegend->iconSize();
QRect textRect = painter->fontMetrics().boundingRect(0, 0, 0, 0, Qt::TextDontClip, mPlottable->name());
int textHeight = qMax(textRect.height(), iconSize.height());
int TopCoord = mRect.y() + (mRect.height() - textHeight)/2*1; // *1 == Qt::AlignVCenter, *0 == Qt::AlignTop, *2 == Qt::AlignBottom
int LeftCoord = mRect.x() + (mRect.width() - iconSize.width() - mParentLegend->iconTextPadding() - textRect.width())/2*1; // *1 == Qt::AlignHCenter, *0 == Qt::AlignLeft, *2 == Qt::AlignRight
QRect iconRect(LeftCoord, TopCoord, iconSize.width(), iconSize.height());
painter->drawText(LeftCoord+iconSize.width()+mParentLegend->iconTextPadding(), TopCoord, textRect.width(), textHeight, Qt::TextDontClip, mPlottable->name());
// draw icon:
painter->save();
painter->setClipRect(iconRect, Qt::IntersectClip);
mPlottable->drawLegendIcon(painter, iconRect);
painter->restore();
// draw icon border:
if (getIconBorderPen().style() != Qt::NoPen)
{
painter->setPen(getIconBorderPen());
painter->setBrush(Qt::NoBrush);
int halfPen = qCeil(painter->pen().widthF()*0.5)+1;
painter->setClipRect(mOuterRect.adjusted(-halfPen, -halfPen, halfPen, halfPen)); // extend default clip rect so thicker pens (especially during selection) are not clipped
painter->drawRect(iconRect);
}
}
TickLabels can overlap:
If you set the TickLabel font at 60 points or if you have a QCPAxisTickerDateTime with a format like "HH:mm:ss.zzz dddd dd MMMM yyyy", you will notice that TickLabels can overlap. One way to solve this is to change:
void QCPAxisPainterPrivate::placeTickLabel(QCPPainter *painter, double position, int distanceToAxis, const QString &text, QSize *tickLabelsSize, int& PreviousLabelPosition)
{
// warning: if you change anything here, also adapt getMaxTickLabelSize() accordingly!
if (text.isEmpty()) return;
QSize finalSize;
QPointF labelAnchor;
switch (type)
{
case QCPAxis::atLeft: labelAnchor = QPointF(axisRect.left()-distanceToAxis-offset, position); break;
case QCPAxis::atRight: labelAnchor = QPointF(axisRect.right()+distanceToAxis+offset, position); break;
case QCPAxis::atTop: labelAnchor = QPointF(position, axisRect.top()-distanceToAxis-offset); break;
case QCPAxis::atBottom: labelAnchor = QPointF(position, axisRect.bottom()+distanceToAxis+offset); break;
}
if (mParentPlot->plottingHints().testFlag(QCP::phCacheLabels) && !painter->modes().testFlag(QCPPainter::pmNoCaching)) // label caching enabled
{
CachedLabel *cachedLabel = mLabelCache.take(text); // attempt to get label from cache
if (!cachedLabel) // no cached label existed, create it
{
cachedLabel = new CachedLabel;
TickLabelData labelData = getTickLabelData(painter->font(), text);
cachedLabel->offset = getTickLabelDrawOffset(labelData)+labelData.rotatedTotalBounds.topLeft();
if (!qFuzzyCompare(1.0, mParentPlot->bufferDevicePixelRatio()))
{
cachedLabel->pixmap = QPixmap(labelData.rotatedTotalBounds.size()*mParentPlot->bufferDevicePixelRatio());
#ifdef QCP_DEVICEPIXELRATIO_SUPPORTED
# ifdef QCP_DEVICEPIXELRATIO_FLOAT
cachedLabel->pixmap.setDevicePixelRatio(mParentPlot->devicePixelRatioF());
# else
cachedLabel->pixmap.setDevicePixelRatio(mParentPlot->devicePixelRatio());
# endif
#endif
} else
cachedLabel->pixmap = QPixmap(labelData.rotatedTotalBounds.size());
cachedLabel->pixmap.fill(Qt::transparent);
QCPPainter cachePainter(&cachedLabel->pixmap);
cachePainter.setPen(painter->pen());
drawTickLabel(&cachePainter, -labelData.rotatedTotalBounds.topLeft().x(), -labelData.rotatedTotalBounds.topLeft().y(), labelData);
}
// if label would be partly clipped by widget border on sides, don't draw it (only for outside tick labels):
bool labelClippedByBorder = false;
if (tickLabelSide == QCPAxis::lsOutside)
{
if (QCPAxis::orientation(type) == Qt::Horizontal)
labelClippedByBorder = labelAnchor.x()+cachedLabel->offset.x()+cachedLabel->pixmap.width()/mParentPlot->bufferDevicePixelRatio() > viewportRect.right() || labelAnchor.x()+cachedLabel->offset.x() < PreviousLabelPosition;
else
labelClippedByBorder = labelAnchor.y()+cachedLabel->offset.y()+cachedLabel->pixmap.height()/mParentPlot->bufferDevicePixelRatio() > PreviousLabelPosition || labelAnchor.y()+cachedLabel->offset.y() < viewportRect.top();
}
if (!labelClippedByBorder)
{
painter->drawPixmap(labelAnchor+cachedLabel->offset, cachedLabel->pixmap);
finalSize = cachedLabel->pixmap.size()/mParentPlot->bufferDevicePixelRatio();
if (QCPAxis::orientation(type) == Qt::Horizontal) {
PreviousLabelPosition = labelAnchor.x()+cachedLabel->offset.x()+cachedLabel->pixmap.width()/mParentPlot->bufferDevicePixelRatio();
} else {
PreviousLabelPosition = labelAnchor.y()+cachedLabel->offset.y();
};
}
mLabelCache.insert(text, cachedLabel); // return label to cache or insert for the first time if newly created
} else // label caching disabled, draw text directly on surface:
{
TickLabelData labelData = getTickLabelData(painter->font(), text);
QPointF finalPosition = labelAnchor + getTickLabelDrawOffset(labelData);
// if label would be partly clipped by widget border on sides, don't draw it (only for outside tick labels):
bool labelClippedByBorder = false;
if (tickLabelSide == QCPAxis::lsOutside)
{
if (QCPAxis::orientation(type) == Qt::Horizontal)
labelClippedByBorder = finalPosition.x()+(labelData.rotatedTotalBounds.width()+labelData.rotatedTotalBounds.left()) > viewportRect.right() || finalPosition.x()+labelData.rotatedTotalBounds.left() < PreviousLabelPosition;
else
labelClippedByBorder = finalPosition.y()+(labelData.rotatedTotalBounds.height()+labelData.rotatedTotalBounds.top()) > PreviousLabelPosition || finalPosition.y()+labelData.rotatedTotalBounds.top() < viewportRect.top();
}
if (!labelClippedByBorder)
{
drawTickLabel(painter, finalPosition.x(), finalPosition.y(), labelData);
finalSize = labelData.rotatedTotalBounds.size();
if (QCPAxis::orientation(type) == Qt::Horizontal) {
PreviousLabelPosition = finalPosition.x()+(labelData.rotatedTotalBounds.width()+labelData.rotatedTotalBounds.left());
} else {
PreviousLabelPosition = finalPosition.y()+labelData.rotatedTotalBounds.top();
};
}
}
// expand passed tickLabelsSize if current tick label is larger:
if (finalSize.width() > tickLabelsSize->width())
tickLabelsSize->setWidth(finalSize.width());
if (finalSize.height() > tickLabelsSize->height())
tickLabelsSize->setHeight(finalSize.height());
}
And in void QCPAxisPainterPrivate::draw(QCPPainter *painter):
int PreviousLabelPosition;
if(QCPAxis::orientation(type) == Qt::Horizontal){
PreviousLabelPosition = viewportRect.left();
} else {
PreviousLabelPosition = viewportRect.bottom();
};
for (int i=0; i<maxLabelIndex; ++i)
placeTickLabel(painter, tickPositions.at(i), distanceToAxis, tickLabels.at(i), &tickLabelsSize, PreviousLabelPosition);
And in the headers:
virtual void placeTickLabel(QCPPainter *painter, double position, int distanceToAxis, const QString &text, QSize *tickLabelsSize, int& PreviousLabelPosition);
Default values are not indicated in the documentation:
I think it would really help if default values were indicated in the documentation. For example, I struggled for a while to understand why my legend was not properly aligned at the top, until I realised that the insetLayout had margins of 12 by default. I think it would be better for this one to put a margin of 0 by default:
In QCustomPlot::QCustomPlot(QWidget *parent):
defaultAxisRect->insetLayout()->setMargins(QMargins(0, 0, 0, 0));
I hope my suggestions help!
Thank you again for developping QCustomPlot!