Bug 33784 – [Qt] Modifying SVG path dumping to equal to other ports

• Component: SVG
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

example: http://build.webkit.org/results/Qt%20Linux%20Release/r53390%20%286095%29/svg/custom/transformedMaskFails-pretty-diff.html
...
on Mac: [data="M0.00,0.00 L150.00,0.00 L150.00,150.00 L0.00,150.00 Z"]
on Qt:  [data="M 0 0L 150 0L 150 150L 0 150L 0 0"]
...

We should remove space after M and L, use fixed point real numbers with 2 decimal, use Z to close path instead of L to starting point.

I have a preliminary patch to fix it except closing path. I try to fix it and then we should update many expected files at the same time.

But unfortunately we have to use platform dependent expected files until fix.

[Nikolas Zimmermann]

Hi Csaba,

thanks for the bug report. We should really rewrite to have a cross-platform Path::debugString() implementation, by supplying a path-applier function, that's used to iterate over all path segments (using Path::apply()) in order to dump them.

Shouldn't be much work, does anyone volunteer? :-)

[Dirk Schulze]

(In reply to comment #1)
> Hi Csaba,
> 
> thanks for the bug report. We should really rewrite to have a cross-platform
> Path::debugString() implementation, by supplying a path-applier function,
> that's used to iterate over all path segments (using Path::apply()) in order to
> dump them.
> 
> Shouldn't be much work, does anyone volunteer? :-)

Also it is not possible for qt to match the LayoutTest results. Qt closes paths, once they cross each other, while SVG doesn't. Not a problem for the visible result, but gives wrong output for DRT.
Qt also closes path's automaticly, if start and ending point match each other. This causes one of the W3C tests to fail.
Altogether, it's realy better to follow Nikos advice, even if I still did not understand how he wants to do it ;-)

[Csaba Osztrogonác]

Created attachment 48236 [details]
make path dump closer to Mac

Now I can't produce a cross-platform Path::debugString() implementation, 
but I would like to make our expected files more similar to Mac.

Proposed patch to modify path dumping and update expected files attached.

[WebKit Review Bot]

Attachment 48236 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/graphics/qt/PathQt.cpp:314:  Missing space after ,  [whitespace/comma] [3]
WebCore/platform/graphics/qt/PathQt.cpp:317:  Missing space after ,  [whitespace/comma] [3]
WebCore/platform/graphics/qt/PathQt.cpp:327:  Missing space after ,  [whitespace/comma] [3]
Total errors found: 3


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dirk Schulze]

Comment on attachment 48236 [details]
make path dump closer to Mac

r=me, but please add spaces after the commas in arg(...)

[Csaba Osztrogonác]

Comment on attachment 48236 [details]
make path dump closer to Mac

Style fixed, long line splitted as Kenneth asked on #qtwebkit, 
and then patch landed in http://trac.webkit.org/changeset/54429

[Dirk Schulze]

Can we close the bug now?

[Csaba Osztrogonác]

(In reply to comment #7)
> Can we close the bug now?

I think we shouldn't close this bug. Not yet. Because we have some more little differences between Mac and Qt SVG path dumping results. Additionally we have a strange problem because 0.00 isn't equal to -0.00. It needs more investigation.

[Andreas Kling]

This was fixed by <http://trac.webkit.org/changeset/69386> since we no longer dump platform-specific data for RenderSVGPath.