-
Notifications
You must be signed in to change notification settings - Fork 101
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
Live updates for materials #342
Conversation
AssetManager static hack to load textures in preview...
Conflicts: jme3-core/src/com/jme3/gde/core/editor/nodes/Diagram.java jme3-materialeditor/src/com/jme3/gde/materialdefinition/editor/ShaderNodeDiagram.java jme3-materialeditor/src/com/jme3/gde/materialdefinition/editor/previews/ColorPreview.java jme3-materialeditor/src/com/jme3/gde/materials/MaterialPreviewRenderer.java
matToRemove.clearParam(matParam.getName()); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what caused the mat previews not to update when changing parameters. They got overwritten when the material was reloaded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general looking very good, just a few minor considerations
public Material reloadMaterial(Material mat) { | ||
Material dummy; | ||
try { | ||
((ProjectAssetManager) mat.getMaterialDef().getAssetManager()).clearCache(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, we could probably use the same technique as in L94, if I'm not mistaken (that is, if we can get an asset key from Material#getMaterialDef), because clearing the whole cache has bigger side effects (such as every j3o having to be reloaded from disk etc), If I still know that right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean, but perhaps we can do that in a separate PR since I haven't changed any of that code?
jme3-materialeditor/src/com/jme3/gde/materials/MaterialPreviewRenderer.java
Outdated
Show resolved
Hide resolved
jme3-materialeditor/src/com/jme3/gde/materials/MaterialPreviewRenderer.java
Outdated
Show resolved
Hide resolved
jme3-materialeditor/src/com/jme3/gde/materialdefinition/editor/ShaderNodeDiagram.java
Outdated
Show resolved
Hide resolved
jme3-materialeditor/src/com/jme3/gde/materialdefinition/editor/ShaderNodeDiagram.java
Outdated
Show resolved
Hide resolved
jme3-materialeditor/src/com/jme3/gde/materialdefinition/editor/ShaderNodeDiagram.java
Outdated
Show resolved
Hide resolved
Yeah, those comments are from before. Btw, you can select the cog wheel and "hide whitespace" to only see the relevant changes. |
I managed to break out this as well. Still one annoying "whitespace" diff in this one.
I made a preliminary merge test with these PR's into the big one and it was fairly smooth, so I'm less worried about that now.