-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Shader compile error with troika-3d-text #59
Comments
(Sorry for the very long error log) |
I found out that this only happens when using the postprocessing library, when enabling the instancing workaround. |
The reason this is happening is because of this part: // Handler for automatically wrapping the base material with our upgrades. We do the wrapping
// lazily on _read_ rather than write to avoid unnecessary wrapping on transient values.
get material() {
let derivedMaterial = this._derivedMaterial
const baseMaterial = this._baseMaterial || defaultMaterial
if (!derivedMaterial || derivedMaterial.baseMaterial !== baseMaterial) {
if (derivedMaterial) {
derivedMaterial.dispose()
}
derivedMaterial = this._derivedMaterial = createTextDerivedMaterial(baseMaterial)
// dispose the derived material when its base material is disposed:
baseMaterial.addEventListener('dispose', function onDispose() {
baseMaterial.removeEventListener('dispose', onDispose)
derivedMaterial.dispose()
})
}
return derivedMaterial
} The postprocessing workaround changes the materials of all the meshes every render two times: https://github.com/vanruesc/postprocessing/blob/master/src/core/OverrideMaterialManager.js |
Ah I see, so it essentially ends up doing...
I can see how that would end up double-deriving the material. Should be a simple fix, I just have to check whether the incoming material is already a derived text material. @giulioz Do you happen to have an online example using your particular postprocessing setup that I could test this with - a Codesandbox or something? |
Potential fix for #59. Setting `mesh.material = mesh.material` would result in the derived text material being derived again, causing shader errors. This fix checks whether the incoming material is already derived and if so then it's just used as is.
I tried to replicate a similar situation with react-three-fiber and I got even another kind of error: https://codesandbox.io/s/troika-text-pp-bug-ymsv1?file=/src/index.js |
That codesandbox is using an old version, that shadowSide bug was fixed already. I've just published version 0.28.0 which I think should fix your postprocessing error, give that a try and reopen this ticket if there's still an issue. |
My fault, I forked from another sandbox and forgot to update... I'll let you know if 0.28.0 fixes the bug, thank you very much in advance. |
Hmm, yeah I suppose that makes sense if postprocessing is assigning a different MeshNormalMaterial instance every frame. The createDerivedMaterial utility will cache by base material instance, not by material class, so if it's not using the same material instance each time it will re-derive. Still, though, ThreeJS's program management should reuse a webgl program if the shader content isn't changing, so there must be something I'm doing that causes slightly different shader content each time. I'll look into that. |
Ah yes, the rewritten shaders use an This could be a tricky one to fix without breaking other things. If you could still provide a testcase for me that would be very helpful. |
I've updated the Codesandbox (https://codesandbox.io/s/troika-text-pp-bug-ymsv1). |
@giulioz Thank you for that CSB example, that's perfect. I've been able to reproduce it locally using that and I have a tentative fix that seems to be working. But this derived material logic is pretty finnicky/fragile so I'm going to put it through its paces during my day job work for the rest of the week, before pushing it out. Hope that doesn't hold you up too badly. |
No problem! Thanks for the assistance and your hard work on this amazing library :) |
… switch Followup performance fix for #59. When used in postprocessing, the base material can be temporarily reassigned, and disposing the original derived material triggers recompilation of the shader program on every next frame.
…erivedMaterial Related to #59. Different instances of the same Material class can easily give identical shader output; however we were injecting a unique id into the rewritten shader code for every instance, preventing the resulting shader code from ever being the same and causing separate compiled webgl programs for each instance. This change chooses a consistent id based on incoming derived material options, allowing the result to match when the input shader code is the same.
Thanks for the patience; I've published v 0.28.1 which should fix your frame rate issue. Please reopen if that's not the case. :) |
When using the 3d-text module I get this error. It seems that something is wrong with DerivedShader:
The text was updated successfully, but these errors were encountered: