Bug 111766

Created attachment 192072 [details] Patch

Comment on attachment 192072 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192072&action=review > Source/WebCore/rendering/ExclusionPolygon.cpp:77 > +FloatPolygon::FloatPolygon(PassOwnPtr<Vector<FloatPoint> > vertices, WindRule fillRule) New classes usually get new files. I would like to have this in a separate file as well. Is it generic enough to move it to WebCore/platform? If not, what would make it generic?

(In reply to comment #2) > (From update of attachment 192072 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=192072&action=review > > > Source/WebCore/rendering/ExclusionPolygon.cpp:77 > > +FloatPolygon::FloatPolygon(PassOwnPtr<Vector<FloatPoint> > vertices, WindRule fillRule) > > New classes usually get new files. I would like to have this in a separate file as well. Is it generic enough to move it to WebCore/platform? If not, what would make it generic? To make this class generic, I'd need to remove the following two methods, since they're rather specific to Exclusion processing: void computeXIntersections(float y, bool isMinY, Vector<ExclusionInterval>&) const; void computeOverlappingEdgeXProjections(float minY, float maxY, Vector<ExclusionInterval>&) const; To move FloatPolygon to its own pair of files, I'd also move/include VertexPair and FloatPolygonEdge. And perhaps VertexPair should be called FloatVertexPair. I'd call it FloatPointPair, but it exists to provide access to a pair of points called "vertices".

Created attachment 192268 [details] Patch Refactored platform/graphics/FloatPolygon class out of rendering/ExclusionPolygon per Dirk's feedback.

Comment on attachment 192268 [details] Patch Attachment 192268 [details] did not pass qt-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17113152

Created attachment 192275 [details] Patch Updated Qt Target.pri build file.

Comment on attachment 192275 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192275&action=review I would like to wait for the bots before giving it my final review. > Source/WebCore/ChangeLog:1295 > (WebCore::FormData::appendKeyValuePairItems): > > +2013-03-07 Hans Muller <hmuller@adobe.com> > + > + [CSS Exclusions] Refactor the ExclusionPolygon class to enable storing multiple boundaries > + https://bugs.webkit.org/show_bug.cgi?id=111766 > + > + Reviewed by NOBODY (OOPS!). > + > + Refactored the ExclusionPolygon class to enable adding support for shape-margin and shape-padding. > + Extracted a new FloatPolygon class which is now used by ExclusionPolygon to represent the shape's > + boundary. It will be used to add m_paddedPolygon and m_marginPolygon members to ExclusionPolygon Merge problem here.

Created attachment 192284 [details] Patch Removed duplicate ChangeLog entry.

Created attachment 192480 [details] Patch Resync'd with latest build file changes. Hopefully that will make the recalcitrant EWS bots happier.

Comment on attachment 192480 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192480&action=review I am sorry. It took me a bit longer to review it. The patch looks great to me. Just some snippets before landing. > Source/WebCore/rendering/ExclusionPolygon.cpp:138 > + for (unsigned i = 0; i < edges.size(); ++i) I would keep the braces here, since the content of the for loop is more than a line. > Source/WebCore/rendering/ExclusionPolygon.h:40 > #include "FloatPoint.h" > +#include "FloatPolygon.h" > #include "FloatRect.h" > -#include "PODIntervalTree.h" > #include "WindRule.h" > -#include <wtf/MathExtras.h> > #include <wtf/OwnPtr.h> > #include <wtf/PassOwnPtr.h> Can't your remove FloatPoint, FloatRect, WindRule, OwnPtr etc. dependencies here? Should be included in FloatPolygon already.

Created attachment 192546 [details] Patch Made the suggested changes.

Comment on attachment 192546 [details] Patch Clearing flags on attachment: 192546 Committed r145411: <http://trac.webkit.org/changeset/145411>

All reviewed patches have been landed. Closing bug.