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.

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

RoundRect for Canvas2D #5619

Closed
Tracked by #5613
fserb opened this issue Jun 8, 2020 · 6 comments · Fixed by #6765
Closed
Tracked by #5613

RoundRect for Canvas2D #5619

fserb opened this issue Jun 8, 2020 · 6 comments · Fixed by #6765
Labels
addition/proposal New features or enhancements topic: canvas

Comments

@fserb
Copy link
Contributor

fserb commented Jun 8, 2020

Allows to render rectangles with rounded corners.

Working proposal: https://github.com/fserb/canvas2D/blob/master/spec/roundrect.md

(cc @whatwg/canvas )

@fserb fserb mentioned this issue Jun 8, 2020
9 tasks
@annevk annevk added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: canvas labels Jun 9, 2020
@litherum
Copy link

litherum commented Jun 9, 2020

The WebKit team feels this is a good direction.

@annevk annevk removed the needs implementer interest Moving the issue forward requires implementers to express interest label Jun 9, 2020
@othermaciej
Copy link
Collaborator

othermaciej commented Jun 9, 2020

It would be nice if the radii didn't have to be in an array (though I realize that would end up being a lot of parameters). There are surely other ways to allow specifying a variable number of them.

@mysteryDate
Copy link
Contributor

@Kaiido had the interesting idea to make it a part of the path2d API: fserb/canvas2D#7

They're correct that path objects are more feature rich, though it would make it an extra stop removed from the developers. Any thoughts here?

@annevk
Copy link
Member

annevk commented Aug 17, 2020

Could you maybe sketch out the two approaches through code samples so it's a bit clearer what the advantages and drawbacks might be?

@Kaiido
Copy link
Member

Kaiido commented Aug 18, 2020

Not sure to which "two approaches" the last comment was referring to, but I think it might be worth clarifying my point anyway.

So to be 100% clear, I am proposing that instead of adding a new CanvasRoundRect mixin, the existing CanvasPath one be extended with a single roundRect method.

interface mixin CanvasPath {
  // addition:
  void roundRect(double x, double y, double w, double h, sequence<double> radius);
}

This would then get inherited by all the Interfaces that do include this mixin: CanvasRenderingContext2D, OffscreenCanvasRenderingContext2D and Path2D.
On the other hand, the CanvasRoundRect mixin could only be included by the two former interfaces and not on Path2D where filling or stroking have no meaning.

Basically, the differences are about the same as fillRect() + strokeRect() + clearRect() vs rect().

On its own mixin (current proposal), the RoundRect primitive would live only in its own sub-path, outside of the context's current default path.
This would prevent it from being used with a lot of the context's methods working with a path. For instance it would be impossible to clip() using a RoundRect:

//### as an addition to CanvasPath
ctx.beginPath();
ctx.roundRect(25, 25, 250, 100, [25]);
ctx.clip();  // now the context is clipped to this RoundRect shape
//### as its own mixin
// can't do...

The same situation would occur with other methods like isPointInPath, isPointInStroke, scrollToPath etc.

Having it on the CanvasPath mixin also allows to generate more complex paths that would get filled or stroked in a single call, avoiding heavy transitions between CPU and GPU:

//### as an addition to CanvasPath
ctx.beginPath();
rects.forEach(({ x, y }) => ctx.roundRect(x, y, 25, 25, [10]));
ctx.fill(); // a single draw operation
//### as its own mixin
rects.forEach(({ x, y }) => ctx.fillRoundRect(x, y, 25, 25, [10]));  // a draw operation per rect

And of course, it allows to store these in Path2D objects, which couldn't be done with the current proposal.


The only "advantages" of having it on its own mixin would be:

  • One liner filling or stroking of a RoundRect vs 3 lines:
//### as an addition to CanvasPath
ctx.beginPath();
ctx.roundRect(25, 25, 50, 50, [10]);
ctx.fill(); // or .stroke()
//### as its own mixin
ctx.fillRoundRect(25, 25, 50, 50, [10]); // or .strokeRoundRect(...
  • One liner clearing of the context vs 5 lines
//### as an addition to CanvasPath
ctx.beginPath();
ctx.roundRect(25, 25, 50, 50, [10]);
ctx.globalCompositeOperation = "destination-out";
ctx.fill();
ctx.globalCompositeOperation = "source-over";
//### as its own mixin
ctx.clearRoundRect(25, 25, 50, 50, [10]);

I'm not sure these "advantages" outweigh the drawbacks, moreover when most of web-devs are already used to have similar methods like ellipse on the CanvasPath mixin.

@mysteryDate
Copy link
Contributor

I'm convinced by this argument. My only other thought is that the CanvasPath stuff is somewhat opaquely implemented and perhaps it'll get less use as a CanvasPath object than as its own stand-alone thing (the whole point is to provide an obvious shortcut, after all). I don't have any actual evidence to back this up though.

Is there any reason why we can't have both?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements topic: canvas
Development

Successfully merging a pull request may close this issue.

6 participants