Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add conic-gradient #6661

Merged
merged 18 commits into from
May 18, 2021
Merged

Add conic-gradient #6661

Changes from 13 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c34a139
merge to masteer branch
yiyix Apr 2, 2020
2263557
small fix
yiyix Mar 17, 2021
27a4c5c
changes
yiyix Apr 6, 2021
c464652
Add more text modifier properties
yiyix Apr 2, 2020
2f236c9
pull
yiyix Apr 6, 2021
7429bc0
address comments
yiyix Apr 8, 2021
44daada
address comments
yiyix Apr 14, 2021
b44ec37
update text preparation algorithm
yiyix Apr 15, 2021
c3238b9
Merge remote-tracking branch 'upstream/main'
yiyix Apr 20, 2021
be3c905
update text preparation algorithm
yiyix Apr 21, 2021
aad64f0
formatting nits
annevk Apr 26, 2021
7ac879d
Merge remote-tracking branch 'upstream/main'
yiyix May 5, 2021
68eb67d
add conic gradient
yiyix May 7, 2021
c404de9
fix typo
yiyix May 7, 2021
15686d4
address comments
yiyix May 8, 2021
8bb28c0
update conicgradient to take radians
yiyix May 12, 2021
5dee3b5
Editorial tweaks
domenic May 12, 2021
f3d6c6e
Fix nits
domenic May 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3388,6 +3388,7 @@ a.setAttribute('href', 'https://example.com/'); // change the content attribute
<li><dfn data-x-href="https://drafts.csswg.org/css-images/#intrinsic-height">intrinsic height</dfn></li>
<li><dfn data-x-href="https://drafts.csswg.org/css-images/#intrinsic-width">intrinsic width</dfn></li>
<li>The <dfn data-x-href="https://drafts.csswg.org/css-images-3/#the-image-orientation">'image-orientation'</dfn> property</li>
<li><dfn data-x-href="https://drafts.csswg.org/css-images-4/#funcdef-conic-gradient">conic-gradient</dfn></li>
yiyix marked this conversation as resolved.
Show resolved Hide resolved
<li>The <dfn data-x-href="https://drafts.csswg.org/css-images/#the-object-fit">'object-fit'</dfn> property</li>
</ul>

Expand Down Expand Up @@ -60353,6 +60354,7 @@ interface mixin <dfn>CanvasFillStrokeStyles</dfn> {
attribute (DOMString or CanvasGradient or CanvasPattern) <span data-x="dom-context-2d-fillStyle">fillStyle</span>; // (default black)
<span>CanvasGradient</span> <span data-x="dom-context-2d-createLinearGradient">createLinearGradient</span>(double x0, double y0, double x1, double y1);
<span>CanvasGradient</span> <span data-x="dom-context-2d-createRadialGradient">createRadialGradient</span>(double x0, double y0, double r0, double x1, double y1, double r1);
<span>CanvasGradient</span> <span data-x="dom-context-2d-createConicGradient">createConicFradient</span>(double startAngle, double x, double y);
yiyix marked this conversation as resolved.
Show resolved Hide resolved
<span>CanvasPattern</span>? <span data-x="dom-context-2d-createPattern">createPattern</span>(<span>CanvasImageSource</span> image, [<span>LegacyNullToEmptyString</span>] DOMString repetition);
<!--
// v8 we received one request from Ralf Richard G&oml;bel for a new kind of pattern: a hatch.
Expand Down Expand Up @@ -63308,8 +63310,8 @@ try {

<hr>

<p>There are two types of gradients, linear gradients and radial gradients, both represented by
objects implementing the opaque <code>CanvasGradient</code> interface.</p>
<p>There are three types of gradients, linear gradients, radial gradients and conic gradients,
yiyix marked this conversation as resolved.
Show resolved Hide resolved
represented by objects implementing the opaque <code>CanvasGradient</code> interface.</p>

<p id="interpolation">Once a gradient has been created (see below), stops are placed along it to
define how the colors are distributed along the gradient. <span w-nodev>The color of the
Expand Down Expand Up @@ -63350,6 +63352,13 @@ try {
<p>If either of the radii are negative, throws an
<span>"<code>IndexSizeError</code>"</span> <code>DOMException</code> exception.</p>
</dd>

<dt><var>gradient</var> = <var>context</var> . <code subdfn data-x="dom-context-2d-createConicGradient">createConicGradient</code>(<var>startAngle</var>, <var>x</var>, <var>y</var>)</dt>

<dd>
<p>Returns a <code>CanvasGradient</code> object that represents a
conic gradient that paints clockwise along the rotation around the center
represented by the arguments.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: indentation and wrapping is off and misses an end tag.

</dl>

<div w-nodev>
Expand Down Expand Up @@ -63387,7 +63396,6 @@ try {
</li>
</ol>


<p>The <dfn method for="CanvasFillStrokeStyles"><code
data-x="dom-context-2d-createLinearGradient">createLinearGradient(<var>x0</var>, <var>y0</var>,
<var>x1</var>, <var>y1</var>)</code></dfn> method takes four arguments that represent the start
Expand Down Expand Up @@ -63448,6 +63456,18 @@ try {
<p>The resulting radial gradient must then be transformed as described by the <span
data-x="dom-context-2d-transformation">current transformation matrix</span> when rendering.</p>

<p>The <dfn method for="CanvasFillStrokeStyles"><code
data-x="dom-context-2d-createConicGradient">createConicGradient(<var>startAngle</var>, <var>x</var>,
<var>y</var>)</code></dfn> method takes three arguments, the first argument, <var>startAngle</var>,
represents the angle in degree at which begins the gradient, and the last two arguements,
yiyix marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

@Kaiido Kaiido May 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where it has been defined that angle should be in degree.
In #5431 (comment) @nt1m was asking if this should be in radians or degrees, but it seems there hasn't been any response in that thread, and given current Nightly still uses radians in the implementation, I guess they haven't been made aware of that decision to use degrees.

For what it's worth, if it's still time to have this discussion, I feel as a web-author that having the angle in degrees is very confusing: If I'm not mistaken, it is the only place in the whole canvas API that we would have an angle expressed in degrees instead of in radians. So if we want to play with that angle and e.g the transformation of the context, or the start/end angles of an ellipse, we'd have to convert between degrees and radians, not to mention the mental gymnastic of remembering where to use what...
If the goal was to have a simpler mapping from CSS, then note that only the <angle> 0 can have the unit omitted, and that CSS has a rad unit.

Note that there is already a discrepancy with arc (and ellipse), where both startAngle and endAngle are relative to the x-axis, while CSS conic gradients asks that "0deg points to the top of the page", but that one sounds a bit less dramatic to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(@Kaiido to be clear, now is definitely the time. Thanks for raising this! See also https://whatwg.org/working-mode#changes if you want to get more familiar with the process in general.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for radians. I'm not an expert, but all the JS APIs I know of use (e.g. the Math functions) use radians, and canvas should definitely be internally consistent.

The inconistency between axes does seem problematic though. Would it make sense to make the mapping (startAngle + Math.PI / 2) radians so that it matches other canvas APIs? We could also even add a counterclockwise boolean like those APIs have.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I implemented this many months ago and cannot remember why I chose degrees. I think I was trying to match some CSS behaviour.
I also would prefer radians, personally. +1 to rotating everything by PI/2 as well to match arc and ellipse.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strong agree that an angle number without additional type info should be interpreted as radians, coming from a JS API.

Argh at the mismatch of canvas api start angle. CSS copied from SVG, using "bearing angles". I think we should match existing canvas precedent here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am so glad we have this discussion here now before i change more Canvas API.

+1 for radians and rotating everything by PI/2. Canvas API should be consistent. Changing spec. I will also change our implementation to map this.

(<var>x</var>, <var>y</var>), represent the center of the gradient. The method, when invoked, must
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: exceeds a 100 columns.

return a conic <code>CanvasGradient</code> initialized with the specified center.</p>
yiyix marked this conversation as resolved.
Show resolved Hide resolved

It follows the same rendering rule as CSS <span>conic-gradient</span>. The <var>position</var> is
the center defined by <var>x</var> and <var>y</var> in the coordinate space. This function is
domenic marked this conversation as resolved.
Show resolved Hide resolved
equivalent to CSS conic-gradient(from <var>startAngle</var> deg at <var>x</var> px <var>y</var> px, <var>angular-color-stop-list</var>),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the convention (e.g. in https://html.spec.whatwg.org/#flow-content-3:is-modal) is to wrap CSS expressions in single quotes '.

domenic marked this conversation as resolved.
Show resolved Hide resolved
where <var>angular-color-stop-list</var> is to be applied to the <code>CanvasGradient</code>.
domenic marked this conversation as resolved.
Show resolved Hide resolved
domenic marked this conversation as resolved.
Show resolved Hide resolved

<p>Gradients must be painted only where the relevant stroking or filling effects requires that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does anything need to be said about the coordinate spaces involved? Like, are CSS pixels the same as the canvas pixels?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it helps the clarification, i can add something. I see CSS pixel defined and used in Canvas, (ex: actualBoundingBoxRight). I can may add it in the previous paragraph where i explain what is x and y

they be drawn.</p>

Expand Down