Bug 23927

Created attachment 27605 [details] Patch for getTransformToElement fix This patch fixes one test on PPC Mac

Created attachment 27608 [details] Patch to skip SVG tests until problem is resolved This patch skips 16 svg tests until the number printing problem is resolved

Comment on attachment 27608 [details] Patch to skip SVG tests until problem is resolved r=me, but make sure you keep this bug open, and assign it to yourself.

Applied getTransfrmToElement patch Sending LayoutTests/ChangeLog Sending LayoutTests/svg/custom/getTransformToElement.svg Transmitting file data .. Committed revision 40914.

Applied patch to skip some tests Sending LayoutTests/ChangeLog Sending LayoutTests/platform/mac/Skipped Transmitting file data .. Committed revision 40915.

I'm leaving this bug open. LayoutTests are now clear (WRT these issues), but some tests are being skipped, so the underlying problem still needs to be resolved.

Comment on attachment 27605 [details] Patch for getTransformToElement fix Clearing review flag to remove from commit queue.

Comment on attachment 27608 [details] Patch to skip SVG tests until problem is resolved Clearing review flag to remove from commit queue.

Hm, any news regarding this bug? I'd love to see it resolved. The problem is the way Path::debugString() (which is used by DRT -> SVGRenderTreeAsText -> RenderPath -> Path::debugString()) works: it's manually traversing the path items, building up the d=".." string thatt will be dumped in the -expected.txt files (for any SVG test). We should make that code cross-platform (why should every platform have to write it's own debugString() method, that's only used for layout tests anyways), and fix the rounding issues alltogether. We shouldn't skip SVG tests for a long period like that :(

I'm also running into display problems with the precision of large numbers here: https://bugs.webkit.org/show_bug.cgi?id=25645 What would it involve to change Path::debugString() to output numbers in scientific (exponential) notation for sufficiently large (or small) numbers in a consistent manner across all platforms? Are people generally against this since it would mean rebaselining a bunch of tests? That seems better than skipping them.

(In reply to comment #10) > I'm also running into display problems with the precision of large numbers > here: https://bugs.webkit.org/show_bug.cgi?id=25645 > > What would it involve to change Path::debugString() to output numbers in > scientific (exponential) notation for sufficiently large (or small) numbers in > a consistent manner across all platforms? Are people generally against this > since it would mean rebaselining a bunch of tests? That seems better than > skipping them. I don't believe that the issues here is relatet to the one in bug 25645. But to answer your question, you have to change the platform-specific debug infos at WebCore/platform/graphcis/<platform>/Path*.cpp But some platforms, like cairo, don't support float values, they use fixed point instead. So you would break many DRT-tests by changing debugString.

I agree that making a cross-platform version of Path::debugString() makes the most sense.

(In reply to comment #11) > I don't believe that the issues here is relatet to the one in bug 25645. But to > answer your question, you have to change the platform-specific debug infos at > WebCore/platform/graphcis/<platform>/Path*.cpp > But some platforms, like cairo, don't support float values, they use fixed > point instead. So you would break many DRT-tests by changing debugString. It's unrelated except that they both output using debugString(). That's what I thought about debugString() -- that changing it would break everything. Thanks for confirming that. I guess what we should aim for is round-trip preservation of values, so if I enter 12345678901234567890 then debugString should output "12345678901234567890[.00]". That's probably very difficult to do.

(In reply to comment #12) > I agree that making a cross-platform version of Path::debugString() makes the > most sense. Not sure if it makes sense and realy depends on how we are doing it. E.g. if we save every point , every line or curve or transformation in an extra array, we don't know what the different platforms are realy doing. The question is, if we care about the real output, or if we care about what webkit wants to draw (independent of what we see on the screen). Both things can be from interest. Note, that transformations on a path would also cause transformations on previous stored values on the array, for the platform independent solution.

Just brainstormed this again with Dirk and there's a new idea: How about utilizing the already present coordinate information (path segments + coordinates) from the SVG DOM when dumping the RenderPath debugString? Instead of pulling it from the engines (ask Cg for path segments..) we could ask just access the node() of the RenderPath, which is a SVGPathElement. This contains a SVGPathSegList with all necessary information. The only drawback is that we have to manually transform the coordinates into absolute coords on screen, to get the same results we dump now (same coord space). The benefit is that OpenVG which doesn't even offer access to the path segments can easily be supported. The drawback is that we loose information from the engine itself. We could do following: Keep the current debugString implementation seperated, implement a new cross-platfrom debugstring utiliyzing SVGPathSegList and compare the transformed coordinates from the engine and our internally stored coordinates from SVGPathSegList using a tolerance (aka. "fuzzy compare"). If we see significant deviations, we could dump two debug strings in the DRT results, indicating an error, if not we could just dump the cross-platform version. Details need to be worked out, but we think this may be the right step in the right direction. Comments appreciated!

I'm going to reenable the tests in question, it makes no sense to skip them just because of 0.0 vs -0.0 issues. The tests are helpful, and we want them nontheless. The debugString() problem is being worked on at the moment, and we need these tests to verify the problem is gone, see bug 47340.

Marked as duplicate of 47467. *** This bug has been marked as a duplicate of bug 47467 ***