Bug 124080 - Web Inspector: [CSS Shapes] Support raster shape visualizations
Summary: Web Inspector: [CSS Shapes] Support raster shape visualizations
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Bear Travis
URL:
Keywords: InRadar
Depends on: 124071
Blocks: 124070
  Show dependency treegraph
 
Reported: 2013-11-08 16:05 PST by Bear Travis
Modified: 2013-12-04 14:22 PST (History)
10 users (show)

See Also:


Attachments
Initial patch (8.32 KB, patch)
2013-11-25 11:40 PST, Bear Travis
no flags Details | Formatted Diff | Diff
Rectangle version (8.36 KB, patch)
2013-11-26 11:37 PST, Bear Travis
no flags Details | Formatted Diff | Diff
Updated patch (7.57 KB, patch)
2013-11-26 14:55 PST, Bear Travis
joepeck: review+
Details | Formatted Diff | Diff
Incorporating feedback (7.80 KB, patch)
2013-12-04 13:30 PST, Bear Travis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bear Travis 2013-11-08 16:05:26 PST
Properly visualize the raster shapes (url values) for the CSS Shape properties.
Comment 1 Radar WebKit Bug Importer 2013-11-08 16:05:58 PST
<rdar://problem/15429461>
Comment 2 Bear Travis 2013-11-25 11:40:12 PST
Created attachment 217819 [details]
Initial patch
Comment 3 Bear Travis 2013-11-26 11:37:14 PST
Created attachment 217893 [details]
Rectangle version
Comment 4 Hans Muller 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 
}
Comment 5 Bear Travis 2013-11-26 14:55:56 PST
Created attachment 217905 [details]
Updated patch
Comment 6 Joseph Pecoraro 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.
Comment 7 Bear Travis 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.
Comment 8 WebKit Commit Bot 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>
Comment 9 WebKit Commit Bot 2013-12-04 14:22:11 PST
All reviewed patches have been landed.  Closing bug.