Path::createEllipse() - why not leverage port paths?
Hey folks, Is there a reason we're manually creating the ellipses in Path::createEllipse() instead of using the port-specific Path::addEllipse()? For the SVG version of the benchmark at http://www.themaninblue.com/writing/perspective/2010/03/22/ we're spending roughly 25% of runtime in Path::createEllipse(), whereas if I let it use Qt's Path::addEllipse() it drops to 2.5%. (Qt is also much better at rendering the paths generated by QPainterPath::addEllipse() instead of the 100x LineTo paths from Path::createEllipse - time spent filling paths goes down from 30% to 25%.) All in all, these are 15 additional FPS that I'd love to get if possible. So lay it on me :-) Curiously yours, -Kling
The main reason is DumpRenderTree. If we use the Path::createEllipse logic of the different platforms, we end up with different LayoutTest results on SVG between platform, since we create them by traversing the path data of the certain platform. So changing this is painful and causes a lot of new platform dependent results. But I would support this! :-) Btw, we have the same problem with Path::createCircle. Dirk Am Sonntag, den 19.09.2010, 05:11 +0200 schrieb Andreas Kling:
Hey folks,
Is there a reason we're manually creating the ellipses in Path::createEllipse() instead of using the port-specific Path::addEllipse()?
For the SVG version of the benchmark at http://www.themaninblue.com/writing/perspective/2010/03/22/ we're spending roughly 25% of runtime in Path::createEllipse(), whereas if I let it use Qt's Path::addEllipse() it drops to 2.5%.
(Qt is also much better at rendering the paths generated by QPainterPath::addEllipse() instead of the 100x LineTo paths from Path::createEllipse - time spent filling paths goes down from 30% to 25%.)
All in all, these are 15 additional FPS that I'd love to get if possible. So lay it on me :-)
Curiously yours, -Kling _______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Am Sonntag, den 19.09.2010, 09:32 +0200 schrieb Dirk Schulze:
The main reason is DumpRenderTree. Let me rephrase it: The only reason why we still use this logic is DumpRenderTree.
If we use the Path::createEllipse logic of the different platforms, we end up with different LayoutTest results on SVG between platform, since we create them by traversing the path data of the certain platform. Just an examples: IIRC CoreGraphics uses cubic or qudratic curves to draw an ellipse, Cairo uses arc and scales the path context to create an ellipse, not sure what Skia or Qt are doing internally, but because of the traversing of the path data, we definitely end up with different results on DRT.
Dirk
Am 19.09.2010 um 09:41 schrieb Dirk Schulze:
Am Sonntag, den 19.09.2010, 09:32 +0200 schrieb Dirk Schulze:
The main reason is DumpRenderTree. Let me rephrase it: The only reason why we still use this logic is DumpRenderTree.
If we use the Path::createEllipse logic of the different platforms, we end up with different LayoutTest results on SVG between platform, since we create them by traversing the path data of the certain platform. Just an examples: IIRC CoreGraphics uses cubic or qudratic curves to draw an ellipse, Cairo uses arc and scales the path context to create an ellipse, not sure what Skia or Qt are doing internally, but because of the traversing of the path data, we definitely end up with different results on DRT.
That's part of the reason, why I proposed to use a cross-platform version of dumping the path data strings. We should utilize the SVGPathByteStream, which already contains a representation of the _parsed_ path data to dump the DRT data. There will always be differences, across platforms, when asking the underlying graphics system for the path segments. What we really want to see in the dumps is the parsed path data that gets passed to the graphics systems. To summarize: Let's go for platform specific variants of Path::createCircle/Path::createEllipse, but change the DRT dumps. For primitive shapes, we shouldn't dump any path data _at all_ (eg. circle / ellipse / rect etc) but instead just dump "RenderPath {rect} x=0, y=0, width=50, height=50". That would make our DRT dumps much easier to read. Andreas, interessted to work on that? Cheers, Niko
On 09/19/2010 11:28 AM, ext Nikolas Zimmermann wrote:
To summarize: Let's go for platform specific variants of Path::createCircle/Path::createEllipse, but change the DRT dumps. For primitive shapes, we shouldn't dump any path data _at all_ (eg. circle / ellipse / rect etc) but instead just dump "RenderPath {rect} x=0, y=0, width=50, height=50". That would make our DRT dumps much easier to read.
Andreas, interessted to work on that?
I'm on it. Thanks for the input guys! -Kling
participants (3)
-
Andreas Kling
-
Dirk Schulze
-
Nikolas Zimmermann