Re: review of CSS Masking

Hello Cameron,

Thank you very much for your review of the CSS Masking specification.

On Mar 18, 2014, at 3:27 PM, Cameron McCormack <cam@mcc.id.au> wrote:

> 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.

It can’t, fixed.

> 
> == 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.

Especially animations of CSS basic shapes are easier for authors to specify and easier for implementations to animate as well as more performant than masking.

> 
> == 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.

The spec text wasn’t updated in any other CSS spec. I changed the text slightly. It says: “[…] keywords such as inherit”.

> 
> == 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.

The very first sentence says that: “ region to which paint can be applied”.

> 
>> 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’.

I changed the wording. It should be more clear now.

> 
>> 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?

I changed boundary to geometry of the element.

> 
>> 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.)

Yes, I changed the text to the text that Chris suggested.

> 
> == 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/

Fixed.

> 
> Does clip-path really apply to <mask> elements?

Yes, indeed it does! At the moment that is just implemented by Internet Explorer.

> 
>> 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.

You are right. The other values can not be animated. I clarified the text.

> 
> 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.

This is not part of CSS Shapes yet.

> 
> 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>?

We do not allow animating them yet.

> 
> 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?

I will rather remove the linking to SVG2 to avoid any dependency of the spec to a work in progress document. This will be part of the next commit probably.

> 
>> 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.

I changed the behavior in the spec now. The computed value is not affected anymore but the rendering is. We do that in different other places in CSS already such as transform-style.

> 
>> 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.)

:) Clarified it in the spec.

> 
> == 6.1 The <clipPath> element ==
> 
>> Categories: None
> 
> Shouldn't this say "Container element" instead of "None”?

This is not defined as container element in SVG 1.1 and behaves slightly different than the other elements in the group. For now we probably should not change that. Other specifications like transforms and blending do depend on the current definition of container elements.

> 
> The content model doesn't need to mention <animateColor> any more, now that it's been removed from the spec.

It doesn’t once we publish SVG2. For now, the specification relies on SVG 1.1 which still defines animateColor.

> 
> In the attributes list, clipPathUnits should have quotes around it to match the other ones.

Fixed.

> 
>> 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.

This paragraph and the paragraph before target graphics elements within the <clipPath> element. <clipPath> just allows graphics elements as children at this point.

> 
> == 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.

As mentioned above, I would like to avoid any reference of SVG2. SVG1.1 defines the fill-rule property and even more important: the algorithms for even-odd/nonzero. Copying these paragraphs doesn’t seem to be an option. SVG2 will have the same paragraphs for fill-rule as well by the way.

> 
> == 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.)

You are right that we swapped the names of mask-source-type and mask-source. I added more notes to make it more obvious.

> 
> == 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.

I changed the spec text. It does no longer use “may”.

> 
> == 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?

It clarifies that mask-clip does not effect mask-origin even though the image itself is effected.

> 
> == 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.

I linked to the definition of "used value" in question. I agree that mask-image should not be in the list. I removed it.

> 
>> 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.

Yes, that was an oversight. I fixed it.

> 
> == 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.

I changed the wording and clarified the description more.

> 
> == 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.

Yes, thanks.

> 
> == 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.

I added more notes that explain the relation ship for both, mask-image and mask-box-source. I hope this makes it more clear.

> 
> == 9.1 The mask element ==
> 
> Some attributes in the element definition table are missing quotes around their names.

I fixed that.

> 
> == 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>

This was deferred. Across all CSS specifications we have 3 different ways to select a certain element. CSS3 UI for instance has one for the navigational properties/values, GCPM another. The CSS WG wants to investigate if we can combine the use cases and come up with one type.

Greetings,
Dirk

Received on Friday, 11 April 2014 09:17:20 UTC