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

[RFC]: Add Testing Feature ctx.__getClipRegion() #54

Closed
tmarti opened this issue Nov 5, 2019 · 4 comments · Fixed by #58
Closed

[RFC]: Add Testing Feature ctx.__getClipRegion() #54

tmarti opened this issue Nov 5, 2019 · 4 comments · Fixed by #58

Comments

@tmarti
Copy link

tmarti commented Nov 5, 2019

Hello,

According to...

https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/save

The drawing state that gets saved onto a stack consists of:

  • ...
  • The current clipping region.
  • ...

... the clipping region is also involved in ctx.save() and ctx.restore() operations, but that seems not to be implemented in jest-canvas-mock.

A very easy solution to this would be to create a _clipStack object on the main class to keep the saved clips. Not sure though which should be the initial clip (on the bottom of the stack).

PS: sorry to open so many issues lately, but this library is a fine piece of work even more incredible after solving those small issues 👌. Thank you!

@jtenner
Copy link
Collaborator

jtenner commented Nov 5, 2019

Right. So implementing this feature isn't exactly a couple hours of work, but it should be doable. The problem is definitely that save and restore make this implementation quite complicated.

I am happy to help add this feature, but I'd rather mentor someone to learn how to help maintain this software through pair programming. Is this something that interests you?

@jtenner jtenner added this to the 2.3.0 milestone Nov 5, 2019
@tmarti
Copy link
Author

tmarti commented Nov 6, 2019

The problem with the clip

Right. So implementing this feature isn't exactly a couple hours of work, but it should be doable. The problem is definitely that save and restore make this implementation quite complicated.

I think it could be easier than it sounds:

You already mantain all the other stacks, so it's just a matter of adding a _clipStack and:

  • push to into the _clipStack during save()

  • popping from the _clipStack during restore ()

  • updating the top of the _clipStack during clip (...) processing

  • initialize the _clipStack:

    Not sure though which should be the initial clip (on the bottom of the stack).

    Not sure if the stack should be statically initialized as...

    _clipStack = [ null ];

    ... or with an empty path.

Also not sure if the clip should reset after some draw operation... I would bet it shouldn't.

The collaboration opportunity

I am happy to help add this feature, but I'd rather mentor someone to learn how to help maintain this software through pair programming. Is this something that interests you?

Right now I'm quite busy with managing some developments at work, and dedicate most of the free time to experiment with prototypes. So many thanks for the offer and I'm of course open to requests for ideas and clarifications, but I cannot ensure to be able to consistently dedicate time to this pair programming you are suggesting. So I must decline the proposition.

Nevertheless, I'm open to questions about ideas and clarifications.

Many many thanks for you efforts on mantaining such a fine library 🙂.

@jtenner
Copy link
Collaborator

jtenner commented Nov 15, 2019

I'm still trying to think how this should be accomplished, becuase it's not exactly obvious what instructions should be copied over into a CanvasInstruction[][] stack. It could use the current path, but what about repeated calls to clip()?

The clip function essentially "closes" the path. So I think the clip() instruction should be pushed to the array too.

Also, if more instructions are added to the path and it is clipped before it's reset with .beginPath() should it copy the whole __path entirely again into the _clipStack?

ctx.beginPath();
ctx.arc(1, 2, 3, 4, 5);
ctx.clip(); // copy the previous instructions
ctx.rect(1, 2, 3, 4);
ctx.clip(); // only copy the rect and the current clip

It seems that this might require keeping track of a written index inside the path buffer for the clipping region.

@jtenner
Copy link
Collaborator

jtenner commented Nov 21, 2019

I will probably start development on this next week, and do a minor version bump.

@jtenner jtenner changed the title [missing feature]: save/restore clipping region [RFC]: Add Testing Feature ctx.__getClipRegion() Nov 21, 2019
@jtenner jtenner modified the milestones: 2.3.0, 2.4.0 Nov 21, 2019
@jtenner jtenner self-assigned this Nov 21, 2019
@jtenner jtenner modified the milestones: 2.4.0, 2.3.0 Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants