Bug 5359 - WebKit+SVG is missing marker support
Summary: WebKit+SVG is missing marker support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P4 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-13 01:20 PDT by Eric Seidel (no email)
Modified: 2005-11-28 18:00 PST (History)
0 users

See Also:


Attachments
Improved marker support (5.43 KB, patch)
2005-10-23 03:23 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Proposed patch for marker support (7.44 KB, patch)
2005-11-22 09:10 PST, Julien Palmas
no flags Details | Formatted Diff | Diff
Improved Patch (8.42 KB, patch)
2005-11-22 09:33 PST, Julien Palmas
no flags Details | Formatted Diff | Diff
Should solve the markerUnits issue (9.11 KB, patch)
2005-11-22 15:12 PST, Julien Palmas
eric: review-
Details | Formatted Diff | Diff
Patch improved with Eric's ideas (14.09 KB, patch)
2005-11-28 12:37 PST, Julien Palmas
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2005-10-13 01:20:59 PDT
WebKit+SVG is missing marker support

I believe ksvg2 already actually has marker support for AGG (and possibly Cairo), but I just haven't 
gotten around to implementing it for the kcanvas Quartz backend.

This would be a relatively easy fix for someone with some c++ skills looking to dive into KCanvas.

There are several w3c tests for markers, at least these two:
painting-marker-01-f.svg
painting-marker-02-f.svg

Right now you just see a bunch of these logs when running the tests:
2005-10-13 01:19:44.566 DumpKCanvasTree[15508] *** Marker support not implemented.
2005-10-13 01:19:44.567 DumpKCanvasTree[15508] *** Marker support not implemented.
Comment 1 Rob Buis 2005-10-23 03:22:25 PDT
Hi, 
 
I recently did some work to fix marker support in ksvg2 again 
somewhat. It is not perfect, for instance clipping is not correct, 
but it should fix some testcases. I'll add it so hopefully it helps 
in porting to OS X.  
Cheers, 
 
Rob. 
Comment 2 Rob Buis 2005-10-23 03:23:05 PDT
Created attachment 4445 [details]
Improved marker support
Comment 3 Julien Palmas 2005-11-22 09:10:36 PST
Created attachment 4766 [details]
Proposed patch for marker support

This patch essentially gets the path vertices coords and use them to draw the
markers at the correct position.
More testing is necessary to see if marker scaling, rotating and so on work
correctly.
Comment 4 Julien Palmas 2005-11-22 09:33:06 PST
Created attachment 4767 [details]
Improved Patch

Pulled out some code from ksvg2. It should improve scaling among other things.
SVGMarkerElementImpl::canvasResource
Comment 5 Julien Palmas 2005-11-22 15:12:56 PST
Created attachment 4776 [details]
Should solve the markerUnits issue
Comment 6 Eric Seidel (no email) 2005-11-23 04:20:10 PST
Comment on attachment 4776 [details]
Should solve the markerUnits issue

We discussed this on IRC.  Still needs some tweaks.
Comment 7 Julien Palmas 2005-11-28 12:37:17 PST
Created attachment 4836 [details]
Patch improved with Eric's ideas

This patch uses a CGPathApplierFunction to draw the markers.
2 structures are used to help the drawing : MarkerData	and DrawMarkersData.

In KCanvasRenderingStyle::updateStroke : 
Solves a problem when using a <marker markerUnits="strokeWidth"> with a <path
stroke="none" stroke-width="8">

KCanvasMarker::draw doesn't use objectMatrix as an argument anymore.
Modifies KCanvasMarker::draw to setDrawsContents(true) before marker rendering
and setDrawsContents(false) after.

KCanvasMarker::refX(), KCanvasMarker::refY() => KCanvasMarker::setRef(double
refX, double refY)
KCanvasMarker::setScaleX(), KCanvasMarker:: setScaleY() =>
KCanvasMarker::setScale(float scaleX, float scaleY)

Adds CGPointSubtractPoints to QuartzSupport.
Comment 8 Eric Seidel (no email) 2005-11-28 13:07:10 PST
Comment on attachment 4836 [details]
Patch improved with Eric's ideas

Looks great!
Comment 9 Eric Seidel (no email) 2005-11-28 18:00:57 PST
Your patch produced a regression in all of the layout tests when I attempted to land.

The line at fault was:

+    strokePainter()->setStrokeWidth(cssPrimitiveToLength(item, m_style->svgStyle()->strokeWidth(), 
1.0));

which created a strokePainter() even when there was no stroke.  (I know this is very confusing code... 
KCanvasRenderingStyle all needs to go away, and will *soon*.)

I instead made cssPrimitiveToLength a static method on KCanvasRenderingStyle and just called it from 
your drawMarkersIfNeeded function to compute the strokeWidth there, which works even when 
stroke=none.

I made a couple other minor style updates and landed.