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!
I realised I made a mistake for QCPPlottableLegendItem. It works correctly with this:
void QCPPlottableLegendItem::draw(QCPPainter *painter)
{
if (!mPlottable) return;
painter->setFont(getFont());
painter->setPen(QPen(getTextColor()));
QSize iconSize = mParentLegend->iconSize();
QRect textRectMax = painter->fontMetrics().boundingRect(0, 0, 0, 0, Qt::AlignBottom, mPlottable->name());
int maxHeight = qMax(textRectMax.height(), iconSize.height());
int LeftCoord = mRect.x() + (mRect.width() - iconSize.width() - mParentLegend->iconTextPadding() - textRectMax.width())/2*0; // *1 == Qt::AlignHCenter, *0 == Qt::AlignLeft, *2 == Qt::AlignRight
QRect iconRect(LeftCoord, mRect.y() + (maxHeight - iconSize.height())/2*1, iconSize.width(), iconSize.height()); // *1 == Qt::AlignVCenter, *0 == Qt::AlignTop, *2 == Qt::AlignBottom
QRect textRect(LeftCoord+iconSize.width()+mParentLegend->iconTextPadding(), mRect.y() + (maxHeight - textRectMax.height())/2*1, textRectMax.width(), textRectMax.height()); // *1 == Qt::AlignVCenter, *0 == Qt::AlignTop, *2 == Qt::AlignBottom
painter->drawText(textRect, Qt::AlignBottom, 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);
}
}
And one last suggestion for a custom number formatting of axis tick labels. I believe the current method of subclassing QCPAxisTicker and re-implementing getTickLabel() is not very user-friendly.
One way to make it better could be to add a QString mformatString member variable to the QCPAxis class and changing getTickLabel() into:
QString QCPAxisTicker::getTickLabel(double tick, const QLocale &locale, QChar formatChar, int precision, QString &formatString)
{
return QString(formatString).arg(locale.toString(tick, formatChar.toLatin1(), precision));
}
and redefining createLabelVector() as:
QVector<QString> QCPAxisTicker ::createLabelVector(const QVector<double> &ticks, const QLocale &locale, QChar formatChar, int precision, QString &formatString), which calls getTickLabel(tickCoord, locale, formatChar, precision, formatString)
and generate() as:
void QCPAxisTicker ::generate(const QCPRange &range, const QLocale &locale, QChar formatChar, int precision, QString &formatString, QVector<double> &ticks, QVector<double> *subTicks, QVector<QString> *tickLabels), which calls createLabelVector(ticks, locale, formatChar, precision, formatString)
and with setupTickVectors() calling generate(mRange, mParentPlot->locale(), mNumberFormatChar, mNumberPrecision, mformatString, mTickVector, mSubTicks ? &mSubTickVector : nullptr, mTickLabels ? &mTickVectorLabels : nullptr)
The getter would be:
QString QCPAxis::formatString() const
{
return mformatString;
};
The setter would be:
void QCPAxis::setFormatString(const QString &formatString){
mformatString = formatString;
return;
};
And with the following declarations in the QCPAxis class:
Q_PROPERTY(QString formatString READ formatString WRITE setFormatString) QString mformatString; QString formatString() const; void setFormatString(const QString &formatString);
mformatString("%1") in the QCPAxis constructor.
And finally in setupFullAxesBox():
xAxis2->setFormatString(xAxis->formatString()); yAxis2->setFormatString(yAxis->formatString());
That way, if you do for example customPlot ->yAxis->setFormatString("$%1"), you easily get a custom tick label representing dollars.
Thank you so much for your solution to the overlapping tick labels issue! This problem has been bothering me for quite some time. After trying out your code, the chart works perfectly!