Skip to content

Fix transforms in clip() by using shape system for primitives using manual method #8236

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

Open
VANSH3104 wants to merge 9 commits into
base: dev-2.0
Choose a base branch
from
+111 −30
Open

Fix transforms in clip() by using shape system for primitives using manual method #8236

Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a5e1523
Fix transforms in clip() by using shape system for primitives using m…
VANSH3104 Nov 6, 2025
c4b5ca5
Merge branch 'dev-2.0' into transform/Clip
perminder-17 Nov 11, 2025
cc98f19
Handle negative scales in clipping by using absolute values for scale…
VANSH3104 Nov 12, 2025
34c4706
Merge branch 'dev-2.0' into transform/Clip
perminder-17 Nov 13, 2025
e5f5a41
Prevent double transform application in clip() with absolute coordinates
VANSH3104 Nov 19, 2025
22f8093
Merge branch 'dev-2.0' into transform/Clip
perminder-17 Nov 26, 2025
fc826d4
Reset transforms during clip application to ensure proper coordinate …
VANSH3104 Nov 27, 2025
5a49130
implement clipping with transform support for all 2D shapes
VANSH3104 Dec 1, 2025
902d2f6
Merge branch 'dev-2.0' into transform/Clip
perminder-17 Dec 5, 2025
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
57 changes: 46 additions & 11 deletions src/core/p5.Renderer2D.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ class Renderer2D extends Renderer {
// Start a new path. Everything from here on out should become part of this
// one path so that we can clip to the whole thing.
this.clipPath = new Path2D();
this.initialTransform = this.drawingContext.getTransform();

if (this._clipInvert) {
// Slight hack: draw a big rectangle over everything with reverse winding
Expand All @@ -330,7 +331,10 @@ class Renderer2D extends Renderer {
}

endClip() {
const currentTransform = this.drawingContext.getTransform();
this.drawingContext.setTransform(1, 0, 0, 1, 0, 0);
this.drawingContext.clip(this.clipPath);
this.drawingContext.setTransform(currentTransform);
this.clipPath = null;

super.endClip();
Expand Down Expand Up @@ -705,7 +709,7 @@ class Renderer2D extends Renderer {
}

ellipse(args) {
const ctx = this.clipPath || this.drawingContext;
const ctx = this.drawingContext;
const doFill = !!this.states.fillColor,
doStroke = this.states.strokeColor;
const x = parseFloat(args[0]),
Expand All @@ -725,17 +729,48 @@ class Renderer2D extends Renderer {
centerY = y + h / 2,
radiusX = w / 2,
radiusY = h / 2;
if (!this._clipping) ctx.beginPath();

ctx.ellipse(centerX, centerY, radiusX, radiusY, 0, 0, 2 * Math.PI);
ctx.closePath();

if (!this._clipping && doFill) {
ctx.fill();
}
if (!this._clipping && doStroke) {
ctx.stroke();
if (this._clipping) {
const tempPath = new Path2D();
const current = this.drawingContext.getTransform();
// Transform coordinates manually but preserve scale signs
const transformPoint = (x, y) => {
return {
x: current.a * x + current.c * y + current.e,
y: current.b * x + current.d * y + current.f
};
};
const transformedCenter = transformPoint(centerX, centerY);
// Calculate transformed radii WITHOUT Math.abs() to preserve negative scaling
const scaleX = Math.sqrt(current.a * current.a + current.c * current.c);
const scaleY = Math.sqrt(current.b * current.b + current.d * current.d);

// Preserve the sign of the scaling for mirroring effects
const signX = current.a < 0 ? -1 : 1;
const signY = current.d < 0 ? -1 : 1;

const transformedRadiusX = scaleX * radiusX * signX;
const transformedRadiusY = scaleY * radiusY * signY;
tempPath.ellipse(
transformedCenter.x,
transformedCenter.y,
Math.abs(transformedRadiusX),
Math.abs(transformedRadiusY),
0, 0, 2 * Math.PI
);
this.clipPath.addPath(tempPath);
} else {
ctx.beginPath();
ctx.ellipse(centerX, centerY, radiusX, radiusY, 0, 0, 2 * Math.PI);
ctx.closePath();
if (doFill) {
ctx.fill();
}
if (doStroke) {
ctx.stroke();
}
Comment on lines 740 to 756
Copy link
Collaborator

@perminder-17 perminder-17 Nov 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @VANSH3104 I am sorry for the delay in the response, I didn't got time to review this one. I just did some testing with this, the behaviour looks fine to me.

But, you should implement it not in a manual way but something like storing this.drawingContext.getTransform().inverse() at the start of clip() and then multiply that with this.drawingContext.getTransform(). The exact way which you do in your first commit: a5e1523

So, that commit will apply extra transforms internally, like you would be getting scale(2) ---> as scale(4).

Fix for that would be:

in the beginClip() we should fetching the transformations.

beginClip(){
//..
this._clipBaseTransform = this.drawingContext.getTransform();
//....

And in the endClip() we must set that.

    const saved = this.drawingContext.getTransform(); // here we will save the current transform to a variable.
    this.drawingContext.setTransform(this._clipBaseTransform); // While applying the clip, the canvas was given the same transform as _clipBaseTransform.
    this.drawingContext.clip(this.clipPath);   // Now the clipPath will apply on the correct space.                 
    this.drawingContext.setTransform(saved);  // In this puts your original/current transform back. Restoring that.

This could be the flow, no extra transform/rotations will be applied. Can you please check? Don't go with the manula logic, use the multiply logic which you used in your first commit. Thanks for the hlep.

Copy link
Contributor Author

@VANSH3104 VANSH3104 Nov 27, 2025

Choose a reason for hiding this comment

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

@perminder-17 thanks for the review i have implement yor suggestion of transform storing. I also test this and it work on all cases that we discussed previously

}

return this;
}

line(x1, y1, x2, y2) {
Expand Down
Loading