p5.js icon indicating copy to clipboard operation
p5.js copied to clipboard

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

Open VANSH3104 opened this issue 2 months ago • 16 comments

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:

Screenshot 2025-11-06 at 1 09 33 PM

PR Checklist

VANSH3104 avatar Nov 06 '25 07:11 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

VANSH3104 avatar Nov 06 '25 07:11 VANSH3104

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

VANSH3104 avatar Nov 11 '25 19:11 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 avatar Nov 11 '25 19:11 VANSH3104

Screenshot 2025-11-12 at 1 32 57 AM it shows this @perminder-17

VANSH3104 avatar Nov 11 '25 20:11 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

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 avatar Nov 12 '25 08:11 perminder-17

Screenshot 2025-11-12 at 1 32 57 AM 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?

perminder-17 avatar Nov 12 '25 09:11 perminder-17

@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 avatar Nov 12 '25 12:11 VANSH3104

Screenshot 2025-11-12 at 6 32 47 PM

@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

VANSH3104 avatar Nov 12 '25 13:11 VANSH3104

@perminder-17 I implemented the clip transform fix we discussed. Following our conversation about the double transform issue where scale(2) was behaving like scale(4), I used the manual coordinate transformation approach. Instead of complex matrix math, I transform all coordinates to absolute screen space upfront, then reset the transform in endClip() before applying the clip. This ensures transforms are applied only once during path creation, not again during clipping.

VANSH3104 avatar Nov 19 '25 08:11 VANSH3104

@perminder-17 am I on the right track ?

VANSH3104 avatar Nov 19 '25 08:11 VANSH3104

@perminder-17 is this need any changes?

VANSH3104 avatar Nov 21 '25 12:11 VANSH3104

@perminder-17 should i apply this on other shapes also ?

VANSH3104 avatar Nov 25 '25 09:11 VANSH3104

@perminder-17 should i apply it to all shapes it look good from my side looking for your suggestion.

VANSH3104 avatar Nov 27 '25 08:11 VANSH3104

@davepagurek i add this to all other shapes also can you please check that too?

VANSH3104 avatar Dec 01 '25 05:12 VANSH3104

@perminder-17 can you check this please

VANSH3104 avatar Dec 03 '25 11:12 VANSH3104

@perminder-17 @davepagurek could either of you please review this code please?

VANSH3104 avatar Dec 03 '25 11:12 VANSH3104

Hi @davepagurek , whenever you have a moment, I'd be grateful for your insights on this. Please let me know if you see anything that should be adjusted.

VANSH3104 avatar Dec 16 '25 18:12 VANSH3104