Bug 33115 – SVG <marker> orient=auto computes wrong angle at path closure (Z command)

• Component: SVG
• Status: NEW

[Nikolas Zimmermann]

Marker elements don't support paths which contain PathElementAddQuadCurveToPoint elements. See SVGMarkerLayoutInfo, updateMarkerDataForElement() method.

[Ahmad Saleem]

Following Blink commit: https://chromium.googlesource.com/chromium/src.git/+/7003bed16de3861e47d6bb0287ba4d6bffcfea04

Removed this FIXME:

https://searchfox.org/wubkat/source/Source/WebCore/rendering/svg/SVGMarkerData.h#118

and changed it into:

case PathElement::Type::AddQuadCurveToPoint:
            m_inslopePoints[0] = points[0];
            m_inslopePoints[1] = points[1];
            m_origin = points[1];
            break;

[Ahmad Saleem]

case PathElement::Type::AddQuadCurveToPoint:
    // Compute tangents for quadratic bezier curve
    // Start tangent: from origin to control point
    // End tangent: from control point to end point
    {
        FloatSize startTangent = points[0] - m_origin;
        FloatSize endTangent = points[1] - points[0];
        
        if (startTangent.isZero()) {
            // If start tangent is zero, use the end tangent for both
            m_inslopePoints[0] = m_origin;
            m_inslopePoints[1] = points[1];
        } else if (endTangent.isZero()) {
            // If end tangent is zero, use the start tangent for both
            m_inslopePoints[0] = m_origin;
            m_inslopePoints[1] = points[0];
        } else {
            // Normal case: use control point for in-slope calculation
            m_inslopePoints[0] = points[0];
            m_inslopePoints[1] = points[1];
        }
        
        m_origin = points[1];
    }
    break;

[Karl Dubost]

The original title about PathElementAddQuadCurveToPoint describes dead code on CG platforms. it's dead code on macOS/iOS. The real bug is that CloseSubpath feeds (0,0) into the slope calculations because the CG  backend passes empty points for the Z command.

CoreGraphics converts all quadratic béziers to cubics internally, so CGPathApply never emits kCGPathElementAddQuadCurveToPoint. The Q segments go through the cubic case (line 137-141) which correctly sets m_inslopePoints.

The FIXME is technically correct but only affects non-CG backends (GTK/WPE with Cairo/Skia).


  Bug B: CloseSubpath uses (0,0) — two locations

  Location 1 — line 61-62: updateOutslope receives element.points[0] which is (0,0) for CloseSubpath. It should use m_subpathStart instead:

  Current:
      if (element.type != PathElement::Type::MoveToPoint)
          markerData.updateOutslope(element.points[0]);

  Fix:
      if (element.type == PathElement::Type::CloseSubpath)
          markerData.updateOutslope(markerData.m_subpathStart);
      else if (element.type != PathElement::Type::MoveToPoint)
          markerData.updateOutslope(element.points[0]);

  Location 2 — line 149-150: updateInslope also receives points[0] which is (0,0). It should use m_subpathStart:

  Current:
      case PathElement::Type::CloseSubpath:
          updateInslope(points[0]);

  Fix:
      case PathElement::Type::CloseSubpath:
          updateInslope(m_subpathStart);

  Bug A: Q segments don't set inslope (for GTK/WPE)

  Location — line 133-136: Add m_inslopePoints assignment:

  Current:
      case PathElement::Type::AddQuadCurveToPoint:
          // FIXME: https://bugs.webkit.org/show_bug.cgi?id=33115
          m_origin = points[1];
          break;

  Fix:
      case PathElement::Type::AddQuadCurveToPoint:
          m_inslopePoints[0] = points[0];
          m_inslopePoints[1] = points[1];
          m_origin = points[1];
          break;

[Karl Dubost]

Created attachment 478892 [details]
bugA-testcase

[Karl Dubost]

Created attachment 478893 [details]
testcase A + B