RESOLVED FIXED 124080
Web Inspector: [CSS Shapes] Support raster shape visualizations
https://bugs.webkit.org/show_bug.cgi?id=124080
Summary Web Inspector: [CSS Shapes] Support raster shape visualizations
Bear Travis
Reported 2013-11-08 16:05:26 PST
Properly visualize the raster shapes (url values) for the CSS Shape properties.
Attachments
Initial patch (8.32 KB, patch)
2013-11-25 11:40 PST, Bear Travis
no flags
Rectangle version (8.36 KB, patch)
2013-11-26 11:37 PST, Bear Travis
no flags
Updated patch (7.57 KB, patch)
2013-11-26 14:55 PST, Bear Travis
joepeck: review+
Incorporating feedback (7.80 KB, patch)
2013-12-04 13:30 PST, Bear Travis
no flags
Radar WebKit Bug Importer
Comment 1 2013-11-08 16:05:58 PST
Bear Travis
Comment 2 2013-11-25 11:40:12 PST
Created attachment 217819 [details] Initial patch
Bear Travis
Comment 3 2013-11-26 11:37:14 PST
Created attachment 217893 [details] Rectangle version
Hans Muller
Comment 4 2013-11-26 12:07:50 PST
(In reply to comment #3) > Created an attachment (id=217893) [details] > Rectangle version This line from the ChangeLog could probably just be left out: > To do this, create lines from the leftmost to rightmost point on each line of the image. I think RasterShapeIntervals::buildPath(), could be simpler. You could use Path::adRect() instead of the local addRectangle() method. Since all of this stuff is inlined and optimized, you need not create local variables like pos and end. And you could stick with "y" since that matches the term bounds() uses for vertical coordinates. You could use the existing RasterShapeIntervals limitIntervalAt() method. So modulo collapsing rectangles, something like: if (bounds().isEmpty()) return; for (int y = bounds().y(); y < bounds().maxY(), ++y) { if (intervalsAt(y).isEmpty()) continue; path.addRect(limitIntervalAt(y)); // IntRect to FloatRect conversion }
Bear Travis
Comment 5 2013-11-26 14:55:56 PST
Created attachment 217905 [details] Updated patch
Joseph Pecoraro
Comment 6 2013-12-04 12:06:01 PST
Comment on attachment 217905 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=217905&action=review r=me > Source/WebCore/rendering/shapes/RasterShape.cpp:266 > + for (; endY < bounds().maxY(); endY++) { > + if (intervalsAt(endY).isEmpty() || limitIntervalAt(endY) != extent) > + break; > + } It looks to me like limitIntervalAt will give you one big rect at a y, even if there are holes between shapes on that y. So essentially you'll get an outline of all the shapes, but you won't get perfectly accurate information within the outlines. In the future it might be nice to show exactly where the shape boundaries are in an overlay. But that was not the intent of this patch. > Source/WebCore/rendering/shapes/RasterShape.h:58 > + void buildPath(Path&) const; Maybe this could be made something like "buildBoundsPath" / "buildOutlineLimitPath" to be more specific? I could imagine a "buildShapesPath" which build path as specific as possible. > LayoutTests/inspector-protocol/model/highlight-shape-outside.html:38 > +.raster-simple { > + -webkit-shape-outside: url("data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 100 100' preserveAspectRatio='none' width='100px' height='100px'><rect x='25' y='25' width='50' height='50' fill='blue' /></svg>"); > +} I'd like to see a test with a data:image/png like some of the other -webkit-shape-outside tests. Maybe just a simple 25x25 png.
Bear Travis
Comment 7 2013-12-04 13:30:38 PST
Created attachment 218439 [details] Incorporating feedback Added a .png test, and modified the name to buildBoundsPath. Eventually, I may try to get the exact shape outlines up and running, but this seemed like a good place to start.
WebKit Commit Bot
Comment 8 2013-12-04 14:22:08 PST
Comment on attachment 218439 [details] Incorporating feedback Clearing flags on attachment: 218439 Committed r160127: <http://trac.webkit.org/changeset/160127>
WebKit Commit Bot
Comment 9 2013-12-04 14:22:11 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.