overte-org / overte

Overte open source virtual worlds platform.
https://overte.org/
Other
138 stars 50 forks source link

Clean up GeometryCache and remove _glColor4f #845

Closed HifiExperiments closed 6 months ago

HifiExperiments commented 7 months ago
  1. Cleanup lots of unused cruft in GeometryCache and simplify the remainder, including using per-instance attributes for all the renderXXXX methods when possible instead of repeating data
  2. Remove uses of hacky _glColor4f and replace with per-instance buffers managed by owners

The latter change should fix #11 and maybe #799 (I wasn't able to reproduce either with this PR but we should test more), as well as make it easier to switch to Vulkan. _glColor4f was used for specifying a fallback vertex color for when models or shapes didn't have them. This is because our shaders assume the existence of vertex colors, with a default of white. By specifying this fallback color, we can avoid having different shader variants for hasVertexColor/!hasVertexColor, which saves us from expensive pipeline switches. But the state management was always messy, leading to colors "bleeding" between objects. Now everything manages its own true color buffer, simplifying the state management.

Testing

Make sure shapes and models work, test vertex colors, make sure other entity types like text, image, and line (laser pointers) still work.

ksuprynowicz commented 7 months ago

I tested it and almost everything works perfectly in both deferred and forward renderer, including lasers and create app gizmos. The one thing that is broken currently is rendering objects with lightmaps. Something very weird happens to them both in deferred and forward renderer: 20240302_161451_MixedReality 20240302_161515_MixedReality 20240302_161728_MixedReality 20240302_161737_MixedReality

HifiExperiments commented 7 months ago

@ksuprynowicz ooooo interesting! I will take a look

JulianGro commented 7 months ago

The trees in the tutorial don't appear for me on this PR. Might be glTF related though?

ksuprynowicz commented 7 months ago

Lightmapped model for testing this is available in Overte Hub next to the office tower

ksuprynowicz commented 6 months ago

I built and tested again and now the issue is gone and everything works perfectly. Does anyone else experience problems on this PR or we can merge it?

HifiExperiments commented 6 months ago

@ksuprynowicz hmmmmmm I haven't changed anything so that's a little suspicious! I haven't gotten a chance to revisit this yet but since @JulianGro also reported issues maybe let's hold off? I guess if others test and everything is fine before I take another look then it's good to go from my side, just want to be sure.

also did you build this branch directly or rebase on master or anything like that? I guess it's possible the other fixes that have been merged could have affected this

ksuprynowicz commented 6 months ago

More testing is a good idea. I think that clearing KTX cache may help with the issue Julian mentioned. For me bloom on this PR was broken, and was fixed by reloading KTX cache. Trees in tutorials work correctly for me.

ksuprynowicz commented 6 months ago

It turns out that some models don't render correctly on this PR, especially http://oaktown.pl/models/new_hub/sea.glb Master branch: overte-snap-by-X74hc595-on-2024-03-12_01-31-12 PR 845: overte-snap-by--on-2024-03-12_01-31-14

One of default avatars also renders in NSFW way: glitch

HifiExperiments commented 6 months ago

@ksuprynowicz 😨

I took a look and it seems like the fallback color, which is supposed to be (1, 1, 1, 1), is showing up in the shaders with an alpha < 1, so any transparent mesh with no vertex colors is becoming extra transparent. the fallback color is set as 0xFFFFFFFF, which is supposed to be interpreted as 4 uint8s, which should be (255, 255, 255, 255), and then normalized to (1, 1, 1, 1). but for some reason the alpha isn't interpreted correctly...

HifiExperiments commented 6 months ago

@ksuprynowicz ok I think everything should be working now!

ksuprynowicz commented 6 months ago

I tested, and both the avatar and water in Overte Hub works perfectly now!

HifiExperiments commented 6 months ago

@ksuprynowicz nice!! then this is ready to merge after code review