Never try to create/render 0 pixel sized fonts

This throws Qt warnings, and results in large fonts being created when they actually should be invisible
qgis · Aug 18, 2021 · 7c75c4c · 7c75c4c
1 parent d90a87e
commit 7c75c4c
Show file tree

Hide file tree

Showing 6 changed files with 129 additions and 40 deletions.
diff --git a/python/core/auto_generated/textrenderer/qgstextformat.sip.in b/python/core/auto_generated/textrenderer/qgstextformat.sip.in
@@ -165,7 +165,7 @@ of rendered text.
 .. seealso:: :py:func:`toQFont`
 %End
 
-    QFont scaledFont( const QgsRenderContext &context, double scaleFactor = 1.0 ) const;
+    QFont scaledFont( const QgsRenderContext &context, double scaleFactor = 1.0) const;
 %Docstring
 Returns a font with the size scaled to match the format's size settings (including
 units and map unit scale) for a specified render context.

diff --git a/src/core/layertree/qgslayertreemodellegendnode.cpp b/src/core/layertree/qgslayertreemodellegendnode.cpp
@@ -529,10 +529,14 @@ QVariant QgsSymbolLegendNode::data( int role ) const
           QPainter painter( &pix );
           painter.setRenderHint( QPainter::Antialiasing );
           context->setPainter( &painter );
-          const QFontMetricsF fm( mTextOnSymbolTextFormat.scaledFont( *context ) );
-          const qreal yBaselineVCenter = ( mIconSize.height() + fm.ascent() - fm.descent() ) / 2;
-          QgsTextRenderer::drawText( QPointF( mIconSize.width() / 2, yBaselineVCenter ), 0, QgsTextRenderer::AlignCenter,
-                                     QStringList() << mTextOnSymbolLabel, *context, mTextOnSymbolTextFormat );
+          bool isNullSize = false;
+          const QFontMetricsF fm( mTextOnSymbolTextFormat.scaledFont( *context, 1.0, &isNullSize ) );
+          if ( !isNullSize )
+          {
+            const qreal yBaselineVCenter = ( mIconSize.height() + fm.ascent() - fm.descent() ) / 2;
+            QgsTextRenderer::drawText( QPointF( mIconSize.width() / 2, yBaselineVCenter ), 0, QgsTextRenderer::AlignCenter,
+                                       QStringList() << mTextOnSymbolLabel, *context, mTextOnSymbolTextFormat );
+          }
         }
       }
       else
@@ -732,10 +736,14 @@ QSizeF QgsSymbolLegendNode::drawSymbol( const QgsLegendSettings &settings, ItemC
 
     if ( !mTextOnSymbolLabel.isEmpty() )
     {
-      const QFontMetricsF fm( mTextOnSymbolTextFormat.scaledFont( *context ) );
-      const qreal yBaselineVCenter = ( height * dotsPerMM + fm.ascent() - fm.descent() ) / 2;
-      QgsTextRenderer::drawText( QPointF( width * dotsPerMM / 2, yBaselineVCenter ), 0, QgsTextRenderer::AlignCenter,
-                                 QStringList() << mTextOnSymbolLabel, *context, mTextOnSymbolTextFormat );
+      bool isNullSize = false;
+      const QFontMetricsF fm( mTextOnSymbolTextFormat.scaledFont( *context, 1.0, &isNullSize ) );
+      if ( !isNullSize )
+      {
+        const qreal yBaselineVCenter = ( height * dotsPerMM + fm.ascent() - fm.descent() ) / 2;
+        QgsTextRenderer::drawText( QPointF( width * dotsPerMM / 2, yBaselineVCenter ), 0, QgsTextRenderer::AlignCenter,
+                                   QStringList() << mTextOnSymbolLabel, *context, mTextOnSymbolTextFormat );
+      }
     }
   }
 

diff --git a/src/core/textrenderer/qgstextformat.cpp b/src/core/textrenderer/qgstextformat.cpp
@@ -159,19 +159,36 @@ QFont QgsTextFormat::font() const
   return d->textFont;
 }
 
-QFont QgsTextFormat::scaledFont( const QgsRenderContext &context, double scaleFactor ) const
+QFont QgsTextFormat::scaledFont( const QgsRenderContext &context, double scaleFactor, bool *isZeroSize ) const
 {
+  if ( isZeroSize )
+    *isZeroSize = false;
+
   QFont font = d->textFont;
   if ( scaleFactor == 1 )
   {
     int fontPixelSize = QgsTextRenderer::sizeToPixel( d->fontSize, context, d->fontSizeUnits,
                         d->fontSizeMapUnitScale );
+    if ( fontPixelSize == 0 )
+    {
+      if ( isZeroSize )
+        *isZeroSize = true;
+      return QFont();
+    }
+
     font.setPixelSize( fontPixelSize );
   }
   else
   {
     double fontPixelSize = context.convertToPainterUnits( d->fontSize, d->fontSizeUnits, d->fontSizeMapUnitScale );
-    font.setPixelSize( std::round( scaleFactor * fontPixelSize + 0.5 ) );
+    if ( qgsDoubleNear( fontPixelSize, 0 ) )
+    {
+      if ( isZeroSize )
+        *isZeroSize = true;
+      return QFont();
+    }
+    const int roundedPixelSize = static_cast< int >( std::round( scaleFactor * fontPixelSize + 0.5 ) );
+    font.setPixelSize( roundedPixelSize );
   }
 
   font.setLetterSpacing( QFont::AbsoluteSpacing, context.convertToPainterUnits( d->textFont.letterSpacing(), d->fontSizeUnits, d->fontSizeMapUnitScale ) * scaleFactor );

diff --git a/src/core/textrenderer/qgstextformat.h b/src/core/textrenderer/qgstextformat.h
@@ -189,11 +189,12 @@ class CORE_EXPORT QgsTextFormat
      * QgsTextRenderer::FONT_WORKAROUND_SCALE and then manually scale painter devices or calculations
      * based on the resultant font metrics. Failure to do so will result in poor quality text rendering
      * at small font sizes.
+     * \param isZeroSize will be set to true if the font is scaled down to a near 0 size, and nothing should be rendered. Not available in Python bindings.
      * \returns font with scaled size
      * \see font()
      * \see size()
      */
-    QFont scaledFont( const QgsRenderContext &context, double scaleFactor = 1.0 ) const;
+    QFont scaledFont( const QgsRenderContext &context, double scaleFactor = 1.0, bool *isZeroSize SIP_PYARGREMOVE = nullptr ) const;
 
     /**
      * Sets the font used for rendering text. Note that the size of the font

diff --git a/src/core/textrenderer/qgstextrenderer.cpp b/src/core/textrenderer/qgstextrenderer.cpp
@@ -304,7 +304,11 @@ double QgsTextRenderer::drawBuffer( QgsRenderContext &context, const QgsTextRend
     referenceScaleOverride.emplace( QgsScopedRenderContextReferenceScaleOverride( context, -1.0 ) );
   }
 
-  const QFont font = format.scaledFont( context, scaleFactor );
+  bool isNullSize = false;
+  const QFont font = format.scaledFont( context, scaleFactor, &isNullSize );
+  if ( isNullSize )
+    return 0;
+
   referenceScaleOverride.reset();
 
   QPainterPath path;
@@ -460,7 +464,11 @@ void QgsTextRenderer::drawMask( QgsRenderContext &context, const QgsTextRenderer
     referenceScaleOverride.emplace( QgsScopedRenderContextReferenceScaleOverride( context, -1.0 ) );
   }
 
-  const QFont font = format.scaledFont( context, scaleFactor );
+  bool isNullSize = false;
+  const QFont font = format.scaledFont( context, scaleFactor, &isNullSize );
+  if ( isNullSize )
+    return;
+
   referenceScaleOverride.reset();
 
   double xOffset = 0;
@@ -535,7 +543,11 @@ double QgsTextRenderer::textWidth( const QgsRenderContext &context, const QgsTex
 {
   //calculate max width of text lines
   const double scaleFactor = ( context.flags() & QgsRenderContext::ApplyScalingWorkaroundForTextRendering ) ? FONT_WORKAROUND_SCALE : 1.0;
-  const QFont baseFont = format.scaledFont( context, scaleFactor );
+
+  bool isNullSize = false;
+  const QFont baseFont = format.scaledFont( context, scaleFactor, &isNullSize );
+  if ( isNullSize )
+    return 0;
 
   double width = 0;
   switch ( format.orientation() )
@@ -602,7 +614,11 @@ double QgsTextRenderer::textHeight( const QgsRenderContext &context, const QgsTe
 double QgsTextRenderer::textHeight( const QgsRenderContext &context, const QgsTextFormat &format, QChar character, bool includeEffects )
 {
   const double scaleFactor = ( context.flags() & QgsRenderContext::ApplyScalingWorkaroundForTextRendering ) ? FONT_WORKAROUND_SCALE : 1.0;
-  const QFont baseFont = format.scaledFont( context, scaleFactor );
+  bool isNullSize = false;
+  const QFont baseFont = format.scaledFont( context, scaleFactor, &isNullSize );
+  if ( isNullSize )
+    return 0;
+
   const QFontMetrics fm( baseFont );
   const double height = ( character.isNull() ? fm.height() : fm.boundingRect( character ).height() ) / scaleFactor;
 
@@ -640,7 +656,10 @@ double QgsTextRenderer::textHeight( const QgsRenderContext &context, const QgsTe
   //calculate max height of text lines
   const double scaleFactor = ( context.flags() & QgsRenderContext::ApplyScalingWorkaroundForTextRendering ) ? FONT_WORKAROUND_SCALE : 1.0;
 
-  const QFont baseFont = format.scaledFont( context, scaleFactor );
+  bool isNullSize = false;
+  const QFont baseFont = format.scaledFont( context, scaleFactor, &isNullSize );
+  if ( isNullSize )
+    return 0;
 
   switch ( format.orientation() )
   {
@@ -766,7 +785,6 @@ void QgsTextRenderer::drawBackground( QgsRenderContext &context, QgsTextRenderer
   if ( mode != Label )
   {
     // need to calculate size of text
-    QFontMetricsF fm( format.scaledFont( context, scaleFactor ) );
     double width = textWidth( context, format, document );
     double height = textHeight( context, format, document, mode );
 
@@ -795,7 +813,9 @@ void QgsTextRenderer::drawBackground( QgsRenderContext &context, QgsTextRenderer
 
       case Point:
       {
-        double originAdjust = fm.ascent() / scaleFactor / 2.0 - fm.leading() / scaleFactor / 2.0;
+        bool isNullSize = false;
+        QFontMetricsF fm( format.scaledFont( context, scaleFactor, &isNullSize ) );
+        double originAdjust = isNullSize ? 0 : ( fm.ascent() / scaleFactor / 2.0 - fm.leading() / scaleFactor / 2.0 );
         switch ( component.hAlign )
         {
           case AlignLeft:
@@ -1287,7 +1307,11 @@ void QgsTextRenderer::drawTextInternal( TextPart drawType,
       referenceScaleOverride.emplace( QgsScopedRenderContextReferenceScaleOverride( context, -1.0 ) );
     }
 
-    const QFont f = format.scaledFont( context, fontScale );
+    bool isNullSize = false;
+    const QFont f = format.scaledFont( context, fontScale, &isNullSize );
+    if ( isNullSize )
+      return;
+
     tmpMetrics = std::make_unique< QFontMetricsF >( f );
     fontMetrics = tmpMetrics.get();
 
@@ -1583,34 +1607,38 @@ void QgsTextRenderer::drawTextInternalHorizontal( QgsRenderContext &context, con
         // to temporarily remove the reference scale here or we'll be applying the scaling twice
         referenceScaleOverride.emplace( QgsScopedRenderContextReferenceScaleOverride( context, -1.0 ) );
       }
-      const QFont font = format.scaledFont( context, fontScale );
+      bool isNullSize = false;
+      const QFont font = format.scaledFont( context, fontScale, &isNullSize );
       referenceScaleOverride.reset();
 
-      textp.scale( 1 / fontScale, 1 / fontScale );
-
-      double xOffset = 0;
-      for ( const QgsTextFragment &fragment : block )
+      if ( !isNullSize )
       {
-        // draw text, QPainterPath method
-        QPainterPath path;
-        path.setFillRule( Qt::WindingFill );
+        textp.scale( 1 / fontScale, 1 / fontScale );
 
-        QFont fragmentFont = font;
-        fragment.characterFormat().updateFontForFormat( fragmentFont, fontScale );
+        double xOffset = 0;
+        for ( const QgsTextFragment &fragment : block )
+        {
+          // draw text, QPainterPath method
+          QPainterPath path;
+          path.setFillRule( Qt::WindingFill );
 
-        if ( extraWordSpace || extraLetterSpace )
-          applyExtraSpacingForLineJustification( fragmentFont, extraWordSpace * fontScale, extraLetterSpace * fontScale );
+          QFont fragmentFont = font;
+          fragment.characterFormat().updateFontForFormat( fragmentFont, fontScale );
 
-        path.addText( xOffset, 0, fragmentFont, fragment.text() );
+          if ( extraWordSpace || extraLetterSpace )
+            applyExtraSpacingForLineJustification( fragmentFont, extraWordSpace * fontScale, extraLetterSpace * fontScale );
 
-        QColor textColor = fragment.characterFormat().textColor().isValid() ? fragment.characterFormat().textColor() : format.color();
-        textColor.setAlphaF( fragment.characterFormat().textColor().isValid() ? textColor.alphaF() * format.opacity() : format.opacity() );
-        textp.setBrush( textColor );
-        textp.drawPath( path );
+          path.addText( xOffset, 0, fragmentFont, fragment.text() );
+
+          QColor textColor = fragment.characterFormat().textColor().isValid() ? fragment.characterFormat().textColor() : format.color();
+          textColor.setAlphaF( fragment.characterFormat().textColor().isValid() ? textColor.alphaF() * format.opacity() : format.opacity() );
+          textp.setBrush( textColor );
+          textp.drawPath( path );
 
-        xOffset += fragment.horizontalAdvance( fragmentFont, true );
+          xOffset += fragment.horizontalAdvance( fragmentFont, true );
+        }
+        textp.end();
       }
-      textp.end();
 
       if ( format.shadow().enabled() && format.shadow().shadowPlacement() == QgsTextShadowSettings::ShadowText )
       {
@@ -1686,7 +1714,11 @@ void QgsTextRenderer::drawTextInternalVertical( QgsRenderContext &context, const
     referenceScaleOverride.emplace( QgsScopedRenderContextReferenceScaleOverride( context, -1.0 ) );
   }
 
-  const QFont font = format.scaledFont( context, fontScale );
+  bool isNullSize = false;
+  const QFont font = format.scaledFont( context, fontScale, &isNullSize );
+  if ( isNullSize )
+    return;
+
   referenceScaleOverride.reset();
 
   double letterSpacing = font.letterSpacing() / fontScale;

diff --git a/tests/src/core/testqgslabelingengine.cpp b/tests/src/core/testqgslabelingengine.cpp
@@ -50,6 +50,7 @@ class TestQgsLabelingEngine : public QObject
     void init();// will be called before each testfunction is executed.
     void cleanup();// will be called after every testfunction.
     void testEngineSettings();
+    void testScaledFont();
     void testBasic();
     void testDiagrams();
     void testRuleBased();
@@ -214,6 +215,36 @@ void TestQgsLabelingEngine::testEngineSettings()
   QCOMPARE( settings3.placementVersion(), QgsLabelingEngineSettings::PlacementEngineVersion1 );
 }
 
+void TestQgsLabelingEngine::testScaledFont()
+{
+  QgsTextFormat format;
+  format.setFont( QgsFontUtils::getStandardTestFont( QStringLiteral( "Bold" ) ).family() );
+  format.setSize( 9.9 );
+  format.setSizeUnit( QgsUnitTypes::RenderUnit::RenderPixels );
+
+  bool isNullSize = true;
+
+  QgsRenderContext context;
+  QFont f = format.scaledFont( context, 1.0, &isNullSize );
+  QCOMPARE( f.pixelSize(), 10 );
+  QVERIFY( !isNullSize );
+
+  isNullSize = true;
+  f = format.scaledFont( context, 10.0, &isNullSize );
+  QCOMPARE( f.pixelSize(), 100 );
+  QVERIFY( !isNullSize );
+
+  isNullSize = false;
+  format.setSize( 0 );
+  format.scaledFont( context, 1.0, &isNullSize );
+  QVERIFY( isNullSize );
+
+  isNullSize = false;
+  format.scaledFont( context, 10.0, &isNullSize );
+  QVERIFY( isNullSize );
+};
+
+
 void TestQgsLabelingEngine::setDefaultLabelParams( QgsPalLayerSettings &settings )
 {
   QgsTextFormat format;