- From: Cameron McCormack <cam@mcc.id.au>
- Date: Tue, 18 Mar 2014 22:27:52 +0800
- To: Dirk Schulze <dschulze@adobe.com>
- CC: "public-fx@w3.org" <public-fx@w3.org>
Hi Dirk, Here are some review comments on CSS Masking. I'm focusing mostly on meaningful things, but I've included a couple of comments about wording. https://dvcs.w3.org/hg/FXTF/raw-file/061c12d148be/css-masking-1/index.html == 1.1 Clipping == > The clip-path property can reference an SVG graphics element or use a > specified basic shapes as clipping path. I don't think the clip-path property can reference any SVG graphics element. It's only a <clipPath> element. == 1.2 Masking == > Note: While masking gives many possibilities for enhanced graphical > effects and in general provides more control over the “visible > portions” of the content, clipping paths can perform better and are > easier to interpolate. It's not clear to me how clipping paths are easier to interpolate. == 3 Values == > In addition to the property-specific values listed in their > definitions, all properties defined in this specification also accept > the inherit keyword as their property value. For readability it has > not been repeated explicitly. Surely also 'initial' and 'unset'. Maybe there is some more up-to-date text you can use here. == 5 Clipping Paths == > The clipping path restricts the region to which paint can be applied, > the so-called clipping region. Conceptually, any parts of the drawing > that lie outside of this region are not drawn. This includes any > content, background, borders, text decoration, outline and visible > scrolling mechanism of the element to which the clipping path is > applied, and those of its descendants. I think this is saying that anything that the element paints is affected by the clipping path. Examples not included in your list are selections, the caret, maybe some SVG specific things. Maybe you should say explicitly that anything painted is affected. > A clipping path is conceptually equivalent to a custom viewport for > the element it applies to. Is it really conceptually equivalent? Maybe "viewport" is not the right term, since that makes me think in the SVG context of something that provides a coordinate space, while in CSS terms of something that might be scrollable depending on the value of 'overflow'. > The boundaries of a clipped element (i.e. an element which references > a clipPath element via a clip-path property, or a child of the > referencing element) must remain the same as if it were not clipped. What is a boundary? > By default, pointer events must not be dispatched on the clipped > (non-visible) regions of a shape. For example, an element with a > dimension of 10px to 10px which is clipped to a circle with a radius > of 5px will not receive click events outside the clipping region. It might be confusing here that "clipped region" and "clipping region" are being used to mean opposite things. (The first is the part that is clipped away and the second is the part that remains after clipping.) == 5.1 Clipping Shape: the clip-path property == > Applies to: All elements. In SVG, it applies to container elements > without the <defs> element and all graphics elements s/without/excluding/ Does clip-path really apply to <mask> elements? > Animatable: See shape-outside [CSS-SHAPES] The definition of animation behaviour for shape-outside doesn't include all of the values that clip-path can take. In this section you should say something about whether the border radius of a given <geometry-box> are also taken into account when a <basic-shape> is specified. Should the definitions of fill-box, stroke-box and view-box have the same kind of wording as for <geometry-box> about what it means to use the keyword without the <basic-shape>? You link to the SVG 1.1 definitions of the viewBox="" attribute and the "SVG viewport" term. Given you are using other definitions from SVG 2, should you reference SVG 2 here for everything? > For SVG elements without associated CSS layout box, the values > content-box, padding-box, border-box and margin-box compute to > fill-box. You say that they compute to fill-box, but the property definition table doesn't say anything about computing keywords to other keywords. The wording "For SVG elements without associated CSS layout box…" sounds like there are some SVG elements that do have an associated CSS layout box (is that the same thing as "CSS box"?). Since there are none, you might want to say "For SVG elements, which do not have associated CSS layout boxes, …" instead. > A computed value of other than none results in the creation of a > stacking context [CSS21] the same way that CSS opacity [CSS3COLOR] > does. How is it the same way? When it has a value other than 0? (That's what I would think as being "the same what the CSS opacity does", but obviously that's not what's meant.) == 6.1 The <clipPath> element == > Categories: None Shouldn't this say "Container element" instead of "None"? The content model doesn't need to mention <animateColor> any more, now that it's been removed from the spec. In the attributes list, clipPathUnits should have quotes around it to match the other ones. > For a given graphics element, the actual clipping path used will be > the intersection of the clipping path specified by its clip-path > property (if any) with any clipping paths on its ancestors, as > specified by the clip-path property on the elements which establish a > new viewport. (See [SVG11]) This should apply to any element you put clip-path on, not just SVG graphics elements. == 6.2 Winding Rules: the clip-rule property == I think this property should be defined without reference to SVG 1.1, since SVG 2 is going to rely on this specification to define clip-rule, and it would be a bit strange to then have to follow back to 1.1 for what nonzero and evenodd mean. == 7.2 Mask Image Interpretation: the mask-type property == Did the definition of this property change? My implementation interprets mask-type as defining whether a given <mask> element is a luminance or alpha mask, so you would put it on the <mask> element, not the element being masked. http://mcc.id.au/blog/2012/12/mask-type (And I think that's how you implemented it initially too: http://trac.webkit.org/changeset/129018) It looks like this became mask-source-type, and mask-type is now used to control referenced <mask> elements. Is there a reason for the naming, or could we have mask-type be the one that applies to <mask> and mask-source-type be the one that applies to masked elements? I think that might make slightly more sense. The example in this section talks about mask-source-type overriding mask-type, but that isn't mentioned at all in the property definition. (It's mentioned in the definition of mask-source-type, but it would be good to mention it here too. Or at least forward reference it; the example at the moment sounds like we should already know that fact.) == 7.5 Masking Area: the mask-clip property == > Determines the mask painting area, which determines the area that is > affected by the mask. The painted content of an element may be > restricted to this area. What does it mean by "may be restricted to this area"? I assume you don't mean "MAY". Is this just the area that the mask affects, and outside that area, the element is painted unmasked? I think you need a more detailed explanation because it's not really clear just from those two sentences. == 7.6 Positioning Area: the mask-origin property == I don't really understand the green box note that talks about the top left of a mask being clipped. Is this a special case that's interesting to point out? == 7.8 Mask Shorthand: the mask property == I don't know if talking about the "used value" is the right thing to cause mask-repeat, mask-position, etc. not have any effect if you're referencing a <mask> element. I think mask-image shouldn't be in that list, because that's how you reference the <mask> element. > The mask shorthand also resets mask-box to its initial value. mask-box is a shorthand, and shorthands don't have initial values. You should list the specific longhands that get reset. == 7.9 The Mask Image Rendering Model == > The application of the mask-image property to an element formatted > with the CSS box model establishes a stacking context… This sounds like even <span style="mask-image: none"> will establish a stacking context. == 7.9.1 Mask processing == > Regions not covered by a mask image are treated as transparent black. Depending on the answer to my question in section 7.5, this might not always be the case. == 8.1 Mask Image Source: the mask-box-source property == How does mask-box-source relate to mask-image? It seems like they both allow referencing an image to be used as the mask image. == 9.1 The mask element == Some attributes in the element definition table are missing quotes around their names. == 10 Security == (I didn't review this section closely.) What happened to the 'child' value that was going to allow us to write things like the following? <g mask="child"> <rect .../> <mask> ... </mask> </g>
Received on Tuesday, 18 March 2014 14:28:29 UTC