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

[VANSH3104]

Resolves #7903

Changes:

Modified ellipse() method in p5.Renderer2D to apply transforms during clipping
Used existing this.initialClipTransform (inverse of initial transform) stored in beginClip()
Applied correct matrix multiplication order: initialClipTransform.multiply(currentTransform)

Screenshots of the change:

PR Checklist

• npm run lint passes
• Inline reference is included / updated
• Unit tests are included / updated

[VANSH3104]

@davepagurek I've implemented the manual fix for the ellipse() method and it's working correctly now!
I think moving to the class-based approach you suggested would be better long-term. Should I extend this manual fix to other primitives (rect, arc, etc.) first, or start working on migrating them to the shape system? and also drawshape function is useless i think its never call while clipping on or off

[perminder-17]

function setup() {
  createCanvas(400, 200);
  pixelDensity(1);
  noLoop();
}

function draw() {
  background(240);
  translate(200, 0); 
  clip(() => {
    scale(-1,1);                 
    ellipse(50, 100, 100, 100);  
  });
  fill(0, 0, 255, 120);
  ellipse(100, 100, 300, 300); 
}

what you get when you use this sketch? I think transformation is not correctly applied?

[perminder-17]

Also, your current solution applies to ellipse only? Do you think we need to add this solution in other shapes as well? arc/quad/line etc?

btw, thanks for working on this one.

[VANSH3104]

@perminder-17 yes it is only applicable for ellipse now I need to change for all too

[VANSH3104]

@perminder-17 I also have another idea class based approach also just want to discuss which one is better. I think This approach is temporary we need to find a perfect solution for that

[VANSH3104]

it shows this @perminder-17

[perminder-17]

@perminder-17 I also have another idea class based approach also just want to discuss which one is better. I think This approach is temporary we need to find a perfect solution for that

I saw @davepagurek's comment about the shape-system approach. It would be great long-term and would also unlock future tasks, but it feels like a fairly large refactor right now with higher regression risk. Since we already have the alternative approach working, how about we stick with that for now and plan to migrate the primitives into the shape drawing system later?

[perminder-17]

it shows this @perminder-17

I think this is not the expected behavior.

I think you're storing inverse of initial clip transform into beginClip(), and multiplying with currentTransform ,

inside endClip() clip() is running with the current transform, and it's applying again i think. I am totally not sure with this but

Since, in the 1.x versions, it works something like: https://editor.p5js.org/aman12345/sketches/Csnrbz9BX , so would be great to test that too.

CC: @davepagurek for the confirmation on the approach as well as the bug if it's exist?

[VANSH3104]

@perminder-17 i think When we using scale(-1,1) inside clip(), the negative scale breaks the matrix calculations and makes the clipping region invisible. Basic clipping are working fine, but it give issue with complex transformations.

[VANSH3104]

@perminder-17 Now it work as expected it can handle -ve values also and i add condition and use absolute values to find the original position

[perminder-17]

Looks like you're now forcing the geometry with negative transformations to make it positive. If the user actually wanted to mirror the orientation, it will actually cause problem to the geometry. I still think the problem is that our transformation is applied twice.

I just tested code, and it looks like:

When I apply scale(3) , the expected result is:

The current behavior in the changes of this PR:

function setup() {
  createCanvas(500, 500);
  background(240);

  clip(()=>{
    scale(3);
    ellipse(20, 40, 40, 40); 
  })
  fill(255, 80, 80);
  rect(0, 0, 500, 500);
}

this is the code I am using, Please ignore the calculations, I was just testing scale() and other transformations behaviour.

scale(2) is equivalent to scale(4), scale(3) is equivalent to scale(9) and I think this is the problem with the negetive stuff as well.

As I was mentioning on previous comments,

I think you're storing inverse of initial clip transform into beginClip(), and multiplying with currentTransform , inside endClip() clip() is running with the current transform, and it's applying again i think. I am totally not sure with this but

[VANSH3104]

@perminder-17 Yes you re right I test it I am working on the solution

[perminder-17]

@perminder-17 Yes you re right I test it I am working on the solution

Feel free to reach out to me or @davepagurek if you face problems on the solution.