cginternals / cginternals/globjects

Reloading shader triggers numerous changed() and does not work

Aperta
#369 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
bug
Lingua principale
C++
Stelle
563
Fork
59
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

I've a shader whose source is a `File` (obtained via `Shader::sourceFromFile`).
If I want to update it, I'm calling `File::reload`. This does not work.
The sequence of operation that this causes is this:

1. `reload` call `changed` (once),
2. This calls `Shader::notifyChanged` that calls `updateSource`
3. This calls `File::string` and this triggers `File::loadFromFileContent` (so far, so good)
4. This then calls `Shader::invalidate` that sets compile flags to false
5. This calls `Shader::changed` and this triggers `Program::invalidate`
6. This set the dirty flag

[Later on, code that `use` program does this:]

1. In `Program::use` it's calling `checkDirty` (which is true)
2. It calls `Program::link` that's not compiling the shaders (but re-linking old compiled shaders)

Solution:
Compile shaders when the program is dirty (seems logical, not all user have a NVidia card that compile shaders when told to link them). Please notice that `Program::compileAttachedShaders` is protected and thus, not callable from user code. We can still call `Shader::compile` but this forces to keep a pointer on the shader (and that's exactly what `Program` is now doing, right?)

Also, I think there is too much abuse of the `ChangeListener` pattern here so the system is convoluted with many boolean to keep track of states. It would be so much easier if the Program was a listener of the Shader's AbstractStringSource directly, since the double listener triggering is useless in that case.
The only thing that can be modified in a Shader is its source. So why should they be in the Listener callchain instead of the source itself? Typically, `Shader::updateSource` or `invalidate` is misleading, since it does not cause reloading the source's text from the AbstractStringSource.

BTW, the error return is not checked in `Shader::updateSource` implementation, thus any error in the shader code will get silenced is using `reload` feature (at `ShadingLanguageIncludeImplementation_ShadingLanguageIncludeARB.cpp` line 26).
I guess you should check it explicitely in `updateSources` and throw a message upon error.
Or, if the Program is compiling the shader automatically, you'll get the error at that time, that's a good workaround too.

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Direzione di ricerca

Traccia il percorso di ricaricamento attraverso File::reload, Shader::notifyChanged, Shader::updateSource, Shader::invalidate, Program::invalidate e Program::use/checkDirty/link. Esamina Program::compileAttachedShaders e ShadingLanguageIncludeImplementation_ShadingLanguageIncludeARB.cpp alla riga 26, quindi riproduci il comportamento di ricaricamento degli shader. Il lavoro è completato quando i programmi contrassegnati come dirty compilano gli shader aggiornati prima del nuovo linking e gli errori di aggiornamento degli shader non vengono ignorati silenziosamente.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
cpp
Ambito
computer-graphics
Tipo di issue
Bug
Difficoltà
5/5
Tempo stimato
Più di una settimana
Stato di attività
Ferma
Chiarezza
Abbastanza chiara
Idoneità per principianti
28/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.