Bug 29696 - [CHROMIUM] Paths fail to render in SVG with large viewbox
Summary: [CHROMIUM] Paths fail to render in SVG with large viewbox
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Stephen White
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-09-23 15:12 PDT by Stephen White
Modified: 2009-09-29 14:11 PDT (History)
2 users (show)

See Also:


Attachments
Fix for SVG/skia coordinate issue (2.28 KB, patch)
2009-09-23 15:13 PDT, Stephen White
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephen White 2009-09-23 15:12:18 PDT
SVG paths with large coordinate values (and correspondingly large viewbox) are not rendered in Chrome.

See http://crbug.com/21174.
Comment 1 Stephen White 2009-09-23 15:13:39 PDT
Created attachment 40021 [details]
Fix for SVG/skia coordinate issue
Comment 2 Eric Seidel (no email) 2009-09-23 17:51:51 PDT
Comment on attachment 40021 [details]
Fix for SVG/skia coordinate issue

I'm just gonna trust you that this is safe.  The Skia seat-belt is so lame. :(  If you want this committed by the commit-queue you'll need to set commit-queue=?.  I can't remember if you're a committer or not.
Comment 3 Stephen White 2009-09-24 06:50:37 PDT
(In reply to comment #2)
> (From update of attachment 40021 [details])
> I'm just gonna trust you that this is safe.

I ran it by the layout trybots on all three platforms, if that makes you feel better.

> The Skia seat-belt is so lame. :( 

The best solution for that (IMHO) is to write floating point (or even better, SSE2) versions of the pixel loops, so there are no fixed-point limitations to worry about.
Comment 4 Eric Seidel (no email) 2009-09-24 13:49:42 PDT
Comment on attachment 40021 [details]
Fix for SVG/skia coordinate issue

According to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/modules/committers.py Stephen is not a committer, but it looks like he was invited August 4th.  So I'll assume he just hasn't added himself to that list yet and leave this for him to commit.
Comment 5 Stephen White 2009-09-24 14:18:53 PDT
(In reply to comment #4)
> (From update of attachment 40021 [details])
> According to
> http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/modules/committers.py
> Stephen is not a committer, but it looks like he was invited August 4th.  So
> I'll assume he just hasn't added himself to that list yet and leave this for
> him to commit.

Actually, I did commit it, in the end.. is there something I should do so that appears here?
Comment 6 Eric Seidel (no email) 2009-09-26 01:28:56 PDT
If you committed this bug, than you should update the bug with the revision number that you committed and close it.  Searching the commit logs, I don't see that this has been committed though.
Comment 7 Roland Steiner 2009-09-27 19:24:27 PDT
(In reply to comment #3)
> The best solution for that (IMHO) is to write floating point (or even better,
> SSE2) versions of the pixel loops, so there are no fixed-point limitations to
> worry about.

I agree that this is probably the best interim fix for this bug. 

However, it means that the paths that are not rendered due to my original patch are (potentially) rendered wrong instead. Until the fixed-point implementation in Skia is changed, there doesn't seem to be much that one could do about it, though.

Very minor nit: in currentPathInLocalCoordinates(): one can wait with copying the patch until the matrix inversion has run, in case that fails. Since that's an exceptional case however, there shouldn't be much real impact one way or the other.
Comment 8 Eric Seidel (no email) 2009-09-29 14:11:59 PDT
This was landed as r48726 and can be closed.