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

Line.vert fix for small units

Open TiborUdvari opened this issue 1 year ago β€’ 1 comments

Resolves #7200

Resolves #[Add issue number here]

Changes:

Screenshots of the change:

PR Checklist

TiborUdvari avatar Sep 03 '24 11:09 TiborUdvari

πŸŽ‰ Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page!

welcome[bot] avatar Sep 03 '24 11:09 welcome[bot]

Hi @TiborUdvari , it's a great work.

Thank you for opening the pull request! I just tested the issues you were facing, specifically the offset for small coordinates being too high. Fortunately, it doesn't seem to reopen the initial issue (https://github.com/processing/p5.js/issues/6956).

Your implementation of dynamicScale and dynamicZAdjustment is great and working well.

I do have one question regarding the "popping" effect you are talking here https://github.com/processing/p5.js/issues/7200#issuecomment-2326473376. I wasn't able to reproduce it using the code in your PR. Could you share the code where you encountered this effect? Additionally, if it’s still reproducible, does the logic @davepagurek mentioned (facingCamera == 1.0 ? 1.0 : mix(0.999, 0.995, facingCamera)) resolve the issue, or do we need to explore other solutions?

perminder-17 avatar Sep 15 '24 22:09 perminder-17

I don't think I understood 100% the issues with the popping effects, so I didn't go further on this. What I didn't like about my solution is that it was a mix between two solutions and I didn't understand the whole problem space, so I didn't go further on it. It does apply a quick fix for what we have now though.

I had to monkey patch p5.xr with the previous version of the shader as a temporary solution, as it really affects most of the AR examples, as they use mostly real world coords, in meters.

Unfortunately didn't have the time to go deeper on this.

TiborUdvari avatar Sep 17 '24 13:09 TiborUdvari

I didn't like about my solution is that it was a mix between two solutions and I didn't understand the whole problem space

This might not be the cleanest solution, but given the current options (since we haven't found any better/cleaner solution yet) I think your solution works well. As for the flickering or popping effects, I couldn't reproduce them on my end. I also reviewed the reference documentation with your code, and everything looks fine to me. Thanks for the fix!

perminder-17 avatar Sep 18 '24 23:09 perminder-17