[webkit-reviews] review canceled: [Bug 34366] [OpenVG] Implement support for paths : [Attachment 47755] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Feb 8 22:46:10 PST 2010
Ariya Hidayat <ariya.hidayat at gmail.com> has canceled Jakob Petsovits
<jpetsovits at rim.com>'s request for review:
Bug 34366: [OpenVG] Implement support for paths
https://bugs.webkit.org/show_bug.cgi?id=34366
Attachment 47755: Patch
https://bugs.webkit.org/attachment.cgi?id=47755&action=review
------- Additional Comments from Ariya Hidayat <ariya.hidayat at gmail.com>
Some minor remarks.
> +Path* PainterOpenVG::currentPath()
> +{
> + return m_currentPath;
> +}
Shouldn't this be a const?
> +bool Path::contains(const FloatPoint& point, WindRule rule) const
> +{
> + notImplemented();
> +
> + return boundingRect().contains(point);
> +}
Which one is correct?
> +bool Path::strokeContains(StrokeStyleApplier* applier, const FloatPoint&
point) const
> +{
> + notImplemented();
> +
> + return (const_cast<Path*>(this))->strokeBoundingRect().contains(point);
> +}
idem.
> +FloatRect Path::strokeBoundingRect(StrokeStyleApplier* applier)
> +{
> + notImplemented();
> +
> + return boundingRect();
> +}
idem.
> +String Path::debugString() const
> +{
> + String debugString = "";
> +
> + /*
> + m_path->makeCompatibleContextCurrent();
> + VGint numSegments = vgGetParameteri(m_path->vgPath(),
VG_PATH_NUM_SEGMENTS);
> +
> + for (int i = 0; i < numSegments; i++) {
> + // Hm, OpenVG provides no means to retrieve path segment
information?
> + // This is a bit unfortunate, we might need to store the segments in
> + // memory if we want to implement this function properly.
> + }
> + */
> +
> + return debugString;
> +}
Maybe just notImplemented() with the said comment?
> +void Path::apply(void* info, PathApplierFunction function) const
> +{
> + /*
> + m_path->makeCompatibleContextCurrent();
> + VGint numSegments = vgGetParameteri(m_path->vgPath(),
VG_PATH_NUM_SEGMENTS);
> +
> + for (int i = 0; i < numSegments; i++) {
> + // Hm, OpenVG provides no means to retrieve path segment
information?
> + // This is *very* unfortunate, we might need to store the segments
in
> + // memory if we want to implement this function properly.
> + // See
http://www.khronos.org/message_boards/viewtopic.php?f=6&t=1887
> + // Bleh.
> + }
> + */
> +}
idem.
More information about the webkit-reviews
mailing list