sonic boom trail issue - Githubissues

webaverse / app

Web metaverse client

https://app.webaverse.com

MIT License

343 stars 207 forks source link

sonic boom trail issue #3667

Closed tcm390 closed 2 years ago

tcm390 commented 2 years ago

issue:

https://user-images.githubusercontent.com/60634884/186944727-c8fa2823-4bd4-4a87-89e5-1dc4a932ea8c.mp4

Result:

https://user-images.githubusercontent.com/60634884/186944754-8b493e04-9399-4488-9d61-70f9c12695f3.mp4

avaer commented 2 years ago

The functionality looks good but:

This PR changes textures (why is it not another PR?)
The code is unreadable due to having so many shaders so cannot be reviewed
Need to check that there is no performance regressions

avaer commented 2 years ago

Actually I'd like to merge this anyway. But it's marked as a Draft. Could you clarify @tcm390?

tcm390 commented 2 years ago

The functionality looks good but:

This PR changes textures (why is it not another PR?)

The code is unreadable due to having so many shaders so cannot be reviewed

Need to check that there is no performance regressions

Got it would check the performance regression

tcm390 commented 2 years ago

I used h panel to test it. I think there is no performance regressions(inspect the index.js, both GPU and CPU performance are improved) and also the hiccup on the first time use of sonicBoom is eliminated

Result:

https://user-images.githubusercontent.com/60634884/187049431-3c96a623-5930-4256-a724-fad75565bc41.mp4

old one:

https://user-images.githubusercontent.com/60634884/187049311-a52a45cf-e060-4457-b80b-374416973b0c.mp4

avaer commented 2 years ago

H can never be used to test performance, because the probes reduce performance.

@tcm390 please keep that in mind from now on.